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 9d4a09889..d1a0124a7 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 e04d35c9f..dd7ff44a7 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 5396d67dc..b970c2923 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 ab6c2a199..17e11b6c0 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 d9358a2b1..ff1408f33 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" - 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 {