From f800ea05edc739c080357bdc01c40f6fe9421853 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Tue, 10 Mar 2026 18:45:17 +0100 Subject: [PATCH] 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. --- api/v1/hypervisor_types.go | 1 + internal/controller/aggregates_controller.go | 13 +- .../controller/aggregates_controller_test.go | 156 ++++++++++++++++++ internal/controller/onboarding_controller.go | 8 +- .../controller/onboarding_controller_test.go | 16 +- internal/controller/traits_controller.go | 14 +- internal/controller/traits_controller_test.go | 82 ++++++++- 7 files changed, 282 insertions(+), 8 deletions(-) diff --git a/api/v1/hypervisor_types.go b/api/v1/hypervisor_types.go index e67bbb04..7d9e18a9 100644 --- a/api/v1/hypervisor_types.go +++ b/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 diff --git a/internal/controller/aggregates_controller.go b/internal/controller/aggregates_controller.go index ba2763aa..e3a3a71e 100644 --- a/internal/controller/aggregates_controller.go +++ b/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, diff --git a/internal/controller/aggregates_controller_test.go b/internal/controller/aggregates_controller_test.go index a5d792e3..5102a3bc 100644 --- a/internal/controller/aggregates_controller_test.go +++ b/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") diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index b0a72eb8..709aa1dd 100644 --- a/internal/controller/onboarding_controller.go +++ b/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() diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index 18222cae..0e4988a1 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/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()) diff --git a/internal/controller/traits_controller.go b/internal/controller/traits_controller.go index d85f16fd..260efa85 100644 --- a/internal/controller/traits_controller.go +++ b/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 } diff --git a/internal/controller/traits_controller_test.go b/internal/controller/traits_controller_test.go index f44cb8ac..54772db3 100644 --- a/internal/controller/traits_controller_test.go +++ b/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