Skip to content

Commit fd760b0

Browse files
authored
test: Add service LB deleter test (#99)
1 parent 0857199 commit fd760b0

File tree

6 files changed

+314
-8
lines changed

6 files changed

+314
-8
lines changed

go.mod

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ go 1.20
88
require (
99
github.com/go-logr/logr v1.2.4
1010
github.com/spf13/pflag v1.0.5
11+
github.com/stretchr/testify v1.8.1
1112
golang.org/x/sync v0.3.0
1213
k8s.io/api v0.27.4
1314
k8s.io/apimachinery v0.27.4
@@ -25,6 +26,7 @@ require (
2526
github.com/cespare/xxhash/v2 v2.2.0 // indirect
2627
github.com/davecgh/go-spew v1.1.1 // indirect
2728
github.com/emicklei/go-restful/v3 v3.9.0 // indirect
29+
github.com/evanphx/json-patch v5.6.0+incompatible // indirect
2830
github.com/evanphx/json-patch/v5 v5.6.0 // indirect
2931
github.com/fsnotify/fsnotify v1.6.0 // indirect
3032
github.com/go-openapi/jsonpointer v0.19.6 // indirect
@@ -49,6 +51,7 @@ require (
4951
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
5052
github.com/onsi/gomega v1.27.8 // indirect
5153
github.com/pkg/errors v0.9.1 // indirect
54+
github.com/pmezard/go-difflib v1.0.0 // indirect
5255
github.com/prometheus/client_golang v1.16.0 // indirect
5356
github.com/prometheus/client_model v0.4.0 // indirect
5457
github.com/prometheus/common v0.42.0 // indirect

go.sum

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.m
4242
github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.mod h1:hliV/p42l8fGbc6Y9bQ70uLwIvmJyVE5k4iMKlh8wCQ=
4343
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
4444
github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
45+
github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
4546
github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww=
4647
github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
4748
github.com/flowstack/go-jsonschema v0.1.1/go.mod h1:yL7fNggx1o8rm9RlgXv7hTBWxdBM0rVwpMwimd3F3N0=

make/go.mk

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ endif
170170

171171
.PHONY: go-fix.%
172172
go-fix.%: ## Runs golangci-lint for a specific module
173-
go-fix.%: ; $(info $(M) linting $* module)
173+
go-fix.%: ; $(info $(M) go fixing $* module)
174174
$(if $(filter-out root,$*),cd $* && )go fix ./...
175175

176176
.PHONY: go-generate

pkg/handlers/servicelbgc/deleter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func failedToDeleteServicesError(svcsFailedToBeDeleted []client.ObjectKey) error
105105
return fmt.Errorf("%w: the following Services could not be deleted "+
106106
"and must cleaned up manually before deleting the cluster: %s",
107107
ErrFailedToDeleteService,
108-
strings.Join(toStringSlice(svcsFailedToBeDeleted), ","),
108+
strings.Join(toStringSlice(svcsFailedToBeDeleted), ", "),
109109
)
110110
}
111111

Lines changed: 301 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,301 @@
1+
// Copyright 2023 D2iQ, Inc. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package servicelbgc
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
"github.com/go-logr/logr"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
corev1 "k8s.io/api/core/v1"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"sigs.k8s.io/cluster-api/api/v1beta1"
16+
"sigs.k8s.io/controller-runtime/pkg/client"
17+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
18+
)
19+
20+
//nolint:funlen // Long tests are OK
21+
func Test_shouldDeleteServicesWithLoadBalancer(t *testing.T) {
22+
t.Parallel()
23+
24+
tests := []struct {
25+
name string
26+
cluster *v1beta1.Cluster
27+
shouldDelete bool
28+
}{{
29+
name: "should delete",
30+
cluster: &v1beta1.Cluster{
31+
ObjectMeta: metav1.ObjectMeta{
32+
Name: "cluster-should-delete",
33+
},
34+
Status: v1beta1.ClusterStatus{
35+
Conditions: v1beta1.Conditions{{
36+
Type: v1beta1.ControlPlaneInitializedCondition,
37+
Status: corev1.ConditionTrue,
38+
}},
39+
Phase: string(v1beta1.ClusterPhaseProvisioned),
40+
},
41+
},
42+
shouldDelete: true,
43+
}, {
44+
name: "should delete: annotation is set to true",
45+
cluster: &v1beta1.Cluster{
46+
ObjectMeta: metav1.ObjectMeta{
47+
Name: "cluster-should-delete",
48+
Annotations: map[string]string{
49+
LoadBalancerGCAnnotation: "true",
50+
},
51+
},
52+
Status: v1beta1.ClusterStatus{
53+
Conditions: v1beta1.Conditions{{
54+
Type: v1beta1.ControlPlaneInitializedCondition,
55+
Status: corev1.ConditionTrue,
56+
}},
57+
Phase: string(v1beta1.ClusterPhaseProvisioned),
58+
},
59+
},
60+
shouldDelete: true,
61+
}, {
62+
name: "should not delete: annotation is set to false",
63+
cluster: &v1beta1.Cluster{
64+
ObjectMeta: metav1.ObjectMeta{
65+
Name: "cluster-should-delete",
66+
Annotations: map[string]string{
67+
LoadBalancerGCAnnotation: "false",
68+
},
69+
},
70+
Status: v1beta1.ClusterStatus{
71+
Phase: string(v1beta1.ClusterPhaseProvisioned),
72+
},
73+
},
74+
shouldDelete: false,
75+
}, {
76+
name: "should not delete: phase is Pending",
77+
cluster: &v1beta1.Cluster{
78+
ObjectMeta: metav1.ObjectMeta{
79+
Name: "cluster-should-delete",
80+
},
81+
Status: v1beta1.ClusterStatus{
82+
Phase: string(v1beta1.ClusterPhasePending),
83+
},
84+
},
85+
shouldDelete: false,
86+
}, {
87+
name: "should not delete: ControlPlaneInitialized condition is False",
88+
cluster: &v1beta1.Cluster{
89+
ObjectMeta: metav1.ObjectMeta{
90+
Name: "cluster-should-delete",
91+
},
92+
Status: v1beta1.ClusterStatus{
93+
Conditions: v1beta1.Conditions{{
94+
Type: v1beta1.ControlPlaneInitializedCondition,
95+
Status: corev1.ConditionFalse,
96+
}},
97+
},
98+
},
99+
shouldDelete: false,
100+
}}
101+
for idx := range tests {
102+
tt := tests[idx]
103+
t.Run(tt.name, func(t *testing.T) {
104+
t.Parallel()
105+
shouldDelete, err := shouldDeleteServicesWithLoadBalancer(tt.cluster)
106+
assert.NoError(t, err)
107+
assert.Equal(t, tt.shouldDelete, shouldDelete)
108+
})
109+
}
110+
}
111+
112+
//nolint:funlen // Long tests are OK
113+
func Test_deleteServicesWithLoadBalancer(t *testing.T) {
114+
t.Parallel()
115+
116+
tests := []struct {
117+
name string
118+
startServices []corev1.Service
119+
endServices []corev1.Service
120+
}{{
121+
name: "no services",
122+
startServices: []corev1.Service(nil),
123+
endServices: []corev1.Service(nil),
124+
}, {
125+
name: "should not delete, all services with ClusterIP",
126+
startServices: []corev1.Service{{
127+
ObjectMeta: metav1.ObjectMeta{
128+
Name: "svc-1",
129+
Namespace: "ns-1",
130+
},
131+
Spec: corev1.ServiceSpec{
132+
Type: corev1.ServiceTypeClusterIP,
133+
},
134+
}, {
135+
ObjectMeta: metav1.ObjectMeta{
136+
Name: "svc-2",
137+
Namespace: "ns-2",
138+
},
139+
Spec: corev1.ServiceSpec{
140+
Type: corev1.ServiceTypeClusterIP,
141+
},
142+
}},
143+
endServices: []corev1.Service{{
144+
ObjectMeta: metav1.ObjectMeta{
145+
Name: "svc-1",
146+
Namespace: "ns-1",
147+
ResourceVersion: "1",
148+
},
149+
Spec: corev1.ServiceSpec{
150+
Type: corev1.ServiceTypeClusterIP,
151+
},
152+
}, {
153+
ObjectMeta: metav1.ObjectMeta{
154+
Name: "svc-2",
155+
Namespace: "ns-2",
156+
ResourceVersion: "1",
157+
},
158+
Spec: corev1.ServiceSpec{
159+
Type: corev1.ServiceTypeClusterIP,
160+
},
161+
}},
162+
}, {
163+
name: "should delete 1 service with LoadBalancer",
164+
startServices: []corev1.Service{{
165+
ObjectMeta: metav1.ObjectMeta{
166+
Name: "svc-1",
167+
Namespace: "ns-1",
168+
},
169+
Spec: corev1.ServiceSpec{
170+
Type: corev1.ServiceTypeClusterIP,
171+
},
172+
}, {
173+
ObjectMeta: metav1.ObjectMeta{
174+
Name: "svc-2",
175+
Namespace: "ns-2",
176+
},
177+
Spec: corev1.ServiceSpec{
178+
Type: corev1.ServiceTypeLoadBalancer,
179+
},
180+
Status: corev1.ServiceStatus{
181+
LoadBalancer: corev1.LoadBalancerStatus{
182+
Ingress: []corev1.LoadBalancerIngress{{Hostname: "lb-123.com"}},
183+
},
184+
},
185+
}},
186+
endServices: []corev1.Service{{
187+
ObjectMeta: metav1.ObjectMeta{
188+
Name: "svc-1",
189+
Namespace: "ns-1",
190+
ResourceVersion: "1",
191+
},
192+
Spec: corev1.ServiceSpec{
193+
Type: corev1.ServiceTypeClusterIP,
194+
},
195+
}},
196+
}}
197+
198+
for idx := range tests {
199+
tt := tests[idx] // Capture range variable
200+
t.Run(tt.name, func(t *testing.T) {
201+
t.Parallel()
202+
fakeClient := fake.NewClientBuilder().Build()
203+
for i := range tt.startServices {
204+
svc := &tt.startServices[i]
205+
require.NoError(
206+
t,
207+
fakeClient.Create(context.Background(), svc),
208+
"error creating Service",
209+
)
210+
}
211+
212+
for {
213+
err := deleteServicesWithLoadBalancer(context.TODO(), fakeClient, logr.Discard())
214+
if err == nil {
215+
break
216+
}
217+
require.ErrorIs(
218+
t,
219+
err,
220+
ErrServicesStillExist,
221+
"only allowed a retry-able error here",
222+
)
223+
}
224+
225+
services := &corev1.ServiceList{}
226+
require.NoError(
227+
t,
228+
fakeClient.List(context.Background(), services),
229+
"error listing Services",
230+
)
231+
assert.Equal(t, tt.endServices, services.Items)
232+
})
233+
}
234+
}
235+
236+
func Test_needsDelete(t *testing.T) {
237+
t.Parallel()
238+
239+
tests := []struct {
240+
name string
241+
service *corev1.Service
242+
shouldDelete bool
243+
}{{
244+
name: "shouldDelete",
245+
service: &corev1.Service{
246+
Spec: corev1.ServiceSpec{
247+
Type: corev1.ServiceTypeLoadBalancer,
248+
},
249+
Status: corev1.ServiceStatus{
250+
LoadBalancer: corev1.LoadBalancerStatus{
251+
Ingress: []corev1.LoadBalancerIngress{{Hostname: "lb-123.com"}},
252+
},
253+
},
254+
},
255+
shouldDelete: true,
256+
}, {
257+
name: "false: ServiceTypeNodePort",
258+
service: &corev1.Service{
259+
Spec: corev1.ServiceSpec{
260+
Type: corev1.ServiceTypeNodePort,
261+
},
262+
Status: corev1.ServiceStatus{
263+
LoadBalancer: corev1.LoadBalancerStatus{
264+
Ingress: []corev1.LoadBalancerIngress{{Hostname: "lb-123.com"}},
265+
},
266+
},
267+
},
268+
shouldDelete: false,
269+
}, {
270+
name: "false: LoadBalancer is empty",
271+
service: &corev1.Service{
272+
Spec: corev1.ServiceSpec{
273+
Type: corev1.ServiceTypeLoadBalancer,
274+
},
275+
Status: corev1.ServiceStatus{},
276+
},
277+
shouldDelete: false,
278+
}}
279+
for idx := range tests {
280+
tt := tests[idx]
281+
t.Run(tt.name, func(t *testing.T) {
282+
t.Parallel()
283+
del := needsDelete(tt.service)
284+
assert.Equal(t, tt.shouldDelete, del)
285+
})
286+
}
287+
}
288+
289+
// this test is mainly here to visually show what the error will look like.
290+
func Test_failedToDeleteServicesError(t *testing.T) {
291+
svcs := []client.ObjectKey{
292+
{Namespace: "ns-1", Name: "svc-1"},
293+
{Namespace: "ns-2", Name: "svc-2"},
294+
{Namespace: "ns-3", Name: "svc-3"},
295+
{Namespace: "ns-4", Name: "svc-4"},
296+
{Namespace: "ns-5", Name: "svc-5"},
297+
}
298+
//nolint:lll // want to show the full error in one line
299+
expectedErrString := "failed to delete kubernetes services: the following Services could not be deleted and must cleaned up manually before deleting the cluster: ns-1/svc-1, ns-2/svc-2, ns-3/svc-3, ns-4/svc-4, ns-5/svc-5"
300+
assert.EqualError(t, failedToDeleteServicesError(svcs), expectedErrString)
301+
}

pkg/handlers/servicelbgc/handler.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,14 @@ func (s *ServiceLoadBalancerGC) BeforeClusterDelete(
7878
err = deleteServicesWithLoadBalancer(ctx, remoteClient, log)
7979
switch {
8080
case errors.Is(err, ErrFailedToDeleteService):
81-
resp.Status = runtimehooksv1.ResponseStatusFailure
82-
resp.Message = err.Error()
81+
resp.SetStatus(runtimehooksv1.ResponseStatusFailure)
82+
resp.SetMessage(err.Error())
83+
resp.SetRetryAfterSeconds(5)
8384
case errors.Is(err, ErrServicesStillExist):
84-
resp.Status = runtimehooksv1.ResponseStatusSuccess
85-
resp.Message = err.Error()
86-
resp.RetryAfterSeconds = 5
85+
resp.SetStatus(runtimehooksv1.ResponseStatusSuccess)
86+
resp.SetMessage(err.Error())
87+
resp.SetRetryAfterSeconds(5)
8788
default:
88-
resp.Status = runtimehooksv1.ResponseStatusSuccess
89+
resp.SetStatus(runtimehooksv1.ResponseStatusSuccess)
8990
}
9091
}

0 commit comments

Comments
 (0)