Traits have to be applied before Aggregates

The aggregates will allow anyone to deploy to the hypervisor,
but the traits define what can (or cannot) be deployed too.
So, before allowing people to deploy by aggregates, we should
set the correct traits.

Author: fwiesel

api/v1/hypervisor_types.go

@@ -72,6 +72,7 @@ const (
 	ConditionReasonTestAggregates     = "TestAggregates"
 	ConditionReasonTerminating        = "Terminating"
 	ConditionReasonEvictionInProgress = "EvictionInProgress"
+	ConditionReasonWaitingForTraits   = "WaitingForTraits"
 )
 
 // HypervisorSpec defines the desired state of Hypervisor

internal/controller/aggregates_controller.go

@@ -153,8 +153,19 @@ func (ac *AggregatesController) determineDesiredState(hv *kvmv1.Hypervisor) ([]s
 			}
 		}
 
-		// If the onboarding is almost complete, it will wait (among other things) for this controller to switch to Spec.Aggregates
+		// If the onboarding is almost complete, it will wait (among other things) for this controller to switch to Spec.Aggregates.
+		// We wait for traits to be applied first to ensure sequential ordering: Traits → Aggregates.
 		if onboardingCondition.Reason == kvmv1.ConditionReasonHandover {
+			if !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated) {
+				// Traits not yet applied — keep test aggregates and signal we're waiting
+				zone := hv.Labels[corev1.LabelTopologyZone]
+				return []string{zone, testAggregateName}, metav1.Condition{
+					Type:    kvmv1.ConditionTypeAggregatesUpdated,
+					Status:  metav1.ConditionFalse,
+					Reason:  kvmv1.ConditionReasonWaitingForTraits,
+					Message: "Waiting for traits to be applied before switching to spec aggregates",
+				}
+			}
 			return hv.Spec.Aggregates, metav1.Condition{
 				Type:    kvmv1.ConditionTypeAggregatesUpdated,
 				Status:  metav1.ConditionTrue,

internal/controller/aggregates_controller_test.go

@@ -239,6 +239,162 @@ var _ = Describe("AggregatesController", func() {
 			})
 		})
 
+		Context("During onboarding Handover phase with traits not ready", func() {
+			BeforeEach(func(ctx SpecContext) {
+				By("Setting onboarding condition to Handover without TraitsUpdated")
+				hypervisor := &kvmv1.Hypervisor{}
+				Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+				meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+					Type:    kvmv1.ConditionTypeOnboarding,
+					Status:  metav1.ConditionTrue,
+					Reason:  kvmv1.ConditionReasonHandover,
+					Message: "Waiting for other controllers to take over",
+				})
+				Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+
+				By("Setting desired aggregates")
+				Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+				hypervisor.Spec.Aggregates = []string{"zone-a", "prod-agg"}
+				Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
+
+				By("Mocking GetAggregates to return aggregates without host")
+				aggregateList := `{
+					"aggregates": [
+						{
+							"name": "zone-a",
+							"availability_zone": "zone-a",
+							"deleted": false,
+							"id": 1,
+							"uuid": "uuid-zone-a",
+							"hosts": []
+						},
+						{
+							"name": "tenant_filter_tests",
+							"availability_zone": "",
+							"deleted": false,
+							"id": 99,
+							"uuid": "uuid-test",
+							"hosts": []
+						}
+					]
+				}`
+				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, aggregateList)
+				})
+
+				By("Mocking AddHost for zone and test aggregates")
+				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": "zone-a", "id": 1, "uuid": "uuid-zone-a", "hosts": ["hv-test"]}}`)
+				})
+				fakeServer.Mux.HandleFunc("POST /os-aggregates/99/action", func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Add("Content-Type", "application/json")
+					w.WriteHeader(http.StatusOK)
+					fmt.Fprint(w, `{"aggregate": {"name": "tenant_filter_tests", "id": 99, "uuid": "uuid-test", "hosts": ["hv-test"]}}`)
+				})
+			})
+
+			It("should keep test aggregates and set WaitingForTraits condition", func(ctx SpecContext) {
+				updated := &kvmv1.Hypervisor{}
+				Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
+
+				aggregateNames := make([]string, len(updated.Status.Aggregates))
+				for i, agg := range updated.Status.Aggregates {
+					aggregateNames[i] = agg.Name
+				}
+
+				Expect(aggregateNames).To(ConsistOf("zone-a", testAggregateName))
+				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.ConditionReasonWaitingForTraits))
+			})
+		})
+
+		Context("During onboarding Handover phase with traits ready", func() {
+			BeforeEach(func(ctx SpecContext) {
+				By("Setting onboarding condition to Handover with TraitsUpdated=True")
+				hypervisor := &kvmv1.Hypervisor{}
+				Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+				meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+					Type:    kvmv1.ConditionTypeOnboarding,
+					Status:  metav1.ConditionTrue,
+					Reason:  kvmv1.ConditionReasonHandover,
+					Message: "Waiting for other controllers to take over",
+				})
+				meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+					Type:    kvmv1.ConditionTypeTraitsUpdated,
+					Status:  metav1.ConditionTrue,
+					Reason:  kvmv1.ConditionReasonSucceeded,
+					Message: "Traits updated successfully",
+				})
+				Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+
+				By("Setting desired aggregates")
+				Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+				hypervisor.Spec.Aggregates = []string{"zone-a", "prod-agg"}
+				Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed())
+
+				By("Mocking GetAggregates")
+				aggregateList := `{
+					"aggregates": [
+						{
+							"name": "zone-a",
+							"availability_zone": "zone-a",
+							"deleted": false,
+							"id": 1,
+							"uuid": "uuid-zone-a",
+							"hosts": []
+						},
+						{
+							"name": "prod-agg",
+							"availability_zone": "",
+							"deleted": false,
+							"id": 2,
+							"uuid": "uuid-prod-agg",
+							"hosts": []
+						}
+					]
+				}`
+				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, aggregateList)
+				})
+
+				By("Mocking AddHost for both spec aggregates")
+				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": "zone-a", "id": 1, "uuid": "uuid-zone-a", "hosts": ["hv-test"]}}`)
+				})
+				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": "prod-agg", "id": 2, "uuid": "uuid-prod-agg", "hosts": ["hv-test"]}}`)
+				})
+			})
+
+			It("should apply spec aggregates and set Succeeded condition", func(ctx SpecContext) {
+				updated := &kvmv1.Hypervisor{}
+				Expect(aggregatesController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
+
+				aggregateNames := make([]string, len(updated.Status.Aggregates))
+				for i, agg := range updated.Status.Aggregates {
+					aggregateNames[i] = agg.Name
+				}
+
+				Expect(aggregateNames).To(ConsistOf("zone-a", "prod-agg"))
+				Expect(meta.IsStatusConditionTrue(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.ConditionReasonSucceeded))
+			})
+		})
+
 		Context("During normal operations", func() {
 			BeforeEach(func(ctx SpecContext) {
 				By("Setting desired aggregates")

internal/controller/onboarding_controller.go

@@ -315,13 +315,17 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri
 	// Check if we're in the RemovingTestAggregate phase
 	onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding)
 	if onboardingCondition != nil && onboardingCondition.Reason == kvmv1.ConditionReasonHandover {
-		// We're waiting for aggregates controller to sync
+		// We're waiting for aggregates and traits controllers to sync
 		if !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeAggregatesUpdated) {
 			log.Info("waiting for aggregates to be updated", "condition", kvmv1.ConditionTypeAggregatesUpdated)
 			return ctrl.Result{RequeueAfter: defaultWaitTime}, nil
 		}
+		if !meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated) {
+			log.Info("waiting for traits to be updated", "condition", kvmv1.ConditionTypeTraitsUpdated)
+			return ctrl.Result{RequeueAfter: defaultWaitTime}, nil
+		}
 
-		// Aggregates have been synced, mark onboarding as complete
+		// Aggregates and traits have been synced, mark onboarding as complete
 		log.Info("aggregates updated successfully", "aggregates", hv.Status.Aggregates)
 		base := hv.DeepCopy()
 

internal/controller/onboarding_controller_test.go

@@ -466,6 +466,12 @@ var _ = Describe("Onboarding Controller", func() {
 					Reason:  kvmv1.ConditionReasonSucceeded,
 					Message: "Aggregates updated successfully",
 				})
+				meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
+					Type:    kvmv1.ConditionTypeTraitsUpdated,
+					Status:  metav1.ConditionTrue,
+					Reason:  kvmv1.ConditionReasonSucceeded,
+					Message: "Traits updated successfully",
+				})
 				Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed())
 
 				By("Reconciling once more to complete onboarding and set Ready")
@@ -511,7 +517,7 @@ var _ = Describe("Onboarding Controller", func() {
 				err = reconcileLoop(ctx, 1)
 				Expect(err).NotTo(HaveOccurred())
 
-				By("Simulating aggregates controller setting condition after removing test aggregate")
+				By("Simulating aggregates and traits controllers setting conditions after removing test aggregate")
 				hv := &kvmv1.Hypervisor{}
 				Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed())
 				hv.Status.Aggregates = []kvmv1.Aggregate{
@@ -523,9 +529,15 @@ var _ = Describe("Onboarding Controller", func() {
 					Reason:  kvmv1.ConditionReasonSucceeded,
 					Message: "Aggregates updated successfully",
 				})
+				meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
+					Type:    kvmv1.ConditionTypeTraitsUpdated,
+					Status:  metav1.ConditionTrue,
+					Reason:  kvmv1.ConditionReasonSucceeded,
+					Message: "Traits updated successfully",
+				})
 				Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed())
 
-				By("Reconciling to complete onboarding after aggregates condition is set")
+				By("Reconciling to complete onboarding after aggregates and traits conditions are set")
 				err = reconcileLoop(ctx, 5)
 				Expect(err).NotTo(HaveOccurred())
 

internal/controller/traits_controller.go

@@ -69,8 +69,18 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 	}
 
-	if !meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) ||
-		meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
+	if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) {
+		return ctrl.Result{}, nil
+	}
+
+	// Only run when onboarding is complete (False) or in Handover phase
+	onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding)
+	if onboardingCondition == nil {
+		// Onboarding hasn't started yet
+		return ctrl.Result{}, nil
+	}
+	if onboardingCondition.Status == metav1.ConditionTrue && onboardingCondition.Reason != kvmv1.ConditionReasonHandover {
+		// Onboarding is in progress (Initial/Testing) — not yet at Handover
 		return ctrl.Result{}, nil
 	}
 

internal/controller/traits_controller_test.go

@@ -154,7 +154,7 @@ var _ = Describe("TraitsController", func() {
 		})
 	})
 
-	Context("Reconcile before onboarding", func() {
+	Context("Reconcile before onboarding (no condition set)", func() {
 		BeforeEach(func(ctx SpecContext) {
 			// Mock resourceproviders.GetTraits
 			fakeServer.Mux.HandleFunc("GET /resource_providers/1234/traits", func(w http.ResponseWriter, r *http.Request) {
@@ -186,6 +186,86 @@ var _ = Describe("TraitsController", func() {
 		})
 	})
 
+	Context("Reconcile during onboarding Initial phase", func() {
+		BeforeEach(func(ctx SpecContext) {
+			// Mock resourceproviders.GetTraits
+			fakeServer.Mux.HandleFunc("GET /resource_providers/1234/traits", func(w http.ResponseWriter, r *http.Request) {
+				defer GinkgoRecover()
+				Fail("should not be called")
+			})
+			// Mock resourceproviders.UpdateTraits
+			fakeServer.Mux.HandleFunc("PUT /resource_providers/1234/traits", func(w http.ResponseWriter, r *http.Request) {
+				defer GinkgoRecover()
+				Fail("should not be called")
+			})
+
+			hypervisor := &kvmv1.Hypervisor{}
+			Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+			meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+				Type:   kvmv1.ConditionTypeOnboarding,
+				Status: metav1.ConditionTrue,
+				Reason: kvmv1.ConditionReasonInitial,
+			})
+			hypervisor.Status.HypervisorID = "1234"
+			hypervisor.Status.Traits = []string{"CUSTOM_FOO", "HW_CPU_X86_VMX"}
+			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should not update traits during Initial phase", func(ctx SpecContext) {
+			req := ctrl.Request{NamespacedName: hypervisorName}
+			_, err := traitsController.Reconcile(ctx, req)
+			Expect(err).NotTo(HaveOccurred())
+
+			updated := &kvmv1.Hypervisor{}
+			Expect(traitsController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
+			Expect(updated.Status.Traits).NotTo(ContainElements("CUSTOM_FOO", "CUSTOM_BAR", "HW_CPU_X86_VMX"))
+			Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated)).To(BeFalse())
+		})
+	})
+
+	Context("Reconcile during onboarding Handover phase", func() {
+		BeforeEach(func(ctx SpecContext) {
+			// Mock resourceproviders.GetTraits
+			fakeServer.Mux.HandleFunc("GET /resource_providers/1234/traits", func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Add("Content-Type", "application/json")
+				w.WriteHeader(http.StatusOK)
+
+				_, err := fmt.Fprint(w, TraitsBody)
+				Expect(err).NotTo(HaveOccurred())
+			})
+			// Mock resourceproviders.UpdateTraits
+			fakeServer.Mux.HandleFunc("PUT /resource_providers/1234/traits", func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Add("Content-Type", "application/json")
+				w.WriteHeader(http.StatusOK)
+
+				_, err := fmt.Fprint(w, TraitsBodyUpdated)
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			hypervisor := &kvmv1.Hypervisor{}
+			Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed())
+			meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{
+				Type:   kvmv1.ConditionTypeOnboarding,
+				Status: metav1.ConditionTrue,
+				Reason: kvmv1.ConditionReasonHandover,
+			})
+			hypervisor.Status.HypervisorID = "1234"
+			hypervisor.Status.Traits = []string{"CUSTOM_FOO", "HW_CPU_X86_VMX"}
+			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should update traits during Handover phase", func(ctx SpecContext) {
+			req := ctrl.Request{NamespacedName: hypervisorName}
+			_, err := traitsController.Reconcile(ctx, req)
+			Expect(err).NotTo(HaveOccurred())
+
+			updated := &kvmv1.Hypervisor{}
+			Expect(traitsController.Client.Get(ctx, hypervisorName, updated)).To(Succeed())
+			Expect(updated.Status.Traits).To(ContainElements("CUSTOM_FOO", "CUSTOM_BAR", "HW_CPU_X86_VMX"))
+			Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated)).To(BeTrue())
+		})
+	})
+
 	Context("Reconcile when terminating", func() {
 		BeforeEach(func(ctx SpecContext) {
 			// Mock resourceproviders.GetTraits