Merge pull request #176 from fluxcd/finalizers

Switch to controller-runtime utils for finalizers

controllers/bucket_controller.go

@@ -38,6 +38,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/fluxcd/pkg/runtime/events"
 	"github.com/fluxcd/pkg/runtime/metrics"
@@ -72,37 +73,18 @@ func (r *BucketReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	log := r.Log.WithValues("controller", strings.ToLower(sourcev1.BucketKind), "request", req.NamespacedName)
 
+	// Add our finalizer if it does not exist
+	if !controllerutil.ContainsFinalizer(&bucket, sourcev1.SourceFinalizer) {
+		controllerutil.AddFinalizer(&bucket, sourcev1.SourceFinalizer)
+		if err := r.Update(ctx, &bucket); err != nil {
+			log.Error(err, "unable to register finalizer")
+			return ctrl.Result{}, err
+		}
+	}
+
 	// Examine if the object is under deletion
-	if bucket.ObjectMeta.DeletionTimestamp.IsZero() {
+	if !bucket.ObjectMeta.DeletionTimestamp.IsZero() {
-		// The object is not being deleted, so if it does not have our finalizer,
+		return r.reconcileDelete(ctx, bucket)
-		// then lets add the finalizer and update the object. This is equivalent
-		// registering our finalizer.
-		if !containsString(bucket.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			bucket.ObjectMeta.Finalizers = append(bucket.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &bucket); err != nil {
-				log.Error(err, "unable to register finalizer")
-				return ctrl.Result{}, err
-			}
-		}
-	} else {
-		// The object is being deleted
-		if containsString(bucket.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			// Our finalizer is still present, so lets handle garbage collection
-			if err := r.gc(bucket, true); err != nil {
-				r.event(bucket, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
-				// Return the error so we retry the failed garbage collection
-				return ctrl.Result{}, err
-			}
-			// Record deleted status
-			r.recordReadiness(bucket, true)
-			// Remove our finalizer from the list and update it
-			bucket.ObjectMeta.Finalizers = removeString(bucket.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &bucket); err != nil {
-				return ctrl.Result{}, err
-			}
-			// Stop reconciliation as the object is being deleted
-			return ctrl.Result{}, nil
-		}
 	}
 
 	// record reconciliation duration
@@ -268,6 +250,26 @@ func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket
 	return sourcev1.BucketReady(bucket, artifact, url, sourcev1.BucketOperationSucceedReason, message), nil
 }
 
+func (r *BucketReconciler) reconcileDelete(ctx context.Context, bucket sourcev1.Bucket) (ctrl.Result, error) {
+	if err := r.gc(bucket, true); err != nil {
+		r.event(bucket, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
+		// Return the error so we retry the failed garbage collection
+		return ctrl.Result{}, err
+	}
+
+	// Record deleted status
+	r.recordReadiness(bucket, true)
+
+	// Remove our finalizer from the list and update it
+	controllerutil.RemoveFinalizer(&bucket, sourcev1.SourceFinalizer)
+	if err := r.Update(ctx, &bucket); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	// Stop reconciliation as the object is being deleted
+	return ctrl.Result{}, nil
+}
+
 func (r *BucketReconciler) auth(ctx context.Context, bucket sourcev1.Bucket) (*minio.Client, error) {
 	opt := minio.Options{
 		Region: bucket.Spec.Region,

controllers/gitrepository_controller.go

@@ -36,6 +36,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/fluxcd/pkg/runtime/events"
 	"github.com/fluxcd/pkg/runtime/metrics"
@@ -71,37 +72,18 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro
 
 	log := r.Log.WithValues("controller", strings.ToLower(sourcev1.GitRepositoryKind), "request", req.NamespacedName)
 
+	// Add our finalizer if it does not exist
+	if !controllerutil.ContainsFinalizer(&repository, sourcev1.SourceFinalizer) {
+		controllerutil.AddFinalizer(&repository, sourcev1.SourceFinalizer)
+		if err := r.Update(ctx, &repository); err != nil {
+			log.Error(err, "unable to register finalizer")
+			return ctrl.Result{}, err
+		}
+	}
+
 	// Examine if the object is under deletion
-	if repository.ObjectMeta.DeletionTimestamp.IsZero() {
+	if !repository.ObjectMeta.DeletionTimestamp.IsZero() {
-		// The object is not being deleted, so if it does not have our finalizer,
+		return r.reconcileDelete(ctx, repository)
-		// then lets add the finalizer and update the object. This is equivalent
-		// registering our finalizer.
-		if !containsString(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			repository.ObjectMeta.Finalizers = append(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &repository); err != nil {
-				log.Error(err, "unable to register finalizer")
-				return ctrl.Result{}, err
-			}
-		}
-	} else {
-		// The object is being deleted
-		if containsString(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			// Our finalizer is still present, so lets handle garbage collection
-			if err := r.gc(repository, true); err != nil {
-				r.event(repository, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
-				// Return the error so we retry the failed garbage collection
-				return ctrl.Result{}, err
-			}
-			// Record deleted status
-			r.recordReadiness(repository, true)
-			// Remove our finalizer from the list and update it
-			repository.ObjectMeta.Finalizers = removeString(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &repository); err != nil {
-				return ctrl.Result{}, err
-			}
-			// Stop reconciliation as the object is being deleted
-			return ctrl.Result{}, nil
-		}
 	}
 
 	// record reconciliation duration
@@ -266,6 +248,26 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour
 	return sourcev1.GitRepositoryReady(repository, artifact, url, sourcev1.GitOperationSucceedReason, message), nil
 }
 
+func (r *GitRepositoryReconciler) reconcileDelete(ctx context.Context, repository sourcev1.GitRepository) (ctrl.Result, error) {
+	if err := r.gc(repository, true); err != nil {
+		r.event(repository, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
+		// Return the error so we retry the failed garbage collection
+		return ctrl.Result{}, err
+	}
+
+	// Record deleted status
+	r.recordReadiness(repository, true)
+
+	// Remove our finalizer from the list and update it
+	controllerutil.RemoveFinalizer(&repository, sourcev1.SourceFinalizer)
+	if err := r.Update(ctx, &repository); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	// Stop reconciliation as the object is being deleted
+	return ctrl.Result{}, nil
+}
+
 // verify returns an error if the PGP signature can't be verified
 func (r *GitRepositoryReconciler) verify(ctx context.Context, publicKeySecret types.NamespacedName, commit *object.Commit) error {
 	if commit.PGPSignature == "" {

controllers/helmchart_controller.go

@@ -39,6 +39,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/fluxcd/pkg/runtime/events"
 	"github.com/fluxcd/pkg/runtime/metrics"
@@ -76,37 +77,18 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	log := r.Log.WithValues("controller", strings.ToLower(sourcev1.HelmChartKind), "request", req.NamespacedName)
 
+	// Add our finalizer if it does not exist
+	if !controllerutil.ContainsFinalizer(&chart, sourcev1.SourceFinalizer) {
+		controllerutil.AddFinalizer(&chart, sourcev1.SourceFinalizer)
+		if err := r.Update(ctx, &chart); err != nil {
+			log.Error(err, "unable to register finalizer")
+			return ctrl.Result{}, err
+		}
+	}
+
 	// Examine if the object is under deletion
-	if chart.ObjectMeta.DeletionTimestamp.IsZero() {
+	if !chart.ObjectMeta.DeletionTimestamp.IsZero() {
-		// The object is not being deleted, so if it does not have our finalizer,
+		return r.reconcileDelete(ctx, chart)
-		// then lets add the finalizer and update the object. This is equivalent
-		// registering our finalizer.
-		if !containsString(chart.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			chart.ObjectMeta.Finalizers = append(chart.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &chart); err != nil {
-				log.Error(err, "unable to register finalizer")
-				return ctrl.Result{}, err
-			}
-		}
-	} else {
-		// The object is being deleted
-		if containsString(chart.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			// Our finalizer is still present, so lets handle garbage collection
-			if err := r.gc(chart, true); err != nil {
-				r.event(chart, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
-				// Return the error so we retry the failed garbage collection
-				return ctrl.Result{}, err
-			}
-			// Record deleted status
-			r.recordReadiness(chart, true)
-			// Remove our finalizer from the list and update it
-			chart.ObjectMeta.Finalizers = removeString(chart.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &chart); err != nil {
-				return ctrl.Result{}, err
-			}
-			// Stop reconciliation as the object is being deleted
-			return ctrl.Result{}, nil
-		}
 	}
 
 	// record reconciliation duration
@@ -491,6 +473,27 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
 	return sourcev1.HelmChartReady(chart, newArtifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil
 }
 
+func (r *HelmChartReconciler) reconcileDelete(ctx context.Context, chart sourcev1.HelmChart) (ctrl.Result, error) {
+	// Our finalizer is still present, so lets handle garbage collection
+	if err := r.gc(chart, true); err != nil {
+		r.event(chart, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
+		// Return the error so we retry the failed garbage collection
+		return ctrl.Result{}, err
+	}
+
+	// Record deleted status
+	r.recordReadiness(chart, true)
+
+	// Remove our finalizer from the list and update it
+	controllerutil.RemoveFinalizer(&chart, sourcev1.SourceFinalizer)
+	if err := r.Update(ctx, &chart); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	// Stop reconciliation as the object is being deleted
+	return ctrl.Result{}, nil
+}
+
 // resetStatus returns a modified v1beta1.HelmChart and a boolean indicating
 // if the status field has been reset.
 func (r *HelmChartReconciler) resetStatus(chart sourcev1.HelmChart) (sourcev1.HelmChart, bool) {

controllers/helmrepository_controller.go

@@ -35,6 +35,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/yaml"
 
 	"github.com/fluxcd/pkg/runtime/events"
@@ -73,37 +74,18 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 
 	log := r.Log.WithValues("controller", strings.ToLower(sourcev1.HelmRepositoryKind), "request", req.NamespacedName)
 
+	// Add our finalizer if it does not exist
+	if !controllerutil.ContainsFinalizer(&repository, sourcev1.SourceFinalizer) {
+		controllerutil.AddFinalizer(&repository, sourcev1.SourceFinalizer)
+		if err := r.Update(ctx, &repository); err != nil {
+			log.Error(err, "unable to register finalizer")
+			return ctrl.Result{}, err
+		}
+	}
+
 	// Examine if the object is under deletion
-	if repository.ObjectMeta.DeletionTimestamp.IsZero() {
+	if !repository.ObjectMeta.DeletionTimestamp.IsZero() {
-		// The object is not being deleted, so if it does not have our finalizer,
+		return r.reconcileDelete(ctx, repository)
-		// then lets add the finalizer and update the object. This is equivalent
-		// registering our finalizer.
-		if !containsString(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			repository.ObjectMeta.Finalizers = append(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &repository); err != nil {
-				log.Error(err, "unable to register finalizer")
-				return ctrl.Result{}, err
-			}
-		}
-	} else {
-		// The object is being deleted
-		if containsString(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer) {
-			// Our finalizer is still present, so lets handle garbage collection
-			if err := r.gc(repository, true); err != nil {
-				r.event(repository, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
-				// Return the error so we retry the failed garbage collection
-				return ctrl.Result{}, err
-			}
-			// Record deleted status
-			r.recordReadiness(repository, true)
-			// Remove our finalizer from the list and update it
-			repository.ObjectMeta.Finalizers = removeString(repository.ObjectMeta.Finalizers, sourcev1.SourceFinalizer)
-			if err := r.Update(ctx, &repository); err != nil {
-				return ctrl.Result{}, err
-			}
-			// Stop reconciliation as the object is being deleted
-			return ctrl.Result{}, nil
-		}
 	}
 
 	// record reconciliation duration
@@ -264,6 +246,27 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 	return sourcev1.HelmRepositoryReady(repository, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil
 }
 
+func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, repository sourcev1.HelmRepository) (ctrl.Result, error) {
+	// Our finalizer is still present, so lets handle garbage collection
+	if err := r.gc(repository, true); err != nil {
+		r.event(repository, events.EventSeverityError, fmt.Sprintf("garbage collection for deleted resource failed: %s", err.Error()))
+		// Return the error so we retry the failed garbage collection
+		return ctrl.Result{}, err
+	}
+
+	// Record deleted status
+	r.recordReadiness(repository, true)
+
+	// Remove our finalizer from the list and update it
+	controllerutil.RemoveFinalizer(&repository, sourcev1.SourceFinalizer)
+	if err := r.Update(ctx, &repository); err != nil {
+		return ctrl.Result{}, err
+	}
+
+	// Stop reconciliation as the object is being deleted
+	return ctrl.Result{}, nil
+}
+
 // resetStatus returns a modified v1beta1.HelmRepository and a boolean indicating
 // if the status field has been reset.
 func (r *HelmRepositoryReconciler) resetStatus(repository sourcev1.HelmRepository) (sourcev1.HelmRepository, bool) {

controllers/util.go

@@ -1,36 +0,0 @@
-/*
-Copyright 2020 The Flux CD contributors.
-
-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
-
-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 controllers
-
-func containsString(slice []string, s string) bool {
-	for _, item := range slice {
-		if item == s {
-			return true
-		}
-	}
-	return false
-}
-
-func removeString(slice []string, s string) (result []string) {
-	for _, item := range slice {
-		if item == s {
-			continue
-		}
-		result = append(result, item)
-	}
-	return
-}