Merge pull request #208 from fluxcd/gc-bugfix

controllers/bucket_controller.go

@@ -108,7 +108,7 @@ func (r *BucketReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 
 	// purge old artifacts from storage
-	if err := r.gc(bucket, false); err != nil {
+	if err := r.gc(bucket); err != nil {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
@@ -252,7 +252,7 @@ func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket
 }
 
 func (r *BucketReconciler) reconcileDelete(ctx context.Context, bucket sourcev1.Bucket) (ctrl.Result, error) {
-	if err := r.gc(bucket, true); err != nil {
+	if err := r.gc(bucket); 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
@@ -349,10 +349,13 @@ func (r *BucketReconciler) resetStatus(bucket sourcev1.Bucket) (sourcev1.Bucket,
 	return bucket, false
 }
 
-// gc performs a garbage collection on all but current artifacts of the given bucket.
+// gc performs a garbage collection for the given v1beta1.Bucket.
-func (r *BucketReconciler) gc(bucket sourcev1.Bucket, all bool) error {
+// It removes all but the current artifact except for when the
-	if all {
+// deletion timestamp is set, which will result in the removal of
-		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(bucket.Kind, bucket.GetObjectMeta(), "", ""))
+// all artifacts for the resource.
+func (r *BucketReconciler) gc(bucket sourcev1.Bucket) error {
+	if !bucket.DeletionTimestamp.IsZero() {
+		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(bucket.Kind, bucket.GetObjectMeta(), "", "*"))
 	}
 	if bucket.GetArtifact() != nil {
 		return r.Storage.RemoveAllButCurrent(*bucket.GetArtifact())

controllers/gitrepository_controller.go

@@ -107,7 +107,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro
 	}
 
 	// purge old artifacts from storage
-	if err := r.gc(repository, false); err != nil {
+	if err := r.gc(repository); err != nil {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
@@ -250,7 +250,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour
 }
 
 func (r *GitRepositoryReconciler) reconcileDelete(ctx context.Context, repository sourcev1.GitRepository) (ctrl.Result, error) {
-	if err := r.gc(repository, true); err != nil {
+	if err := r.gc(repository); 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
@@ -308,11 +308,13 @@ func (r *GitRepositoryReconciler) resetStatus(repository sourcev1.GitRepository)
 	return repository, false
 }
 
-// gc performs a garbage collection on all but current artifacts of
+// gc performs a garbage collection for the given v1beta1.GitRepository.
-// the given repository.
+// It removes all but the current artifact except for when the
-func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository, all bool) error {
+// deletion timestamp is set, which will result in the removal of
-	if all {
+// all artifacts for the resource.
-		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", ""))
+func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository) error {
+	if !repository.DeletionTimestamp.IsZero() {
+		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "*"))
 	}
 	if repository.GetArtifact() != nil {
 		return r.Storage.RemoveAllButCurrent(*repository.GetArtifact())

controllers/helmchart_controller.go

@@ -114,7 +114,7 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	}
 
 	// Purge all but current artifact from storage
-	if err := r.gc(chart, false); err != nil {
+	if err := r.gc(chart); err != nil {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
@@ -589,7 +589,7 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
 
 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 {
+	if err := r.gc(chart); 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
@@ -624,11 +624,13 @@ func (r *HelmChartReconciler) resetStatus(chart sourcev1.HelmChart) (sourcev1.He
 	return chart, false
 }
 
-// gc performs a garbage collection on all but the current artifact of
+// gc performs a garbage collection for the given v1beta1.HelmChart.
-// the given chart.
+// It removes all but the current artifact except for when the
-func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart, all bool) error {
+// deletion timestamp is set, which will result in the removal of
-	if all {
+// all artifacts for the resource.
-		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), "", ""))
+func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart) error {
+	if !chart.DeletionTimestamp.IsZero() {
+		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), "", "*"))
 	}
 	if chart.GetArtifact() != nil {
 		return r.Storage.RemoveAllButCurrent(*chart.GetArtifact())

controllers/helmrepository_controller.go

@@ -108,7 +108,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 	}
 
 	// purge old artifacts from storage
-	if err := r.gc(repository, false); err != nil {
+	if err := r.gc(repository); err != nil {
 		log.Error(err, "unable to purge old artifacts")
 	}
 
@@ -248,7 +248,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 
 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 {
+	if err := r.gc(repository); 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
@@ -282,11 +282,13 @@ func (r *HelmRepositoryReconciler) resetStatus(repository sourcev1.HelmRepositor
 	return repository, false
 }
 
-// gc performs a garbage collection on all but current artifacts of
+// gc performs a garbage collection for the given v1beta1.HelmRepository.
-// the given repository.
+// It removes all but the current artifact except for when the
-func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository, all bool) error {
+// deletion timestamp is set, which will result in the removal of
-	if all {
+// all artifacts for the resource.
-		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", ""))
+func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository) error {
+	if !repository.DeletionTimestamp.IsZero() {
+		return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "*"))
 	}
 	if repository.GetArtifact() != nil {
 		return r.Storage.RemoveAllButCurrent(*repository.GetArtifact())