diff --git a/pkg/applier/kpt_applier.go b/pkg/applier/kpt_applier.go index 94782f7b94..1757c05d78 100644 --- a/pkg/applier/kpt_applier.go +++ b/pkg/applier/kpt_applier.go @@ -278,6 +278,7 @@ func handleApplySkippedEvent(obj *unstructured.Unstructured, id core.ID, err err return SkipErrorForResource(err, id, actuation.ActuationStrategyApply) } +// processPruneEvent handles PruneEvents from the Applier func processPruneEvent(ctx context.Context, e event.PruneEvent, s *stats.PruneEventStats, objectStatusMap ObjectStatusMap, cs *clientSet) status.Error { id := idFrom(e.Identifier) klog.V(4).Infof("prune %v for object: %v", e.Status, id) @@ -411,7 +412,7 @@ func (a *Applier) applyInner(ctx context.Context, objs []client.Object) (map[sch // through annotation. enabledObjs, disabledObjs := partitionObjs(objs) if len(disabledObjs) > 0 { - klog.Infof("%v objects to be disabled: %v", len(disabledObjs), core.GKNNs(disabledObjs)) + klog.Infof("Applier disabling %d objects: %v", len(disabledObjs), core.GKNNs(disabledObjs)) disabledCount, err := cs.handleDisabledObjects(ctx, a.inventory, disabledObjs) if err != nil { a.addError(err) @@ -422,7 +423,7 @@ func (a *Applier) applyInner(ctx context.Context, objs []client.Object) (map[sch Succeeded: disabledCount, } } - klog.Infof("%v objects to be applied: %v", len(enabledObjs), core.GKNNs(enabledObjs)) + klog.Infof("Applier applying %d objects: %v", len(enabledObjs), core.GKNNs(enabledObjs)) resources, err := toUnstructured(enabledObjs) if err != nil { a.addError(err) @@ -460,7 +461,7 @@ func (a *Applier) applyInner(ctx context.Context, objs []client.Object) (map[sch switch e.Type { case event.InitType: for _, ag := range e.InitEvent.ActionGroups { - klog.Info("InitEvent", ag) + klog.Info("Applier InitEvent", ag) } case event.ActionGroupType: klog.Info(e.ActionGroupEvent) @@ -502,7 +503,7 @@ func (a *Applier) applyInner(ctx context.Context, objs []client.Object) (map[sch klog.V(4).Info(logEvent) a.addError(processPruneEvent(ctx, e.PruneEvent, s.PruneEvent, objStatusMap, cs)) default: - klog.V(4).Infof("skipped %v event", e.Type) + klog.V(4).Infof("Applier skipped %v event", e.Type) } } @@ -517,12 +518,12 @@ func (a *Applier) applyInner(ctx context.Context, objs []client.Object) (map[sch errs := a.Errors() if errs == nil { - klog.V(4).Infof("all resources are up to date.") + klog.V(4).Infof("Applier completed without error") } if s.Empty() { - klog.V(4).Infof("The applier made no new progress") + klog.V(4).Infof("Applier made no new progress") } else { - klog.Infof("The applier made new progress: %s", s.String()) + klog.Infof("Applier made new progress: %s", s.String()) objStatusMap.Log(klog.V(0)) } return gvks, errs @@ -558,6 +559,7 @@ func (a *Applier) invalidateErrors() { a.errs = nil } +// Apply all managed resource objects and return any errors. // Apply implements Interface. func (a *Applier) Apply(ctx context.Context, desiredResource []client.Object) (map[schema.GroupVersionKind]struct{}, status.MultiError) { a.applyMux.Lock() diff --git a/pkg/declared/resources.go b/pkg/declared/resources.go index 8c2e904f5b..0b8e38e76d 100644 --- a/pkg/declared/resources.go +++ b/pkg/declared/resources.go @@ -41,7 +41,9 @@ type Resources struct { objectSet map[core.ID]*unstructured.Unstructured } -// Update performs an atomic update on the resource declaration set. +// Update performs an atomic update on the resource declaration set, converts +// the objects to Unstructured and validating that not all namespaces are +// deleted at once. func (r *Resources) Update(ctx context.Context, objects []client.Object) ([]client.Object, status.Error) { // First build up the new map using a local pointer/reference. newSet := make(map[core.ID]*unstructured.Unstructured) diff --git a/pkg/metrics/tagkeys.go b/pkg/metrics/tagkeys.go index 26c5586188..4fc558954b 100644 --- a/pkg/metrics/tagkeys.go +++ b/pkg/metrics/tagkeys.go @@ -114,7 +114,7 @@ func StatusTagKey(err error) string { // StatusTagValueFromSummary returns error if the summary indicates at least 1 // error, otherwise success. func StatusTagValueFromSummary(summary *v1beta1.ErrorSummary) string { - if summary.TotalCount == 0 { + if summary == nil || summary.TotalCount == 0 { return StatusSuccess } return StatusError diff --git a/pkg/parse/namespace.go b/pkg/parse/namespace.go index 89a61dbc23..b3d6e0a3c1 100644 --- a/pkg/parse/namespace.go +++ b/pkg/parse/namespace.go @@ -41,6 +41,7 @@ import ( ) // NewNamespaceRunner creates a new runnable parser for parsing a Namespace repo. +// TODO: replace with builder pattern to avoid too many arguments. func NewNamespaceRunner(clusterName, syncName, reconcilerName string, scope declared.Scope, fileReader reader.Reader, c client.Client, pollingPeriod, resyncPeriod, retryPeriod, statusUpdatePeriod time.Duration, fs FileSource, dc discovery.DiscoveryInterface, resources *declared.Resources, app applier.KptApplier, rem remediator.Interface) (Parser, error) { converter, err := declared.NewValueConverter(dc) if err != nil { @@ -161,13 +162,14 @@ func (p *namespace) setSourceStatusWithRetries(ctx context.Context, newStatus so currentRS := rs.DeepCopy() setSourceStatusFields(&rs.Status.Source, p, newStatus, denominator) + errorSources, errorSummary := summarizeErrors(rs.Status.Status, newStatus.commit) + syncErrorCount := totalErrors(errorSummary) - continueSyncing := (rs.Status.Source.ErrorSummary.TotalCount == 0) - var errorSource []v1beta1.ErrorSource - if len(rs.Status.Source.Errors) > 0 { - errorSource = []v1beta1.ErrorSource{v1beta1.SourceError} - } - reposync.SetSyncing(&rs, continueSyncing, "Source", "Source", newStatus.commit, errorSource, rs.Status.Source.ErrorSummary, newStatus.lastUpdate) + // Syncing is stopped/prevented if there are any errors from the same commit. + // TODO: Handle non-blocking errors + syncing := (syncErrorCount == 0) + + reposync.SetSyncing(&rs, syncing, "Source", "Source", newStatus.commit, errorSources, errorSummary, newStatus.lastUpdate) // Avoid unnecessary status updates. if !currentRS.Status.Source.LastUpdate.IsZero() && cmp.Equal(currentRS.Status, rs.Status, compare.IgnoreTimestampUpdates) { @@ -191,7 +193,8 @@ func (p *namespace) setSourceStatusWithRetries(ctx context.Context, newStatus so if err := p.client.Status().Update(ctx, &rs); err != nil { // If the update failure was caused by the size of the RepoSync object, we would truncate the errors and retry. if isRequestTooLargeError(err) { - klog.Infof("Failed to update RepoSync source status (total error count: %d, denominator: %d): %s.", rs.Status.Source.ErrorSummary.TotalCount, denominator, err) + klog.Infof("Failed to update RepoSync source status (total error count: %d, denominator: %d): %s.", + syncErrorCount, denominator, err) return p.setSourceStatusWithRetries(ctx, newStatus, denominator*2) } return status.APIServerError(err, "failed to update RepoSync source status from parser") @@ -201,7 +204,7 @@ func (p *namespace) setSourceStatusWithRetries(ctx context.Context, newStatus so // setRenderingStatus implements the Parser interface func (p *namespace) setRenderingStatus(ctx context.Context, oldStatus, newStatus renderingStatus) error { - if oldStatus.equal(newStatus) { + if oldStatus.Equal(newStatus) { return nil } @@ -223,13 +226,14 @@ func (p *namespace) setRenderingStatusWithRetires(ctx context.Context, newStatus currentRS := rs.DeepCopy() setRenderingStatusFields(&rs.Status.Rendering, p, newStatus, denominator) + errorSources, errorSummary := summarizeErrors(rs.Status.Status, newStatus.commit) + syncErrorCount := totalErrors(errorSummary) - continueSyncing := (rs.Status.Rendering.ErrorSummary.TotalCount == 0) - var errorSource []v1beta1.ErrorSource - if len(rs.Status.Rendering.Errors) > 0 { - errorSource = []v1beta1.ErrorSource{v1beta1.RenderingError} - } - reposync.SetSyncing(&rs, continueSyncing, "Rendering", newStatus.message, newStatus.commit, errorSource, rs.Status.Rendering.ErrorSummary, newStatus.lastUpdate) + // Syncing is stopped/prevented if there are any errors from the same commit. + // TODO: Handle non-blocking errors + syncing := (syncErrorCount == 0) + + reposync.SetSyncing(&rs, syncing, "Rendering", newStatus.message, newStatus.commit, errorSources, errorSummary, newStatus.lastUpdate) // Avoid unnecessary status updates. if !currentRS.Status.Rendering.LastUpdate.IsZero() && cmp.Equal(currentRS.Status, rs.Status, compare.IgnoreTimestampUpdates) { @@ -253,7 +257,8 @@ func (p *namespace) setRenderingStatusWithRetires(ctx context.Context, newStatus if err := p.client.Status().Update(ctx, &rs); err != nil { // If the update failure was caused by the size of the RepoSync object, we would truncate the errors and retry. if isRequestTooLargeError(err) { - klog.Infof("Failed to update RepoSync rendering status (total error count: %d, denominator: %d): %s.", rs.Status.Rendering.ErrorSummary.TotalCount, denominator, err) + klog.Infof("Failed to update RepoSync rendering status (total error count: %d, denominator: %d): %s.", + syncErrorCount, denominator, err) return p.setRenderingStatusWithRetires(ctx, newStatus, denominator*2) } return status.APIServerError(err, "failed to update RepoSync rendering status from parser") @@ -283,16 +288,19 @@ func (p *namespace) setSyncStatusWithRetries(ctx context.Context, newStatus sync currentRS := rs.DeepCopy() setSyncStatusFields(&rs.Status.Status, newStatus, denominator) + errorSources, errorSummary := summarizeErrors(rs.Status.Status, newStatus.commit) + syncErrorCount := totalErrors(errorSummary) - errorSources, errorSummary := summarizeErrors(rs.Status.Source, rs.Status.Sync) + var message string if newStatus.syncing { - reposync.SetSyncing(rs, true, "Sync", "Syncing", rs.Status.Sync.Commit, errorSources, errorSummary, rs.Status.Sync.LastUpdate) + message = "Syncing" } else { - if errorSummary.TotalCount == 0 { + message = "Sync Completed" + if syncErrorCount == 0 { rs.Status.LastSyncedCommit = rs.Status.Sync.Commit } - reposync.SetSyncing(rs, false, "Sync", "Sync Completed", rs.Status.Sync.Commit, errorSources, errorSummary, rs.Status.Sync.LastUpdate) } + reposync.SetSyncing(rs, newStatus.syncing, "Sync", message, rs.Status.Sync.Commit, errorSources, errorSummary, rs.Status.Sync.LastUpdate) // Avoid unnecessary status updates. if !currentRS.Status.Sync.LastUpdate.IsZero() && cmp.Equal(currentRS.Status, rs.Status, compare.IgnoreTimestampUpdates) { @@ -312,14 +320,15 @@ func (p *namespace) setSyncStatusWithRetries(ctx context.Context, newStatus sync } if klog.V(5).Enabled() { - klog.Infof("Updating status for RepoSync %s/%s:\nDiff (- Expected, + Actual):\n%s", + klog.Infof("Updating sync status for RepoSync %s/%s:\nDiff (- Old, + New):\n%s", rs.Namespace, rs.Name, cmp.Diff(currentRS.Status, rs.Status)) } if err := p.client.Status().Update(ctx, rs); err != nil { // If the update failure was caused by the size of the RepoSync object, we would truncate the errors and retry. if isRequestTooLargeError(err) { - klog.Infof("Failed to update RepoSync sync status (total error count: %d, denominator: %d): %s.", rs.Status.Sync.ErrorSummary.TotalCount, denominator, err) + klog.Infof("Failed to update RepoSync sync status (total error count: %d, denominator: %d): %s.", + syncErrorCount, denominator, err) return p.setSyncStatusWithRetries(ctx, newStatus, denominator*2) } return status.APIServerError(err, fmt.Sprintf("failed to update the RepoSync sync status for the %v namespace", p.scope)) diff --git a/pkg/parse/root.go b/pkg/parse/root.go index b6f4513aa5..505f7870f7 100644 --- a/pkg/parse/root.go +++ b/pkg/parse/root.go @@ -174,13 +174,14 @@ func (p *root) setSourceStatusWithRetries(ctx context.Context, newStatus sourceS currentRS := rs.DeepCopy() setSourceStatusFields(&rs.Status.Source, p, newStatus, denominator) + errorSources, errorSummary := summarizeErrors(rs.Status.Status, newStatus.commit) + syncErrorCount := totalErrors(errorSummary) - continueSyncing := (rs.Status.Source.ErrorSummary.TotalCount == 0) - var errorSource []v1beta1.ErrorSource - if len(rs.Status.Source.Errors) > 0 { - errorSource = []v1beta1.ErrorSource{v1beta1.SourceError} - } - rootsync.SetSyncing(&rs, continueSyncing, "Source", "Source", newStatus.commit, errorSource, rs.Status.Source.ErrorSummary, newStatus.lastUpdate) + // Syncing is stopped/prevented if there are any errors from the same commit. + // TODO: Handle non-blocking errors + syncing := (syncErrorCount == 0) + + rootsync.SetSyncing(&rs, syncing, "Source", "Source", newStatus.commit, errorSources, errorSummary, newStatus.lastUpdate) // Avoid unnecessary status updates. if !currentRS.Status.Source.LastUpdate.IsZero() && cmp.Equal(currentRS.Status, rs.Status, compare.IgnoreTimestampUpdates) { @@ -197,14 +198,15 @@ func (p *root) setSourceStatusWithRetries(ctx context.Context, newStatus sourceS } if klog.V(5).Enabled() { - klog.Infof("Updating source status for RootSync %s/%s:\nDiff (- Expected, + Actual):\n%s", + klog.Infof("Updating source status for RootSync %s/%s:\nDiff (- Old, + New):\n%s", rs.Namespace, rs.Name, cmp.Diff(currentRS.Status, rs.Status)) } if err := p.client.Status().Update(ctx, &rs); err != nil { // If the update failure was caused by the size of the RootSync object, we would truncate the errors and retry. if isRequestTooLargeError(err) { - klog.Infof("Failed to update RootSync source status (total error count: %d, denominator: %d): %s.", rs.Status.Source.ErrorSummary.TotalCount, denominator, err) + klog.Infof("Failed to update RootSync source status (total error count: %d, denominator: %d): %s.", + syncErrorCount, denominator, err) return p.setSourceStatusWithRetries(ctx, newStatus, denominator*2) } return status.APIServerError(err, "failed to update RootSync source status from parser") @@ -241,19 +243,23 @@ func setSourceStatusFields(source *v1beta1.SourceStatus, p Parser, newStatus sou source.Git = nil source.Oci = nil } - errorSummary := &v1beta1.ErrorSummary{ - TotalCount: len(cse), - Truncated: denominator != 1, - ErrorCountAfterTruncation: len(cse) / denominator, + if len(cse) > 0 { + source.Errors = cse[0 : len(cse)/denominator] + source.ErrorSummary = &v1beta1.ErrorSummary{ + TotalCount: len(cse), + Truncated: denominator != 1, + ErrorCountAfterTruncation: len(cse) / denominator, + } + } else { + source.Errors = nil + source.ErrorSummary = nil } - source.Errors = cse[0 : len(cse)/denominator] - source.ErrorSummary = errorSummary source.LastUpdate = newStatus.lastUpdate } // setRenderingStatus implements the Parser interface func (p *root) setRenderingStatus(ctx context.Context, oldStatus, newStatus renderingStatus) error { - if oldStatus.equal(newStatus) { + if oldStatus.Equal(newStatus) { return nil } @@ -275,13 +281,14 @@ func (p *root) setRenderingStatusWithRetires(ctx context.Context, newStatus rend currentRS := rs.DeepCopy() setRenderingStatusFields(&rs.Status.Rendering, p, newStatus, denominator) + errorSources, errorSummary := summarizeErrors(rs.Status.Status, newStatus.commit) + syncErrorCount := totalErrors(errorSummary) - continueSyncing := (rs.Status.Rendering.ErrorSummary.TotalCount == 0) - var errorSource []v1beta1.ErrorSource - if len(rs.Status.Rendering.Errors) > 0 { - errorSource = []v1beta1.ErrorSource{v1beta1.RenderingError} - } - rootsync.SetSyncing(&rs, continueSyncing, "Rendering", newStatus.message, newStatus.commit, errorSource, rs.Status.Rendering.ErrorSummary, newStatus.lastUpdate) + // Syncing is stopped/prevented if there are any errors from the same commit. + // TODO: Handle non-blocking errors + syncing := (syncErrorCount == 0) + + rootsync.SetSyncing(&rs, syncing, "Rendering", newStatus.message, newStatus.commit, errorSources, errorSummary, newStatus.lastUpdate) // Avoid unnecessary status updates. if !currentRS.Status.Rendering.LastUpdate.IsZero() && cmp.Equal(currentRS.Status, rs.Status, compare.IgnoreTimestampUpdates) { @@ -298,14 +305,15 @@ func (p *root) setRenderingStatusWithRetires(ctx context.Context, newStatus rend } if klog.V(5).Enabled() { - klog.Infof("Updating rendering status for RootSync %s/%s:\nDiff (- Expected, + Actual):\n%s", + klog.Infof("Updating rendering status for RootSync %s/%s:\nDiff (- Old, + New):\n%s", rs.Namespace, rs.Name, cmp.Diff(currentRS.Status, rs.Status)) } if err := p.client.Status().Update(ctx, &rs); err != nil { // If the update failure was caused by the size of the RootSync object, we would truncate the errors and retry. if isRequestTooLargeError(err) { - klog.Infof("Failed to update RootSync rendering status (total error count: %d, denominator: %d): %s.", rs.Status.Rendering.ErrorSummary.TotalCount, denominator, err) + klog.Infof("Failed to update RootSync rendering status (total error count: %d, denominator: %d): %s.", + syncErrorCount, denominator, err) return p.setRenderingStatusWithRetires(ctx, newStatus, denominator*2) } return status.APIServerError(err, "failed to update RootSync rendering status from parser") @@ -343,12 +351,17 @@ func setRenderingStatusFields(rendering *v1beta1.RenderingStatus, p Parser, newS rendering.Oci = nil } rendering.Message = newStatus.message - errorSummary := &v1beta1.ErrorSummary{ - TotalCount: len(cse), - Truncated: denominator != 1, + if len(cse) > 0 { + rendering.Errors = cse[0 : len(cse)/denominator] + rendering.ErrorSummary = &v1beta1.ErrorSummary{ + TotalCount: len(cse), + Truncated: denominator != 1, + ErrorCountAfterTruncation: len(cse) / denominator, + } + } else { + rendering.Errors = nil + rendering.ErrorSummary = nil } - rendering.Errors = cse[0 : len(cse)/denominator] - rendering.ErrorSummary = errorSummary rendering.LastUpdate = newStatus.lastUpdate } @@ -374,16 +387,19 @@ func (p *root) setSyncStatusWithRetries(ctx context.Context, newStatus syncStatu currentRS := rs.DeepCopy() setSyncStatusFields(&rs.Status.Status, newStatus, denominator) + errorSources, errorSummary := summarizeErrors(rs.Status.Status, newStatus.commit) + syncErrorCount := totalErrors(errorSummary) - errorSources, errorSummary := summarizeErrors(rs.Status.Source, rs.Status.Sync) + var message string if newStatus.syncing { - rootsync.SetSyncing(rs, true, "Sync", "Syncing", rs.Status.Sync.Commit, errorSources, errorSummary, rs.Status.Sync.LastUpdate) + message = "Syncing" } else { - if errorSummary.TotalCount == 0 { + message = "Sync Completed" + if syncErrorCount == 0 { rs.Status.LastSyncedCommit = rs.Status.Sync.Commit } - rootsync.SetSyncing(rs, false, "Sync", "Sync Completed", rs.Status.Sync.Commit, errorSources, errorSummary, rs.Status.Sync.LastUpdate) } + rootsync.SetSyncing(rs, newStatus.syncing, "Sync", message, rs.Status.Sync.Commit, errorSources, errorSummary, rs.Status.Sync.LastUpdate) // Avoid unnecessary status updates. if !currentRS.Status.Sync.LastUpdate.IsZero() && cmp.Equal(currentRS.Status, rs.Status, compare.IgnoreTimestampUpdates) { @@ -403,14 +419,15 @@ func (p *root) setSyncStatusWithRetries(ctx context.Context, newStatus syncStatu } if klog.V(5).Enabled() { - klog.Infof("Updating sync status for RootSync %s/%s:\nDiff (- Expected, + Actual):\n%s", + klog.Infof("Updating sync status for RootSync %s/%s:\nDiff (- Old, + New):\n%s", rs.Namespace, rs.Name, cmp.Diff(currentRS.Status, rs.Status)) } if err := p.client.Status().Update(ctx, rs); err != nil { // If the update failure was caused by the size of the RootSync object, we would truncate the errors and retry. if isRequestTooLargeError(err) { - klog.Infof("Failed to update RootSync sync status (total error count: %d, denominator: %d): %s.", rs.Status.Sync.ErrorSummary.TotalCount, denominator, err) + klog.Infof("Failed to update RootSync sync status (total error count: %d, denominator: %d): %s.", + syncErrorCount, denominator, err) return p.setSyncStatusWithRetries(ctx, newStatus, denominator*2) } return status.APIServerError(err, "failed to update RootSync sync status") @@ -429,35 +446,62 @@ func setSyncStatusFields(syncStatus *v1beta1.Status, newStatus syncStatus, denom } func setSyncStatusErrors(syncStatus *v1beta1.Status, cse []v1beta1.ConfigSyncError, denominator int) { - syncStatus.Sync.ErrorSummary = &v1beta1.ErrorSummary{ - TotalCount: len(cse), - Truncated: denominator != 1, + if len(cse) > 0 { + syncStatus.Sync.ErrorSummary = &v1beta1.ErrorSummary{ + TotalCount: len(cse), + Truncated: denominator != 1, + } + syncStatus.Sync.Errors = cse[0 : len(cse)/denominator] + } else { + syncStatus.Sync.Errors = nil + syncStatus.Sync.ErrorSummary = nil } - syncStatus.Sync.Errors = cse[0 : len(cse)/denominator] } -// summarizeErrors summarizes the errors from `sourceStatus` and `syncStatus`, and returns an ErrorSource slice and an ErrorSummary. -func summarizeErrors(sourceStatus v1beta1.SourceStatus, syncStatus v1beta1.SyncStatus) ([]v1beta1.ErrorSource, *v1beta1.ErrorSummary) { +// summarizeErrors summarizes the rendering, source, and sync errors that +// correspond to the specified commit. +// Returns an ErrorSource slice and ErrorSummary. +func summarizeErrors(rsStatus v1beta1.Status, commit string) ([]v1beta1.ErrorSource, *v1beta1.ErrorSummary) { + var errorSummary v1beta1.ErrorSummary var errorSources []v1beta1.ErrorSource - if len(sourceStatus.Errors) > 0 { + if len(rsStatus.Rendering.Errors) > 0 && rsStatus.Rendering.Commit == commit { + errorSources = append(errorSources, v1beta1.RenderingError) + summary := rsStatus.Rendering.ErrorSummary + if summary != nil { + errorSummary.TotalCount += summary.TotalCount + errorSummary.ErrorCountAfterTruncation += summary.ErrorCountAfterTruncation + errorSummary.Truncated = errorSummary.Truncated || summary.Truncated + } + } + if len(rsStatus.Source.Errors) > 0 && rsStatus.Source.Commit == commit { errorSources = append(errorSources, v1beta1.SourceError) + summary := rsStatus.Source.ErrorSummary + if summary != nil { + errorSummary.TotalCount += summary.TotalCount + errorSummary.ErrorCountAfterTruncation += summary.ErrorCountAfterTruncation + errorSummary.Truncated = errorSummary.Truncated || summary.Truncated + } } - if len(syncStatus.Errors) > 0 { + if len(rsStatus.Sync.Errors) > 0 && rsStatus.Sync.Commit == commit { errorSources = append(errorSources, v1beta1.SyncError) + summary := rsStatus.Sync.ErrorSummary + if summary != nil { + errorSummary.TotalCount += summary.TotalCount + errorSummary.ErrorCountAfterTruncation += summary.ErrorCountAfterTruncation + errorSummary.Truncated = errorSummary.Truncated || summary.Truncated + } } + if errorSummary.TotalCount > 0 { + return errorSources, &errorSummary + } + return nil, nil +} - errorSummary := &v1beta1.ErrorSummary{} - for _, summary := range []*v1beta1.ErrorSummary{sourceStatus.ErrorSummary, syncStatus.ErrorSummary} { - if summary == nil { - continue - } - errorSummary.TotalCount += summary.TotalCount - errorSummary.ErrorCountAfterTruncation += summary.ErrorCountAfterTruncation - if summary.Truncated { - errorSummary.Truncated = true - } +func totalErrors(summary *v1beta1.ErrorSummary) int { + if summary != nil { + return summary.TotalCount } - return errorSources, errorSummary + return 0 } // addImplicitNamespaces hydrates the given FileObjects by injecting implicit diff --git a/pkg/parse/root_test.go b/pkg/parse/root_test.go index fbbde5f2a9..640ca319c9 100644 --- a/pkg/parse/root_test.go +++ b/pkg/parse/root_test.go @@ -947,32 +947,92 @@ func (a *fakeApplier) Syncing() bool { func TestSummarizeErrors(t *testing.T) { testCases := []struct { name string - sourceStatus v1beta1.SourceStatus - syncStatus v1beta1.SyncStatus + commit string + rsStatus v1beta1.Status expectedErrorSources []v1beta1.ErrorSource expectedErrorSummary *v1beta1.ErrorSummary }{ { - name: "both sourceStatus and syncStatus are empty", - sourceStatus: v1beta1.SourceStatus{}, - syncStatus: v1beta1.SyncStatus{}, + name: "all status fields are empty", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{}, + Sync: v1beta1.SyncStatus{}, + }, expectedErrorSources: nil, - expectedErrorSummary: &v1beta1.ErrorSummary{}, + expectedErrorSummary: nil, }, { - name: "sourceStatus is not empty (no trucation), syncStatus is empty", - sourceStatus: v1beta1.SourceStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "1021", ErrorMessage: "1021-error-message"}, - {Code: "1022", ErrorMessage: "1022-error-message"}, + name: "rendering is not empty (no trucation), source and sync are empty", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1001", ErrorMessage: "1001-error-message"}, + {Code: "1002", ErrorMessage: "1002-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 2, - Truncated: false, - ErrorCountAfterTruncation: 2, + Source: v1beta1.SourceStatus{}, + Sync: v1beta1.SyncStatus{}, + }, + expectedErrorSources: []v1beta1.ErrorSource{v1beta1.RenderingError}, + expectedErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + }, + { + name: "rendering is not empty and trucates errors, source and sync are empty", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1001", ErrorMessage: "1001-error-message"}, + {Code: "1002", ErrorMessage: "1002-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, + Source: v1beta1.SourceStatus{}, + Sync: v1beta1.SyncStatus{}, + }, + expectedErrorSources: []v1beta1.ErrorSource{v1beta1.RenderingError}, + expectedErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + }, + { + name: "source is not empty (no trucation), sync is empty", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", + }, + Sync: v1beta1.SyncStatus{}, }, - syncStatus: v1beta1.SyncStatus{}, expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SourceError}, expectedErrorSummary: &v1beta1.ErrorSummary{ TotalCount: 2, @@ -981,19 +1041,24 @@ func TestSummarizeErrors(t *testing.T) { }, }, { - name: "sourceStatus is not empty and trucates errors, syncStatus is empty", - sourceStatus: v1beta1.SourceStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "1021", ErrorMessage: "1021-error-message"}, - {Code: "1022", ErrorMessage: "1022-error-message"}, - }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 100, - Truncated: true, - ErrorCountAfterTruncation: 2, + name: "source is not empty and trucates errors, sync is empty", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, + Sync: v1beta1.SyncStatus{}, }, - syncStatus: v1beta1.SyncStatus{}, expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SourceError}, expectedErrorSummary: &v1beta1.ErrorSummary{ TotalCount: 100, @@ -1002,17 +1067,22 @@ func TestSummarizeErrors(t *testing.T) { }, }, { - name: "sourceStatus is empty, syncStatus is not empty (no trucation)", - sourceStatus: v1beta1.SourceStatus{}, - syncStatus: v1beta1.SyncStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "2009", ErrorMessage: "apiserver error"}, - {Code: "2009", ErrorMessage: "webhook error"}, - }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 2, - Truncated: false, - ErrorCountAfterTruncation: 2, + name: "source is empty, sync is not empty (no trucation)", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{}, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, }, expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SyncError}, @@ -1023,17 +1093,22 @@ func TestSummarizeErrors(t *testing.T) { }, }, { - name: "sourceStatus is empty, syncStatus is not empty and trucates errors", - sourceStatus: v1beta1.SourceStatus{}, - syncStatus: v1beta1.SyncStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "2009", ErrorMessage: "apiserver error"}, - {Code: "2009", ErrorMessage: "webhook error"}, - }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 100, - Truncated: true, - ErrorCountAfterTruncation: 2, + name: "source is empty, sync is not empty and trucates errors", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{}, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, }, expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SyncError}, @@ -1044,27 +1119,33 @@ func TestSummarizeErrors(t *testing.T) { }, }, { - name: "neither sourceStatus nor syncStatus is empty or trucates errors", - sourceStatus: v1beta1.SourceStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "1021", ErrorMessage: "1021-error-message"}, - {Code: "1022", ErrorMessage: "1022-error-message"}, - }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 2, - Truncated: false, - ErrorCountAfterTruncation: 2, - }, - }, - syncStatus: v1beta1.SyncStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "2009", ErrorMessage: "apiserver error"}, - {Code: "2009", ErrorMessage: "webhook error"}, + name: "neither source nor sync is empty or trucates errors", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 2, - Truncated: false, - ErrorCountAfterTruncation: 2, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, }, expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SourceError, v1beta1.SyncError}, @@ -1075,27 +1156,70 @@ func TestSummarizeErrors(t *testing.T) { }, }, { - name: "neither sourceStatus nor syncStatus is empty, sourceStatus trucates errors", - sourceStatus: v1beta1.SourceStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "1021", ErrorMessage: "1021-error-message"}, - {Code: "1022", ErrorMessage: "1022-error-message"}, + name: "neither source nor sync is empty, source trucates errors", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 100, - Truncated: true, - ErrorCountAfterTruncation: 2, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, }, - syncStatus: v1beta1.SyncStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "2009", ErrorMessage: "apiserver error"}, - {Code: "2009", ErrorMessage: "webhook error"}, + expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SourceError, v1beta1.SyncError}, + expectedErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 102, + Truncated: true, + ErrorCountAfterTruncation: 4, + }, + }, + { + name: "neither source nor sync is empty, sync trucates errors", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{}, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 2, + Truncated: false, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 2, - Truncated: false, - ErrorCountAfterTruncation: 2, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, }, expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SourceError, v1beta1.SyncError}, @@ -1106,74 +1230,237 @@ func TestSummarizeErrors(t *testing.T) { }, }, { - name: "neither sourceStatus nor syncStatus is empty, syncStatus trucates errors", - sourceStatus: v1beta1.SourceStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "1021", ErrorMessage: "1021-error-message"}, - {Code: "1022", ErrorMessage: "1022-error-message"}, + name: "neither rendering nor source nor sync is empty, all trucate errors", + commit: "a", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1001", ErrorMessage: "1001-error-message"}, + {Code: "1002", ErrorMessage: "1002-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 2, - Truncated: false, - ErrorCountAfterTruncation: 2, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", + }, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, }, - syncStatus: v1beta1.SyncStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "2009", ErrorMessage: "apiserver error"}, - {Code: "2009", ErrorMessage: "webhook error"}, + expectedErrorSources: []v1beta1.ErrorSource{ + v1beta1.RenderingError, + v1beta1.SourceError, + v1beta1.SyncError, + }, + expectedErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 300, + Truncated: true, + ErrorCountAfterTruncation: 6, + }, + }, + { + name: "new source commit, hide old render and sync errors", + commit: "b", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1001", ErrorMessage: "1001-error-message"}, + {Code: "1002", ErrorMessage: "1002-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, - - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 100, - Truncated: true, - ErrorCountAfterTruncation: 2, + Source: v1beta1.SourceStatus{ + Errors: nil, + ErrorSummary: nil, + Commit: "b", + }, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", }, }, - expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SourceError, v1beta1.SyncError}, + expectedErrorSources: nil, + expectedErrorSummary: nil, + }, + { + name: "new source commit with errors, hide old render and sync errors", + commit: "b", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1001", ErrorMessage: "1001-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 1, + }, + Commit: "a", + }, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "b", + }, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + {Code: "2009", ErrorMessage: "apply error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", + }, + }, + expectedErrorSources: []v1beta1.ErrorSource{ + v1beta1.SourceError, + }, expectedErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 102, + TotalCount: 100, Truncated: true, - ErrorCountAfterTruncation: 4, + ErrorCountAfterTruncation: 2, }, }, { - name: "neither sourceStatus nor syncStatus is empty, both trucates errors", - sourceStatus: v1beta1.SourceStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "1021", ErrorMessage: "1021-error-message"}, - {Code: "1022", ErrorMessage: "1022-error-message"}, + name: "new render commit with errors, hide old sync errors", + commit: "b", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1001", ErrorMessage: "1001-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 1, + }, + Commit: "b", }, - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 100, - Truncated: true, - ErrorCountAfterTruncation: 2, + Source: v1beta1.SourceStatus{ + Errors: nil, + ErrorSummary: nil, + Commit: "b", }, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + {Code: "2009", ErrorMessage: "apply error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "a", + }, + }, + expectedErrorSources: []v1beta1.ErrorSource{ + v1beta1.RenderingError, + }, + expectedErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 1, }, - syncStatus: v1beta1.SyncStatus{ - Errors: []v1beta1.ConfigSyncError{ - {Code: "2009", ErrorMessage: "apiserver error"}, - {Code: "2009", ErrorMessage: "webhook error"}, + }, + { + name: "new sync commit with errors, include non-blocking source errors", + commit: "b", + rsStatus: v1beta1.Status{ + Rendering: v1beta1.RenderingStatus{ + Errors: nil, + ErrorSummary: nil, + Commit: "b", }, - - ErrorSummary: &v1beta1.ErrorSummary{ - TotalCount: 100, - Truncated: true, - ErrorCountAfterTruncation: 2, + Source: v1beta1.SourceStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "1021", ErrorMessage: "1021-error-message"}, + {Code: "1022", ErrorMessage: "1022-error-message"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 2, + }, + Commit: "b", + }, + Sync: v1beta1.SyncStatus{ + Errors: []v1beta1.ConfigSyncError{ + {Code: "2009", ErrorMessage: "apiserver error"}, + {Code: "2009", ErrorMessage: "webhook error"}, + {Code: "2009", ErrorMessage: "apply error"}, + }, + ErrorSummary: &v1beta1.ErrorSummary{ + TotalCount: 100, + Truncated: true, + ErrorCountAfterTruncation: 3, + }, + Commit: "b", }, }, - expectedErrorSources: []v1beta1.ErrorSource{v1beta1.SourceError, v1beta1.SyncError}, + expectedErrorSources: []v1beta1.ErrorSource{ + v1beta1.SourceError, + v1beta1.SyncError, + }, expectedErrorSummary: &v1beta1.ErrorSummary{ TotalCount: 200, Truncated: true, - ErrorCountAfterTruncation: 4, + ErrorCountAfterTruncation: 5, }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - gotErrorSources, gotErrorSummary := summarizeErrors(tc.sourceStatus, tc.syncStatus) + gotErrorSources, gotErrorSummary := summarizeErrors(tc.rsStatus, tc.commit) if diff := cmp.Diff(tc.expectedErrorSources, gotErrorSources); diff != "" { t.Errorf("summarizeErrors() got %v, expected %v", gotErrorSources, tc.expectedErrorSources) } diff --git a/pkg/parse/run.go b/pkg/parse/run.go index bcec323b65..33e4ad6901 100644 --- a/pkg/parse/run.go +++ b/pkg/parse/run.go @@ -142,42 +142,50 @@ func Run(ctx context.Context, p Parser) { } } +// run executes the parts of the pipeline managed by the reconciler. +// +// Phases: +// 1. Fetch (git-sync), then the reconciler reads the result from disk +// 2. Render (hydration-controller), then the reconciler reads the result from disk +// 3. Parse (reconciler) +// 4. Sync (reconciler) +// 5. Remediate (remediator) func run(ctx context.Context, p Parser, trigger string, state *reconcilerState) { var syncDir cmpath.Absolute - gs := sourceStatus{} - gs.commit, syncDir, gs.errs = hydrate.SourceCommitAndDir(p.options().SourceType, p.options().SourceDir, p.options().SyncDir, p.options().reconcilerName) + fetchStatus := sourceStatus{} + fetchStatus.commit, syncDir, fetchStatus.errs = hydrate.SourceCommitAndDir(p.options().SourceType, p.options().SourceDir, p.options().SyncDir, p.options().reconcilerName) // If failed to fetch the source commit and directory, set `.status.source` to fail early. // Otherwise, set `.status.rendering` before `.status.source` because the parser needs to // read and parse the configs after rendering is done and there might have errors. - if gs.errs != nil { - gs.lastUpdate = metav1.Now() + if fetchStatus.errs != nil { + fetchStatus.lastUpdate = metav1.Now() var setSourceStatusErr error - if state.needToSetSourceStatus(gs) { - setSourceStatusErr = p.setSourceStatus(ctx, gs) + if state.needToSetSourceStatus(fetchStatus) { + setSourceStatusErr = p.setSourceStatus(ctx, fetchStatus) if setSourceStatusErr == nil { - state.sourceStatus = gs - state.syncingConditionLastUpdate = gs.lastUpdate + state.sourceStatus = fetchStatus + state.syncingConditionLastUpdate = fetchStatus.lastUpdate } } - state.invalidate(status.Append(gs.errs, setSourceStatusErr)) + state.invalidate(status.Append(fetchStatus.errs, setSourceStatusErr)) return } - rs := renderingStatus{ - commit: gs.commit, - } + renderStatus := renderingStatus{} + renderStatus.commit = fetchStatus.commit + // set the rendering status by checking the done file. doneFilePath := p.options().RepoRoot.Join(cmpath.RelativeSlash(hydrate.DoneFile)).OSPath() _, err := os.Stat(doneFilePath) - if os.IsNotExist(err) || (err == nil && hydrate.DoneCommit(doneFilePath) != gs.commit) { - rs.message = RenderingInProgress - rs.lastUpdate = metav1.Now() - setRenderingStatusErr := p.setRenderingStatus(ctx, state.renderingStatus, rs) + if os.IsNotExist(err) || (err == nil && hydrate.DoneCommit(doneFilePath) != fetchStatus.commit) { + renderStatus.message = RenderingInProgress + renderStatus.lastUpdate = metav1.Now() + setRenderingStatusErr := p.setRenderingStatus(ctx, state.renderingStatus, renderStatus) if setRenderingStatusErr == nil { state.reset() - state.renderingStatus = rs - state.syncingConditionLastUpdate = rs.lastUpdate + state.renderingStatus = renderStatus + state.syncingConditionLastUpdate = renderStatus.lastUpdate } else { var m status.MultiError state.invalidate(status.Append(m, setRenderingStatusErr)) @@ -185,15 +193,15 @@ func run(ctx context.Context, p Parser, trigger string, state *reconcilerState) return } if err != nil { - rs.message = RenderingFailed - rs.lastUpdate = metav1.Now() - rs.errs = status.InternalHydrationError(err, "unable to read the done file: %s", doneFilePath) - setRenderingStatusErr := p.setRenderingStatus(ctx, state.renderingStatus, rs) + renderStatus.message = RenderingFailed + renderStatus.lastUpdate = metav1.Now() + renderStatus.errs = status.InternalHydrationError(err, "unable to read the done file: %s", doneFilePath) + setRenderingStatusErr := p.setRenderingStatus(ctx, state.renderingStatus, renderStatus) if setRenderingStatusErr == nil { - state.renderingStatus = rs - state.syncingConditionLastUpdate = rs.lastUpdate + state.renderingStatus = renderStatus + state.syncingConditionLastUpdate = renderStatus.lastUpdate } - state.invalidate(status.Append(rs.errs, setRenderingStatusErr)) + state.invalidate(status.Append(renderStatus.errs, setRenderingStatusErr)) return } @@ -201,7 +209,7 @@ func run(ctx context.Context, p Parser, trigger string, state *reconcilerState) oldSyncDir := state.cache.source.syncDir // `read` is called no matter what the trigger is. ps := sourceState{ - commit: gs.commit, + commit: fetchStatus.commit, syncDir: syncDir, } if errs := read(ctx, p, trigger, state, ps); errs != nil { @@ -233,37 +241,37 @@ func run(ctx context.Context, p Parser, trigger string, state *reconcilerState) // read reads config files from source if no rendering is needed, or from hydrated output if rendering is done. // It also updates the .status.rendering and .status.source fields. func read(ctx context.Context, p Parser, trigger string, state *reconcilerState, sourceState sourceState) status.MultiError { - hydrationStatus, sourceStatus := readFromSource(ctx, p, trigger, state, sourceState) - hydrationStatus.lastUpdate = metav1.Now() + renderStatus, readStatus := readFromSource(ctx, p, trigger, state, sourceState) + renderStatus.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. - setRenderingStatusErr := p.setRenderingStatus(ctx, state.renderingStatus, hydrationStatus) + setRenderingStatusErr := p.setRenderingStatus(ctx, state.renderingStatus, renderStatus) if setRenderingStatusErr == nil { - state.renderingStatus = hydrationStatus - state.syncingConditionLastUpdate = hydrationStatus.lastUpdate + state.renderingStatus = renderStatus + state.syncingConditionLastUpdate = renderStatus.lastUpdate } - renderingErrs := status.Append(hydrationStatus.errs, setRenderingStatusErr) + renderingErrs := status.Append(renderStatus.errs, setRenderingStatusErr) if renderingErrs != nil { return renderingErrs } - if sourceStatus.errs == nil { + if readStatus.errs == nil { return nil } // Only call `setSourceStatus` if `readFromSource` fails. // If `readFromSource` succeeds, `parse` may still fail. - sourceStatus.lastUpdate = metav1.Now() + readStatus.lastUpdate = metav1.Now() var setSourceStatusErr error - if state.needToSetSourceStatus(sourceStatus) { - setSourceStatusErr := p.setSourceStatus(ctx, sourceStatus) + if state.needToSetSourceStatus(readStatus) { + setSourceStatusErr := p.setSourceStatus(ctx, readStatus) if setSourceStatusErr == nil { - state.sourceStatus = sourceStatus - state.syncingConditionLastUpdate = sourceStatus.lastUpdate + state.sourceStatus = readStatus + state.syncingConditionLastUpdate = readStatus.lastUpdate } } - return status.Append(sourceStatus.errs, setSourceStatusErr) + return status.Append(readStatus.errs, setSourceStatusErr) } // readFromSource reads the source or hydrated configs, checks whether the sourceState in @@ -273,41 +281,40 @@ func readFromSource(ctx context.Context, p Parser, trigger string, state *reconc opts := p.options() start := time.Now() - hydrationStatus := renderingStatus{ - commit: sourceState.commit, - } - sourceStatus := sourceStatus{ - commit: sourceState.commit, - } + renderStatus := renderingStatus{} + renderStatus.commit = sourceState.commit + + readStatus := sourceStatus{} + readStatus.commit = sourceState.commit // Check if the hydratedRoot directory exists. // If exists, read the hydrated directory. Otherwise, read the source directory. absHydratedRoot, err := cmpath.AbsoluteOS(opts.HydratedRoot) if err != nil { - hydrationStatus.message = RenderingFailed - hydrationStatus.errs = status.InternalHydrationError(err, "hydrated-dir must be an absolute path") - return hydrationStatus, sourceStatus + renderStatus.message = RenderingFailed + renderStatus.errs = status.InternalHydrationError(err, "hydrated-dir must be an absolute path") + return renderStatus, readStatus } var hydrationErr hydrate.HydrationError if _, err := os.Stat(absHydratedRoot.OSPath()); err == nil { sourceState, hydrationErr = opts.readHydratedDir(absHydratedRoot, opts.HydratedLink, opts.reconcilerName) if hydrationErr != nil { - hydrationStatus.message = RenderingFailed - hydrationStatus.errs = status.HydrationError(hydrationErr.Code(), hydrationErr) - return hydrationStatus, sourceStatus + renderStatus.message = RenderingFailed + renderStatus.errs = status.HydrationError(hydrationErr.Code(), hydrationErr) + return renderStatus, readStatus } - hydrationStatus.message = RenderingSucceeded + renderStatus.message = RenderingSucceeded } else if !os.IsNotExist(err) { - hydrationStatus.message = RenderingFailed - hydrationStatus.errs = status.InternalHydrationError(err, "unable to evaluate the hydrated path %s", absHydratedRoot.OSPath()) - return hydrationStatus, sourceStatus + renderStatus.message = RenderingFailed + renderStatus.errs = status.InternalHydrationError(err, "unable to evaluate the hydrated path %s", absHydratedRoot.OSPath()) + return renderStatus, readStatus } else { - hydrationStatus.message = RenderingSkipped + renderStatus.message = RenderingSkipped } if sourceState.syncDir == state.cache.source.syncDir { - return hydrationStatus, sourceStatus + return renderStatus, readStatus } klog.Infof("New source changes (%s) detected, reset the cache", sourceState.syncDir.OSPath()) @@ -316,13 +323,13 @@ func readFromSource(ctx context.Context, p Parser, trigger string, state *reconc state.resetCache() // Read all the files under state.syncDir - sourceStatus.errs = opts.readConfigFiles(&sourceState) - if sourceStatus.errs == nil { + readStatus.errs = opts.readConfigFiles(&sourceState) + if readStatus.errs == nil { // Set `state.cache.source` after `readConfigFiles` succeeded state.cache.source = sourceState } - metrics.RecordParserDuration(ctx, trigger, "read", metrics.StatusTagKey(sourceStatus.errs), start) - return hydrationStatus, sourceStatus + metrics.RecordParserDuration(ctx, trigger, "read", metrics.StatusTagKey(readStatus.errs), start) + return renderStatus, readStatus } func parseSource(ctx context.Context, p Parser, trigger string, state *reconcilerState) status.MultiError { @@ -354,20 +361,22 @@ func parseSource(ctx context.Context, p Parser, trigger string, state *reconcile func parseAndUpdate(ctx context.Context, p Parser, trigger string, state *reconcilerState) status.MultiError { sourceErrs := parseSource(ctx, p, trigger, state) - newSourceStatus := sourceStatus{ - commit: state.cache.source.commit, - errs: sourceErrs, - lastUpdate: metav1.Now(), - } - if state.needToSetSourceStatus(newSourceStatus) { - if err := p.setSourceStatus(ctx, newSourceStatus); err != nil { + parseStatus := sourceStatus{ + commonStatus: commonStatus{ + commit: state.cache.source.commit, + errs: sourceErrs, + lastUpdate: metav1.Now(), + }, + } + if state.needToSetSourceStatus(parseStatus) { + if err := p.setSourceStatus(ctx, parseStatus); err != nil { // If `p.setSourceStatus` fails, we terminate the reconciliation. // If we call `update` in this case and `update` succeeds, `Status.Source.Commit` would end up be older // than `Status.Sync.Commit`. return status.Append(sourceErrs, err) } - state.sourceStatus = newSourceStatus - state.syncingConditionLastUpdate = newSourceStatus.lastUpdate + state.sourceStatus = parseStatus + state.syncingConditionLastUpdate = parseStatus.lastUpdate } if status.HasBlockingErrors(sourceErrs) { @@ -383,7 +392,7 @@ func parseAndUpdate(ctx context.Context, p Parser, trigger string, state *reconc syncErrs := p.options().Update(ctx, &state.cache) metrics.RecordParserDuration(ctx, trigger, "update", metrics.StatusTagKey(syncErrs), start) - // This is to terminate `updateSyncStatusPeriodically`. + // Terminate `updateSyncStatusPeriodically`. cancel() klog.V(3).Info("Updating sync status (after sync)") @@ -400,10 +409,12 @@ func parseAndUpdate(ctx context.Context, p Parser, trigger string, state *reconc func setSyncStatus(ctx context.Context, p Parser, state *reconcilerState, syncing bool, syncErrs status.MultiError) error { // Update the RSync status, if necessary newSyncStatus := syncStatus{ - syncing: syncing, - commit: state.cache.source.commit, - errs: syncErrs, - lastUpdate: metav1.Now(), + syncing: syncing, + commonStatus: commonStatus{ + commit: state.cache.source.commit, + errs: syncErrs, + lastUpdate: metav1.Now(), + }, } if state.needToSetSyncStatus(newSyncStatus) { if err := p.SetSyncStatus(ctx, newSyncStatus); err != nil { @@ -429,8 +440,8 @@ func setSyncStatus(ctx context.Context, p Parser, state *reconcilerState, syncin return nil } -// updateSyncStatusPeriodically update the sync status periodically until the -// cancellation function of the context is called. +// updateSyncStatusPeriodically updates the sync status periodically until the +// context is cancelled. func updateSyncStatusPeriodically(ctx context.Context, p Parser, state *reconcilerState) { updatePeriod := p.options().statusUpdatePeriod updateTimer := time.NewTimer(updatePeriod) @@ -438,7 +449,7 @@ func updateSyncStatusPeriodically(ctx context.Context, p Parser, state *reconcil for { select { case <-ctx.Done(): - // ctx.Done() is closed when the cancellation function of the context is called. + // ctx.Done() is closed when the context is cancelled. return case <-updateTimer.C: diff --git a/pkg/parse/state.go b/pkg/parse/state.go index d470c534ce..1229fc0b05 100644 --- a/pkg/parse/state.go +++ b/pkg/parse/state.go @@ -28,36 +28,40 @@ const ( maxRetryInterval = time.Duration(5) * time.Minute ) -type sourceStatus struct { +type commonStatus struct { commit string errs status.MultiError lastUpdate metav1.Time } -func (gs sourceStatus) equal(other sourceStatus) bool { - return gs.commit == other.commit && status.DeepEqual(gs.errs, other.errs) +func (cs commonStatus) Equal(other commonStatus) bool { + return cs.commit == other.commit && status.DeepEqual(cs.errs, other.errs) +} + +type sourceStatus struct { + commonStatus +} + +func (gs sourceStatus) Equal(other sourceStatus) bool { + return gs.commonStatus.Equal(other.commonStatus) } type renderingStatus struct { - commit string - message string - errs status.MultiError - lastUpdate metav1.Time + commonStatus + message string } -func (rs renderingStatus) equal(other renderingStatus) bool { - return rs.commit == other.commit && rs.message == other.message && status.DeepEqual(rs.errs, other.errs) +func (rs renderingStatus) Equal(other renderingStatus) bool { + return rs.commonStatus.Equal(other.commonStatus) && rs.message == other.message } type syncStatus struct { - syncing bool - commit string - errs status.MultiError - lastUpdate metav1.Time + commonStatus + syncing bool } -func (gs syncStatus) equal(other syncStatus) bool { - return gs.syncing == other.syncing && gs.commit == other.commit && status.DeepEqual(gs.errs, other.errs) +func (ss syncStatus) Equal(other syncStatus) bool { + return ss.commonStatus.Equal(other.commonStatus) && ss.syncing == other.syncing } type reconcilerState struct { @@ -159,10 +163,10 @@ func (s *reconcilerState) resetAllButSourceState() { // needToSetSourceStatus returns true if `p.setSourceStatus` should be called. func (s *reconcilerState) needToSetSourceStatus(newStatus sourceStatus) bool { - return !newStatus.equal(s.sourceStatus) || s.sourceStatus.lastUpdate.IsZero() || s.sourceStatus.lastUpdate.Before(&s.syncingConditionLastUpdate) + return !newStatus.Equal(s.sourceStatus) || s.sourceStatus.lastUpdate.IsZero() || s.sourceStatus.lastUpdate.Before(&s.syncingConditionLastUpdate) } // needToSetSyncStatus returns true if `p.SetSyncStatus` should be called. func (s *reconcilerState) needToSetSyncStatus(newStatus syncStatus) bool { - return !newStatus.equal(s.syncStatus) || s.syncStatus.lastUpdate.IsZero() || s.syncStatus.lastUpdate.Before(&s.syncingConditionLastUpdate) + return !newStatus.Equal(s.syncStatus) || s.syncStatus.lastUpdate.IsZero() || s.syncStatus.lastUpdate.Before(&s.syncingConditionLastUpdate) }