Rely on spec.maintenance=termination instead of terminating

Main motivation is to let anyone set the hypervisor into termination,
and for that the spec should be the authorative source for the custom-resource.
That means, one can manually offboard the node now by setting that spec
too.

But also, the terminating condition on the node is not guaranteed to stay,
as it is part of the gardener contract. We should keep the use of that
to as few places as possible, so if that changes we do not have to rewrite
all our code.

Author: fwiesel

internal/controller/aggregates_controller.go

@@ -115,7 +115,7 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request)
 func (ac *AggregatesController) determineDesiredState(hv *kvmv1.Hypervisor) ([]string, metav1.Condition) {
 	// If terminating AND evicted, remove from all aggregates
 	// We must wait for eviction to complete before removing aggregates
-	if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
+	if hv.Spec.Maintenance == kvmv1.MaintenanceTermination {
 		evictingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting)
 		// Only remove aggregates if eviction is complete (Evicting=False)
 		// If Evicting condition is not set or still True, keep current aggregates

internal/controller/aggregates_controller_test.go

@@ -650,35 +650,86 @@ var _ = Describe("AggregatesController", func() {
 
 		Context("when terminating", func() {
 			BeforeEach(func(ctx SpecContext) {
-				By("Setting terminating condition")
+				By("Setting spec.maintenance=termination")
 				hypervisor := &kvmv1.Hypervisor{}
 				Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
-				meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
-					Type:    kvmv1.ConditionTypeTerminating,
-					Status:  metav1.ConditionTrue,
-					Reason:  "dontcare",
-					Message: "dontcare",
+				hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination
+				Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
+			})
+
+			Context("eviction not yet complete", func() {
+				BeforeEach(func(ctx SpecContext) {
+					By("Pre-setting the EvictionInProgress condition to match what controller will determine")
+					hypervisor := &kvmv1.Hypervisor{}
+					Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+					meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+						Type:    kvmv1.ConditionTypeAggregatesUpdated,
+						Status:  metav1.ConditionFalse,
+						Reason:  kvmv1.ConditionReasonEvictionInProgress,
+						Message: "Aggregates unchanged while terminating and eviction in progress",
+					})
+					Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
 				})
-				Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
 
-				By("Pre-setting the EvictionInProgress condition to match what controller will determine")
-				Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
-				meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
-					Type:    kvmv1.ConditionTypeAggregatesUpdated,
-					Status:  metav1.ConditionFalse,
-					Reason:  kvmv1.ConditionReasonEvictionInProgress,
-					Message: "Aggregates unchanged while terminating and eviction in progress",
+				It("should keep aggregates unchanged and set EvictionInProgress condition", func(ctx SpecContext) {
+					updated := &kvmv1.Hypervisor{}
+					Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
+					Expect(updated.Status.Aggregates).To(BeEmpty())
+					Expect(meta.IsStatusConditionFalse(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeTrue())
+					cond := meta.FindStatusCondition(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)
+					Expect(cond.Reason).To(Equal(kvmv1.ConditionReasonEvictionInProgress))
 				})
-				Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
 			})
 
-			It("should neither update Aggregates and nor set status condition", func(ctx SpecContext) {
-				updated := &kvmv1.Hypervisor{}
-				Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
-				Expect(updated.Status.Aggregates).To(BeEmpty())
-				Expect(meta.IsStatusConditionFalse(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeTrue())
-				cond := meta.FindStatusCondition(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)
-				Expect(cond.Reason).To(Equal(kvmv1.ConditionReasonEvictionInProgress))
+			Context("eviction complete (regression: aggregates must be cleared)", func() {
+				BeforeEach(func(ctx SpecContext) {
+					By("Setting Evicting=False to signal eviction is complete")
+					hypervisor := &kvmv1.Hypervisor{}
+					Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+					meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+						Type:    kvmv1.ConditionTypeEvicting,
+						Status:  metav1.ConditionFalse,
+						Reason:  kvmv1.ConditionReasonSucceeded,
+						Message: "Evicted",
+					})
+					hypervisor.Status.Aggregates = []kvmv1.Aggregate{
+						{Name: "staging", UUID: "uuid-staging"},
+						{Name: "qa-de-1b", UUID: "uuid-qa-de-1b"},
+					}
+					Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+
+					By("Mocking GetAggregates and RemoveHost calls")
+					fakeServer.Mux.HandleFunc("GET /os-aggregates", func(w http.ResponseWriter, r *http.Request) {
+						w.Header().Add("Content-Type", "application/json")
+						w.WriteHeader(http.StatusOK)
+						fmt.Fprint(w, `{
+							"aggregates": [
+								{"name": "staging", "id": 1, "uuid": "uuid-staging", "hosts": ["hv-test"]},
+								{"name": "qa-de-1b", "id": 2, "uuid": "uuid-qa-de-1b", "hosts": ["hv-test"]}
+							]
+						}`)
+					})
+					fakeServer.Mux.HandleFunc("POST /os-aggregates/1/action", func(w http.ResponseWriter, r *http.Request) {
+						w.Header().Add("Content-Type", "application/json")
+						w.WriteHeader(http.StatusOK)
+						fmt.Fprint(w, `{"aggregate": {"name": "staging", "id": 1, "uuid": "uuid-staging", "hosts": []}}`)
+					})
+					fakeServer.Mux.HandleFunc("POST /os-aggregates/2/action", func(w http.ResponseWriter, r *http.Request) {
+						w.Header().Add("Content-Type", "application/json")
+						w.WriteHeader(http.StatusOK)
+						fmt.Fprint(w, `{"aggregate": {"name": "qa-de-1b", "id": 2, "uuid": "uuid-qa-de-1b", "hosts": []}}`)
+					})
+				})
+
+				It("should remove all aggregates and set Terminating condition", func(ctx SpecContext) {
+					updated := &kvmv1.Hypervisor{}
+					Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
+					Expect(updated.Status.Aggregates).To(BeEmpty())
+					Expect(meta.IsStatusConditionFalse(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)).To(BeTrue())
+					cond := meta.FindStatusCondition(updated.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated)
+					Expect(cond).NotTo(BeNil())
+					Expect(cond.Reason).To(Equal(kvmv1.ConditionReasonTerminating))
+				})
 			})
 		})
 	})

internal/controller/onboarding_controller.go

@@ -92,7 +92,7 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	// check if hv is terminating
-	if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
+	if hv.Spec.Maintenance == kvmv1.MaintenanceTermination {
 		return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost)
 	}
 

internal/controller/traits_controller.go

@@ -69,7 +69,7 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 	}
 
-	if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
+	if hv.Spec.Maintenance == kvmv1.MaintenanceTermination {
 		return ctrl.Result{}, nil
 	}
 

internal/controller/traits_controller_test.go

@@ -280,17 +280,16 @@ var _ = Describe("TraitsController", func() {
 			})
 
 			hypervisor := &kvmv1.Hypervisor{}
+			Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+			hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination
+			Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
+
 			Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
 			meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
 				Type:   kvmv1.ConditionTypeOnboarding,
 				Status: metav1.ConditionFalse,
 				Reason: "UnitTest",
 			})
-			meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
-				Type:   kvmv1.ConditionTypeTerminating,
-				Status: metav1.ConditionTrue,
-				Reason: "UnitTest",
-			})
 			hypervisor.Status.Traits = []string{"CUSTOM_FOO", "HW_CPU_X86_VMX"}
 			hypervisor.Status.HypervisorID = "1234"
 			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())