diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index ef031870..600902f0 100644 --- a/internal/controller/traits_controller.go +++ b/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 { diff --git a/internal/controller/traits_controller_test.go b/internal/controller/traits_controller_test.go index ec37094e..72cfffbd 100644 --- a/internal/controller/traits_controller_test.go +++ b/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