Add finalizer on version

Jira: OSPRH-6344

controllers/core/openstackcontrolplane_controller.go

@@ -5,15 +5,14 @@ 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,
 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,23 @@ 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")
+	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"

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
 	}

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{}{

tests/functional/openstackoperator_controller_test.go

@@ -1065,6 +1065,40 @@ 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()))
+
+				th.ExpectCondition(
+					names.OpenStackControlplaneName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionInitialized,
+					k8s_corev1.ConditionTrue,
+				)
+			}, timeout, interval).Should(Succeed())
+
+			th.DeleteInstance(GetOpenStackControlPlane(names.OpenStackControlplaneName))
+
+			instance := &corev1.OpenStackVersion{}
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, names.OpenStackControlplaneName, instance)).Should(BeNil()) //nolint:all
+			}, timeout, interval).Should(Succeed())
+
+		})
+	})
+
 })
 
 var _ = Describe("OpenStackOperator Webhook", func() {

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) {