Merge pull request #659 from fluxcd/gc-fail-event-fix

Avoid event logging GC failure
fluxcd · Apr 7, 2022 · 73aa3c4 · 73aa3c4
2 parents 741033e + 44207f4
commit 73aa3c4
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 22 deletions.
diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go
@@ -634,12 +634,10 @@ func (r *BucketReconciler) garbageCollect(ctx context.Context, obj *sourcev1.Buc
 	if obj.GetArtifact() != nil {
 		delFiles, err := r.Storage.GarbageCollect(ctx, *obj.GetArtifact(), time.Second*5)
 		if err != nil {
-			e := &serror.Event{
+			return &serror.Event{
 				Err:    fmt.Errorf("garbage collection of artifacts failed: %w", err),
 				Reason: "GarbageCollectionFailed",
 			}
-			r.eventLogf(ctx, obj, corev1.EventTypeWarning, e.Reason, e.Err.Error())
-			return e
 		}
 		if len(delFiles) > 0 {
 			r.eventLogf(ctx, obj, events.EventTypeTrace, "GarbageCollectionSucceeded",

diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go
@@ -710,12 +710,10 @@ func (r *GitRepositoryReconciler) garbageCollect(ctx context.Context, obj *sourc
 	if obj.GetArtifact() != nil {
 		delFiles, err := r.Storage.GarbageCollect(ctx, *obj.GetArtifact(), time.Second*5)
 		if err != nil {
-			e := &serror.Event{
+			return &serror.Event{
 				Err:    fmt.Errorf("garbage collection of artifacts failed: %w", err),
 				Reason: "GarbageCollectionFailed",
 			}
-			r.eventLogf(ctx, obj, corev1.EventTypeWarning, e.Reason, e.Err.Error())
-			return e
 		}
 		if len(delFiles) > 0 {
 			r.eventLogf(ctx, obj, events.EventTypeTrace, "GarbageCollectionSucceeded",

diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go
@@ -806,12 +806,10 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
 	if obj.GetArtifact() != nil {
 		delFiles, err := r.Storage.GarbageCollect(ctx, *obj.GetArtifact(), time.Second*5)
 		if err != nil {
-			e := &serror.Event{
+			return &serror.Event{
 				Err:    fmt.Errorf("garbage collection of artifacts failed: %w", err),
 				Reason: "GarbageCollectionFailed",
 			}
-			r.eventLogf(ctx, obj, corev1.EventTypeWarning, e.Reason, e.Err.Error())
-			return e
 		}
 		if len(delFiles) > 0 {
 			r.eventLogf(ctx, obj, events.EventTypeTrace, "GarbageCollectionSucceeded",

diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go
@@ -520,12 +520,10 @@ func (r *HelmRepositoryReconciler) garbageCollect(ctx context.Context, obj *sour
 	if obj.GetArtifact() != nil {
 		delFiles, err := r.Storage.GarbageCollect(ctx, *obj.GetArtifact(), time.Second*5)
 		if err != nil {
-			e := &serror.Event{
+			return &serror.Event{
 				Err:    fmt.Errorf("garbage collection of artifacts failed: %w", err),
 				Reason: "GarbageCollectionFailed",
 			}
-			r.eventLogf(ctx, obj, corev1.EventTypeWarning, e.Reason, e.Err.Error())
-			return e
 		}
 		if len(delFiles) > 0 {
 			r.eventLogf(ctx, obj, events.EventTypeTrace, "GarbageCollectionSucceeded",

diff --git a/controllers/storage.go b/controllers/storage.go
@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"hash"
 	"io"
+	"io/fs"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -32,16 +33,12 @@ import (
 	"time"
 
 	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/fluxcd/pkg/lockedfile"
+	"github.com/fluxcd/pkg/untar"
 	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
-	kerrors "k8s.io/apimachinery/pkg/util/errors"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 
-	"github.com/fluxcd/pkg/lockedfile"
-
-	"io/fs"
-
-	"github.com/fluxcd/pkg/untar"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 	sourcefs "github.com/fluxcd/source-controller/internal/fs"
 	"github.com/fluxcd/source-controller/pkg/sourceignore"
@@ -181,7 +178,7 @@ func (s *Storage) getGarbageFiles(artifact sourcev1.Artifact, totalCountLimit, m
 			return nil
 		}
 		if totalFiles >= totalCountLimit {
-			return fmt.Errorf("Reached file walking limit, already walked over: %d", totalFiles)
+			return fmt.Errorf("reached file walking limit, already walked over: %d", totalFiles)
 		}
 		info, err := d.Info()
 		if err != nil {
@@ -190,8 +187,8 @@ func (s *Storage) getGarbageFiles(artifact sourcev1.Artifact, totalCountLimit, m
 		}
 		createdAt := info.ModTime().UTC()
 		diff := now.Sub(createdAt)
-		// compare the time difference between now and the time at which the file was created
-		// with the provided ttl. delete if difference is greater than the ttl.
+		// Compare the time difference between now and the time at which the file was created
+		// with the provided TTL. Delete if the difference is greater than the TTL.
 		expired := diff > ttl
 		if !info.IsDir() && info.Mode()&os.ModeSymlink != os.ModeSymlink {
 			if path != localPath && expired {