From 5a22a299331f27cd34b0e5c5388ae6067076f452 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Thu, 18 Apr 2024 07:25:55 -0400 Subject: [PATCH] Add finalizer on version Jira: OSPRH-6344 --- .../core/openstackcontrolplane_controller.go | 55 ++++++++++++++++--- .../core/openstackversion_controller.go | 5 +- tests/functional/base_test.go | 7 +++ .../openstackoperator_controller_test.go | 40 ++++++++++++++ .../openstackversion_controller_test.go | 8 ++- 5 files changed, 106 insertions(+), 9 deletions(-) diff --git a/controllers/core/openstackcontrolplane_controller.go b/controllers/core/openstackcontrolplane_controller.go index ec4ba0f17..b9efde702 100644 --- a/controllers/core/openstackcontrolplane_controller.go +++ b/controllers/core/openstackcontrolplane_controller.go @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, @@ -13,7 +13,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ - package core import ( @@ -31,7 +30,7 @@ import ( ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1" keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" - "github.com/openstack-k8s-operators/lib-common/modules/common/helper" + common_helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" corev1 "k8s.io/api/core/v1" designatev1 "github.com/openstack-k8s-operators/designate-operator/api/v1beta1" @@ -60,6 +59,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" @@ -135,7 +135,7 @@ func (r *OpenStackControlPlaneReconciler) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, err } - helper, err := helper.NewHelper( + helper, err := common_helper.NewHelper( instance, r.Client, r.Kclient, @@ -150,7 +150,8 @@ func (r *OpenStackControlPlaneReconciler) Reconcile(ctx context.Context, req ctr // // initialize Conditions // - if instance.Status.Conditions == nil { + isNewInstance := instance.Status.Conditions == nil + if isNewInstance { instance.Status.Conditions = condition.Conditions{} } @@ -185,6 +186,11 @@ func (r *OpenStackControlPlaneReconciler) Reconcile(ctx context.Context, req ctr instance.InitConditions() instance.Status.ObservedGeneration = instance.Generation + // If we're not deleting this and the service object doesn't have our finalizer, add it. + if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { + return ctrl.Result{}, nil + } + Log.Info("Looking up the current OpenStackVersion") ctrlResult, version, err := openstack.ReconcileVersion(ctx, instance, helper) if err != nil { @@ -193,6 +199,23 @@ func (r *OpenStackControlPlaneReconciler) Reconcile(ctx context.Context, req ctr return ctrlResult, nil } + versionHelper, err := common_helper.NewHelper( + version, + r.Client, + r.Kclient, + r.Scheme, + Log, + ) + if err != nil { + Log.Error(err, "unable to create helper") + return ctrl.Result{}, err + } + + // Handle version delete + if !instance.DeletionTimestamp.IsZero() { + return r.reconcileDelete(ctx, instance, version, helper, versionHelper) + } + // wait until the version is initialized so we have images on the version.Status if !version.Status.Conditions.IsTrue(corev1beta1.OpenStackVersionInitialized) { return ctrlResult, nil @@ -241,7 +264,7 @@ func (r *OpenStackControlPlaneReconciler) Reconcile(ctx context.Context, req ctr } -func (r *OpenStackControlPlaneReconciler) reconcileOVNControllers(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion, helper *helper.Helper) (ctrl.Result, error) { +func (r *OpenStackControlPlaneReconciler) reconcileOVNControllers(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion, helper *common_helper.Helper) (ctrl.Result, error) { OVNControllerReady, err := openstack.ReconcileOVNController(ctx, instance, version, helper) if err != nil { @@ -259,7 +282,7 @@ func (r *OpenStackControlPlaneReconciler) reconcileOVNControllers(ctx context.Co } -func (r *OpenStackControlPlaneReconciler) reconcileNormal(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion, helper *helper.Helper) (ctrl.Result, error) { +func (r *OpenStackControlPlaneReconciler) reconcileNormal(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion, helper *common_helper.Helper) (ctrl.Result, error) { ctrlResult, err := openstack.ReconcileCAs(ctx, instance, helper) if err != nil { @@ -418,6 +441,24 @@ func (r *OpenStackControlPlaneReconciler) reconcileNormal(ctx context.Context, i return ctrl.Result{}, nil } +func (r *OpenStackControlPlaneReconciler) reconcileDelete(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion, helper *common_helper.Helper, versionHelper *common_helper.Helper) (ctrl.Result, error) { + helper.GetLogger().Info("reconcile delete") + if controllerutil.RemoveFinalizer(version, versionHelper.GetFinalizer()) { + err := versionHelper.PatchInstance(ctx, version) + if err != nil { + return ctrl.Result{}, err + } + } + helper.GetLogger().Info(fmt.Sprintf("finalizer removed '%s' successfully", versionHelper.GetFinalizer())) + + // remove instance finalizer + controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) + helper.GetLogger().Info(fmt.Sprintf("finalizer removed '%s' successfully", helper.GetFinalizer())) + + return ctrl.Result{}, nil + +} + // fields to index to reconcile when change const ( passwordSecretField = ".spec.secret" diff --git a/controllers/core/openstackversion_controller.go b/controllers/core/openstackversion_controller.go index c47fb030e..04b844603 100644 --- a/controllers/core/openstackversion_controller.go +++ b/controllers/core/openstackversion_controller.go @@ -26,6 +26,7 @@ import ( "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -166,7 +167,9 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req instance.Status.Conditions.Init(&cl) instance.Status.ObservedGeneration = instance.Generation - if isNewInstance { + // If we're not deleting this and the service object doesn't have our finalizer, add it. + if instance.DeletionTimestamp.IsZero() && isNewInstance { + controllerutil.AddFinalizer(instance, versionHelper.GetFinalizer()) // Register overall status immediately to have an early feedback e.g. in the cli return ctrl.Result{}, nil } diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go index 2d5090515..6c50435d5 100644 --- a/tests/functional/base_test.go +++ b/tests/functional/base_test.go @@ -17,6 +17,7 @@ limitations under the License. package functional_test import ( + "context" "encoding/base64" . "github.com/onsi/gomega" //revive:disable:dot-imports @@ -249,6 +250,12 @@ func OpenStackVersionConditionGetter(name types.NamespacedName) condition.Condit return instance.Status.Conditions } +func OpenStackVersionRemoveFinalizer(ctx context.Context, name types.NamespacedName) error { + instance := GetOpenStackVersion(name) + instance.SetFinalizers([]string{}) + return th.K8sClient.Update(ctx, instance) +} + func CreateOpenStackControlPlane(name types.NamespacedName, spec map[string]interface{}) client.Object { raw := map[string]interface{}{ diff --git a/tests/functional/openstackoperator_controller_test.go b/tests/functional/openstackoperator_controller_test.go index ffb1b9aec..58ed2829c 100644 --- a/tests/functional/openstackoperator_controller_test.go +++ b/tests/functional/openstackoperator_controller_test.go @@ -1065,6 +1065,46 @@ var _ = Describe("OpenStackOperator controller", func() { }, timeout, interval).Should(Succeed()) }) }) + + When("OpenStackControlplane instance is deleted", func() { + BeforeEach(func() { + DeferCleanup( + th.DeleteInstance, + CreateOpenStackControlPlane(names.OpenStackControlplaneName, GetDefaultOpenStackControlPlaneSpec()), + ) + }) + + It("deletes the OpenStackVersion resource", func() { + + // openstackversion exists + Eventually(func(g Gomega) { + osversion := GetOpenStackVersion(names.OpenStackControlplaneName) + g.Expect(osversion).Should(Not(BeNil())) + g.Expect(osversion.OwnerReferences).Should(HaveLen(1)) + + th.ExpectCondition( + names.OpenStackControlplaneName, + ConditionGetterFunc(OpenStackVersionConditionGetter), + corev1.OpenStackVersionInitialized, + k8s_corev1.ConditionTrue, + ) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + ctlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) + g.Expect(ctlplane.Finalizers).Should(HaveLen(1)) + th.DeleteInstance(ctlplane) + }, timeout, interval).Should(Succeed()) + + // deleting the OpenStackControlPlane should remove the OpenStackVersion finalizer we are good + Eventually(func(g Gomega) { + osversion := GetOpenStackVersion(names.OpenStackControlplaneName) + g.Expect(osversion.Finalizers).Should(BeEmpty()) + }, timeout, interval).Should(Succeed()) + + }) + }) + }) var _ = Describe("OpenStackOperator Webhook", func() { diff --git a/tests/functional/openstackversion_controller_test.go b/tests/functional/openstackversion_controller_test.go index c0a8e6a7d..4fb9e259e 100644 --- a/tests/functional/openstackversion_controller_test.go +++ b/tests/functional/openstackversion_controller_test.go @@ -39,12 +39,18 @@ var _ = Describe("OpenStackOperator controller", func() { }) - When("A default OpenStackVersion instance is created", func() { + When("A default OpenStackVersion instance is created with no Controlplane", func() { BeforeEach(func() { DeferCleanup( th.DeleteInstance, CreateOpenStackVersion(names.OpenStackVersionName, GetDefaultOpenStackVersionSpec()), ) + // we remove the finalizer as this is needed without the Controlplane + DeferCleanup( + OpenStackVersionRemoveFinalizer, + ctx, + names.OpenStackVersionName, + ) }) It("should initialize container images", func() { Eventually(func(g Gomega) {