diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index ef0c767..b6fda77 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -53,6 +53,8 @@ var errRequeue = errors.New("requeue requested") const ( defaultWaitTime = 1 * time.Minute ConditionTypeOnboarding = "Onboarding" + ConditionReasonAborted = "aborted" + ConditionReasonError = "error" ConditionReasonInitial = "initial" ConditionReasonOnboarding = "onboarding" ConditionReasonTesting = "testing" @@ -87,17 +89,18 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, k8sclient.IgnoreNotFound(err) } + computeHost := hv.Name + // check if lifecycle management is enabled if !hv.Spec.LifecycleEnabled { - return ctrl.Result{}, nil + return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost) } // check if hv is terminating if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) { - return ctrl.Result{}, nil + return ctrl.Result{}, r.abortOnboarding(ctx, hv, computeHost) } - computeHost := hv.Name // We bail here out, because the openstack api is not the best to poll if hv.Status.HypervisorID == "" || hv.Status.ServiceID == "" { if err := retry.RetryOnConflict(retry.DefaultRetry, func() error { @@ -144,6 +147,46 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) } } +func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, computeHost string) error { + status := meta.FindStatusCondition(hv.Status.Conditions, ConditionTypeOnboarding) + // Never onboarded + if status == nil { + return nil + } + + changed := meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: ConditionReasonOnboarding, + Message: "Onboarding aborted", + }) + + if meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: ConditionReasonAborted, + Message: "Aborted due to LifecycleEnabled being false", + }) { + changed = true + } + + if !changed { + // Already aborted + return nil + } + if err := r.deleteTestServers(ctx, computeHost); err != nil { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: ConditionTypeOnboarding, + Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" + Reason: ConditionReasonAborted, + Message: err.Error(), + }) + + return errors.Join(err, r.Status().Update(ctx, hv)) + } + return r.Status().Update(ctx, hv) +} + func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, host string) error { zone, found := hv.Labels[corev1.LabelTopologyZone] if !found || zone == "" { @@ -261,27 +304,18 @@ func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervis func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, hv *kvmv1.Hypervisor) (ctrl.Result, error) { log := logger.FromContext(ctx) - serverPrefix := fmt.Sprintf("%v-%v", testPrefixName, host) - - serverPages, err := servers.ListSimple(r.testComputeClient, servers.ListOpts{ - Name: serverPrefix, - }).AllPages(ctx) - - if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { - return ctrl.Result{}, err - } - - serverList, err := servers.ExtractServers(serverPages) + err := r.deleteTestServers(ctx, host) if err != nil { - return ctrl.Result{}, err - } - - for _, server := range serverList { - log.Info("deleting server", "name", server.Name) - err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr() - if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { - return ctrl.Result{}, err + if meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: ConditionTypeOnboarding, + Status: metav1.ConditionTrue, // No cleanup, so we are still "onboarding" + Reason: ConditionReasonAborted, + Message: err.Error(), + }) { + return ctrl.Result{}, errors.Join(err, r.Status().Update(ctx, hv)) } + + return ctrl.Result{}, err } aggs, err := aggregatesByName(ctx, r.computeClient) @@ -319,6 +353,35 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri return ctrl.Result{}, r.Status().Update(ctx, hv) } +func (r *OnboardingController) deleteTestServers(ctx context.Context, host string) error { + log := logger.FromContext(ctx) + serverPrefix := fmt.Sprintf("%v-%v", testPrefixName, host) + + serverPages, err := servers.ListSimple(r.testComputeClient, servers.ListOpts{ + Name: serverPrefix, + }).AllPages(ctx) + + if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { + return err + } + + serverList, err := servers.ExtractServers(serverPages) + if err != nil { + return err + } + + errs := make([]error, 0) + for _, server := range serverList { + log.Info("deleting server", "name", server.Name) + err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr() + if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { + errs = append(errs, err) + } + } + + return errors.Join(errs...) +} + func (r *OnboardingController) ensureNovaProperties(ctx context.Context, hv *kvmv1.Hypervisor) error { hypervisorAddress := hv.Labels[corev1.LabelHostname] if hypervisorAddress == "" { @@ -392,7 +455,7 @@ func (r *OnboardingController) createOrGetTestServer(ctx context.Context, zone, continue } if server.Name != serverName || foundServer != nil { - log.Info("deleting server", "name", server.Name) + log.Info("deleting outdated server", "name", server.Name) err = servers.Delete(ctx, r.testComputeClient, server.ID).ExtractErr() if err != nil && !gophercloud.ResponseCodeIs(err, http.StatusNotFound) { log.Error(err, "failed deleting instance due to", "instance", server.ID) diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index a33a43e..d3eb8ff 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/internal/controller/onboarding_controller_test.go @@ -31,19 +31,20 @@ import ( ) var _ = Describe("Onboarding Controller", func() { + const ( + hypervisorName = "some-test" + ) + var ( onboardingReconciler *OnboardingController + namespacedName = types.NamespacedName{Name: hypervisorName} ) Context("When reconciling a hypervisor", func() { - const hypervisorName = "some-test" - ctx := context.Background() //nolint:govet reconcileLoop := func(steps int) (res ctrl.Result, err error) { - req := ctrl.Request{ - NamespacedName: types.NamespacedName{Name: hypervisorName}, - } + req := ctrl.Request{NamespacedName: namespacedName} for range steps { res, err = onboardingReconciler.Reconcile(ctx, req) if err != nil { @@ -60,19 +61,18 @@ var _ = Describe("Onboarding Controller", func() { } By("creating the resource for the Kind Hypervisor") - resource := &kvmv1.Hypervisor{ + hv := &kvmv1.Hypervisor{ ObjectMeta: metav1.ObjectMeta{ Name: hypervisorName, }, Spec: kvmv1.HypervisorSpec{}, } - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - }) + Expect(k8sClient.Create(ctx, hv)).To(Succeed()) - AfterEach(func() { - hv := &kvmv1.Hypervisor{ObjectMeta: metav1.ObjectMeta{Name: hypervisorName}} - By("Cleanup the specific hypervisor CRO") - Expect(client.IgnoreAlreadyExists(k8sClient.Delete(ctx, hv))).To(Succeed()) + DeferCleanup(func(ctx context.Context) { + By("Cleanup the specific hypervisor CRO") + Expect(client.IgnoreAlreadyExists(k8sClient.Delete(ctx, hv))).To(Succeed()) + }) }) It("should successfully reconcile the resource", func() {