Skip to content

Commit

Permalink
Expose transient errors in the R*Sync status field (#887)
Browse files Browse the repository at this point in the history
The intent is to indicate the processing status.
  • Loading branch information
nan-yu authored and google-oss-prow[bot] committed Sep 29, 2023
1 parent f864367 commit fd30981
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 49 deletions.
8 changes: 0 additions & 8 deletions pkg/parse/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,14 +296,6 @@ func read(ctx context.Context, p Parser, trigger string, state *reconcilerState,
status.InternalHydrationError(err, "error setting %s annotation", metadata.RequiresRenderingAnnotationKey))
}
}
// Return the transient errors here to avoid surfacing them to the R*Sync status field.
// The transient errors might be auto-resolved in the next retry loop, so no need to expose to users.
if status.HasTransientErrors(hydrationStatus.errs) {
return hydrationStatus.errs
}
if status.HasTransientErrors(sourceStatus.errs) {
return sourceStatus.errs
}
hydrationStatus.lastUpdate = metav1.Now()
// update the rendering status before source status because the parser needs to
// read and parse the configs after rendering is done and there might have errors.
Expand Down
40 changes: 22 additions & 18 deletions pkg/parse/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,26 +249,30 @@ func TestRun(t *testing.T) {
expectedMsg: "Sync Completed",
},
{
id: "7",
name: "error because hydration enabled with wet source",
renderingEnabled: true,
hasKustomization: false,
hydratedRootExist: false,
hydrationDone: true,
needRetry: true,
expectedMsg: "Rendering is still in progress",
expectedErrors: "KNV2016: sync source contains only wet configs and hydration-controller is running\n\nFor more information, see https://g.co/cloud/acm-errors#knv2016",
id: "7",
name: "error because hydration enabled with wet source",
renderingEnabled: true,
hasKustomization: false,
hydratedRootExist: false,
hydrationDone: true,
needRetry: true,
expectedMsg: "Rendering not required but is currently enabled",
expectedErrorSourceRefs: []v1beta1.ErrorSource{v1beta1.RenderingError},
expectedErrors: "1 error(s)\n\n\n[1] KNV2016: sync source contains only wet configs and hydration-controller is running\n\nFor more information, see https://g.co/cloud/acm-errors#knv2016\n",
expectedStateRenderingErrs: status.HydrationError(status.TransientErrorCode, fmt.Errorf("sync source contains only wet configs and hydration-controller is running")),
},
{
id: "8",
name: "error because hydration disabled with dry source",
renderingEnabled: false,
hasKustomization: true,
hydratedRootExist: true,
hydrationDone: true,
needRetry: true,
expectedMsg: "Rendering is still in progress",
expectedErrors: "KNV2016: sync source contains dry configs and hydration-controller is not running\n\nFor more information, see https://g.co/cloud/acm-errors#knv2016",
id: "8",
name: "error because hydration disabled with dry source",
renderingEnabled: false,
hasKustomization: true,
hydratedRootExist: true,
hydrationDone: true,
needRetry: true,
expectedMsg: "Rendering required but is currently disabled",
expectedErrorSourceRefs: []v1beta1.ErrorSource{v1beta1.RenderingError},
expectedErrors: "1 error(s)\n\n\n[1] KNV2016: sync source contains dry configs and hydration-controller is not running\n\nFor more information, see https://g.co/cloud/acm-errors#knv2016\n",
expectedStateRenderingErrs: status.HydrationError(status.TransientErrorCode, fmt.Errorf("sync source contains dry configs and hydration-controller is not running")),
},
}

Expand Down
6 changes: 0 additions & 6 deletions pkg/parse/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,6 @@ func (s *reconcilerState) resetPartialCache() {

// needToSetSourceStatus returns true if `p.setSourceStatus` should be called.
func (s *reconcilerState) needToSetSourceStatus(newStatus sourceStatus) bool {
if status.HasTransientErrors(newStatus.errs) {
return false
}
// Update if not initialized
if s.sourceStatus.lastUpdate.IsZero() {
return true
Expand All @@ -180,9 +177,6 @@ func (s *reconcilerState) needToSetSourceStatus(newStatus sourceStatus) bool {

// needToSetSyncStatus returns true if `p.SetSyncStatus` should be called.
func (s *reconcilerState) needToSetSyncStatus(newStatus syncStatus) bool {
if status.HasTransientErrors(newStatus.errs) {
return false
}
// Update if not initialized
if s.syncStatus.lastUpdate.IsZero() {
return true
Expand Down
17 changes: 0 additions & 17 deletions pkg/status/multierror.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,6 @@ var nonBlockingErrorCodes = map[string]struct{}{
EncodeDeclaredFieldErrorCode: {},
}

// HasTransientErrors return whether `errs` include any transient errors.
//
// A transient error is not exposed in the R*Sync API, and is expected to be
// auto-resolved in the next retry loop.
func HasTransientErrors(errs MultiError) bool {
if errs == nil {
return false
}

for _, err := range errs.Errors() {
if err.Code() == TransientErrorCode {
return true
}
}
return false
}

// HasBlockingErrors return whether `errs` include any blocking errors.
//
// An error is blocking if it requires the users to do something so that
Expand Down

0 comments on commit fd30981

Please sign in to comment.