diff --git a/internal/controller/offboarding_controller_test.go b/internal/controller/offboarding_controller_test.go index beab9eca..b3b4465a 100644 --- a/internal/controller/offboarding_controller_test.go +++ b/internal/controller/offboarding_controller_test.go @@ -109,6 +109,12 @@ var _ = Describe("Offboarding Controller", func() { Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) By("Setting terminating condition") + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + Message: "Onboarding completed", + }) meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeTerminating, Status: metav1.ConditionTrue, diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index 3c03246f..38ebdfb9 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -153,6 +153,12 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy return nil } + // Successful onboarding is terminal. Maintenance and offboarding track the + // host's subsequent lifecycle without erasing its successful onboarding. + if status.Reason == kvmv1.ConditionReasonSucceeded { + return nil + } + base := hv.DeepCopy() meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index 1cc36d9b..b1d8b347 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/internal/controller/onboarding_controller_test.go @@ -234,6 +234,82 @@ var _ = Describe("Onboarding Controller", func() { }) }) + Context("aborting onboarding", func() { + DescribeTable("preserves successful onboarding without OpenStack cleanup", + func(ctx SpecContext, maintenance string, lifecycleEnabled bool) { + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + hv.Spec.Maintenance = maintenance + hv.Spec.LifecycleEnabled = lifecycleEnabled + Expect(k8sClient.Update(ctx, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + Message: "Onboarding completed", + LastTransitionTime: metav1.NewTime(time.Date(2026, time.September, 21, 12, 59, 6, 0, time.UTC)), + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + originalStatus := hv.Status.DeepCopy() + originalResourceVersion := hv.ResourceVersion + + // The default mock handler fails any OpenStack request, including cleanup. + Expect(reconcileLoop(ctx, 2)).To(Succeed()) + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + Expect(hv.Status).To(Equal(*originalStatus)) + Expect(hv.ResourceVersion).To(Equal(originalResourceVersion)) + }, + Entry("when termination begins", kvmv1.MaintenanceTermination, true), + Entry("when lifecycle management is disabled", kvmv1.MaintenanceUnset, false), + ) + + DescribeTable("cleans up test servers and aborts incomplete onboarding", + func(ctx SpecContext, reason, maintenance string, lifecycleEnabled bool, message string) { + listRequests, deleteRequests := 0, 0 + fakeServer.Mux.HandleFunc("GET /servers", func(w http.ResponseWriter, r *http.Request) { + listRequests++ + Expect(r.URL.Query().Get("name")).To(Equal(testPrefixName + "-" + hypervisorName)) + w.Header().Set("Content-Type", "application/json") + _, err := fmt.Fprint(w, `{"servers": [{"id": "test-server-id", "name": "ohooc-test-host"}]}`) + Expect(err).NotTo(HaveOccurred()) + }) + fakeServer.Mux.HandleFunc("DELETE /servers/test-server-id", func(w http.ResponseWriter, _ *http.Request) { + deleteRequests++ + w.WriteHeader(http.StatusAccepted) + }) + + hv := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + hv.Spec.Maintenance = maintenance + hv.Spec.LifecycleEnabled = lifecycleEnabled + Expect(k8sClient.Update(ctx, hv)).To(Succeed()) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: reason, + Message: "Onboarding in progress", + }) + Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) + + Expect(reconcileLoop(ctx, 2)).To(Succeed()) + Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) + condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonAborted)) + Expect(condition.Message).To(Equal(message)) + Expect(listRequests).To(Equal(1)) + Expect(deleteRequests).To(Equal(1)) + }, + Entry("Initial during termination", kvmv1.ConditionReasonInitial, kvmv1.MaintenanceTermination, true, "Aborted due to MaintenanceTermination"), + Entry("Testing during termination", kvmv1.ConditionReasonTesting, kvmv1.MaintenanceTermination, true, "Aborted due to MaintenanceTermination"), + Entry("Handover during termination", kvmv1.ConditionReasonHandover, kvmv1.MaintenanceTermination, true, "Aborted due to MaintenanceTermination"), + Entry("Initial with lifecycle disabled", kvmv1.ConditionReasonInitial, kvmv1.MaintenanceUnset, false, "Aborted due to LifecycleEnabled being false"), + Entry("Testing with lifecycle disabled", kvmv1.ConditionReasonTesting, kvmv1.MaintenanceUnset, false, "Aborted due to LifecycleEnabled being false"), + Entry("Handover with lifecycle disabled", kvmv1.ConditionReasonHandover, kvmv1.MaintenanceUnset, false, "Aborted due to LifecycleEnabled being false"), + ) + }) + Context("initial setup of a new hypervisor", func() { BeforeEach(func() { fakeServer.Mux.HandleFunc("GET /os-hypervisors/detail", func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/controller/ready/controller_test.go b/internal/controller/ready/controller_test.go index 7e8783e9..4b5586e0 100644 --- a/internal/controller/ready/controller_test.go +++ b/internal/controller/ready/controller_test.go @@ -344,6 +344,33 @@ var _ = Describe("ComputeReadyCondition", func() { }) Context("Priority 6: All checks pass - Ready", func() { + DescribeTable("preserves readiness semantics after successful onboarding", + func(lifecycleEnabled bool, maintenance, conditionType string, expectedStatus metav1.ConditionStatus, expectedReason string) { + hv.Spec.LifecycleEnabled = lifecycleEnabled + hv.Spec.Maintenance = maintenance + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + }) + if conditionType != "" { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionTrue, + }) + } + + result := ComputeReadyCondition(hv) + Expect(result.Status).To(Equal(expectedStatus)) + Expect(result.Reason).To(Equal(expectedReason)) + }, + Entry("lifecycle disabled does not invalidate readiness", false, kvmv1.MaintenanceUnset, "", metav1.ConditionTrue, kvmv1.ConditionReasonReadyReady), + Entry("termination maintenance remains not ready", true, kvmv1.MaintenanceTermination, "", metav1.ConditionFalse, kvmv1.ConditionReasonReadyMaintenance), + Entry("manual maintenance remains not ready", true, kvmv1.MaintenanceManual, "", metav1.ConditionFalse, kvmv1.ConditionReasonReadyMaintenance), + Entry("terminating condition takes precedence", true, kvmv1.MaintenanceUnset, kvmv1.ConditionTypeTerminating, metav1.ConditionFalse, kvmv1.ConditionReasonTerminating), + Entry("offboarded condition takes precedence", true, kvmv1.MaintenanceUnset, kvmv1.ConditionTypeOffboarded, metav1.ConditionFalse, "Offboarded"), + ) + It("should return Ready=True when all conditions are satisfied", func() { meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding,