controller: post-reboot handling and deployedDigest

docs/ARCHITECTURE.md

@@ -396,14 +396,16 @@ The controller determines whether a node is up to date by comparing
 `spec.desiredImage` against `status.booted.imageDigest`. It does not
 rely on the daemon's `Idle` condition for this.
 
-The controller classifies each node into one of five effective states.
+The controller classifies each node into one of six effective states.
 The `Degraded` condition is checked first (takes priority over activity
 state).
 
 | Effective state | Determination                                             | Reconciler action                                    |
 |-----------------|-----------------------------------------------------------|------------------------------------------------------|
 | Degraded        | BootcNode `Degraded=True`                                 | Mark pool degraded                                   |
-| Idle            | `desiredImage == booted` and `Idle=True`                  | If in reboot slot: free slot only once node is Ready |
+| UpToDate        | `desiredImage == booted`                                  | If in reboot slot: free slot only once node is Ready |
+| Pending         | No booted status, or `desiredImage != booted` with no     | Wait for daemon to report or react                   |
+|                 | actionable Idle reason (daemon hasn't reported/reacted)   |                                                      |
 | Staging         | `desiredImage != booted`, `Idle=False reason=Staging`     | Wait (non-disruptive)                                |
 | Staged          | `desiredImage != booted`, `Idle=False reason=Staged`      | If reboot slot available: assign slot; else wait     |
 | Rebooting       | `desiredImage != booted`, `Idle=False reason=Rebooting`   | Wait for node to come back                           |
@@ -414,9 +416,9 @@ governed by three rules:
 1. **A node can only take a reboot slot when healthy** -- only Staged
    nodes (not Degraded) are candidates for reboot slots.
 2. **A node can only release a reboot slot when healthy** -- after
-   reboot, the slot is held until the node is Idle (`Idle=True`, not
-   Degraded) and the K8s Node is Ready. A node that is Degraded or
-   not Ready post-reboot holds its slot indefinitely.
+   reboot, the slot is held until the node is Idle (`desiredImage ==
+   booted`, not Degraded) and the K8s Node is Ready. A node that is
+   Degraded or not Ready post-reboot holds its slot indefinitely.
 3. **2 unhealthy nodes in reboot slots stop the rollout** -- when 2 or
    more nodes occupying reboot slots are unhealthy (Degraded or not
    Ready), the controller stops assigning new slots. A single

internal/controller/rollout.go

@@ -28,7 +28,8 @@ import (
 // pass.
 type rolloutState struct {
 	// nodes are sorted into these buckets
-	idle         []*bootcv1alpha1.BootcNode
+	upToDate     []*bootcv1alpha1.BootcNode
+	pending      []*bootcv1alpha1.BootcNode
 	staging      []*bootcv1alpha1.BootcNode
 	staged       []*bootcv1alpha1.BootcNode
 	rebooting    []*bootcv1alpha1.BootcNode
@@ -42,7 +43,7 @@ type rolloutState struct {
 // nodeCount returns the total number of nodes in the pool, including
 // unclassified ones. Used for resolving percentage-based maxUnavailable.
 func (rs *rolloutState) nodeCount() int {
-	return len(rs.idle) + len(rs.staging) + len(rs.staged) +
+	return len(rs.upToDate) + len(rs.pending) + len(rs.staging) + len(rs.staged) +
 		len(rs.rebooting) + len(rs.degraded) + len(rs.unclassified)
 }
 
@@ -67,7 +68,8 @@ func (r *BootcNodePoolReconciler) driveRollout(ctx context.Context, pool *bootcv
 	candidates := selectDrainCandidates(rs.staged, avail)
 
 	log.V(1).Info("Rollout state",
-		"idle", len(rs.idle),
+		"upToDate", len(rs.upToDate),
+		"pending", len(rs.pending),
 		"staging", len(rs.staging),
 		"staged", len(rs.staged),
 		"rebooting", len(rs.rebooting),
@@ -298,8 +300,10 @@ func buildRolloutState(log logr.Logger, ownedBootcNodes map[string]*bootcv1alpha
 		log.V(1).Info("Classified node", "node", bn.Name, "state", state.String())
 
 		switch state {
-		case nodeStateIdle:
-			rs.idle = append(rs.idle, bn)
+		case nodeStateUpToDate:
+			rs.upToDate = append(rs.upToDate, bn)
+		case nodeStatePending:
+			rs.pending = append(rs.pending, bn)
 		case nodeStateStaging:
 			rs.staging = append(rs.staging, bn)
 		case nodeStateStaged:
@@ -388,9 +392,13 @@ func nodeNames(nodes []*bootcv1alpha1.BootcNode) []string {
 type nodeState int
 
 const (
-	// nodeStateIdle means the node is running the desired image and the
-	// daemon has no active update cycle.
-	nodeStateIdle nodeState = iota
+	// nodeStateUpToDate means the node is running the desired image.
+	nodeStateUpToDate nodeState = iota
+
+	// nodeStatePending means the node's state is indeterminate: the
+	// daemon hasn't reported yet (no booted status), or hasn't reacted
+	// to a desiredImage change.
+	nodeStatePending
 
 	// nodeStateStaging means the daemon is pulling/staging the image.
 	nodeStateStaging
@@ -409,8 +417,10 @@ const (
 
 func (s nodeState) String() string {
 	switch s {
-	case nodeStateIdle:
-		return "Idle"
+	case nodeStateUpToDate:
+		return "UpToDate"
+	case nodeStatePending:
+		return "Pending"
 	case nodeStateStaging:
 		return "Staging"
 	case nodeStateStaged:
@@ -432,12 +442,12 @@ func classifyNode(bn *bootcv1alpha1.BootcNode) (nodeState, error) {
 	}
 
 	if bn.Status.Booted == nil {
-		// The only way this can happen really is on a brand new BootcNode and
-		// the daemon is still being provisioned. Let's just treat this as Idle
-		// for now and it should resolve in a future reconciliation (though
-		// ideally eventually we can handle 'stuck' states like this... see
+		// The only way this can happen really is on a brand new
+		// BootcNode and the daemon is still being provisioned. It
+		// should resolve in a future reconciliation (though ideally
+		// eventually we can handle 'stuck' states like this... see
 		// related comment below).
-		return nodeStateIdle, nil
+		return nodeStatePending, nil
 	}
 
 	// We could pass in the pool here and use targetDigest instead to avoid
@@ -456,7 +466,7 @@ func classifyNode(bn *bootcv1alpha1.BootcNode) (nodeState, error) {
 	if digested.Digest().String() == bn.Status.Booted.ImageDigest {
 		// Image matches; nothing for the controller to act on
 		// regardless of whether the daemon has settled yet.
-		return nodeStateIdle, nil
+		return nodeStateUpToDate, nil
 	}
 
 	// OK, the node isn't yet booting the desired digest. Let's dig into where
@@ -474,14 +484,12 @@ func classifyNode(bn *bootcv1alpha1.BootcNode) (nodeState, error) {
 		}
 	}
 
-	// Image doesn't match and daemon is either Idle, has no conditions,
-	// or has an unrecognized Idle reason. Classify as Idle since the
-	// daemon should eventually react to the spec change.
-
-	// Hmm, daemon is idle... weird. It could just be that we're racing with the
-	// daemon reconciliation. Or something more broken might be happening (e.g.
-	// daemon not running at all). For now we don't try to detect 'stuck' nodes,
-	// but may in the future. It'll still show up as holding up the pool's
+	// Image doesn't match and daemon is either Idle, has no conditions, or
+	// has an unrecognized Idle reason. This likely means we're racing with
+	// the daemon reconciliation (it hasn't reacted to the spec change
+	// yet), or something more broken is happening (e.g. daemon not running
+	// at all). For now we don't try to detect 'stuck' nodes, but may in
+	// the future. It'll still show up as holding up the pool's
 	// `deployedDigest` field and the updatingCount stat.
-	return nodeStateIdle, nil
+	return nodeStatePending, nil
 }

internal/controller/rollout_envtest_test.go

@@ -27,10 +27,10 @@ func TestSimpleRollout(t *testing.T) {
 
 	const (
 		poolName = "rollout-3node"
-		// All nodes are booting digest B; pool targets digest A.
-		targetRef   = testImageDigestRefA
-		targetImage = testImageDigestRefA
-		bootedImage = testImageDigestRefB
+		// All nodes are booted on digest A; pool targets digest B.
+		oldImage    = testImageDigestRefA
+		newImage    = testImageDigestRefB
+		newImageRef = testImageDigestRefB
 	)
 
 	// Create 3 worker nodes.
@@ -45,7 +45,7 @@ func TestSimpleRollout(t *testing.T) {
 	}
 
 	// Create pool targeting digest A with maxUnavailable: 1.
-	pool := testutil.NewPool(poolName, targetRef,
+	pool := testutil.NewPool(poolName, newImageRef,
 		testutil.WithWorkerSelector(),
 		testutil.WithMaxUnavailable(intstr.FromInt32(1)),
 	)
@@ -66,7 +66,7 @@ func TestSimpleRollout(t *testing.T) {
 	// for the new one. This is the state where nodes have staged the
 	// target image and are waiting for a reboot slot.
 	for _, name := range nodeNames {
-		simulateDaemonStatus(g, ctx, name, testDigestB, bootcv1alpha1.NodeReasonStaged)
+		simulateDaemonStatus(g, ctx, name, testDigestA, bootcv1alpha1.NodeReasonStaged)
 	}
 
 	// Wait for exactly one node to be cordoned (reboot slot assigned).

internal/controller/rollout_test.go

@@ -26,9 +26,12 @@ func TestBuildRolloutState(t *testing.T) {
 	// test focuses more on aggregation: bucketing, slot counting, and
 	// nodeCount.
 	nodes := map[string]*bootcv1alpha1.BootcNode{
-		"idle": testutil.NewNode("idle", desiredImage,
+		"uptodate": testutil.NewNode("uptodate", desiredImage,
 			testutil.WithBootedDigest(testDigestA),
 			testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)),
+		"pending": testutil.NewNode("pending", desiredImage,
+			testutil.WithBootedDigest(otherDigest),
+			testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)),
 		"staged": testutil.NewNode("staged", desiredImage,
 			testutil.WithBootedDigest(otherDigest),
 			testutil.WithNodeCondition(bootcv1alpha1.NodeIdle, metav1.ConditionFalse, bootcv1alpha1.NodeReasonStaged)),
@@ -44,12 +47,13 @@ func TestBuildRolloutState(t *testing.T) {
 
 	rs := buildRolloutState(logr.Discard(), nodes)
 
-	g.Expect(rs.idle).To(HaveLen(1))
+	g.Expect(rs.upToDate).To(HaveLen(1))
+	g.Expect(rs.pending).To(HaveLen(1))
 	g.Expect(rs.staged).To(HaveLen(1))
 	g.Expect(rs.rebooting).To(HaveLen(2))
 	g.Expect(rs.occupiedSlots).To(Equal(2))
 	g.Expect(rs.unclassified).To(BeEmpty())
-	g.Expect(rs.nodeCount()).To(Equal(4))
+	g.Expect(rs.nodeCount()).To(Equal(5))
 }
 
 func TestResolveMaxUnavailable(t *testing.T) {
@@ -180,24 +184,28 @@ func TestClassifyNode(t *testing.T) {
 		want         nodeState
 	}{
 		{
-			name:         "Idle: image matches, Idle=True",
+			name:         "UpToDate: image matches, Idle=True",
 			bootedDigest: desiredDigest,
 			conditions:   []metav1.Condition{idleCond(metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)},
-			want:         nodeStateIdle,
+			want:         nodeStateUpToDate,
 		},
-		// Daemon is idle but there's a diff; for now mark as Idle. See related
-		// comment in classifyNode().
 		{
-			name:         "Idle: image differs, Idle=True",
+			name:         "Pending: image differs, Idle=True (daemon hasn't reacted)",
 			bootedDigest: otherDigest,
 			conditions:   []metav1.Condition{idleCond(metav1.ConditionTrue, bootcv1alpha1.NodeReasonIdle)},
-			want:         nodeStateIdle,
+			want:         nodeStatePending,
 		},
 		{
-			name:         "Idle: no booted status yet (daemon starting)",
+			name:         "Pending: no booted status yet (daemon starting)",
 			bootedDigest: "",
 			conditions:   nil,
-			want:         nodeStateIdle,
+			want:         nodeStatePending,
+		},
+		{
+			name:         "Pending: image differs, no conditions",
+			bootedDigest: otherDigest,
+			conditions:   nil,
+			want:         nodeStatePending,
 		},
 		{
 			name:         "Staging: image differs, Idle=False reason=Staging",