Skip to content

Commit 3ba6ef7

Browse files
fwieselfwiesel
committed
Normalize tests
- metav1 instead of the less descript v1. - Full name of controller instead of shorted name. - DeferCleanup right after creation - Always fail unhandled mocked http requests
1 parent e8ca39c commit 3ba6ef7

File tree

5 files changed

+119
-113
lines changed

5 files changed

+119
-113
lines changed

‎internal/controller/aggregates_controller_test.go‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
. "github.com/onsi/ginkgo/v2"
2727
. "github.com/onsi/gomega"
2828
"k8s.io/apimachinery/pkg/api/meta"
29-
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
29+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3030
"k8s.io/apimachinery/pkg/types"
3131
ctrl "sigs.k8s.io/controller-runtime"
3232

@@ -98,49 +98,53 @@ var _ = Describe("AggregatesController", func() {
9898
)
9999

100100
var (
101-
tc *AggregatesController
102-
fakeServer testhelper.FakeServer
103-
hypervisorName = types.NamespacedName{Name: "hv-test"}
101+
aggregatesController *AggregatesController
102+
fakeServer testhelper.FakeServer
103+
hypervisorName = types.NamespacedName{Name: "hv-test"}
104104
)
105-
// Setup and teardown
106105

107106
BeforeEach(func(ctx SpecContext) {
108107
By("Setting up the OpenStack http mock server")
109108
fakeServer = testhelper.SetupHTTP()
110109
DeferCleanup(fakeServer.Teardown)
111110

111+
// Install default handler to fail unhandled requests
112+
fakeServer.Mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
113+
Fail("Unhandled request to fake server: " + r.Method + " " + r.URL.Path)
114+
})
115+
112116
hypervisor := &kvmv1.Hypervisor{
113-
ObjectMeta: v1.ObjectMeta{
117+
ObjectMeta: metav1.ObjectMeta{
114118
Name: hypervisorName.Name,
115119
},
116120
Spec: kvmv1.HypervisorSpec{
117121
LifecycleEnabled: true,
118122
},
119123
}
120124
Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed())
125+
DeferCleanup(func(ctx SpecContext) {
126+
Expect(k8sClient.Delete(ctx, hypervisor)).To(Succeed())
127+
})
128+
121129
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
122-
meta.SetStatusCondition(&hypervisor.Status.Conditions, v1.Condition{
130+
meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
123131
Type: kvmv1.ConditionTypeOnboarding,
124-
Status: v1.ConditionFalse,
132+
Status: metav1.ConditionFalse,
125133
Reason: "dontcare",
126134
Message: "dontcare",
127135
})
128136
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
129137

130138
By("Creating the AggregatesController")
131-
tc = &AggregatesController{
139+
aggregatesController = &AggregatesController{
132140
Client: k8sClient,
133141
Scheme: k8sClient.Scheme(),
134142
computeClient: client.ServiceClient(fakeServer),
135143
}
136-
137-
DeferCleanup(func(ctx SpecContext) {
138-
Expect(tc.Client.Delete(ctx, hypervisor)).To(Succeed())
139-
})
140144
})
141145

142146
JustBeforeEach(func(ctx SpecContext) {
143-
_, err := tc.Reconcile(ctx, ctrl.Request{NamespacedName: hypervisorName})
147+
_, err := aggregatesController.Reconcile(ctx, ctrl.Request{NamespacedName: hypervisorName})
144148
Expect(err).NotTo(HaveOccurred())
145149
})
146150

@@ -190,7 +194,7 @@ var _ = Describe("AggregatesController", func() {
190194

191195
It("should update Aggregates and set status condition as Aggregates differ", func(ctx SpecContext) {
192196
updated := &kvmv1.Hypervisor{}
193-
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
197+
Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
194198
Expect(updated.Status.Aggregates).To(ContainElements("test-aggregate1"))
195199
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeTrue())
196200
})
@@ -234,7 +238,7 @@ var _ = Describe("AggregatesController", func() {
234238

235239
It("should update Aggregates and set status condition when Aggregates differ", func(ctx SpecContext) {
236240
updated := &kvmv1.Hypervisor{}
237-
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
241+
Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
238242
Expect(updated.Status.Aggregates).To(BeEmpty())
239243
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeTrue())
240244
})
@@ -245,13 +249,13 @@ var _ = Describe("AggregatesController", func() {
245249
hypervisor := &kvmv1.Hypervisor{}
246250
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
247251
// Remove the onboarding condition
248-
hypervisor.Status.Conditions = []v1.Condition{}
252+
hypervisor.Status.Conditions = []metav1.Condition{}
249253
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
250254
})
251255

252256
It("should neither update Aggregates and nor set status condition", func(ctx SpecContext) {
253257
updated := &kvmv1.Hypervisor{}
254-
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
258+
Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
255259
Expect(updated.Status.Aggregates).To(BeEmpty())
256260
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeFalse())
257261
})
@@ -262,9 +266,9 @@ var _ = Describe("AggregatesController", func() {
262266
hypervisor := &kvmv1.Hypervisor{}
263267
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
264268
// Remove the onboarding condition
265-
meta.SetStatusCondition(&hypervisor.Status.Conditions, v1.Condition{
269+
meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
266270
Type: kvmv1.ConditionTypeTerminating,
267-
Status: v1.ConditionTrue,
271+
Status: metav1.ConditionTrue,
268272
Reason: "dontcare",
269273
Message: "dontcare",
270274
})
@@ -273,7 +277,7 @@ var _ = Describe("AggregatesController", func() {
273277

274278
It("should neither update Aggregates and nor set status condition", func(ctx SpecContext) {
275279
updated := &kvmv1.Hypervisor{}
276-
Expect(tc.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
280+
Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
277281
Expect(updated.Status.Aggregates).To(BeEmpty())
278282
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeFalse())
279283
})

‎internal/controller/decomission_controller_test.go‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@ import (
3636
kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
3737
)
3838

39-
const (
40-
EOF = "EOF"
41-
serviceId = "service-1234"
42-
hypervisorName = "node-test"
43-
namespaceName = "namespace-test"
44-
AggregateListWithHv = `
39+
var _ = Describe("Decommission Controller", func() {
40+
const (
41+
EOF = "EOF"
42+
serviceId = "service-1234"
43+
hypervisorName = "node-test"
44+
namespaceName = "namespace-test"
45+
AggregateListWithHv = `
4546
{
4647
"aggregates": [
4748
{
@@ -54,7 +55,7 @@ const (
5455
]
5556
}
5657
`
57-
AggregateRemoveHostBody = `
58+
AggregateRemoveHostBody = `
5859
{
5960
"aggregate": {
6061
"name": "test-aggregate2",
@@ -63,28 +64,32 @@ const (
6364
"id": 100001
6465
}
6566
}`
66-
)
67+
)
6768

68-
var _ = Describe("Decommission Controller", func() {
6969
var (
70-
r *NodeDecommissionReconciler
71-
resourceName = types.NamespacedName{Name: hypervisorName}
72-
reconcileReq = ctrl.Request{
70+
decommissionReconciler *NodeDecommissionReconciler
71+
resourceName = types.NamespacedName{Name: hypervisorName}
72+
reconcileReq = ctrl.Request{
7373
NamespacedName: resourceName,
7474
}
7575
fakeServer testhelper.FakeServer
7676
)
7777

7878
BeforeEach(func(ctx SpecContext) {
7979
fakeServer = testhelper.SetupHTTP()
80-
os.Setenv("KVM_HA_SERVICE_URL", fakeServer.Endpoint()+"instance-ha")
80+
DeferCleanup(fakeServer.Teardown)
8181

82+
// Install default handler to fail unhandled requests
83+
fakeServer.Mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
84+
Fail("Unhandled request to fake server: " + r.Method + " " + r.URL.Path)
85+
})
86+
87+
os.Setenv("KVM_HA_SERVICE_URL", fakeServer.Endpoint()+"instance-ha")
8288
DeferCleanup(func() {
8389
os.Unsetenv("KVM_HA_SERVICE_URL")
84-
fakeServer.Teardown()
8590
})
8691

87-
r = &NodeDecommissionReconciler{
92+
decommissionReconciler = &NodeDecommissionReconciler{
8893
Client: k8sClient,
8994
Scheme: k8sClient.Scheme(),
9095
computeClient: client.ServiceClient(fakeServer),
@@ -94,7 +99,6 @@ var _ = Describe("Decommission Controller", func() {
9499
By("creating the namespace for the reconciler")
95100
ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespaceName}}
96101
Expect(k8sclient.IgnoreAlreadyExists(k8sClient.Create(ctx, ns))).To(Succeed())
97-
98102
DeferCleanup(func(ctx SpecContext) {
99103
Expect(k8sClient.Delete(ctx, ns)).To(Succeed())
100104
})
@@ -116,8 +120,8 @@ var _ = Describe("Decommission Controller", func() {
116120

117121
Context("When marking the hypervisor terminating", func() {
118122
JustBeforeEach(func(ctx SpecContext) {
119-
By("reconciling first reconciling the to add the finalizer")
120-
_, err := r.Reconcile(ctx, reconcileReq)
123+
By("reconciling first to add the finalizer")
124+
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
121125
Expect(err).NotTo(HaveOccurred())
122126

123127
hypervisor := &kvmv1.Hypervisor{}
@@ -208,11 +212,15 @@ var _ = Describe("Decommission Controller", func() {
208212
fakeServer.Mux.HandleFunc("DELETE /resource_providers/rp-uuid", func(w http.ResponseWriter, r *http.Request) {
209213
w.WriteHeader(http.StatusAccepted)
210214
})
215+
216+
fakeServer.Mux.HandleFunc("DELETE /os-services/service-1234", func(w http.ResponseWriter, r *http.Request) {
217+
w.WriteHeader(http.StatusNoContent)
218+
})
211219
})
212220

213221
It("should set the hypervisor ready condition", func(ctx SpecContext) {
214222
By("reconciling the created resource")
215-
_, err := r.Reconcile(ctx, reconcileReq)
223+
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
216224
Expect(err).NotTo(HaveOccurred())
217225
hypervisor := &kvmv1.Hypervisor{}
218226
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
@@ -228,7 +236,7 @@ var _ = Describe("Decommission Controller", func() {
228236
It("should set the hypervisor offboarded condition", func(ctx SpecContext) {
229237
By("reconciling the created resource")
230238
for range 3 {
231-
_, err := r.Reconcile(ctx, reconcileReq)
239+
_, err := decommissionReconciler.Reconcile(ctx, reconcileReq)
232240
Expect(err).NotTo(HaveOccurred())
233241
}
234242
Expect(getHypervisorsCalled).To(BeNumerically(">", 0))

‎internal/controller/eviction_controller_test.go‎

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ var _ = Describe("Eviction Controller", func() {
6161
}
6262
}`
6363
)
64+
6465
var (
66+
evictionReconciler *EvictionReconciler
6567
typeNamespacedName = types.NamespacedName{
6668
Name: resourceName,
6769
Namespace: namespaceName,
@@ -70,11 +72,28 @@ var _ = Describe("Eviction Controller", func() {
7072
Name: resourceName,
7173
Namespace: namespaceName,
7274
}
73-
reconcileRequest = ctrl.Request{NamespacedName: typeNamespacedName}
74-
controllerReconciler *EvictionReconciler
75-
fakeServer testhelper.FakeServer
75+
reconcileRequest = ctrl.Request{NamespacedName: typeNamespacedName}
76+
fakeServer testhelper.FakeServer
7677
)
7778

79+
BeforeEach(func(ctx SpecContext) {
80+
By("Setting up the OpenStack http mock server")
81+
fakeServer = testhelper.SetupHTTP()
82+
DeferCleanup(fakeServer.Teardown)
83+
84+
// Install default handler to fail unhandled requests
85+
fakeServer.Mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
86+
Fail("Unhandled request to fake server: " + r.Method + " " + r.URL.Path)
87+
})
88+
89+
By("Creating the EvictionReconciler")
90+
evictionReconciler = &EvictionReconciler{
91+
Client: k8sClient,
92+
Scheme: k8sClient.Scheme(),
93+
computeClient: client.ServiceClient(fakeServer),
94+
}
95+
})
96+
7897
AfterEach(func(ctx SpecContext) {
7998
resource := &kvmv1.Eviction{}
8099
err := k8sClient.Get(ctx, typeNamespacedName, resource)
@@ -84,9 +103,9 @@ var _ = Describe("Eviction Controller", func() {
84103
}
85104
} else {
86105
By("Cleanup the specific resource instance Eviction")
87-
Expect(controllerReconciler).NotTo(BeNil())
106+
Expect(evictionReconciler).NotTo(BeNil())
88107
Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
89-
_, err := controllerReconciler.Reconcile(ctx, reconcileRequest)
108+
_, err := evictionReconciler.Reconcile(ctx, reconcileRequest)
90109
Expect(err).NotTo(HaveOccurred())
91110
Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).Should(HaveOccurred())
92111
}
@@ -147,32 +166,6 @@ var _ = Describe("Eviction Controller", func() {
147166
})
148167

149168
Describe("Reconciliation", func() {
150-
BeforeEach(func(ctx SpecContext) {
151-
By("Setting up the OpenStack http mock server")
152-
fakeServer = testhelper.SetupHTTP()
153-
154-
DeferCleanup(func(ctx SpecContext) {
155-
fakeServer.Teardown()
156-
})
157-
158-
// Install default handler to fail unhandled requests
159-
fakeServer.Mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
160-
Fail("Unhandled request to fake server: " + r.Method + " " + r.URL.Path)
161-
})
162-
163-
By("Creating the EvictionReconciler")
164-
165-
controllerReconciler = &EvictionReconciler{
166-
Client: k8sClient,
167-
Scheme: k8sClient.Scheme(),
168-
computeClient: client.ServiceClient(fakeServer),
169-
}
170-
171-
DeferCleanup(func() {
172-
controllerReconciler = nil
173-
})
174-
})
175-
176169
Describe("an eviction for an onboarded 'test-hypervisor'", func() {
177170
BeforeEach(func(ctx SpecContext) {
178171
By("creating the hypervisor resource")
@@ -222,7 +215,7 @@ var _ = Describe("Eviction Controller", func() {
222215

223216
It("should fail reconciliation", func(ctx SpecContext) {
224217
for range 3 {
225-
_, err := controllerReconciler.Reconcile(ctx, reconcileRequest)
218+
_, err := evictionReconciler.Reconcile(ctx, reconcileRequest)
226219
Expect(err).NotTo(HaveOccurred())
227220
}
228221

@@ -294,7 +287,7 @@ var _ = Describe("Eviction Controller", func() {
294287
for i, expectation := range expectations {
295288
By(fmt.Sprintf("Reconciliation step %d", i+1))
296289
// Reconcile the resource
297-
result, err := controllerReconciler.Reconcile(ctx, reconcileRequest)
290+
result, err := evictionReconciler.Reconcile(ctx, reconcileRequest)
298291
Expect(result).To(Equal(ctrl.Result{}))
299292
Expect(err).NotTo(HaveOccurred())
300293

@@ -327,7 +320,7 @@ var _ = Describe("Eviction Controller", func() {
327320

328321
It("should succeed the reconciliation", func(ctx SpecContext) {
329322
for range 1 {
330-
_, err := controllerReconciler.Reconcile(ctx, reconcileRequest)
323+
_, err := evictionReconciler.Reconcile(ctx, reconcileRequest)
331324
Expect(err).NotTo(HaveOccurred())
332325
}
333326

@@ -345,7 +338,7 @@ var _ = Describe("Eviction Controller", func() {
345338
))
346339

347340
for range 3 {
348-
_, err = controllerReconciler.Reconcile(ctx, reconcileRequest)
341+
_, err = evictionReconciler.Reconcile(ctx, reconcileRequest)
349342
Expect(err).NotTo(HaveOccurred())
350343
}
351344
err = k8sClient.Get(ctx, typeNamespacedName, resource)

0 commit comments

Comments
 (0)