adding sriov operator config finalizer, to control generated cluster …

…level objects cleanup Signed-off-by: Ido Heyvi <[email protected]>
k8snetworkplumbingwg · Oct 29, 2024 · 0d9a707 · 0d9a707
1 parent 09a3af9
commit 0d9a707
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 8 deletions.
diff --git a/api/v1/helper.go b/api/v1/helper.go
@@ -26,11 +26,12 @@ import (
 )
 
 const (
-	LASTNETWORKNAMESPACE    = "operator.sriovnetwork.openshift.io/last-network-namespace"
-	NETATTDEFFINALIZERNAME  = "netattdef.finalizers.sriovnetwork.openshift.io"
-	POOLCONFIGFINALIZERNAME = "poolconfig.finalizers.sriovnetwork.openshift.io"
-	ESwithModeLegacy        = "legacy"
-	ESwithModeSwitchDev     = "switchdev"
+	LASTNETWORKNAMESPACE        = "operator.sriovnetwork.openshift.io/last-network-namespace"
+	NETATTDEFFINALIZERNAME      = "netattdef.finalizers.sriovnetwork.openshift.io"
+	POOLCONFIGFINALIZERNAME     = "poolconfig.finalizers.sriovnetwork.openshift.io"
+	OPERATORCONFIGFINALIZERNAME = "operatorconfig.finalizers.sriovnetwork.openshift.io"
+	ESwithModeLegacy            = "legacy"
+	ESwithModeSwitchDev         = "switchdev"
 
 	SriovCniStateEnable  = "enable"
 	SriovCniStateDisable = "disable"

diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go
@@ -40,6 +40,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/go-logr/logr"
 	machinev1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
 
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
@@ -83,8 +84,6 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			logger.Info("default SriovOperatorConfig object not found. waiting for creation.")
-
-			err := r.deleteAllWebhooks(ctx)
 			return reconcile.Result{}, err
 		}
 		// Error reading the object - requeue the request.
@@ -94,6 +93,19 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 
 	snolog.SetLogLevel(defaultConfig.Spec.LogLevel)
 
+	// examine DeletionTimestamp to determine if object is under deletion
+	if !defaultConfig.ObjectMeta.DeletionTimestamp.IsZero() {
+		// The object is being deleted
+		return r.handleSriovOperatorConfigDeletion(ctx, defaultConfig, logger)
+	}
+	// add finalizer if needed
+	if !sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) {
+		defaultConfig.ObjectMeta.Finalizers = append(defaultConfig.ObjectMeta.Finalizers, sriovnetworkv1.OPERATORCONFIGFINALIZERNAME)
+		if err := r.Update(ctx, defaultConfig); err != nil {
+			return reconcile.Result{}, err
+		}
+	}
+
 	r.FeatureGate.Init(defaultConfig.Spec.FeatureGates)
 	logger.Info("enabled featureGates", "featureGates", r.FeatureGate.String())
 
@@ -434,6 +446,27 @@ func (r *SriovOperatorConfigReconciler) syncOpenShiftSystemdService(ctx context.
 	return r.setLabelInsideObject(ctx, cr, objs)
 }
 
+func (r *SriovOperatorConfigReconciler) handleSriovOperatorConfigDeletion(ctx context.Context,
+	defaultConfig *sriovnetworkv1.SriovOperatorConfig, logger logr.Logger) (ctrl.Result, error) {
+	var err error
+	if sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) {
+		// our finalizer is present, so lets handle any external dependency
+		logger.Info("delete SriovOperatorConfig CR", "Namespace", defaultConfig.Namespace, "Name", defaultConfig.Name)
+		// make sure webhooks objects are deleted prior of removing finalizer
+		err = r.deleteAllWebhooks(ctx)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+		// remove our finalizer from the list and update it.
+		defaultConfig.ObjectMeta.Finalizers, _ = sriovnetworkv1.RemoveString(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers)
+		if err := r.Update(ctx, defaultConfig); err != nil {
+			return reconcile.Result{}, err
+		}
+	}
+
+	return reconcile.Result{}, err
+}
+
 func (r SriovOperatorConfigReconciler) setLabelInsideObject(ctx context.Context, cr *sriovnetworkv1.SriovOperatorConfig, objs []*uns.Unstructured) error {
 	logger := log.Log.WithName("setLabelInsideObject")
 	for _, obj := range objs {

diff --git a/controllers/sriovoperatorconfig_controller_test.go b/controllers/sriovoperatorconfig_controller_test.go
@@ -102,9 +102,15 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 
 	Context("When is up", func() {
 		BeforeEach(func() {
+			var err error
 			config := &sriovnetworkv1.SriovOperatorConfig{}
-			err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
+			err = util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
 			Expect(err).NotTo(HaveOccurred())
+			// in case controller yet to add object's finalizer (e.g whenever test deferCleanup is creating new 'default' config object)
+			if len(config.Finalizers) == 0 {
+				err = util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
+				Expect(err).NotTo(HaveOccurred())
+			}
 			config.Spec = sriovnetworkv1.SriovOperatorConfigSpec{
 				EnableInjector:        true,
 				EnableOperatorWebhook: true,
@@ -240,6 +246,38 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 				client.ObjectKey{Name: "network-resources-injector-config"})
 		})
 
+		It("should add/delete finalizer 'operatorconfig' when SriovOperatorConfig/default is added/deleted", func() {
+			DeferCleanup(k8sClient.Create, context.Background(), makeDefaultSriovOpConfig())
+
+			// verify that finalizer has been added upon object creation
+			config := &sriovnetworkv1.SriovOperatorConfig{}
+			Eventually(func() []string {
+				// wait for SriovOperatorConfig flags to get updated
+				err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "default", Namespace: testNamespace}, config)
+				if err != nil {
+					return nil
+				}
+				return config.Finalizers
+			}, util.APITimeout, util.RetryInterval).Should(Equal([]string{sriovnetworkv1.OPERATORCONFIGFINALIZERNAME}))
+
+			err := k8sClient.Delete(context.Background(), &sriovnetworkv1.SriovOperatorConfig{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "default"},
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// verify that finalizer has been removed
+			var empty []string
+			config = &sriovnetworkv1.SriovOperatorConfig{}
+			Eventually(func() []string {
+				// wait for SriovOperatorConfig flags to get updated
+				err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "default", Namespace: testNamespace}, config)
+				if err != nil {
+					return nil
+				}
+				return config.Finalizers
+			}, util.APITimeout, util.RetryInterval).Should(Equal(empty))
+		})
+
 		It("should be able to update the node selector of sriov-network-config-daemon", func() {
 			By("specify the configDaemonNodeSelector")
 			nodeSelector := map[string]string{"node-role.kubernetes.io/worker": ""}