From 79e81fb5e9036569d1177a3257b4156db6dd372d Mon Sep 17 00:00:00 2001 From: Bobbins228 Date: Thu, 20 Jul 2023 11:47:01 +0100 Subject: [PATCH] Updated Finalizer and moved removeString to Utils file --- controllers/appwrapper_controller.go | 47 +++++----------------------- controllers/machinepools.go | 9 +++--- controllers/utils.go | 11 +++++++ 3 files changed, 23 insertions(+), 44 deletions(-) diff --git a/controllers/appwrapper_controller.go b/controllers/appwrapper_controller.go index 684aa8c..57d7bf2 100644 --- 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 index 220e859..61b6c46 100644 --- a/controllers/machinepools.go +++ b/controllers/machinepools.go @@ -3,6 +3,9 @@ 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" @@ -10,8 +13,6 @@ import ( 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 index bdb5827..22e2d69 100644 --- 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 +}