Fix sync status lost on reconciler pod replacement (#1276)

- Fix reconciler ignoring/erasing status from previous reconciler instances by priming the in-memory cache from the RSyncs status. Example: status.sync.commit Exception: errors are not parsable and may still be lost.
GoogleContainerTools · Jul 2, 2024 · f9daa34 · f9daa34
1 parent 71ac1c9
commit f9daa34
Show file tree

Hide file tree

Showing 9 changed files with 434 additions and 99 deletions.
diff --git a/pkg/parse/cache.go b/pkg/parse/cache.go
@@ -34,7 +34,7 @@ import (
 type cacheForCommit struct {
 	// source tracks the state of the source repo.
 	// This field is only set after the reconciler successfully reads all the source files.
-	source sourceState
+	source *sourceState
 
 	// hasParserResult indicates whether the cache includes the parser result.
 	hasParserResult bool

diff --git a/pkg/parse/namespace.go b/pkg/parse/namespace.go
@@ -20,6 +20,9 @@ import (
 	"strconv"
 
 	"github.com/google/go-cmp/cmp"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
 	"kpt.dev/configsync/pkg/api/configsync"
 	"kpt.dev/configsync/pkg/api/configsync/v1beta1"
@@ -54,7 +57,7 @@ func (p *namespace) options() *Options {
 }
 
 // parseSource implements the Parser interface
-func (p *namespace) parseSource(ctx context.Context, state sourceState) ([]ast.FileObject, status.MultiError) {
+func (p *namespace) parseSource(ctx context.Context, state *sourceState) ([]ast.FileObject, status.MultiError) {
 	p.mux.Lock()
 	defer p.mux.Unlock()
 
@@ -132,7 +135,7 @@ func (p *namespace) setSourceStatusWithRetries(ctx context.Context, newStatus *S
 
 	currentRS := rs.DeepCopy()
 
-	setSourceStatusFields(&rs.Status.Source, p, newStatus, denominator)
+	setSourceStatusFields(&rs.Status.Source, newStatus, denominator)
 
 	continueSyncing := (rs.Status.Source.ErrorSummary.TotalCount == 0)
 	var errorSource []v1beta1.ErrorSource
@@ -156,8 +159,8 @@ func (p *namespace) setSourceStatusWithRetries(ctx context.Context, newStatus *S
 	}
 
 	if klog.V(5).Enabled() {
-		klog.Infof("Updating source status for RepoSync %s/%s:\nDiff (- Expected, + Actual):\n%s",
-			rs.Namespace, rs.Name, cmp.Diff(currentRS.Status, rs.Status))
+		klog.V(5).Infof("Updating source status:\nDiff (- Removed, + Added):\n%s",
+			cmp.Diff(currentRS.Status, rs.Status))
 	}
 
 	if err := p.Client.Status().Update(ctx, &rs, client.FieldOwner(configsync.FieldManager)); err != nil {
@@ -209,7 +212,7 @@ func (p *namespace) setRenderingStatusWithRetires(ctx context.Context, newStatus
 
 	currentRS := rs.DeepCopy()
 
-	setRenderingStatusFields(&rs.Status.Rendering, p, newStatus, denominator)
+	setRenderingStatusFields(&rs.Status.Rendering, newStatus, denominator)
 
 	continueSyncing := (rs.Status.Rendering.ErrorSummary.TotalCount == 0)
 	var errorSource []v1beta1.ErrorSource
@@ -233,8 +236,8 @@ func (p *namespace) setRenderingStatusWithRetires(ctx context.Context, newStatus
 	}
 
 	if klog.V(5).Enabled() {
-		klog.Infof("Updating rendering status for RepoSync %s/%s:\nDiff (- Expected, + Actual):\n%s",
-			rs.Namespace, rs.Name, cmp.Diff(currentRS.Status, rs.Status))
+		klog.V(5).Infof("Updating rendering status:\nDiff (- Removed, + Added):\n%s",
+			cmp.Diff(currentRS.Status, rs.Status))
 	}
 
 	if err := p.Client.Status().Update(ctx, &rs, client.FieldOwner(configsync.FieldManager)); err != nil {
@@ -248,6 +251,32 @@ func (p *namespace) setRenderingStatusWithRetires(ctx context.Context, newStatus
 	return nil
 }
 
+// ReconcilerStatusFromCluster gets the RepoSync sync status from the cluster.
+func (p *namespace) ReconcilerStatusFromCluster(ctx context.Context) (*ReconcilerStatus, error) {
+	rs := &v1beta1.RepoSync{}
+	if err := p.Client.Get(ctx, reposync.ObjectKey(p.Scope, p.SyncName), rs); err != nil {
+		if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) {
+			return nil, nil
+		}
+		return nil, status.APIServerError(err, fmt.Sprintf("failed to get the RepoSync object for the %v namespace", p.Scope))
+	}
+
+	// Read Syncing condition
+	syncing := false
+	var syncingConditionLastUpdate metav1.Time
+	for _, condition := range rs.Status.Conditions {
+		if condition.Type == v1beta1.RepoSyncSyncing {
+			if condition.Status == metav1.ConditionTrue {
+				syncing = true
+			}
+			syncingConditionLastUpdate = condition.LastUpdateTime
+			break
+		}
+	}
+
+	return reconcilerStatusFromRSyncStatus(rs.Status.Status, p.options().SourceType, syncing, syncingConditionLastUpdate), nil
+}
+
 // SetSyncStatus implements the Parser interface
 // SetSyncStatus sets the RepoSync sync status.
 // `errs` includes the errors encountered during the apply step;
@@ -308,8 +337,8 @@ func (p *namespace) setSyncStatusWithRetries(ctx context.Context, newStatus *Syn
 	}
 
 	if klog.V(5).Enabled() {
-		klog.Infof("Updating status for RepoSync %s/%s:\nDiff (- Expected, + Actual):\n%s",
-			rs.Namespace, rs.Name, cmp.Diff(currentRS.Status, rs.Status))
+		klog.V(5).Infof("Updating sync status:\nDiff (- Removed, + Added):\n%s",
+			cmp.Diff(currentRS.Status, rs.Status))
 	}
 
 	if err := p.Client.Status().Update(ctx, rs, client.FieldOwner(configsync.FieldManager)); err != nil {

diff --git a/pkg/parse/opts.go b/pkg/parse/opts.go
@@ -93,7 +93,8 @@ type Options struct {
 
 // Parser represents a parser that can be pointed at and continuously parse a source.
 type Parser interface {
-	parseSource(ctx context.Context, state sourceState) ([]ast.FileObject, status.MultiError)
+	parseSource(ctx context.Context, state *sourceState) ([]ast.FileObject, status.MultiError)
+	ReconcilerStatusFromCluster(ctx context.Context) (*ReconcilerStatus, error)
 	setSourceStatus(ctx context.Context, newStatus *SourceStatus) error
 	setRenderingStatus(ctx context.Context, oldStatus, newStatus *RenderingStatus) error
 	SetSyncStatus(ctx context.Context, newStatus *SyncStatus) error