From bb6a37f1c51cb318225bb33aa45931427f6e965d Mon Sep 17 00:00:00 2001 From: Sunny Date: Tue, 12 Sep 2023 16:34:12 +0000 Subject: [PATCH] Return generic error for patch failures Introduce a new event reason for patch operation failure and update all the returned errors from serial patcher to be a generic error so that they are handled like any other error with an associated warning event. Signed-off-by: Sunny --- api/v1/condition_types.go | 4 ++++ internal/controller/bucket_controller.go | 6 +++--- internal/controller/gitrepository_controller.go | 10 +++++----- internal/controller/helmchart_controller.go | 6 +++--- internal/controller/helmrepository_controller.go | 9 ++++----- internal/controller/ocirepository_controller.go | 6 +++--- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/api/v1/condition_types.go b/api/v1/condition_types.go index 21bb0bfb9..72c7e67a2 100644 --- a/api/v1/condition_types.go +++ b/api/v1/condition_types.go @@ -104,4 +104,8 @@ const ( // CacheOperationFailedReason signals a failure in cache operation. CacheOperationFailedReason string = "CacheOperationFailed" + + // PatchOperationFailedReason signals a failure in patching a kubernetes API + // object. + PatchOperationFailedReason string = "PatchOperationFailed" ) diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index d6598aea8..29c3c5da2 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -268,11 +268,11 @@ func (r *BucketReconciler) reconcile(ctx context.Context, sp *patch.SerialPatche rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } case recAtVal != obj.Status.GetLastHandledReconcileRequest(): if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } } @@ -402,7 +402,7 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, sp *patch.Seria rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg) conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } return sreconcile.ResultSuccess, nil } diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index cd6062dfd..60736b95c 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -264,11 +264,11 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Seria rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } case recAtVal != obj.Status.GetLastHandledReconcileRequest(): if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } } @@ -425,7 +425,7 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patc rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg) conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } return sreconcile.ResultSuccess, nil } @@ -527,7 +527,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch } rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } } conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition) @@ -601,7 +601,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch } rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } } return sreconcile.ResultSuccess, nil diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index 7ef584384..556253efe 100644 --- a/internal/controller/helmchart_controller.go +++ b/internal/controller/helmchart_controller.go @@ -283,11 +283,11 @@ func (r *HelmChartReconciler) reconcile(ctx context.Context, sp *patch.SerialPat rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest(): if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } } @@ -397,7 +397,7 @@ func (r *HelmChartReconciler) reconcileStorage(ctx context.Context, sp *patch.Se rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg) conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } return sreconcile.ResultSuccess, nil } diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index c8462bea6..8e252979a 100644 --- a/internal/controller/helmrepository_controller.go +++ b/internal/controller/helmrepository_controller.go @@ -246,11 +246,11 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Seri rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest(): if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } } @@ -368,7 +368,7 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *pat rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg) conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } return sreconcile.ResultSuccess, nil } @@ -493,8 +493,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patc } rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - ctrl.LoggerFrom(ctx).Error(err, "failed to patch") - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } // Create potential new artifact. diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index f10735408..8fddb4936 100644 --- a/internal/controller/ocirepository_controller.go +++ b/internal/controller/ocirepository_controller.go @@ -263,11 +263,11 @@ func (r *OCIRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Seria rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest(): if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } } @@ -913,7 +913,7 @@ func (r *OCIRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patc rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg) conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil { - return sreconcile.ResultEmpty, err + return sreconcile.ResultEmpty, serror.NewGeneric(err, sourcev1.PatchOperationFailedReason) } return sreconcile.ResultSuccess, nil }