Set TraitsUpdated when the recorded traits match the spec

internal/controller/traits_controller.go

@@ -85,29 +85,30 @@ func (tc *TraitsController) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return ctrl.Result{}, nil
 	}
 
-	customTraitsApplied := slices.Collect(func(yield func(string) bool) {
-		for _, trait := range hv.Status.Traits {
-			if strings.HasPrefix(trait, customPrefix) && !yield(trait) {
-				return
-			}
-		}
-	})
+	statusCustomTraits := customTraitsOf(hv.Status.Traits)
 
-	if slices.Equal(hv.Spec.CustomTraits, customTraitsApplied) {
-		// Nothing to be done
-		return ctrl.Result{}, nil
+	if slicesEqualUnordered(hv.Spec.CustomTraits, statusCustomTraits) {
+		// Nothing to apply; only ensure the success condition is set,
+		// since it gates the handover phase.
+		if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated) {
+			return ctrl.Result{}, nil
+		}
+		return ctrl.Result{}, tc.applyTraitsStatus(ctx, hv, hv.Status.Traits,
+			getTraitCondition(nil, "Traits in sync"))
 	}
 
-	toAdd := utils.Difference(customTraitsApplied, hv.Spec.CustomTraits)
-	toRemove := utils.Difference(hv.Spec.CustomTraits, customTraitsApplied)
-
-	// fetch current traits, to ensure we don't add duplicates
+	// fetch the traits currently set in placement
 	current, err := resourceproviders.GetTraits(ctx, tc.serviceClient, hv.Status.HypervisorID).Extract()
 	if err != nil {
 		return ctrl.Result{}, errors.Join(err,
 			tc.applyTraitsStatus(ctx, hv, hv.Status.Traits, getTraitCondition(err, "Failed to get current traits from placement")))
 	}
 
+	// the applied set is what placement currently reports
+	appliedCustomTraits := customTraitsOf(current.Traits)
+	toAdd := utils.Difference(appliedCustomTraits, hv.Spec.CustomTraits)
+	toRemove := utils.Difference(hv.Spec.CustomTraits, appliedCustomTraits)
+
 	var targetTraits []string
 	slices.Sort(current.Traits)
 	for _, trait := range current.Traits {
@@ -185,6 +186,17 @@ func getTraitCondition(err error, msg string) metav1.Condition {
 	}
 }
 
+// customTraitsOf returns the subset of traits carrying the CUSTOM_ prefix.
+func customTraitsOf(traits []string) []string {
+	return slices.Collect(func(yield func(string) bool) {
+		for _, trait := range traits {
+			if strings.HasPrefix(trait, customPrefix) && !yield(trait) {
+				return
+			}
+		}
+	})
+}
+
 // registerWithManager registers the controller with the Manager without acquiring OpenStack clients.
 // This is useful for testing where clients are injected directly.
 func (tc *TraitsController) registerWithManager(mgr ctrl.Manager) error {

internal/controller/traits_controller_test.go

@@ -264,6 +264,85 @@ var _ = Describe("TraitsController", func() {
 		})
 	})
 
+	Context("Reconcile during onboarding Handover phase with no custom traits", 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())
+			hypervisor.Spec.CustomTraits = nil
+			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.ConditionTrue,
+				Reason: kvmv1.ConditionReasonHandover,
+			})
+			hypervisor.Status.HypervisorID = "1234"
+			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should set the TraitsUpdated condition without contacting placement", 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(BeEmpty())
+			Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated)).To(BeTrue())
+			Expect(meta.FindStatusCondition(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated).Reason).To(Equal(kvmv1.ConditionReasonSucceeded))
+		})
+	})
+
+	Context("Reconcile when the recorded custom traits already match the spec", 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.ConditionReasonHandover,
+			})
+			hypervisor.Status.HypervisorID = "1234"
+			hypervisor.Status.Traits = []string{"CUSTOM_BAR", "HW_CPU_X86_VMX", "CUSTOM_FOO"}
+			Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed())
+		})
+
+		It("should set the TraitsUpdated condition without contacting placement", 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(Equal([]string{"CUSTOM_BAR", "HW_CPU_X86_VMX", "CUSTOM_FOO"}))
+			Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated)).To(BeTrue())
+			Expect(meta.FindStatusCondition(updated.Status.Conditions, kvmv1.ConditionTypeTraitsUpdated).Reason).To(Equal(kvmv1.ConditionReasonSucceeded))
+		})
+	})
+
 	Context("Reconcile when GetTraits returns an error", func() {
 		BeforeEach(func(ctx SpecContext) {
 			// Mock resourceproviders.GetTraits to return a server error