Application: refactor finalizer logic

Author: tkan145

controllers/capabilities/application_controller.go

@@ -79,6 +79,26 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 		reqLogger.V(1).Info(string(jsonData))
 	}
+
+	// Ignore deleted Applications, this can happen when foregroundDeletion is enabled
+	// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
+	if application.GetDeletionTimestamp() != nil {
+		if controllerutil.ContainsFinalizer(application, applicationFinalizer) {
+			err = r.removeApplicationFrom3scale(application)
+			if err != nil {
+				r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to delete application", "%v", err)
+				return ctrl.Result{}, err
+			}
+
+			if controllerutil.RemoveFinalizer(application, applicationFinalizer) {
+				if err = r.UpdateResource(application); err != nil {
+					return ctrl.Result{}, err
+				}
+			}
+		}
+		return ctrl.Result{}, nil
+	}
+
 	// get Account
 	accountResource := &capabilitiesv1beta1.DeveloperAccount{}
 	projectMeta := types.NamespacedName{
@@ -89,25 +109,11 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	err = r.Client().Get(r.Context(), projectMeta, accountResource)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			// If the application CR is marked for deletion and the dev account associated doesn't exist, remove the application CR since there is nothing else to do with application.
-			if application.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(application, applicationFinalizer) {
-				controllerutil.RemoveFinalizer(application, applicationFinalizer)
-				err = r.UpdateResource(application)
-				if err != nil {
-					return ctrl.Result{}, err
-				}
-
-				return ctrl.Result{}, nil
-			}
 			reqLogger.Error(err, "error developer account not found ")
 			statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, application, nil, "", err)
 			statusResult, statusUpdateErr := statusReconciler.Reconcile()
 			if statusUpdateErr != nil {
-				if err != nil {
-					return ctrl.Result{}, fmt.Errorf("failed to reconcile application: %v. Failed to update application status: %w", err, statusUpdateErr)
-				}
-
-				return ctrl.Result{}, fmt.Errorf("failed to update application status: %w", statusUpdateErr)
+				return ctrl.Result{}, fmt.Errorf("failed to reconcile application: %v. Failed to update application status: %w", err, statusUpdateErr)
 			}
 			if statusResult.Requeue {
 				return statusResult, nil
@@ -127,28 +133,6 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, err
 	}
 
-	// Ignore deleted Applications, this can happen when foregroundDeletion is enabled
-	// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
-	if application.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(application, applicationFinalizer) {
-		err = r.removeApplicationFrom3scale(application, req, *threescaleAPIClient)
-		if err != nil {
-			r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to delete application", "%v", err)
-			return ctrl.Result{}, err
-		}
-
-		controllerutil.RemoveFinalizer(application, applicationFinalizer)
-		err = r.UpdateResource(application)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-
-		return ctrl.Result{}, nil
-	}
-
-	if application.GetDeletionTimestamp() != nil {
-		return ctrl.Result{}, nil
-	}
-
 	metadataUpdated, err := r.reconcileMetadata(application, accountResource)
 	if err != nil {
 		r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to update ownerReferences in application", "%v", err)
@@ -268,21 +252,23 @@ func (r *ApplicationReconciler) applicationReconciler(applicationResource *capab
 	return statusReconciler, err
 }
 
-func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabilitiesv1beta1.Application, req ctrl.Request, threescaleAPIClient threescaleapi.ThreeScaleClient) error {
+func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabilitiesv1beta1.Application) error {
 	logger := r.Logger().WithValues("application", client.ObjectKey{Name: application.Name, Namespace: application.Namespace})
 
 	// get Account
 	account := &capabilitiesv1beta1.DeveloperAccount{}
 	projectMeta := types.NamespacedName{
 		Name:      application.Spec.AccountCR.Name,
-		Namespace: req.Namespace,
+		Namespace: application.Namespace,
 	}
 
 	err := r.Client().Get(r.Context(), projectMeta, account)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			logger.Info("Account resource not found. Ignoring since object must have been deleted")
+			return nil
 		}
+		return err
 	}
 
 	// Attempt to remove application only if application.Status.ID is present
@@ -296,6 +282,18 @@ func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabil
 		return fmt.Errorf("could not remove application because ID is missing in the satus of developer account %s", account.Name)
 	}
 
+	// get providerAccountRef from account
+	providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), account.Namespace, account.Spec.ProviderAccountRef, r.Logger())
+	if err != nil {
+		return err
+	}
+
+	insecureSkipVerify := controllerhelper.GetInsecureSkipVerifyAnnotation(application.GetAnnotations())
+	threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount, insecureSkipVerify)
+	if err != nil {
+		return err
+	}
+
 	err = threescaleAPIClient.DeleteApplication(*account.Status.ID, *application.Status.ID)
 	if err != nil && !threescaleapi.IsNotFound(err) {
 		return err

controllers/capabilities/application_controller_test.go

@@ -258,50 +258,3 @@ func TestApplicationReconciler_applicationReconciler(t *testing.T) {
 		})
 	}
 }
-
-func TestApplicationReconciler_removeApplicationFrom3scale(t *testing.T) {
-	// admin portal
-	ap, _ := client.NewAdminPortalFromStr("https://3scale-admin.test.3scale.net")
-	type fields struct {
-		BaseReconciler *reconcilers.BaseReconciler
-	}
-	type args struct {
-		application         *capabilitiesv1beta1.Application
-		req                 controllerruntime.Request
-		threescaleAPIClient *client.ThreeScaleClient
-	}
-	tests := []struct {
-		name    string
-		fields  fields
-		args    args
-		wantErr bool
-	}{
-		{
-			name: "Delete Application successfully",
-			fields: fields{
-				BaseReconciler: getBaseReconciler(getApplicationCR(), getProviderAccount(), getApiManger(), getApplicationProductList(), getApplicationDeveloperAccount(), getProviderAccountRefSecret()),
-			},
-			args: args{
-				application: getApplicationDeleteCR(),
-				req: controllerruntime.Request{
-					NamespacedName: types.NamespacedName{
-						Name:      "test",
-						Namespace: "test",
-					},
-				},
-				threescaleAPIClient: client.NewThreeScale(ap, "test", mockHttpClientApplication(getApplicationPlanListByProductJson(), getApplicationJson("live"))),
-			},
-			wantErr: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			r := &ApplicationReconciler{
-				BaseReconciler: tt.fields.BaseReconciler,
-			}
-			if err := r.removeApplicationFrom3scale(tt.args.application, tt.args.req, *tt.args.threescaleAPIClient); (err != nil) != tt.wantErr {
-				t.Errorf("removeApplicationFrom3scale() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}