diff --git a/e2e/testcases/oci_signature_verification_test.go b/e2e/testcases/oci_signature_verification_test.go index 024525762..dd1095066 100644 --- a/e2e/testcases/oci_signature_verification_test.go +++ b/e2e/testcases/oci_signature_verification_test.go @@ -63,7 +63,7 @@ func TestAddPreSyncAnnotationRepoSync(t *testing.T) { nt.Must(rootSyncGitRepo.Add(nomostest.StructuredNSPath(repoSyncID.Namespace, repoSyncID.Name), repoSyncOCI)) nt.Must(rootSyncGitRepo.CommitAndPush("Set the RepoSync to sync from OCI")) nt.Must(nt.Watcher.WatchObject(kinds.RepoSyncV1Beta1(), repoSyncID.Name, namespaceRepo, - testwatcher.WatchPredicates(testpredicates.RepoSyncHasSourceError(status.SourceErrorCode, "no signatures found")))) + testwatcher.WatchPredicates(testpredicates.RepoSyncHasSourceError(status.APIServerErrorCode, "no signatures found")))) nt.T.Log("Create second image with latest tag.") bookstoreSA := k8sobjects.ServiceAccountObject("bookstore-sa") @@ -81,8 +81,8 @@ func TestAddPreSyncAnnotationRepoSync(t *testing.T) { nt.Must( nt.Watcher.WatchObject(kinds.RepoSyncV1Beta1(), repoSyncID.Name, namespaceRepo, testwatcher.WatchPredicates( - testpredicates.RepoSyncHasSourceError(status.SourceErrorCode, "no signatures found"), - testpredicates.RepoSyncHasSourceError(status.SourceErrorCode, image1.Digest)))) + testpredicates.RepoSyncHasSourceError(status.APIServerErrorCode, "no signatures found"), + testpredicates.RepoSyncHasSourceError(status.APIServerErrorCode, image1.Digest)))) if err = signImage(nt, image1); err != nil { nt.T.Fatalf("Failed to sign second test image %s", err) @@ -139,7 +139,7 @@ func TestAddPreSyncAnnotationRootSync(t *testing.T) { rootSyncOCI := nt.RootSyncObjectOCI(rootSyncKey.Name, image.OCIImageID().WithoutDigest(), "", image.Digest) nt.Must(nt.KubeClient.Apply(rootSyncOCI)) nt.Must(nt.Watcher.WatchObject(kinds.RootSyncV1Beta1(), rootSyncID.Name, configsync.ControllerNamespace, - testwatcher.WatchPredicates(testpredicates.RootSyncHasSourceError(status.SourceErrorCode, "no signatures found")))) + testwatcher.WatchPredicates(testpredicates.RootSyncHasSourceError(status.APIServerErrorCode, "no signatures found")))) nt.T.Log("Create second image with latest tag.") bookstoreSA := k8sobjects.ServiceAccountObject("bookstore-sa") @@ -157,8 +157,8 @@ func TestAddPreSyncAnnotationRootSync(t *testing.T) { nt.Must( nt.Watcher.WatchObject(kinds.RootSyncV1Beta1(), rootSyncID.Name, configsync.ControllerNamespace, testwatcher.WatchPredicates( - testpredicates.RootSyncHasSourceError(status.SourceErrorCode, "no signatures found"), - testpredicates.RootSyncHasSourceError(status.SourceErrorCode, image1.Digest)))) + testpredicates.RootSyncHasSourceError(status.APIServerErrorCode, "no signatures found"), + testpredicates.RootSyncHasSourceError(status.APIServerErrorCode, image1.Digest)))) if err = signImage(nt, image1); err != nil { nt.T.Fatalf("Failed to sign second test image %s", err) diff --git a/pkg/parse/repo_sync_client.go b/pkg/parse/repo_sync_client.go index b79c7d8ae..75eef5f7c 100644 --- a/pkg/parse/repo_sync_client.go +++ b/pkg/parse/repo_sync_client.go @@ -48,15 +48,15 @@ type repoSyncStatusClient struct { // // SetSourceStatus sets the source status with a given source state and set of errors. If errs is empty, all errors // will be removed from the status. -func (p *repoSyncStatusClient) SetSourceStatus(ctx context.Context, newStatus *SourceStatus) error { +func (p *repoSyncStatusClient) SetSourceStatus(ctx context.Context, newStatus *SourceStatus) status.Error { p.mux.Lock() defer p.mux.Unlock() return p.setSourceStatusWithRetries(ctx, newStatus, defaultDenominator) } -func (p *repoSyncStatusClient) setSourceStatusWithRetries(ctx context.Context, newStatus *SourceStatus, denominator int) error { +func (p *repoSyncStatusClient) setSourceStatusWithRetries(ctx context.Context, newStatus *SourceStatus, denominator int) status.Error { if denominator <= 0 { - return fmt.Errorf("The denominator must be a positive number") + return status.InternalErrorf("denominator must be positive: %d", denominator) } opts := p.options @@ -111,7 +111,7 @@ func (p *repoSyncStatusClient) setSourceStatusWithRetries(ctx context.Context, n return nil } -func (p *repoSyncStatusClient) SetSourceAnnotations(ctx context.Context, commit string) error { +func (p *repoSyncStatusClient) SetImageToSyncAnnotation(ctx context.Context, commit string) status.Error { opts := p.options rs := &v1beta1.RepoSync{} rs.Namespace = string(opts.Scope) @@ -132,15 +132,13 @@ func (p *repoSyncStatusClient) SetSourceAnnotations(ctx context.Context, commit err := opts.Client.Patch(ctx, rs, client.RawPatch(types.MergePatchType, []byte(patch)), client.FieldOwner(configsync.FieldManager)) - if err != nil { - return fmt.Errorf("failed to patch RepoSync annotations: %w\nPatch content: %s", err, patch) + return status.APIServerErrorf(err, "failed to patch RepoSync annotation: %s", metadata.ImageToSyncAnnotationKey) } - return nil } -func (p *repoSyncStatusClient) SetRequiresRendering(ctx context.Context, renderingRequired bool) error { +func (p *repoSyncStatusClient) SetRequiresRenderingAnnotation(ctx context.Context, renderingRequired bool) status.Error { opts := p.options rs := &v1beta1.RepoSync{} if err := opts.Client.Get(ctx, reposync.ObjectKey(opts.Scope, opts.SyncName), rs); err != nil { @@ -153,11 +151,17 @@ func (p *repoSyncStatusClient) SetRequiresRendering(ctx context.Context, renderi } existing := rs.DeepCopy() core.SetAnnotation(rs, metadata.RequiresRenderingAnnotationKey, newVal) - return opts.Client.Patch(ctx, rs, client.MergeFrom(existing), client.FieldOwner(configsync.FieldManager)) + err := opts.Client.Patch(ctx, rs, + client.MergeFrom(existing), + client.FieldOwner(configsync.FieldManager)) + if err != nil { + return status.APIServerErrorf(err, "failed to patch RepoSync annotation: %s", metadata.RequiresRenderingAnnotationKey) + } + return nil } // SetRenderingStatus implements the Parser interface -func (p *repoSyncStatusClient) SetRenderingStatus(ctx context.Context, oldStatus, newStatus *RenderingStatus) error { +func (p *repoSyncStatusClient) SetRenderingStatus(ctx context.Context, oldStatus, newStatus *RenderingStatus) status.Error { if oldStatus.Equals(newStatus) { return nil } @@ -167,9 +171,9 @@ func (p *repoSyncStatusClient) SetRenderingStatus(ctx context.Context, oldStatus return p.setRenderingStatusWithRetries(ctx, newStatus, defaultDenominator) } -func (p *repoSyncStatusClient) setRenderingStatusWithRetries(ctx context.Context, newStatus *RenderingStatus, denominator int) error { +func (p *repoSyncStatusClient) setRenderingStatusWithRetries(ctx context.Context, newStatus *RenderingStatus, denominator int) status.Error { if denominator <= 0 { - return fmt.Errorf("The denominator must be a positive number") + return status.InternalErrorf("denominator must be positive: %d", denominator) } opts := p.options @@ -219,8 +223,8 @@ func (p *repoSyncStatusClient) setRenderingStatusWithRetries(ctx context.Context return nil } -// ReconcilerStatusFromCluster gets the RepoSync sync status from the cluster. -func (p *repoSyncStatusClient) ReconcilerStatusFromCluster(ctx context.Context) (*ReconcilerStatus, error) { +// GetReconcilerStatus gets the RepoSync sync status from the cluster. +func (p *repoSyncStatusClient) GetReconcilerStatus(ctx context.Context) (*ReconcilerStatus, status.Error) { opts := p.options rs := &v1beta1.RepoSync{} if err := opts.Client.Get(ctx, reposync.ObjectKey(opts.Scope, opts.SyncName), rs); err != nil { @@ -249,15 +253,15 @@ func (p *repoSyncStatusClient) ReconcilerStatusFromCluster(ctx context.Context) // SetSyncStatus implements the Parser interface // SetSyncStatus sets the RepoSync sync status. // `errs` includes the errors encountered during the apply step; -func (p *repoSyncStatusClient) SetSyncStatus(ctx context.Context, newStatus *SyncStatus) error { +func (p *repoSyncStatusClient) SetSyncStatus(ctx context.Context, newStatus *SyncStatus) status.Error { p.mux.Lock() defer p.mux.Unlock() return p.setSyncStatusWithRetries(ctx, newStatus, defaultDenominator) } -func (p *repoSyncStatusClient) setSyncStatusWithRetries(ctx context.Context, newStatus *SyncStatus, denominator int) error { +func (p *repoSyncStatusClient) setSyncStatusWithRetries(ctx context.Context, newStatus *SyncStatus, denominator int) status.Error { if denominator <= 0 { - return fmt.Errorf("The denominator must be a positive number") + return status.InternalErrorf("denominator must be positive: %d", denominator) } opts := p.options diff --git a/pkg/parse/root_sync_client.go b/pkg/parse/root_sync_client.go index 753dba983..10a343391 100644 --- a/pkg/parse/root_sync_client.go +++ b/pkg/parse/root_sync_client.go @@ -75,15 +75,15 @@ type rootSyncStatusClient struct { } // SetSourceStatus implements the Parser interface -func (p *rootSyncStatusClient) SetSourceStatus(ctx context.Context, newStatus *SourceStatus) error { +func (p *rootSyncStatusClient) SetSourceStatus(ctx context.Context, newStatus *SourceStatus) status.Error { p.mux.Lock() defer p.mux.Unlock() return p.setSourceStatusWithRetries(ctx, newStatus, defaultDenominator) } -func (p *rootSyncStatusClient) setSourceStatusWithRetries(ctx context.Context, newStatus *SourceStatus, denominator int) error { +func (p *rootSyncStatusClient) setSourceStatusWithRetries(ctx context.Context, newStatus *SourceStatus, denominator int) status.Error { if denominator <= 0 { - return fmt.Errorf("The denominator must be a positive number") + return status.InternalErrorf("denominator must be positive: %d", denominator) } opts := p.options @@ -176,7 +176,7 @@ func setSourceStatusFields(source *v1beta1.SourceStatus, newStatus *SourceStatus source.LastUpdate = newStatus.LastUpdate } -func (p *rootSyncStatusClient) SetSourceAnnotations(ctx context.Context, commit string) error { +func (p *rootSyncStatusClient) SetImageToSyncAnnotation(ctx context.Context, commit string) status.Error { opts := p.options rs := &v1beta1.RootSync{} rs.Namespace = configsync.ControllerNamespace @@ -197,15 +197,13 @@ func (p *rootSyncStatusClient) SetSourceAnnotations(ctx context.Context, commit err := opts.Client.Patch(ctx, rs, client.RawPatch(types.MergePatchType, []byte(patch)), client.FieldOwner(configsync.FieldManager)) - if err != nil { - return fmt.Errorf("failed to patch RootSync annotations: %w\nPatch content: %s", err, patch) + return status.APIServerErrorf(err, "failed to patch RootSync annotation: %s", metadata.ImageToSyncAnnotationKey) } - return nil } -func (p *rootSyncStatusClient) SetRequiresRendering(ctx context.Context, renderingRequired bool) error { +func (p *rootSyncStatusClient) SetRequiresRenderingAnnotation(ctx context.Context, renderingRequired bool) status.Error { opts := p.options rs := &v1beta1.RootSync{} if err := opts.Client.Get(ctx, rootsync.ObjectKey(opts.SyncName), rs); err != nil { @@ -218,11 +216,17 @@ func (p *rootSyncStatusClient) SetRequiresRendering(ctx context.Context, renderi } existing := rs.DeepCopy() core.SetAnnotation(rs, metadata.RequiresRenderingAnnotationKey, newVal) - return opts.Client.Patch(ctx, rs, client.MergeFrom(existing), client.FieldOwner(configsync.FieldManager)) + err := opts.Client.Patch(ctx, rs, + client.MergeFrom(existing), + client.FieldOwner(configsync.FieldManager)) + if err != nil { + return status.APIServerErrorf(err, "failed to patch RootSync annotation: %s", metadata.RequiresRenderingAnnotationKey) + } + return nil } // SetRenderingStatus implements the Parser interface -func (p *rootSyncStatusClient) SetRenderingStatus(ctx context.Context, oldStatus, newStatus *RenderingStatus) error { +func (p *rootSyncStatusClient) SetRenderingStatus(ctx context.Context, oldStatus, newStatus *RenderingStatus) status.Error { if oldStatus.Equals(newStatus) { return nil } @@ -232,9 +236,9 @@ func (p *rootSyncStatusClient) SetRenderingStatus(ctx context.Context, oldStatus return p.setRenderingStatusWithRetries(ctx, newStatus, defaultDenominator) } -func (p *rootSyncStatusClient) setRenderingStatusWithRetries(ctx context.Context, newStatus *RenderingStatus, denominator int) error { +func (p *rootSyncStatusClient) setRenderingStatusWithRetries(ctx context.Context, newStatus *RenderingStatus, denominator int) status.Error { if denominator <= 0 { - return fmt.Errorf("The denominator must be a positive number") + return status.InternalErrorf("denominator must be positive: %d", denominator) } opts := p.options @@ -327,8 +331,8 @@ func setRenderingStatusFields(rendering *v1beta1.RenderingStatus, newStatus *Ren rendering.LastUpdate = newStatus.LastUpdate } -// ReconcilerStatusFromCluster gets the RootSync sync status from the cluster. -func (p *rootSyncStatusClient) ReconcilerStatusFromCluster(ctx context.Context) (*ReconcilerStatus, error) { +// GetReconcilerStatus gets the RootSync sync status from the cluster. +func (p *rootSyncStatusClient) GetReconcilerStatus(ctx context.Context) (*ReconcilerStatus, status.Error) { opts := p.options rs := &v1beta1.RootSync{} if err := opts.Client.Get(ctx, rootsync.ObjectKey(opts.SyncName), rs); err != nil { @@ -461,15 +465,15 @@ func reconcilerStatusFromRSyncStatus(rsyncStatus v1beta1.Status, sourceType conf // SetSyncStatus implements the Parser interface // SetSyncStatus sets the RootSync sync status. // `errs` includes the errors encountered during the apply step; -func (p *rootSyncStatusClient) SetSyncStatus(ctx context.Context, newStatus *SyncStatus) error { +func (p *rootSyncStatusClient) SetSyncStatus(ctx context.Context, newStatus *SyncStatus) status.Error { p.mux.Lock() defer p.mux.Unlock() return p.setSyncStatusWithRetries(ctx, newStatus, defaultDenominator) } -func (p *rootSyncStatusClient) setSyncStatusWithRetries(ctx context.Context, newStatus *SyncStatus, denominator int) error { +func (p *rootSyncStatusClient) setSyncStatusWithRetries(ctx context.Context, newStatus *SyncStatus, denominator int) status.Error { if denominator <= 0 { - return fmt.Errorf("The denominator must be a positive number") + return status.InternalErrorf("denominator must be positive: %d", denominator) } opts := p.options @@ -589,9 +593,9 @@ func summarizeErrorsForCommit(sourceStatus v1beta1.SourceStatus, renderingStatus } // prependRootSyncRemediatorStatus adds the conflict error detected by the remediator to the front of the sync errors. -func prependRootSyncRemediatorStatus(ctx context.Context, c client.Client, syncName string, conflictErrs []status.ManagementConflictError, denominator int) error { +func prependRootSyncRemediatorStatus(ctx context.Context, c client.Client, syncName string, conflictErrs []status.ManagementConflictError, denominator int) status.Error { if denominator <= 0 { - return fmt.Errorf("The denominator must be a positive number") + return status.InternalErrorf("denominator must be positive: %d", denominator) } var rs v1beta1.RootSync diff --git a/pkg/parse/root_sync_client_test.go b/pkg/parse/root_sync_client_test.go index 6fc31ed9b..82cf077db 100644 --- a/pkg/parse/root_sync_client_test.go +++ b/pkg/parse/root_sync_client_test.go @@ -569,8 +569,8 @@ For more information, see https://g.co/cloud/acm-errors#knv1060` []status.ManagementConflictError{conflictAB}, defaultDenominator) require.NoError(t, err) var updatedRootSync v1beta1.RootSync - err = fakeClient.Get(ctx, rootsync.ObjectKey(rootSyncName), &updatedRootSync) - require.NoError(t, err) + statuserr := fakeClient.Get(ctx, rootsync.ObjectKey(rootSyncName), &updatedRootSync) + require.NoError(t, statuserr) testutil.AssertEqual(t, tc.expectedErrors, updatedRootSync.Status.Sync.Errors) }) } diff --git a/pkg/parse/run.go b/pkg/parse/run.go index c588684af..342762d09 100644 --- a/pkg/parse/run.go +++ b/pkg/parse/run.go @@ -93,9 +93,9 @@ func DefaultRunFunc(ctx context.Context, r Reconciler, trigger string) RunResult // Initialize status // TODO: Populate status from RSync status if state.status == nil { - reconcilerStatus, err := r.SyncStatusClient().ReconcilerStatusFromCluster(ctx) + reconcilerStatus, err := r.SyncStatusClient().GetReconcilerStatus(ctx) if err != nil { - state.invalidate(status.Append(nil, err)) + state.invalidate(err) return result } state.status = reconcilerStatus @@ -119,9 +119,8 @@ func DefaultRunFunc(ctx context.Context, r Reconciler, trigger string) RunResult // If updating the object fails, it's likely due to a signature verification error // from the webhook. In this case, add the error as a source error. if gs.Errs == nil { - err := r.SyncStatusClient().SetSourceAnnotations(ctx, gs.Commit) - if err != nil { - gs.Errs = status.Append(gs.Errs, status.SourceError.Wrap(err).Build()) + if err := r.SyncStatusClient().SetImageToSyncAnnotation(ctx, gs.Commit); err != nil { + gs.Errs = status.Append(gs.Errs, err) } } @@ -177,8 +176,7 @@ func DefaultRunFunc(ctx context.Context, r Reconciler, trigger string) RunResult state.status.RenderingStatus = rs state.status.SyncingConditionLastUpdate = rs.LastUpdate } else { - var m status.MultiError - state.invalidate(status.Append(m, setRenderingStatusErr)) + state.invalidate(setRenderingStatusErr) } return result } @@ -252,9 +250,8 @@ func (r *reconciler) Read(ctx context.Context, trigger string, sourceState *sour if opts.RenderingEnabled != hydrationStatus.RequiresRendering { // the reconciler is misconfigured. set the annotation so that the reconciler-manager // will recreate this reconciler with the correct configuration. - if err := r.SyncStatusClient().SetRequiresRendering(ctx, hydrationStatus.RequiresRendering); err != nil { - hydrationStatus.Errs = status.Append(hydrationStatus.Errs, - status.InternalHydrationError(err, "error setting %s annotation", metadata.RequiresRenderingAnnotationKey)) + if err := r.SyncStatusClient().SetRequiresRenderingAnnotation(ctx, hydrationStatus.RequiresRendering); err != nil { + hydrationStatus.Errs = status.Append(hydrationStatus.Errs, err) } } hydrationStatus.LastUpdate = nowMeta(opts) diff --git a/pkg/parse/sync_status_client.go b/pkg/parse/sync_status_client.go index 48b5216d5..b84f0a60b 100644 --- a/pkg/parse/sync_status_client.go +++ b/pkg/parse/sync_status_client.go @@ -16,20 +16,22 @@ package parse import ( "context" + + "kpt.dev/configsync/pkg/status" ) // SyncStatusClient provides methods to read and write RSync object status. type SyncStatusClient interface { - // ReconcilerStatusFromCluster reads the status of the reconciler from the RSync status. - ReconcilerStatusFromCluster(ctx context.Context) (*ReconcilerStatus, error) + // GetReconcilerStatus reads the status of the reconciler from the RSync status. + GetReconcilerStatus(ctx context.Context) (*ReconcilerStatus, status.Error) // SetSourceStatus sets the source status and syncing condition on the RSync. - SetSourceStatus(ctx context.Context, newStatus *SourceStatus) error + SetSourceStatus(ctx context.Context, newStatus *SourceStatus) status.Error // SetRenderingStatus sets the rendering status and syncing condition on the RSync. - SetRenderingStatus(ctx context.Context, oldStatus, newStatus *RenderingStatus) error + SetRenderingStatus(ctx context.Context, oldStatus, newStatus *RenderingStatus) status.Error // SetSyncStatus sets the sync status and syncing condition on the RSync. - SetSyncStatus(ctx context.Context, newStatus *SyncStatus) error - // SetRequiresRendering sets the requires-rendering annotation on the RSync. - SetRequiresRendering(ctx context.Context, renderingRequired bool) error - // SetSourceAnnotations sets the source annotations on the RSync. - SetSourceAnnotations(ctx context.Context, commit string) error + SetSyncStatus(ctx context.Context, newStatus *SyncStatus) status.Error + // SetRequiresRenderingAnnotation sets the requires-rendering annotation on the RSync. + SetRequiresRenderingAnnotation(ctx context.Context, renderingRequired bool) status.Error + // SetImageToSyncAnnotation sets the source annotations on the RSync. + SetImageToSyncAnnotation(ctx context.Context, commit string) status.Error }