Updated Finalizer and moved removeString to Utils file

project-codeflare · Jul 20, 2023 · 79e81fb · 79e81fb
1 parent 136e6d1
commit 79e81fb
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 44 deletions.
diff --git a/controllers/appwrapper_controller.go b/controllers/appwrapper_controller.go
@@ -56,7 +56,7 @@ const (
 	minResyncPeriod   = 10 * time.Minute
 	pullSecretKey     = ".dockerconfigjson"
 	pullSecretAuthKey = "cloud.openshift.com"
-	finalizerName     = "mcad.codeflare.dev/finalizer"
+	finalizerName     = "instascale.codeflare.dev/finalizer"
 )
 
 //+kubebuilder:rbac:groups=instascale.ibm.com.instascale.ibm.com,resources=appwrappers,verbs=get;list;watch;create;update;patch;delete
@@ -93,9 +93,7 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	if !useMachineSets && ocmClusterID == "" {
 		getOCMClusterID(r)
 	}
-	if !containsString(appwrapper.ObjectMeta.Finalizers, finalizerName) {
-		appwrapper.ObjectMeta.Finalizers = append(appwrapper.ObjectMeta.Finalizers, finalizerName)
-	}
+
 	//onAdd replacement
 	if appwrapper.Status.State == arbv1.AppWrapperStateEnqueued || appwrapper.Status.State == "" {
 		//scaledAppwrapper = append(scaledAppwrapper, aw.Name)
@@ -104,9 +102,6 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		if useMachineSets {
 			if r.canScaleMachineset(ctx, demandPerInstanceType) {
 				r.scaleUp(ctx, &appwrapper, demandPerInstanceType)
-				if !containsString(appwrapper.ObjectMeta.Finalizers, finalizerName) {
-					appwrapper.ObjectMeta.Finalizers = append(appwrapper.ObjectMeta.Finalizers, finalizerName)
-				}
 			} else {
 				klog.Infof("Cannot scale up replicas max replicas allowed is %v", maxScaleNodesAllowed)
 			}
@@ -120,52 +115,24 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	}
 
-	//onDelete replacement
+	// Checks for finalizer on appwrapper where its deletion timestamp != 0
 	if !appwrapper.ObjectMeta.DeletionTimestamp.IsZero() {
-		if containsString(appwrapper.ObjectMeta.Finalizers, finalizerName) {
+		if contains(appwrapper.ObjectMeta.Finalizers, finalizerName) {
 			if err := r.finalizeScalingDownMachines(ctx, &appwrapper); err != nil {
 				return ctrl.Result{}, err
 			}
 
-			// Remove the finalizer from the object's metadata
+			// Remove the finalizer from the AppWrapper's metadata
 			appwrapper.ObjectMeta.Finalizers = removeString(appwrapper.ObjectMeta.Finalizers, finalizerName)
 			if err := r.Update(ctx, &appwrapper); err != nil {
 				return ctrl.Result{}, err
 			}
-		} else {
-			// The object is being deleted but doesn't have our finalizer, so we add it
-			appwrapper.ObjectMeta.Finalizers = append(appwrapper.ObjectMeta.Finalizers, finalizerName)
-			if err := r.Update(ctx, &appwrapper); err != nil {
-				return ctrl.Result{}, err
-			}
+			return ctrl.Result{}, nil
 		}
-		return ctrl.Result{}, nil
 	}
-
 	return ctrl.Result{}, nil
 }
 
-// containsString checks if a string slice contains a given string
-func containsString(slice []string, str string) bool {
-	for _, s := range slice {
-		if s == str {
-			return true
-		}
-	}
-	return false
-}
-
-// removeString removes a string from a string slice
-func removeString(slice []string, str string) []string {
-	result := make([]string, 0, len(slice))
-	for _, s := range slice {
-		if s != str {
-			result = append(result, s)
-		}
-	}
-	return result
-}
-
 func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context, appwrapper *arbv1.AppWrapper) error {
 	if useMachineSets {
 		if reuse {
@@ -183,7 +150,7 @@ func (r *AppWrapperReconciler) finalizeScalingDownMachines(ctx context.Context,
 			r.scaleDown(ctx, appwrapper)
 		}
 	} else {
-		deleteMachinePool(appwrapper)
+		r.deleteMachinePool(ctx, appwrapper)
 	}
 	return nil
 }

diff --git a/controllers/machinepools.go b/controllers/machinepools.go
@@ -3,15 +3,16 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"os"
+	"strings"
+
 	ocmsdk "github.com/openshift-online/ocm-sdk-go"
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/openshift-online/ocm-sdk-go/logging"
 	configv1 "github.com/openshift/api/config/v1"
 	arbv1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog"
-	"os"
-	"strings"
 )
 
 func scaleMachinePool(aw *arbv1.AppWrapper, userRequestedInstanceType string, replicas int) {
@@ -52,7 +53,7 @@ func scaleMachinePool(aw *arbv1.AppWrapper, userRequestedInstanceType string, re
 	klog.Infof("Created MachinePool: %v", response)
 }
 
-func deleteMachinePool(aw *arbv1.AppWrapper) {
+func (r *AppWrapperReconciler) deleteMachinePool(ctx context.Context, aw *arbv1.AppWrapper) {
 
 	logger, err := ocmsdk.NewGoLoggerBuilder().
 		Debug(false).
@@ -77,7 +78,7 @@ func deleteMachinePool(aw *arbv1.AppWrapper) {
 	machinePoolsList.Range(func(index int, item *cmv1.MachinePool) bool {
 		id, _ := item.GetID()
 		if strings.Contains(id, aw.Name) {
-			targetMachinePool, err := connection.ClustersMgmt().V1().Clusters().Cluster(ocmClusterID).MachinePools().MachinePool(id).Delete().SendContext(context.Background())
+			targetMachinePool, err := connection.ClustersMgmt().V1().Clusters().Cluster(ocmClusterID).MachinePools().MachinePool(id).Delete().SendContext(ctx)
 			if err != nil {
 				klog.Infof("Error deleting target machinepool %v", targetMachinePool)
 			}

diff --git a/controllers/utils.go b/controllers/utils.go
@@ -135,3 +135,14 @@ func contains(s []string, str string) bool {
 
 	return false
 }
+
+// removeString removes a string from a string slice
+func removeString(slice []string, str string) []string {
+	result := make([]string, 0, len(slice))
+	for _, s := range slice {
+		if s != str {
+			result = append(result, s)
+		}
+	}
+	return result
+}