Skip to content

Commit 0e58200

Browse files
notandynotandy
committed
Fix tests to not expect Ready condition from individual controllers
Since the ReadyController now owns the Ready condition, update tests for maintenance, offboarding, onboarding, and hypervisor controllers to only check for the conditions those controllers actually set (Evicting, Offboarded, Onboarding, Terminating) rather than the computed Ready condition. - Remove tests checking Ready condition from maintenance controller - Update offboarding tests to check Offboarded condition instead - Remove Ready condition checks from onboarding controller tests - Remove test that checked Ready condition preservation in hypervisor controller (now handled by ReadyController) - Set initial Offboarded condition in failure mode tests so reconcile hits the error paths
1 parent 7d59616 commit 0e58200

4 files changed

Lines changed: 24 additions & 110 deletions

File tree

‎internal/controller/hypervisor_controller_test.go‎

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
. "github.com/onsi/ginkgo/v2"
2222
. "github.com/onsi/gomega"
2323
corev1 "k8s.io/api/core/v1"
24-
"k8s.io/apimachinery/pkg/api/meta"
2524
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2625
"k8s.io/apimachinery/pkg/types"
2726
ctrl "sigs.k8s.io/controller-runtime"
@@ -311,12 +310,7 @@ var _ = Describe("Hypervisor Controller", func() {
311310
// Get the Hypervisor resource
312311
updatedHypervisor := &kvmv1.Hypervisor{}
313312
Expect(hypervisorController.Get(ctx, hypervisorName, updatedHypervisor)).To(Succeed())
314-
Expect(updatedHypervisor.Status.Conditions).To(ContainElements(
315-
SatisfyAll(
316-
HaveField("Type", kvmv1.ConditionTypeReady),
317-
HaveField("Reason", terminatingReason),
318-
HaveField("Status", metav1.ConditionFalse),
319-
),
313+
Expect(updatedHypervisor.Status.Conditions).To(ContainElement(
320314
SatisfyAll(
321315
HaveField("Type", kvmv1.ConditionTypeTerminating),
322316
HaveField("Reason", terminatingReason),
@@ -342,39 +336,6 @@ var _ = Describe("Hypervisor Controller", func() {
342336
})
343337
})
344338

345-
Context("and the Hypervisor resource already has a Ready Condition set to false", func() {
346-
BeforeEach(func(ctx SpecContext) {
347-
hypervisor := &kvmv1.Hypervisor{}
348-
Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
349-
meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
350-
Type: kvmv1.ConditionTypeReady,
351-
Status: metav1.ConditionFalse,
352-
Reason: "SomeOtherReason",
353-
})
354-
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
355-
})
356-
357-
It("should not update the existing Ready Condition with the new reason", func(ctx SpecContext) {
358-
for range 2 {
359-
_, err := hypervisorController.Reconcile(ctx, ctrl.Request{
360-
NamespacedName: types.NamespacedName{Name: resource.Name},
361-
})
362-
Expect(err).NotTo(HaveOccurred())
363-
}
364-
365-
// Get the Hypervisor resource again
366-
updatedHypervisor := &kvmv1.Hypervisor{}
367-
Expect(k8sClient.Get(ctx, hypervisorName, updatedHypervisor)).To(Succeed())
368-
Expect(updatedHypervisor.Status.Conditions).To(ContainElement(
369-
SatisfyAll(
370-
HaveField("Type", kvmv1.ConditionTypeReady),
371-
HaveField("Reason", "SomeOtherReason"),
372-
HaveField("Status", metav1.ConditionFalse),
373-
),
374-
))
375-
})
376-
})
377-
378339
It("should successfully reconcile the terminating node", func(ctx SpecContext) {
379340
By("Reconciling the created resource")
380341
for range 2 {
@@ -391,12 +352,7 @@ var _ = Describe("Hypervisor Controller", func() {
391352
// Not sure, if that is a good idea, but that is the current behaviour
392353
// We expect another operator to set the Maintenance field to Termination
393354
Expect(updatedHypervisor.Spec.Maintenance).NotTo(Equal(kvmv1.MaintenanceTermination))
394-
Expect(updatedHypervisor.Status.Conditions).To(ContainElements(
395-
SatisfyAll(
396-
HaveField("Type", kvmv1.ConditionTypeReady),
397-
HaveField("Reason", terminatingReason),
398-
HaveField("Status", metav1.ConditionFalse),
399-
),
355+
Expect(updatedHypervisor.Status.Conditions).To(ContainElement(
400356
SatisfyAll(
401357
HaveField("Type", kvmv1.ConditionTypeTerminating),
402358
HaveField("Reason", terminatingReason),

‎internal/controller/hypervisor_maintenance_controller_test.go‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,6 @@ var _ = Describe("HypervisorMaintenanceController", func() {
157157
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
158158
Expect(meta.IsStatusConditionFalse(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeTrue())
159159
})
160-
161-
It("should set the ConditionTypeReady to true", func(ctx SpecContext) {
162-
updated := &kvmv1.Hypervisor{}
163-
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
164-
Expect(updated.Status.Conditions).To(ContainElement(
165-
SatisfyAll(
166-
HaveField("Type", kvmv1.ConditionTypeReady),
167-
HaveField("Status", metav1.ConditionTrue),
168-
)))
169-
})
170160
}) // Spec.Maintenance=""
171161
})
172162

@@ -189,16 +179,6 @@ var _ = Describe("HypervisorMaintenanceController", func() {
189179
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
190180
Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeTrue())
191181
})
192-
193-
It("should set the ConditionTypeReady to false", func(ctx SpecContext) {
194-
updated := &kvmv1.Hypervisor{}
195-
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
196-
Expect(updated.Status.Conditions).To(ContainElement(
197-
SatisfyAll(
198-
HaveField("Type", kvmv1.ConditionTypeReady),
199-
HaveField("Status", metav1.ConditionFalse),
200-
)))
201-
})
202182
}) // Spec.Maintenance="<mode>"
203183
}
204184

@@ -339,14 +319,13 @@ var _ = Describe("HypervisorMaintenanceController", func() {
339319
Expect(hypervisor.Status.Evicted).To(BeFalse())
340320
})
341321

342-
It("should set the ConditionTypeReady to false and reason to evicting", func(ctx SpecContext) {
322+
It("should set the ConditionTypeEvicting to true", func(ctx SpecContext) {
343323
updated := &kvmv1.Hypervisor{}
344324
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
345325
Expect(updated.Status.Conditions).To(ContainElement(
346326
SatisfyAll(
347-
HaveField("Type", kvmv1.ConditionTypeReady),
348-
HaveField("Status", metav1.ConditionFalse),
349-
HaveField("Reason", kvmv1.ConditionReasonReadyEvicting),
327+
HaveField("Type", kvmv1.ConditionTypeEvicting),
328+
HaveField("Status", metav1.ConditionTrue),
350329
)))
351330
})
352331
})
@@ -393,14 +372,13 @@ var _ = Describe("HypervisorMaintenanceController", func() {
393372
Expect(hypervisor.Status.Evicted).To(BeTrue())
394373
})
395374

396-
It("should set the ConditionTypeReady to false and reason to evicted", func(ctx SpecContext) {
375+
It("should set the ConditionTypeEvicting to false when evicted", func(ctx SpecContext) {
397376
updated := &kvmv1.Hypervisor{}
398377
Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed())
399378
Expect(updated.Status.Conditions).To(ContainElement(
400379
SatisfyAll(
401-
HaveField("Type", kvmv1.ConditionTypeReady),
380+
HaveField("Type", kvmv1.ConditionTypeEvicting),
402381
HaveField("Status", metav1.ConditionFalse),
403-
HaveField("Reason", kvmv1.ConditionReasonReadyEvicted),
404382
)))
405383
})
406384
})

‎internal/controller/offboarding_controller_test.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,15 @@ var _ = Describe("Offboarding Controller", func() {
175175
})
176176
})
177177

178-
It("should set the hypervisor ready condition", func(ctx SpecContext) {
178+
It("should set the hypervisor offboarding condition", func(ctx SpecContext) {
179179
_, err := offboardingReconciler.Reconcile(ctx, reconcileReq)
180180
Expect(err).NotTo(HaveOccurred())
181181

182182
hypervisor := &kvmv1.Hypervisor{}
183183
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
184184
Expect(hypervisor.Status.Conditions).To(ContainElement(
185185
SatisfyAll(
186-
HaveField("Type", kvmv1.ConditionTypeReady),
186+
HaveField("Type", kvmv1.ConditionTypeOffboarded),
187187
HaveField("Status", metav1.ConditionFalse),
188188
HaveField("Reason", "Offboarding"),
189189
),
@@ -199,19 +199,13 @@ var _ = Describe("Offboarding Controller", func() {
199199

200200
hypervisor := &kvmv1.Hypervisor{}
201201
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
202-
Expect(hypervisor.Status.Conditions).To(ContainElements(
202+
Expect(hypervisor.Status.Conditions).To(ContainElement(
203203
SatisfyAll(
204204
HaveField("Type", kvmv1.ConditionTypeOffboarded),
205205
HaveField("Status", metav1.ConditionTrue),
206206
HaveField("Reason", "Offboarded"),
207207
HaveField("Message", "Offboarding successful"),
208208
),
209-
SatisfyAll(
210-
HaveField("Type", kvmv1.ConditionTypeReady),
211-
HaveField("Status", metav1.ConditionFalse),
212-
HaveField("Reason", "Offboarded"),
213-
HaveField("Message", "Offboarding successful"),
214-
),
215209
))
216210
})
217211

@@ -296,7 +290,7 @@ var _ = Describe("Offboarding Controller", func() {
296290
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
297291
Expect(hypervisor.Status.Conditions).To(ContainElement(
298292
SatisfyAll(
299-
HaveField("Type", kvmv1.ConditionTypeReady),
293+
HaveField("Type", kvmv1.ConditionTypeOffboarded),
300294
HaveField("Status", metav1.ConditionFalse),
301295
HaveField("Reason", "Offboarding"),
302296
HaveField("Message", ContainSubstring(expectedMessageSubstring)),
@@ -310,6 +304,15 @@ var _ = Describe("Offboarding Controller", func() {
310304
Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed())
311305
hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination
312306
Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
307+
308+
By("Setting initial offboarding condition")
309+
meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
310+
Type: kvmv1.ConditionTypeOffboarded,
311+
Status: metav1.ConditionFalse,
312+
Reason: "Offboarding",
313+
Message: "Hypervisor is being offboarded, removing host from nova",
314+
})
315+
Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
313316
})
314317

315318
Context("When getting hypervisor by name fails", func() {

‎internal/controller/onboarding_controller_test.go‎

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -282,12 +282,7 @@ var _ = Describe("Onboarding Controller", func() {
282282
Expect(err).NotTo(HaveOccurred())
283283

284284
Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed())
285-
Expect(hv.Status.Conditions).To(ContainElements(
286-
SatisfyAll(
287-
HaveField("Type", kvmv1.ConditionTypeReady),
288-
HaveField("Status", metav1.ConditionFalse),
289-
HaveField("Reason", kvmv1.ConditionReasonOnboarding),
290-
),
285+
Expect(hv.Status.Conditions).To(ContainElement(
291286
SatisfyAll(
292287
HaveField("Type", kvmv1.ConditionTypeOnboarding),
293288
HaveField("Status", metav1.ConditionTrue),
@@ -327,12 +322,7 @@ var _ = Describe("Onboarding Controller", func() {
327322
Expect(err).NotTo(HaveOccurred())
328323

329324
Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed())
330-
Expect(hv.Status.Conditions).To(ContainElements(
331-
SatisfyAll(
332-
HaveField("Type", kvmv1.ConditionTypeReady),
333-
HaveField("Status", metav1.ConditionFalse),
334-
HaveField("Reason", kvmv1.ConditionReasonOnboarding),
335-
),
325+
Expect(hv.Status.Conditions).To(ContainElement(
336326
SatisfyAll(
337327
HaveField("Type", kvmv1.ConditionTypeOnboarding),
338328
HaveField("Status", metav1.ConditionTrue),
@@ -355,11 +345,6 @@ var _ = Describe("Onboarding Controller", func() {
355345
Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed())
356346
hv.Status.HypervisorID = hypervisorId
357347
hv.Status.ServiceID = serviceId
358-
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
359-
Type: kvmv1.ConditionTypeReady,
360-
Status: metav1.ConditionFalse,
361-
Reason: kvmv1.ConditionReasonOnboarding,
362-
})
363348
meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
364349
Type: kvmv1.ConditionTypeOnboarding,
365350
Status: metav1.ConditionTrue,
@@ -503,11 +488,7 @@ var _ = Describe("Onboarding Controller", func() {
503488

504489
By("Verifying final state")
505490
Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed())
506-
Expect(hv.Status.Conditions).To(ContainElements(
507-
SatisfyAll(
508-
HaveField("Type", kvmv1.ConditionTypeReady),
509-
HaveField("Status", metav1.ConditionTrue),
510-
),
491+
Expect(hv.Status.Conditions).To(ContainElement(
511492
SatisfyAll(
512493
HaveField("Type", kvmv1.ConditionTypeOnboarding),
513494
HaveField("Status", metav1.ConditionFalse),
@@ -576,11 +557,7 @@ var _ = Describe("Onboarding Controller", func() {
576557
Expect(err).NotTo(HaveOccurred())
577558

578559
Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed())
579-
Expect(hv.Status.Conditions).To(ContainElements(
580-
SatisfyAll(
581-
HaveField("Type", kvmv1.ConditionTypeReady),
582-
HaveField("Status", metav1.ConditionTrue),
583-
),
560+
Expect(hv.Status.Conditions).To(ContainElement(
584561
SatisfyAll(
585562
HaveField("Type", kvmv1.ConditionTypeOnboarding),
586563
HaveField("Status", metav1.ConditionFalse),

0 commit comments

Comments
 (0)