Merge pull request #92 from fluxcd/enhancement/finalizers

Refactor garbage collectors to finalizers

api/v1alpha1/condition_types.go

@@ -21,6 +21,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+const SourceFinalizer = "finalizers.fluxcd.io"
+
 // SourceCondition contains condition information for a source.
 type SourceCondition struct {
 	// Type of the condition, currently ('Ready').

controllers/gitrepository_controller.go

@@ -38,6 +38,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 
 	"github.com/fluxcd/pkg/recorder"
+
 	sourcev1 "github.com/fluxcd/source-controller/api/v1alpha1"
 	intgit "github.com/fluxcd/source-controller/internal/git"
 )
@@ -66,6 +67,37 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro
 
 	log := r.Log.WithValues("controller", strings.ToLower(sourcev1.GitRepositoryKind), "request", req.NamespacedName)
 
+	// Examine if the object is under deletion
+	if repository.ObjectMeta.DeletionTimestamp.IsZero() {
+		// The object is not being deleted, so if it does not have our finalizer,
+		// 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, recorder.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
+			}
+			// 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
+		}
+	}
+
 	// set initial status
 	if reset, status := r.shouldResetStatus(repository); reset {
 		repository.Status = status
@@ -82,7 +114,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro
 	}
 
 	// purge old artifacts from storage
-	if err := r.gc(repository); err != nil {
+	if err := r.gc(repository, false); err != nil {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
@@ -127,7 +159,6 @@ func (r *GitRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, o
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sourcev1.GitRepository{}).
 		WithEventFilter(SourceChangePredicate{}).
-		WithEventFilter(GarbageCollectPredicate{Scheme: r.Scheme, Log: r.Log, Storage: r.Storage}).
 		WithOptions(controller.Options{MaxConcurrentReconciles: opts.MaxConcurrentReconciles}).
 		Complete(r)
 }
@@ -268,8 +299,11 @@ func (r *GitRepositoryReconciler) verify(ctx context.Context, publicKeySecret ty
 
 // gc performs a garbage collection on all but current artifacts of
 // the given repository.
-func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository) error {
+func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository, all bool) error {
 	if repository.Status.Artifact != nil {
+		if all {
+			return r.Storage.RemoveAll(*repository.Status.Artifact)
+		}
 		return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact)
 	}
 	return nil

controllers/helmchart_controller.go

@@ -68,6 +68,37 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 
 	log := r.Log.WithValues("controller", strings.ToLower(sourcev1.HelmChartKind), "request", req.NamespacedName)
 
+	// Examine if the object is under deletion
+	if chart.ObjectMeta.DeletionTimestamp.IsZero() {
+		// The object is not being deleted, so if it does not have our finalizer,
+		// 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, recorder.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
+			}
+			// 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
+		}
+	}
+
 	// set initial status
 	if reset, status := r.shouldResetStatus(chart); reset {
 		chart.Status = status
@@ -84,7 +115,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 
 	// purge old artifacts from storage
-	if err := r.gc(chart); err != nil {
+	if err := r.gc(chart, false); err != nil {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
@@ -138,7 +169,6 @@ func (r *HelmChartReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sourcev1.HelmChart{}).
 		WithEventFilter(SourceChangePredicate{}).
-		WithEventFilter(GarbageCollectPredicate{Scheme: r.Scheme, Log: r.Log, Storage: r.Storage}).
 		WithOptions(controller.Options{MaxConcurrentReconciles: opts.MaxConcurrentReconciles}).
 		Complete(r)
 }
@@ -315,8 +345,11 @@ func (r *HelmChartReconciler) shouldResetStatus(chart sourcev1.HelmChart) (bool,
 
 // gc performs a garbage collection on all but current artifacts of
 // the given chart.
-func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart) error {
+func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart, all bool) error {
 	if chart.Status.Artifact != nil {
+		if all {
+			return r.Storage.RemoveAll(*chart.Status.Artifact)
+		}
 		return r.Storage.RemoveAllButCurrent(*chart.Status.Artifact)
 	}
 	return nil

controllers/helmrepository_controller.go

@@ -70,6 +70,37 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 
 	log := r.Log.WithValues("controller", strings.ToLower(sourcev1.HelmRepositoryKind), "request", req.NamespacedName)
 
+	// Examine if the object is under deletion
+	if repository.ObjectMeta.DeletionTimestamp.IsZero() {
+		// The object is not being deleted, so if it does not have our finalizer,
+		// 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, recorder.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
+			}
+			// 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
+		}
+	}
+
 	// set initial status
 	if reset, status := r.shouldResetStatus(repository); reset {
 		repository.Status = status
@@ -86,7 +117,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 	}
 
 	// purge old artifacts from storage
-	if err := r.gc(repository); err != nil {
+	if err := r.gc(repository, false); err != nil {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
@@ -130,7 +161,6 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager,
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sourcev1.HelmRepository{}).
 		WithEventFilter(SourceChangePredicate{}).
-		WithEventFilter(GarbageCollectPredicate{Scheme: r.Scheme, Log: r.Log, Storage: r.Storage}).
 		WithOptions(controller.Options{MaxConcurrentReconciles: opts.MaxConcurrentReconciles}).
 		Complete(r)
 }
@@ -263,8 +293,11 @@ func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRep
 
 // gc performs a garbage collection on all but current artifacts of
 // the given repository.
-func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository) error {
+func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository, all bool) error {
 	if repository.Status.Artifact != nil {
+		if all {
+			return r.Storage.RemoveAll(*repository.Status.Artifact)
+		}
 		return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact)
 	}
 	return nil

controllers/predicate.go

@@ -17,11 +17,6 @@ limitations under the License.
 package controllers
 
 import (
-	"fmt"
-
-	"github.com/go-logr/logr"
-	"k8s.io/apimachinery/pkg/runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
@@ -57,30 +52,3 @@ func (SourceChangePredicate) Update(e event.UpdateEvent) bool {
 
 	return false
 }
-
-type GarbageCollectPredicate struct {
-	predicate.Funcs
-	Scheme  *runtime.Scheme
-	Log     logr.Logger
-	Storage *Storage
-}
-
-// Delete removes all artifacts from storage that belong to the
-// referenced object.
-func (gc GarbageCollectPredicate) Delete(e event.DeleteEvent) bool {
-	gvk, err := apiutil.GVKForObject(e.Object, gc.Scheme)
-	if err != nil {
-		gc.Log.Error(err, "unable to get GroupVersionKind for deleted object")
-		return false
-	}
-	// delete artifacts
-	artifact := gc.Storage.ArtifactFor(gvk.Kind, e.Meta, "*", "")
-	if err := gc.Storage.RemoveAll(artifact); err != nil {
-		gc.Log.Error(err, "unable to delete artifacts",
-			gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName()))
-	} else {
-		gc.Log.Info(gvk.Kind+" artifacts deleted",
-			gvk.Kind, fmt.Sprintf("%s/%s", e.Meta.GetNamespace(), e.Meta.GetName()))
-	}
-	return true
-}

controllers/util.go

@@ -0,0 +1,36 @@
+/*
+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
+}