reconciler: Fix flake in reconciler tests due to ID being unset

The fix to JSON status marshalling had the unintended effect that now
the reconciler test suite ran with the ID field being uninitialized,
leading to the reconciler overwriting the status of a changed object.
This resulted in small fraction of the reconciler test runs failing
(e.g. object was marked pending, but instead of being re-reconciled the
previous status was inserted).

This did not affect non-test code as status should always be constructed
with StatusPending() etc.

Fix this by exporting the ID and filling it in the tests.

Fixes: 132edd6 ("reconciler: Fix Status JSON marshalling")

Signed-off-by: Jussi Maki <[email protected]>

reconciler/incremental.go

@@ -174,7 +174,7 @@ func (round *incrementalRound[Obj]) batch(changes iter.Seq2[statedb.Change[Obj],
 			if entry.Result == nil {
 				round.retries.Clear(entry.Object)
 			}
-			round.results[entry.Object] = opResult{rev: entry.Revision, id: status.id, err: entry.Result, original: entry.original}
+			round.results[entry.Object] = opResult{rev: entry.Revision, id: status.ID, err: entry.Result, original: entry.original}
 		}
 	}
 }
@@ -213,7 +213,7 @@ func (round *incrementalRound[Obj]) processSingle(obj Obj, rev statedb.Revision,
 		op = OpUpdate
 		err = round.config.Operations.Update(round.ctx, round.txn, obj)
 		status := round.config.GetObjectStatus(obj)
-		round.results[obj] = opResult{original: orig, id: status.id, rev: rev, err: err}
+		round.results[obj] = opResult{original: orig, id: status.ID, rev: rev, err: err}
 	}
 	round.metrics.ReconciliationDuration(round.moduleID, op, time.Since(start))
 
@@ -256,7 +256,7 @@ func (round *incrementalRound[Obj]) commitStatus() (numErrors int) {
 			// modifying the object during reconciliation as the following will forget
 			// the changes.
 			currentStatus := round.config.GetObjectStatus(current)
-			if currentStatus.Kind == StatusKindPending && currentStatus.id == result.id {
+			if currentStatus.Kind == StatusKindPending && currentStatus.ID == result.id {
 				current = round.config.CloneObject(current)
 				current = round.config.SetObjectStatus(current, status)
 				round.table.Insert(wtxn, current)

reconciler/script_test.go

@@ -107,7 +107,7 @@ func newEngine(t testing.TB, args []string) *script.Engine {
 	)
 
 	cmds["start-reconciler"] = script.Command(
-		script.CmdUsage{Summary: "Mark table as initialized"},
+		script.CmdUsage{Summary: "Start the reconciler"},
 		func(s *script.State, args ...string) (script.WaitFunc, error) {
 			opts := []reconciler.Option{
 				// Speed things up a bit. Quick retry interval does mean we can't

reconciler/status_test.go

@@ -112,7 +112,7 @@ func TestStatusSet(t *testing.T) {
 
 	s := set.Get("foo")
 	assert.Equal(t, s.Kind, StatusKindPending)
-	assert.NotZero(t, s.id)
+	assert.NotZero(t, s.ID)
 
 	set = set.Set("foo", StatusDone())
 	set = set.Set("bar", StatusError(errors.New("fail")))
@@ -123,7 +123,7 @@ func TestStatusSet(t *testing.T) {
 	assert.Regexp(t, "^Errored: bar \\(fail\\), Done: foo \\(.* ago\\)", set.String())
 
 	set = set.Pending()
-	assert.NotZero(t, set.Get("foo").id)
+	assert.NotZero(t, set.Get("foo").ID)
 	assert.Equal(t, set.Get("foo").Kind, StatusKindPending)
 	assert.Equal(t, set.Get("bar").Kind, StatusKindPending)
 	assert.Equal(t, set.Get("baz").Kind, StatusKindPending)

reconciler/testdata/batching.txtar

@@ -93,34 +93,40 @@ id: 1
 faulty: false
 status:
     kind: Pending
+    id: 1
 
 -- obj1_faulty.yaml --
 id: 1
 faulty: true
 status:
     kind: Pending
+    id: 2
 
 -- obj2.yaml --
 id: 2
 faulty: false
 status:
     kind: Pending
+    id: 3
 
 -- obj2_faulty.yaml --
 id: 2
 faulty: true
 status:
     kind: Pending
+    id: 4
 
 -- obj3.yaml --
 id: 3
 faulty: false
 status:
     kind: Pending
+    id: 5
 
 -- obj3_faulty.yaml --
 id: 3
 faulty: true
 status:
     kind: Pending
+    id: 6
 

reconciler/testdata/incremental.txtar

@@ -90,34 +90,40 @@ id: 1
 faulty: false
 status:
     kind: Pending
+    id: 1
 
 -- obj1_faulty.yaml --
 id: 1
 faulty: true
 status:
     kind: Pending
+    id: 2
 
 -- obj2.yaml --
 id: 2
 faulty: false
 status:
     kind: Pending
+    id: 3
 
 -- obj2_faulty.yaml --
 id: 2
 faulty: true
 status:
     kind: Pending
+    id: 4
 
 -- obj3.yaml --
 id: 3
 faulty: false
 status:
     kind: Pending
+    id: 5
 
 -- obj3_faulty.yaml --
 id: 3
 faulty: true
 status:
     kind: Pending
+    id: 6
 

reconciler/testdata/pruning.txtar

@@ -59,10 +59,12 @@ id: 1
 faulty: false
 status:
     kind: Pending
+    id: 1
 
 -- obj2.yaml --
 id: 2
 faulty: false
 status:
     kind: Pending
+    id: 2
 

reconciler/testdata/refresh.txtar

@@ -46,6 +46,7 @@ status:
     kind: Pending
     updatedat: 2024-01-01T10:10:10.0+02:00
     error: ""
+    id: 2
 
 -- obj1_old.yaml --
 id: 1
@@ -55,4 +56,5 @@ status:
     kind: Done
     updatedat: 2000-01-01T10:10:10.0+02:00
     error: ""
+    id: 1
 

reconciler/types.go

@@ -140,7 +140,7 @@ type Status struct {
 	// has really changed when committing the resulting status.
 	// This allows multiple reconcilers to exist for a single
 	// object without repeating work when status is updated.
-	id uint64
+	ID uint64 `json:"id,omitempty" yaml:"id,omitempty"`
 }
 
 func (s Status) IsPendingOrRefreshing() bool {
@@ -169,7 +169,7 @@ func StatusPending() Status {
 		Kind:      StatusKindPending,
 		UpdatedAt: time.Now(),
 		Error:     "",
-		id:        nextID(),
+		ID:        nextID(),
 	}
 }
 
@@ -186,7 +186,7 @@ func StatusRefreshing() Status {
 		Kind:      StatusKindRefreshing,
 		UpdatedAt: time.Now(),
 		Error:     "",
-		id:        nextID(),
+		ID:        nextID(),
 	}
 }
 
@@ -197,7 +197,7 @@ func StatusDone() Status {
 		Kind:      StatusKindDone,
 		UpdatedAt: time.Now(),
 		Error:     "",
-		id:        nextID(),
+		ID:        nextID(),
 	}
 }
 
@@ -208,7 +208,7 @@ func StatusError(err error) Status {
 		Kind:      StatusKindError,
 		UpdatedAt: time.Now(),
 		Error:     err.Error(),
-		id:        nextID(),
+		ID:        nextID(),
 	}
 }
 
@@ -249,7 +249,7 @@ func (s StatusSet) Pending() StatusSet {
 	s.statuses = slices.Clone(s.statuses)
 	for i := range s.statuses {
 		s.statuses[i].Kind = StatusKindPending
-		s.statuses[i].id = s.id
+		s.statuses[i].ID = s.id
 	}
 	return s
 }
@@ -331,7 +331,7 @@ func (s StatusSet) Get(name string) Status {
 		return Status{
 			Kind:      StatusKindPending,
 			UpdatedAt: s.createdAt,
-			id:        s.id,
+			ID:        s.id,
 		}
 	}
 	return s.statuses[idx].Status