From 44207f46d5e7f28c54d4f8a025f6e23c3864fd91 Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 7 Apr 2022 21:01:37 +0530 Subject: [PATCH] Avoid event logging GC failure We try to avoid affecting the source reconciliation when there's a garbage collection related failure. The event logging was resulting in events and notifications related to GC failure when the artifact directory isn't created in the first reconciliation of an object. Signed-off-by: Sunny --- controllers/bucket_controller.go | 4 +--- controllers/gitrepository_controller.go | 4 +--- controllers/helmchart_controller.go | 4 +--- controllers/helmrepository_controller.go | 4 +--- controllers/storage.go | 17 +++++++---------- 5 files changed, 11 insertions(+), 22 deletions(-) diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go index 9d4a0988..d1a0124a 100644 --- 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 index e04d35c9..dd7ff44a 100644 --- 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 index 5396d67d..b970c292 100644 --- 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 index ab6c2a19..17e11b6c 100644 --- 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 index d9358a2b..ff1408f3 100644 --- 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" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kerrors "k8s.io/apimachinery/pkg/util/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "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 {