diff --git a/api/v1/hypervisor_types.go b/api/v1/hypervisor_types.go index e67bbb0..7d9e18a 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 ba2763a..e3a3a71 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 a5d792e..5102a3b 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 b0a72eb..709aa1d 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 18222ca..0e4988a 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 d85f16f..260efa8 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 f44cb8a..54772db 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