diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index 521fb2546..d6598aea8 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -200,7 +200,7 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res summarize.WithReconcileError(retErr), summarize.WithIgnoreNotFound(), summarize.WithProcessors( - summarize.RecordContextualError, + summarize.ErrorActionHandler, summarize.RecordReconcileReq, ), summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{ @@ -279,10 +279,10 @@ func (r *BucketReconciler) reconcile(ctx context.Context, sp *patch.SerialPatche // Create temp working dir tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-%s-", obj.Kind, obj.Namespace, obj.Name)) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to create temporary working directory: %w", err), - Reason: sourcev1.DirCreationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to create temporary working directory: %w", err), + sourcev1.DirCreationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -423,7 +423,7 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, sp *patch.Seria func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher, obj *bucketv1.Bucket, index *index.Digester, dir string) (sreconcile.Result, error) { secret, err := r.getBucketSecret(ctx, obj) if err != nil { - e := &serror.Event{Err: err, Reason: sourcev1.AuthenticationFailedReason} + e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) // Return error as the world as observed may change return sreconcile.ResultEmpty, e @@ -434,34 +434,34 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial switch obj.Spec.Provider { case bucketv1.GoogleBucketProvider: if err = gcp.ValidateSecret(secret); err != nil { - e := &serror.Event{Err: err, Reason: sourcev1.AuthenticationFailedReason} + e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } if provider, err = gcp.NewClient(ctx, secret); err != nil { - e := &serror.Event{Err: err, Reason: "ClientError"} + e := serror.NewGeneric(err, "ClientError") conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } case bucketv1.AzureBucketProvider: if err = azure.ValidateSecret(secret); err != nil { - e := &serror.Event{Err: err, Reason: sourcev1.AuthenticationFailedReason} + e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } if provider, err = azure.NewClient(obj, secret); err != nil { - e := &serror.Event{Err: err, Reason: "ClientError"} + e := serror.NewGeneric(err, "ClientError") conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } default: if err = minio.ValidateSecret(secret); err != nil { - e := &serror.Event{Err: err, Reason: sourcev1.AuthenticationFailedReason} + e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } if provider, err = minio.NewClient(obj, secret); err != nil { - e := &serror.Event{Err: err, Reason: "ClientError"} + e := serror.NewGeneric(err, "ClientError") conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } @@ -469,7 +469,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial // Fetch etag index if err = fetchEtagIndex(ctx, provider, obj, index, dir); err != nil { - e := &serror.Event{Err: err, Reason: bucketv1.BucketOperationFailedReason} + e := serror.NewGeneric(err, bucketv1.BucketOperationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } @@ -501,7 +501,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial }() if err = fetchIndexFiles(ctx, provider, obj, index, dir); err != nil { - e := &serror.Event{Err: err, Reason: bucketv1.BucketOperationFailedReason} + e := serror.NewGeneric(err, bucketv1.BucketOperationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) return sreconcile.ResultEmpty, e } @@ -550,45 +550,45 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, sp *patch.Seri // Ensure target path exists and is a directory if f, err := os.Stat(dir); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to stat source path: %w", err), - Reason: sourcev1.StatOperationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to stat source path: %w", err), + sourcev1.StatOperationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } else if !f.IsDir() { - e := &serror.Event{ - Err: fmt.Errorf("source path '%s' is not a directory", dir), - Reason: sourcev1.InvalidPathReason, - } + e := serror.NewGeneric( + fmt.Errorf("source path '%s' is not a directory", dir), + sourcev1.InvalidPathReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } // Ensure artifact directory exists and acquire lock if err := r.Storage.MkdirAll(artifact); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to create artifact directory: %w", err), - Reason: sourcev1.DirCreationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to create artifact directory: %w", err), + sourcev1.DirCreationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } unlock, err := r.Storage.Lock(artifact) if err != nil { - return sreconcile.ResultEmpty, &serror.Event{ - Err: fmt.Errorf("failed to acquire lock for artifact: %w", err), - Reason: meta.FailedReason, - } + return sreconcile.ResultEmpty, serror.NewGeneric( + fmt.Errorf("failed to acquire lock for artifact: %w", err), + meta.FailedReason, + ) } defer unlock() // Archive directory to storage if err := r.Storage.Archive(&artifact, dir, nil); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("unable to archive artifact to storage: %s", err), - Reason: sourcev1.ArchiveOperationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("unable to archive artifact to storage: %s", err), + sourcev1.ArchiveOperationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -635,10 +635,10 @@ func (r *BucketReconciler) reconcileDelete(ctx context.Context, obj *bucketv1.Bu func (r *BucketReconciler) garbageCollect(ctx context.Context, obj *bucketv1.Bucket) error { if !obj.DeletionTimestamp.IsZero() { if deleted, err := r.Storage.RemoveAll(r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), "", "*")); err != nil { - return &serror.Event{ - Err: fmt.Errorf("garbage collection for deleted resource failed: %s", err), - Reason: "GarbageCollectionFailed", - } + return serror.NewGeneric( + fmt.Errorf("garbage collection for deleted resource failed: %s", err), + "GarbageCollectionFailed", + ) } else if deleted != "" { r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "GarbageCollectionSucceeded", "garbage collected artifacts for deleted resource") @@ -649,10 +649,10 @@ func (r *BucketReconciler) garbageCollect(ctx context.Context, obj *bucketv1.Buc if obj.GetArtifact() != nil { delFiles, err := r.Storage.GarbageCollect(ctx, *obj.GetArtifact(), time.Second*5) if err != nil { - return &serror.Event{ - Err: fmt.Errorf("garbage collection of artifacts failed: %w", err), - Reason: "GarbageCollectionFailed", - } + return serror.NewGeneric( + fmt.Errorf("garbage collection of artifacts failed: %w", err), + "GarbageCollectionFailed", + ) } if len(delFiles) > 0 { r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "GarbageCollectionSucceeded", diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 3dfa9c91e..cd6062dfd 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -561,6 +561,8 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch ) ge.Notification = false ge.Ignore = true + // Log it as this will not be passed to the runtime. + ge.Log = true ge.Event = corev1.EventTypeNormal // Remove any stale fetch failed condition. conditions.Delete(obj, sourcev1.FetchFailedCondition) @@ -815,10 +817,10 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, sp *patc // Copy artifact (sub)contents to configured directory. if err := r.Storage.CopyToPath(artifact, incl.GetFromPath(), toPath); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to copy '%s' include from %s to %s: %w", incl.GitRepositoryRef.Name, incl.GetFromPath(), incl.GetToPath(), err), - Reason: "CopyFailure", - } + e := serror.NewGeneric( + fmt.Errorf("failed to copy '%s' include from %s to %s: %w", incl.GitRepositoryRef.Name, incl.GetFromPath(), incl.GetToPath(), err), + "CopyFailure", + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index 35a896f92..7ef584384 100644 --- a/internal/controller/helmchart_controller.go +++ b/internal/controller/helmchart_controller.go @@ -215,7 +215,7 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( summarize.WithReconcileError(retErr), summarize.WithIgnoreNotFound(), summarize.WithProcessors( - summarize.RecordContextualError, + summarize.ErrorActionHandler, summarize.RecordReconcileReq, ), summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{ @@ -420,19 +420,19 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, sp *patch.Ser // Retrieve the source s, err := r.getSource(ctx, obj) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to get source: %w", err), - Reason: "SourceUnavailable", - } + e := serror.NewGeneric( + fmt.Errorf("failed to get source: %w", err), + "SourceUnavailable", + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) // Return Kubernetes client errors, but ignore others which can only be // solved by a change in generation if apierrs.ReasonForError(err) == metav1.StatusReasonUnknown { - return sreconcile.ResultEmpty, &serror.Stalling{ - Err: fmt.Errorf("failed to get source: %w", err), - Reason: "UnsupportedSourceKind", - } + return sreconcile.ResultEmpty, serror.NewStalling( + fmt.Errorf("failed to get source: %w", err), + "UnsupportedSourceKind", + ) } return sreconcile.ResultEmpty, e } @@ -471,15 +471,15 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, sp *patch.Ser // Handle any build error if retErr != nil { if buildErr := new(chart.BuildError); errors.As(retErr, &buildErr) { - retErr = &serror.Event{ - Err: buildErr, - Reason: buildErr.Reason.Reason, - } + retErr = serror.NewGeneric( + buildErr, + buildErr.Reason.Reason, + ) if chart.IsPersistentBuildErrorReason(buildErr.Reason) { - retErr = &serror.Stalling{ - Err: buildErr, - Reason: buildErr.Reason.Reason, - } + retErr = serror.NewStalling( + buildErr, + buildErr.Reason.Reason, + ) } } } @@ -516,10 +516,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * clientOpts, certsTmpDir, err := getter.GetClientOpts(ctxTimeout, r.Client, repo, normalizedURL) if err != nil && !errors.Is(err, getter.ErrDeprecatedTLSConfig) { - e := &serror.Event{ - Err: err, - Reason: sourcev1.AuthenticationFailedReason, - } + e := serror.NewGeneric( + err, + sourcev1.AuthenticationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -549,10 +549,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // or rework to enable reusing credentials to avoid the unneccessary handshake operations registryClient, credentialsFile, err := r.RegistryClientGenerator(clientOpts.TlsConfig, clientOpts.MustLoginToRegistry()) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to construct Helm client: %w", err), - Reason: meta.FailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to construct Helm client: %w", err), + meta.FailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -574,10 +574,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * if obj.Spec.Verify.SecretRef == nil { provider = fmt.Sprintf("%s keyless", provider) } - e := &serror.Event{ - Err: fmt.Errorf("failed to verify the signature using provider '%s': %w", provider, err), - Reason: sourcev1.VerificationError, - } + e := serror.NewGeneric( + fmt.Errorf("failed to verify the signature using provider '%s': %w", provider, err), + sourcev1.VerificationError, + ) conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -599,10 +599,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * if clientOpts.MustLoginToRegistry() { err = ociChartRepo.Login(clientOpts.RegLoginOpts...) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to login to OCI registry: %w", err), - Reason: sourcev1.AuthenticationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to login to OCI registry: %w", err), + sourcev1.AuthenticationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -683,10 +683,10 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj // Create temporary working directory tmpDir, err := util.TempDirForObj("", obj) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to create temporary working directory: %w", err), - Reason: sourcev1.DirCreationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to create temporary working directory: %w", err), + sourcev1.DirCreationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -695,10 +695,10 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj // Create directory to untar source into sourceDir := filepath.Join(tmpDir, "source") if err := os.Mkdir(sourceDir, 0o700); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to create directory to untar source into: %w", err), - Reason: sourcev1.DirCreationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to create directory to untar source into: %w", err), + sourcev1.DirCreationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -706,25 +706,25 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj // Open the tarball artifact file and untar files into working directory f, err := os.Open(r.Storage.LocalPath(source)) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to open source artifact: %w", err), - Reason: sourcev1.ReadOperationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to open source artifact: %w", err), + sourcev1.ReadOperationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } if err = tar.Untar(f, sourceDir, tar.WithMaxUntarSize(-1)); err != nil { _ = f.Close() - return sreconcile.ResultEmpty, &serror.Event{ - Err: fmt.Errorf("artifact untar error: %w", err), - Reason: meta.FailedReason, - } + return sreconcile.ResultEmpty, serror.NewGeneric( + fmt.Errorf("artifact untar error: %w", err), + meta.FailedReason, + ) } if err = f.Close(); err != nil { - return sreconcile.ResultEmpty, &serror.Event{ - Err: fmt.Errorf("artifact close error: %w", err), - Reason: meta.FailedReason, - } + return sreconcile.ResultEmpty, serror.NewGeneric( + fmt.Errorf("artifact close error: %w", err), + meta.FailedReason, + ) } // Setup dependency manager @@ -834,19 +834,19 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, _ *patch.Se // Ensure artifact directory exists and acquire lock if err := r.Storage.MkdirAll(artifact); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to create artifact directory: %w", err), - Reason: sourcev1.DirCreationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to create artifact directory: %w", err), + sourcev1.DirCreationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } unlock, err := r.Storage.Lock(artifact) if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to acquire lock for artifact: %w", err), - Reason: sourcev1.AcquireLockFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to acquire lock for artifact: %w", err), + sourcev1.AcquireLockFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -854,10 +854,10 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, _ *patch.Se // Copy the packaged chart to the artifact path if err = r.Storage.CopyFromPath(&artifact, b.Path); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("unable to copy Helm chart to storage: %w", err), - Reason: sourcev1.ArchiveOperationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("unable to copy Helm chart to storage: %w", err), + sourcev1.ArchiveOperationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -938,10 +938,10 @@ func (r *HelmChartReconciler) reconcileDelete(ctx context.Context, obj *helmv1.H func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *helmv1.HelmChart) error { if !obj.DeletionTimestamp.IsZero() { if deleted, err := r.Storage.RemoveAll(r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), "", "*")); err != nil { - return &serror.Event{ - Err: fmt.Errorf("garbage collection for deleted resource failed: %w", err), - Reason: "GarbageCollectionFailed", - } + return serror.NewGeneric( + fmt.Errorf("garbage collection for deleted resource failed: %w", err), + "GarbageCollectionFailed", + ) } else if deleted != "" { r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "GarbageCollectionSucceeded", "garbage collected artifacts for deleted resource") @@ -952,10 +952,10 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *helmv1.He if obj.GetArtifact() != nil { delFiles, err := r.Storage.GarbageCollect(ctx, *obj.GetArtifact(), time.Second*5) if err != nil { - return &serror.Event{ - Err: fmt.Errorf("garbage collection of artifacts failed: %w", err), - Reason: "GarbageCollectionFailed", - } + return serror.NewGeneric( + fmt.Errorf("garbage collection of artifacts failed: %w", err), + "GarbageCollectionFailed", + ) } if len(delFiles) > 0 { r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "GarbageCollectionSucceeded", @@ -1275,17 +1275,17 @@ func reasonForBuild(build *chart.Build) string { func chartRepoConfigErrorReturn(err error, obj *helmv1.HelmChart) (sreconcile.Result, error) { switch err.(type) { case *url.Error: - e := &serror.Stalling{ - Err: fmt.Errorf("invalid Helm repository URL: %w", err), - Reason: sourcev1.URLInvalidReason, - } + e := serror.NewStalling( + fmt.Errorf("invalid Helm repository URL: %w", err), + sourcev1.URLInvalidReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e default: - e := &serror.Stalling{ - Err: fmt.Errorf("failed to construct Helm client: %w", err), - Reason: meta.FailedReason, - } + e := serror.NewStalling( + fmt.Errorf("failed to construct Helm client: %w", err), + meta.FailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index 9d45271dc..3d5fc5c7d 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -659,7 +659,7 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar") }, want: sreconcile.ResultEmpty, - wantErr: &serror.Event{Err: errors.New("gitrepositories.source.toolkit.fluxcd.io \"unavailable\" not found")}, + wantErr: &serror.Generic{Err: errors.New("gitrepositories.source.toolkit.fluxcd.io \"unavailable\" not found")}, assertFunc: func(g *WithT, build chart.Build, obj helmv1.HelmChart) { g.Expect(build.Complete()).To(BeFalse()) @@ -963,7 +963,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) { } }, want: sreconcile.ResultEmpty, - wantErr: &serror.Event{Err: errors.New("failed to get authentication secret '/invalid'")}, + wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret '/invalid'")}, assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) { g.Expect(build.Complete()).To(BeFalse()) @@ -1231,7 +1231,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { } }, want: sreconcile.ResultEmpty, - wantErr: &serror.Event{Err: errors.New("failed to get authentication secret '/invalid'")}, + wantErr: &serror.Generic{Err: errors.New("failed to get authentication secret '/invalid'")}, assertFunc: func(g *WithT, obj *helmv1.HelmChart, build chart.Build) { g.Expect(build.Complete()).To(BeFalse()) @@ -1463,7 +1463,7 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { name: "Empty source artifact", source: sourcev1.Artifact{}, want: sreconcile.ResultEmpty, - wantErr: &serror.Event{Err: errors.New("no such file or directory")}, + wantErr: &serror.Generic{Err: errors.New("no such file or directory")}, assertFunc: func(g *WithT, build chart.Build) { g.Expect(build.Complete()).To(BeFalse()) }, @@ -1472,7 +1472,7 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { name: "Invalid artifact type", source: *yamlArtifact, want: sreconcile.ResultEmpty, - wantErr: &serror.Event{Err: errors.New("artifact untar error: requires gzip-compressed body")}, + wantErr: &serror.Generic{Err: errors.New("artifact untar error: requires gzip-compressed body")}, assertFunc: func(g *WithT, build chart.Build) { g.Expect(build.Complete()).To(BeFalse()) }, diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index eb871a1f1..c8462bea6 100644 --- a/internal/controller/helmrepository_controller.go +++ b/internal/controller/helmrepository_controller.go @@ -177,7 +177,7 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque summarize.WithReconcileError(retErr), summarize.WithIgnoreNotFound(), summarize.WithProcessors( - summarize.RecordContextualError, + summarize.ErrorActionHandler, summarize.RecordReconcileReq, ), summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{ @@ -393,10 +393,10 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc obj *helmv1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { normalizedURL, err := repository.NormalizeURL(obj.Spec.URL) if err != nil { - e := &serror.Stalling{ - Err: fmt.Errorf("invalid Helm repository URL: %w", err), - Reason: sourcev1.URLInvalidReason, - } + e := serror.NewStalling( + fmt.Errorf("invalid Helm repository URL: %w", err), + sourcev1.URLInvalidReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -407,10 +407,10 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc ctrl.LoggerFrom(ctx). Info("warning: specifying TLS authentication data via `.spec.secretRef` is deprecated, please use `.spec.certSecretRef` instead") } else { - e := &serror.Event{ - Err: err, - Reason: sourcev1.AuthenticationFailedReason, - } + e := serror.NewGeneric( + err, + sourcev1.AuthenticationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -421,17 +421,17 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc if err != nil { switch err.(type) { case *url.Error: - e := &serror.Stalling{ - Err: fmt.Errorf("invalid Helm repository URL: %w", err), - Reason: sourcev1.URLInvalidReason, - } + e := serror.NewStalling( + fmt.Errorf("invalid Helm repository URL: %w", err), + sourcev1.URLInvalidReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e default: - e := &serror.Stalling{ - Err: fmt.Errorf("failed to construct Helm client: %w", err), - Reason: meta.FailedReason, - } + e := serror.NewStalling( + fmt.Errorf("failed to construct Helm client: %w", err), + meta.FailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -439,10 +439,10 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc // Fetch the repository index from remote. if err := newChartRepo.CacheIndex(); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to fetch Helm repository index: %w", err), - Reason: meta.FailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to fetch Helm repository index: %w", err), + meta.FailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) // Coin flip on transient or persistent error, return error and hope for the best return sreconcile.ResultEmpty, e @@ -465,10 +465,10 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc // Load the cached repository index to ensure it passes validation. if err := chartRepo.LoadFromPath(); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to load Helm repository from index YAML: %w", err), - Reason: helmv1.IndexationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to load Helm repository from index YAML: %w", err), + helmv1.IndexationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -478,10 +478,10 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc // Calculate revision. revision := chartRepo.Digest(intdigest.Canonical) if revision.Validate() != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to calculate revision: %w", err), - Reason: helmv1.IndexationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to calculate revision: %w", err), + helmv1.IndexationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -541,10 +541,10 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pa // Create artifact dir if err := r.Storage.MkdirAll(*artifact); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to create artifact directory: %w", err), - Reason: sourcev1.DirCreationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("failed to create artifact directory: %w", err), + sourcev1.DirCreationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -552,28 +552,28 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pa // Acquire lock. unlock, err := r.Storage.Lock(*artifact) if err != nil { - return sreconcile.ResultEmpty, &serror.Event{ - Err: fmt.Errorf("failed to acquire lock for artifact: %w", err), - Reason: meta.FailedReason, - } + return sreconcile.ResultEmpty, serror.NewGeneric( + fmt.Errorf("failed to acquire lock for artifact: %w", err), + meta.FailedReason, + ) } defer unlock() // Save artifact to storage in JSON format. b, err := chartRepo.ToJSON() if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("unable to get JSON index from chart repo: %w", err), - Reason: sourcev1.ArchiveOperationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("unable to get JSON index from chart repo: %w", err), + sourcev1.ArchiveOperationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } if err = r.Storage.Copy(artifact, bytes.NewBuffer(b)); err != nil { - e := &serror.Event{ - Err: fmt.Errorf("unable to save artifact to storage: %w", err), - Reason: sourcev1.ArchiveOperationFailedReason, - } + e := serror.NewGeneric( + fmt.Errorf("unable to save artifact to storage: %w", err), + sourcev1.ArchiveOperationFailedReason, + ) conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -639,10 +639,10 @@ func (r *HelmRepositoryReconciler) reconcileDelete(ctx context.Context, obj *hel func (r *HelmRepositoryReconciler) garbageCollect(ctx context.Context, obj *helmv1.HelmRepository) error { if !obj.DeletionTimestamp.IsZero() || (obj.Spec.Type != "" && obj.Spec.Type != helmv1.HelmRepositoryTypeDefault) { if deleted, err := r.Storage.RemoveAll(r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), "", "*")); err != nil { - return &serror.Event{ - Err: fmt.Errorf("garbage collection for deleted resource failed: %w", err), - Reason: "GarbageCollectionFailed", - } + return serror.NewGeneric( + fmt.Errorf("garbage collection for deleted resource failed: %w", err), + "GarbageCollectionFailed", + ) } else if deleted != "" { r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "GarbageCollectionSucceeded", "garbage collected artifacts for deleted resource") @@ -657,10 +657,10 @@ func (r *HelmRepositoryReconciler) garbageCollect(ctx context.Context, obj *helm if obj.GetArtifact() != nil { delFiles, err := r.Storage.GarbageCollect(ctx, *obj.GetArtifact(), time.Second*5) if err != nil { - return &serror.Event{ - Err: fmt.Errorf("garbage collection of artifacts failed: %w", err), - Reason: "GarbageCollectionFailed", - } + return serror.NewGeneric( + fmt.Errorf("garbage collection of artifacts failed: %w", err), + "GarbageCollectionFailed", + ) } if len(delFiles) > 0 { r.eventLogf(ctx, obj, eventv1.EventTypeTrace, "GarbageCollectionSucceeded", diff --git a/internal/error/error.go b/internal/error/error.go index 0852ba412..cb3a8cd78 100644 --- a/internal/error/error.go +++ b/internal/error/error.go @@ -90,28 +90,6 @@ func NewStalling(err error, reason string) *Stalling { } } -// Event is an error event. It can be used to construct an event to be -// recorded. -// Deprecated: use Generic error with NewGeneric() for the same behavior and -// replace the RecordContextualError with ErrorActionHandler for result -// processing. -type Event struct { - // Reason is the reason for the event error. - Reason string - // Error is the actual error for the event. - Err error -} - -// Error implements error interface. -func (ee *Event) Error() string { - return ee.Err.Error() -} - -// Unwrap returns the underlying error. -func (ee *Event) Unwrap() error { - return ee.Err -} - // Waiting is the reconciliation wait state error. It contains an error, wait // duration and a reason for the wait. It is a contextual error, used to express // the scenario which contributed to the reconciliation result. @@ -176,13 +154,13 @@ func (g *Generic) Unwrap() error { // NewGeneric constructs a new Generic error with default configuration. func NewGeneric(err error, reason string) *Generic { - // Since it's a error, ensure to log and send failure notification. + // Since it's a generic error, it'll be returned to the runtime and logged + // automatically, do not log it. Send failure notification. return &Generic{ Reason: reason, Err: err, Config: Config{ Event: corev1.EventTypeWarning, - Log: true, Notification: true, }, } diff --git a/internal/reconcile/summarize/processor.go b/internal/reconcile/summarize/processor.go index dcee87360..746ca7c8e 100644 --- a/internal/reconcile/summarize/processor.go +++ b/internal/reconcile/summarize/processor.go @@ -36,27 +36,6 @@ import ( // reconciliation failure. The errors can be recorded as logs and events. type ResultProcessor func(context.Context, kuberecorder.EventRecorder, client.Object, reconcile.Result, error) -// RecordContextualError is a ResultProcessor that records the contextual errors -// based on their types. -// An event is recorded for the errors that are returned to the runtime. The -// runtime handles the logging of the error. -// An event is recorded and an error is logged for errors that are known to be -// swallowed, not returned to the runtime. -func RecordContextualError(ctx context.Context, recorder kuberecorder.EventRecorder, obj client.Object, _ reconcile.Result, err error) { - switch e := err.(type) { - case *serror.Event: - recorder.Eventf(obj, corev1.EventTypeWarning, e.Reason, e.Error()) - case *serror.Waiting: - // Waiting errors are not returned to the runtime. Log it explicitly. - ctrl.LoggerFrom(ctx).Info("reconciliation waiting", "reason", e.Err, "duration", e.RequeueAfter) - recorder.Event(obj, corev1.EventTypeNormal, e.Reason, e.Error()) - case *serror.Stalling: - // Stalling errors are not returned to the runtime. Log it explicitly. - ctrl.LoggerFrom(ctx).Error(e, "reconciliation stalled") - recorder.Eventf(obj, corev1.EventTypeWarning, e.Reason, e.Error()) - } -} - // RecordReconcileReq is a ResultProcessor that checks the reconcile // annotation value and sets it in the object status as // status.lastHandledReconcileAt. diff --git a/internal/reconcile/summarize/summary_test.go b/internal/reconcile/summarize/summary_test.go index 6064fcbd9..c7703a940 100644 --- a/internal/reconcile/summarize/summary_test.go +++ b/internal/reconcile/summarize/summary_test.go @@ -357,7 +357,7 @@ func TestSummarizeAndPatch(t *testing.T) { WithReconcileError(tt.reconcileErr), WithConditions(tt.conditions...), WithIgnoreNotFound(), - WithProcessors(RecordContextualError, RecordReconcileReq), + WithProcessors(ErrorActionHandler, RecordReconcileReq), WithResultBuilder(reconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.Spec.Interval.Duration}), } if tt.bipolarConditions != nil {