diff --git a/api/v1alpha1/bootcnodepool_types.go b/api/v1alpha1/bootcnodepool_types.go index 6fc5f6f..f29598d 100644 --- a/api/v1alpha1/bootcnodepool_types.go +++ b/api/v1alpha1/bootcnodepool_types.go @@ -103,6 +103,16 @@ type RolloutSpec struct { // +optional // +kubebuilder:validation:Minimum=0 DrainTimeoutSeconds *int32 `json:"drainTimeoutSeconds,omitempty"` + + // rebootTimeoutSeconds is the maximum time in seconds to wait for a + // node to boot the desired image after a reboot is issued. If the node + // stays in the Rebooting state longer than this (e.g. it failed to + // boot the new image or never rejoined the cluster), it is reported as + // degraded at the pool level. If not set, the controller waits + // indefinitely. + // +optional + // +kubebuilder:validation:Minimum=0 + RebootTimeoutSeconds *int32 `json:"rebootTimeoutSeconds,omitempty"` } // DisruptionSpec controls the disruption behavior during updates. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 8467775..be71e82 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -326,6 +326,11 @@ func (in *RolloutSpec) DeepCopyInto(out *RolloutSpec) { *out = new(int32) **out = **in } + if in.RebootTimeoutSeconds != nil { + in, out := &in.RebootTimeoutSeconds, &out.RebootTimeoutSeconds + *out = new(int32) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RolloutSpec. diff --git a/config/crd/bases/node.bootc.dev_bootcnodepools.yaml b/config/crd/bases/node.bootc.dev_bootcnodepools.yaml index 9ce421a..db05674 100644 --- a/config/crd/bases/node.bootc.dev_bootcnodepools.yaml +++ b/config/crd/bases/node.bootc.dev_bootcnodepools.yaml @@ -179,6 +179,17 @@ spec: are already mid-staging will complete their staging. Tag resolution continues and status.targetDigest is kept current. type: boolean + rebootTimeoutSeconds: + description: |- + rebootTimeoutSeconds is the maximum time in seconds to wait for a + node to boot the desired image after a reboot is issued. If the node + stays in the Rebooting state longer than this (e.g. it failed to + boot the new image or never rejoined the cluster), it is reported as + degraded at the pool level. If not set, the controller waits + indefinitely. + format: int32 + minimum: 0 + type: integer type: object required: - image diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 4fe6216..966f6d5 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -322,6 +322,13 @@ func (r *BootcNodePoolReconciler) Reconcile( // but counts catch up on the next successful reconcile. syncPoolStatus(&pool, rs) + // Carry a time-based requeue (e.g. reboot timeout) unless something + // sooner is already scheduled. + if rs.requeueAfter > 0 && + (resolveResult.RequeueAfter == 0 || rs.requeueAfter < resolveResult.RequeueAfter) { + resolveResult.RequeueAfter = rs.requeueAfter + } + return complete(resolveResult) } diff --git a/internal/controller/rollout.go b/internal/controller/rollout.go index 0872cf3..14dfc3b 100644 --- a/internal/controller/rollout.go +++ b/internal/controller/rollout.go @@ -45,8 +45,19 @@ type rolloutState struct { degraded []*bootcv1alpha1.BootcNode unclassified []*bootcv1alpha1.BootcNode + // rebootTimedOutNodes are the subset of degraded nodes that got there + // by exceeding the reboot timeout (as opposed to a daemon-reported + // error). They booted the old digest, so findUnhealthySlots can't + // recognize them by digest and relies on this list instead. + rebootTimedOutNodes []*bootcv1alpha1.BootcNode + // BootcNodes with the in-reboot-slot annotation occupiedSlots int + + // requeueAfter, when non-zero, asks the reconciler to requeue the + // pool after this duration. Used for time-based transitions like the + // reboot timeout, which fire without any incoming event. + requeueAfter time.Duration } // nodeCount returns the total number of nodes in the pool, including @@ -72,6 +83,14 @@ func (r *BootcNodePoolReconciler) driveRollout( rs := buildRolloutState(log, ownedBootcNodes) + // Detect nodes stuck in the Rebooting state past the configured + // timeout (e.g. they failed to boot the new image or never rejoined + // the cluster) and reclassify them as degraded. The daemon can't + // self-report while the node is down, so the controller surfaces this + // at the pool level. requeueAfter drives the timeout to fire without + // any further Node event. + rs.requeueAfter = applyRebootTimeouts(rs, resolveRebootTimeout(pool), time.Now()) + // Flag degraded nodes at the pool level. if len(rs.degraded) > 0 { names := nodeNames(rs.degraded) @@ -477,6 +496,15 @@ func findUnhealthySlots(rs *rolloutState, targetDigest string) []unhealthySlot { result = append(result, unhealthySlot{name: bn.Name, reason: "NotReady"}) } } + // Reboot-timed-out nodes booted the old digest (they never came back), + // so the digest check above skips them. Count them here: a node that + // failed to boot the target is strong evidence the image is bad, which + // is exactly what the halt is meant to catch. + for _, bn := range rs.rebootTimedOutNodes { + if metav1.HasAnnotation(bn.ObjectMeta, bootcv1alpha1.AnnotationInRebootSlot) { + result = append(result, unhealthySlot{name: bn.Name, reason: "RebootTimeout"}) + } + } return result } @@ -610,6 +638,64 @@ func (s nodeState) String() string { } } +// resolveRebootTimeout returns the configured reboot timeout, or 0 if the +// timeout is disabled (unset). +func resolveRebootTimeout(pool *bootcv1alpha1.BootcNodePool) time.Duration { + if pool.Spec.Rollout == nil || pool.Spec.Rollout.RebootTimeoutSeconds == nil { + return 0 + } + return time.Duration(*pool.Spec.Rollout.RebootTimeoutSeconds) * time.Second +} + +// rebootTimedOut reports whether a BootcNode has been in the Rebooting +// state longer than timeout. A non-positive timeout disables the check. +func rebootTimedOut(bn *bootcv1alpha1.BootcNode, timeout time.Duration, now time.Time) bool { + if timeout <= 0 { + return false + } + idle := apimeta.FindStatusCondition(bn.Status.Conditions, bootcv1alpha1.NodeIdle) + if idle == nil || idle.Reason != bootcv1alpha1.NodeReasonRebooting { + return false + } + return now.Sub(idle.LastTransitionTime.Time) > timeout +} + +// applyRebootTimeouts moves rebooting nodes that have exceeded the reboot +// timeout out of rs.rebooting and into rs.degraded, so the pool reports +// them as degraded (the daemon can't self-report while the node is down). +// It returns the duration after which the pool should be requeued to +// re-evaluate the soonest not-yet-expired rebooting node, or 0 if the +// timeout is disabled or no rebooting nodes remain. +func applyRebootTimeouts(rs *rolloutState, timeout time.Duration, now time.Time) time.Duration { + if timeout <= 0 { + return 0 + } + + remaining := rs.rebooting[:0] + var requeue time.Duration + for _, bn := range rs.rebooting { + if rebootTimedOut(bn, timeout, now) { + rs.degraded = append(rs.degraded, bn) + rs.rebootTimedOutNodes = append(rs.rebootTimedOutNodes, bn) + continue + } + remaining = append(remaining, bn) + + // Schedule a requeue for when this node would time out, so the + // timeout fires even without a further Node event. + idle := apimeta.FindStatusCondition(bn.Status.Conditions, bootcv1alpha1.NodeIdle) + if idle == nil { + continue + } + if left := timeout - now.Sub(idle.LastTransitionTime.Time); left > 0 && + (requeue == 0 || left < requeue) { + requeue = left + } + } + rs.rebooting = remaining + return requeue +} + // classifyNode determines the effective state of a BootcNode. func classifyNode(bn *bootcv1alpha1.BootcNode) (nodeState, error) { // Check Degraded first — takes priority over activity state. diff --git a/internal/controller/rollout_envtest_test.go b/internal/controller/rollout_envtest_test.go index dc4f2df..b6eec7f 100644 --- a/internal/controller/rollout_envtest_test.go +++ b/internal/controller/rollout_envtest_test.go @@ -201,6 +201,138 @@ func TestDegradedNodeSetsPoolCondition(t *testing.T) { }).Should(HaveKey(bootcv1alpha1.AnnotationInRebootSlot), "non-degraded node should get a reboot slot") } +// TestRebootTimeoutMarksNodeDegraded verifies that a node stuck in the +// Rebooting state longer than rebootTimeoutSeconds is reported as degraded +// at the pool level, even though its daemon is down and can't self-report. +func TestRebootTimeoutMarksNodeDegraded(t *testing.T) { + g := NewWithT(t) + g.SetDefaultEventuallyTimeout(pollTimeout) + g.SetDefaultEventuallyPollingInterval(pollInterval) + ctx := context.Background() + + const ( + poolName = "reboot-timeout-pool" + nodeName = "reboot-timeout-w1" + ) + + node := testutil.NewK8sNode(nodeName, testutil.WorkerLabels()) + g.Expect(k8sClient.Create(ctx, node)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, node) + }) + + // Pool targets digest B with a 1s reboot timeout. + pool := testutil.NewPool(poolName, testImageDigestRefB, + testutil.WithWorkerSelector(), + testutil.WithMaxUnavailable(intstr.FromInt32(1)), + testutil.WithRebootTimeoutSeconds(1), + ) + g.Expect(k8sClient.Create(ctx, pool)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, pool) + }) + + g.Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: nodeName}, &bootcv1alpha1.BootcNode{}) + }).Should(Succeed()) + + // Simulate the daemon issuing a reboot: node still booted on the old + // digest, Idle=False/Rebooting. The node never comes back. + simulateDaemonStatus(g, ctx, nodeName, testDigestA, bootcv1alpha1.NodeReasonRebooting) + + // After the reboot timeout elapses, the pool should mark the node + // degraded and reflect it in degradedCount. + g.Eventually(func() ([]metav1.Condition, error) { + var p bootcv1alpha1.BootcNodePool + err := k8sClient.Get(ctx, client.ObjectKey{Name: poolName}, &p) + return p.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", bootcv1alpha1.PoolNodeDegraded), + HaveField("Message", ContainSubstring(nodeName)), + ))) + + g.Eventually(func() (int32, error) { + var p bootcv1alpha1.BootcNodePool + err := k8sClient.Get(ctx, client.ObjectKey{Name: poolName}, &p) + return p.Status.DegradedCount, err + }).Should(Equal(int32(1))) +} + +// TestRebootTimeoutHaltsRollout verifies that when 2+ slotted nodes exceed +// the reboot timeout (they rebooted toward the target but never came back), +// the controller escalates to Degraded/RolloutHalted rather than silently +// stalling. +func TestRebootTimeoutHaltsRollout(t *testing.T) { + g := NewWithT(t) + g.SetDefaultEventuallyTimeout(pollTimeout) + g.SetDefaultEventuallyPollingInterval(pollInterval) + ctx := context.Background() + + const poolName = "reboot-halt-pool" + + nodeNames := []string{"reboot-halt-w1", "reboot-halt-w2"} + for _, name := range nodeNames { + name := name + node := testutil.NewK8sNode(name, testutil.WorkerLabels()) + g.Expect(k8sClient.Create(ctx, node)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, node) + }) + } + + // Pool targets digest B, both nodes may reboot at once, 1s timeout. + pool := testutil.NewPool(poolName, testImageDigestRefB, + testutil.WithWorkerSelector(), + testutil.WithMaxUnavailable(intstr.FromInt32(2)), + testutil.WithRebootTimeoutSeconds(1), + ) + g.Expect(k8sClient.Create(ctx, pool)).To(Succeed()) + t.Cleanup(func() { + _ = k8sClient.Delete(ctx, pool) + }) + + for _, name := range nodeNames { + name := name + g.Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bootcv1alpha1.BootcNode{}) + }).Should(Succeed()) + } + + // Drive both nodes to Staged so they acquire reboot slots (drain + // completes instantly in envtest since there are no pods). + for _, name := range nodeNames { + simulateDaemonStatus(g, ctx, name, testDigestA, bootcv1alpha1.NodeReasonStaged) + } + for _, name := range nodeNames { + name := name + g.Eventually(func() (map[string]string, error) { + var bn bootcv1alpha1.BootcNode + err := k8sClient.Get(ctx, client.ObjectKey{Name: name}, &bn) + return bn.Annotations, err + }).Should(HaveKey(bootcv1alpha1.AnnotationInRebootSlot), "node %s should get a slot", name) + } + + // Both nodes reboot but never come back: still on the old digest, + // Idle=False/Rebooting, holding their slots. + for _, name := range nodeNames { + simulateDaemonStatus(g, ctx, name, testDigestA, bootcv1alpha1.NodeReasonRebooting) + } + + // After the timeout, both count as unhealthy-in-slot → halt. + g.Eventually(func() ([]metav1.Condition, error) { + var p bootcv1alpha1.BootcNodePool + err := k8sClient.Get(ctx, client.ObjectKey{Name: poolName}, &p) + return p.Status.Conditions, err + }).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", bootcv1alpha1.PoolRolloutHalted), + HaveField("Message", ContainSubstring("RebootTimeout")), + ))) +} + // TestUnhealthyNodesHaltRollout verifies that when 2+ nodes in reboot slots // are unhealthy, the controller stops assigning new slots and sets // Degraded/RolloutHalted on the pool. It also verifies recovery: when diff --git a/internal/controller/rollout_test.go b/internal/controller/rollout_test.go index 9f0fcf7..18ffd24 100644 --- a/internal/controller/rollout_test.go +++ b/internal/controller/rollout_test.go @@ -4,6 +4,7 @@ package controller import ( "testing" + "time" "github.com/go-logr/logr" . "github.com/onsi/gomega" @@ -308,3 +309,216 @@ func TestClassifyNode(t *testing.T) { }) } } + +func TestResolveRebootTimeout(t *testing.T) { + g := NewWithT(t) + + secs := func(v int32) *int32 { return &v } + + // Nil rollout / nil field means the timeout is disabled. + g.Expect(resolveRebootTimeout(&bootcv1alpha1.BootcNodePool{})).To(Equal(time.Duration(0))) + + poolNoField := &bootcv1alpha1.BootcNodePool{ + Spec: bootcv1alpha1.BootcNodePoolSpec{Rollout: &bootcv1alpha1.RolloutSpec{}}, + } + g.Expect(resolveRebootTimeout(poolNoField)).To(Equal(time.Duration(0))) + + pool := &bootcv1alpha1.BootcNodePool{ + Spec: bootcv1alpha1.BootcNodePoolSpec{ + Rollout: &bootcv1alpha1.RolloutSpec{RebootTimeoutSeconds: secs(600)}, + }, + } + g.Expect(resolveRebootTimeout(pool)).To(Equal(10 * time.Minute)) +} + +func TestRebootTimedOut(t *testing.T) { + const desiredImage = testImageDigestRefA + now := time.Now() + + rebootingNode := func(transitioned time.Time) *bootcv1alpha1.BootcNode { + return testutil.NewNode( + "n", desiredImage, + testutil.WithBootedDigest(testDigestB), + testutil.WithNodeConditionAt( + bootcv1alpha1.NodeIdle, + metav1.ConditionFalse, + bootcv1alpha1.NodeReasonRebooting, + transitioned, + ), + ) + } + + tests := []struct { + name string + node *bootcv1alpha1.BootcNode + timeout time.Duration + want bool + }{ + { + name: "rebooting past timeout", + node: rebootingNode(now.Add(-11 * time.Minute)), + timeout: 10 * time.Minute, + want: true, + }, + { + name: "rebooting within timeout", + node: rebootingNode(now.Add(-5 * time.Minute)), + timeout: 10 * time.Minute, + want: false, + }, + { + name: "timeout disabled", + node: rebootingNode(now.Add(-1 * time.Hour)), + timeout: 0, + want: false, + }, + { + name: "not rebooting", + node: testutil.NewNode( + "n", desiredImage, + testutil.WithBootedDigest(testDigestB), + testutil.WithNodeConditionAt( + bootcv1alpha1.NodeIdle, + metav1.ConditionFalse, + bootcv1alpha1.NodeReasonStaged, + now.Add(-1*time.Hour), + ), + ), + timeout: 10 * time.Minute, + want: false, + }, + { + name: "no idle condition", + node: testutil.NewNode("n", desiredImage, testutil.WithBootedDigest(testDigestB)), + timeout: 10 * time.Minute, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(rebootTimedOut(tt.node, tt.timeout, now)).To(Equal(tt.want)) + }) + } +} + +func TestApplyRebootTimeouts(t *testing.T) { + const desiredImage = testImageDigestRefA + now := time.Now() + + rebootingNode := func(name string, transitioned time.Time) *bootcv1alpha1.BootcNode { + return testutil.NewNode( + name, desiredImage, + testutil.WithBootedDigest(testDigestB), + testutil.WithNodeConditionAt( + bootcv1alpha1.NodeIdle, + metav1.ConditionFalse, + bootcv1alpha1.NodeReasonRebooting, + transitioned, + ), + testutil.WithNodeAnnotation(bootcv1alpha1.AnnotationInRebootSlot, ""), + ) + } + + t.Run("expired node moves to degraded, fresh node requeued", func(t *testing.T) { + g := NewWithT(t) + + expired := rebootingNode("expired", now.Add(-11*time.Minute)) + fresh := rebootingNode("fresh", now.Add(-4*time.Minute)) + rs := &rolloutState{rebooting: []*bootcv1alpha1.BootcNode{expired, fresh}} + + requeue := applyRebootTimeouts(rs, 10*time.Minute, now) + + g.Expect(rs.degraded).To(HaveLen(1)) + g.Expect(rs.degraded[0].Name).To(Equal("expired")) + g.Expect(rs.rebooting).To(HaveLen(1)) + g.Expect(rs.rebooting[0].Name).To(Equal("fresh")) + // Timed-out nodes are also tracked so findUnhealthySlots can + // escalate to a halt regardless of their booted digest. + g.Expect(rs.rebootTimedOutNodes).To(HaveLen(1)) + g.Expect(rs.rebootTimedOutNodes[0].Name).To(Equal("expired")) + // fresh transitioned 4m ago with a 10m timeout → 6m remaining. + g.Expect(requeue).To(Equal(6 * time.Minute)) + }) + + t.Run("disabled timeout is a no-op", func(t *testing.T) { + g := NewWithT(t) + + node := rebootingNode("n", now.Add(-1*time.Hour)) + rs := &rolloutState{rebooting: []*bootcv1alpha1.BootcNode{node}} + + requeue := applyRebootTimeouts(rs, 0, now) + + g.Expect(rs.degraded).To(BeEmpty()) + g.Expect(rs.rebooting).To(HaveLen(1)) + g.Expect(requeue).To(Equal(time.Duration(0))) + }) +} + +func TestFindUnhealthySlots(t *testing.T) { + const ( + desiredImage = testImageDigestRefA + targetDigest = testDigestA + oldDigest = testDigestB + ) + + slottedNode := func(name, bootedDigest string) *bootcv1alpha1.BootcNode { + return testutil.NewNode( + name, desiredImage, + testutil.WithBootedDigest(bootedDigest), + testutil.WithNodeAnnotation(bootcv1alpha1.AnnotationInRebootSlot, ""), + ) + } + + t.Run("degraded on target digest counts, on old digest does not", func(t *testing.T) { + g := NewWithT(t) + onTarget := slottedNode("on-target", targetDigest) + onOld := slottedNode("on-old", oldDigest) + rs := &rolloutState{degraded: []*bootcv1alpha1.BootcNode{onTarget, onOld}} + + got := findUnhealthySlots(rs, targetDigest) + + g.Expect(got).To(ConsistOf(unhealthySlot{name: "on-target", reason: "Degraded"})) + }) + + t.Run("reboot-timed-out node counts regardless of digest", func(t *testing.T) { + g := NewWithT(t) + // A timed-out node never booted the target; it sits on the old + // digest but must still count toward the halt threshold. + timedOut := slottedNode("timed-out", oldDigest) + rs := &rolloutState{ + degraded: []*bootcv1alpha1.BootcNode{timedOut}, + rebootTimedOutNodes: []*bootcv1alpha1.BootcNode{timedOut}, + } + + got := findUnhealthySlots(rs, targetDigest) + + g.Expect(got).To(ConsistOf(unhealthySlot{name: "timed-out", reason: "RebootTimeout"})) + }) + + t.Run("upToDate node still in a slot is NotReady", func(t *testing.T) { + g := NewWithT(t) + notReady := slottedNode("not-ready", targetDigest) + rs := &rolloutState{upToDate: []*bootcv1alpha1.BootcNode{notReady}} + + got := findUnhealthySlots(rs, targetDigest) + + g.Expect(got).To(ConsistOf(unhealthySlot{name: "not-ready", reason: "NotReady"})) + }) + + t.Run("unslotted timed-out node is ignored", func(t *testing.T) { + g := NewWithT(t) + unslotted := testutil.NewNode( + "unslotted", + desiredImage, + testutil.WithBootedDigest(oldDigest), + ) + rs := &rolloutState{ + degraded: []*bootcv1alpha1.BootcNode{unslotted}, + rebootTimedOutNodes: []*bootcv1alpha1.BootcNode{unslotted}, + } + + g.Expect(findUnhealthySlots(rs, targetDigest)).To(BeEmpty()) + }) +} diff --git a/test/util/builders.go b/test/util/builders.go index a6a882d..1b946b9 100644 --- a/test/util/builders.go +++ b/test/util/builders.go @@ -5,6 +5,8 @@ package testutil import ( + "time" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -112,6 +114,16 @@ func WithDrainTimeoutSeconds(seconds int32) PoolOption { } } +// WithRebootTimeoutSeconds sets the rollout reboot timeout in seconds. +func WithRebootTimeoutSeconds(seconds int32) PoolOption { + return func(pool *bootcv1alpha1.BootcNodePool) { + if pool.Spec.Rollout == nil { + pool.Spec.Rollout = &bootcv1alpha1.RolloutSpec{} + } + pool.Spec.Rollout.RebootTimeoutSeconds = &seconds + } +} + // WithLabel sets a metadata label on the pool. func WithLabel(key, value string) PoolOption { return func(pool *bootcv1alpha1.BootcNodePool) { @@ -178,6 +190,28 @@ func WithNodeCondition(condType string, status metav1.ConditionStatus, reason st } } +// WithNodeConditionAt appends a condition with an explicit +// LastTransitionTime. Useful for exercising time-based logic such as +// reboot timeouts. +func WithNodeConditionAt( + condType string, + status metav1.ConditionStatus, + reason string, + at time.Time, +) NodeOption { + return func(node *bootcv1alpha1.BootcNode) { + node.Status.Conditions = append( + node.Status.Conditions, + metav1.Condition{ + Type: condType, + Status: status, + Reason: reason, + LastTransitionTime: metav1.NewTime(at), + }, + ) + } +} + // WithNodeAnnotation sets a single annotation on the node. func WithNodeAnnotation(key, value string) NodeOption { return func(node *bootcv1alpha1.BootcNode) {