Implement rollout halting and degraded node surfacing #96
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,15 @@ import ( | |
| bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" | ||
| ) | ||
|
|
||
| // Hardcode to 2 for now; might make this configurable later, or dynamically | ||
| // adjusted. The rationale is that 1 unhealthy node might be a one-off. But 2 | ||
| // unhealthy nodes becomes a pattern that might indicate a bad image. Now, this | ||
| // _probably_ should scale with number of nodes somehow. E.g. a 500 node | ||
| // cluster could tolerate more unhealthy nodes. Just starting conservative here | ||
| // and we can adjust. See | ||
| // https://github.com/bootc-dev/bootc-operator/issues/99. | ||
| const unhealthySlotHaltThreshold = 2 | ||
|
|
||
| // rolloutState holds the classified BootcNodes for a single reconcile | ||
| // pass. | ||
| type rolloutState struct { | ||
|
|
@@ -59,20 +68,46 @@ func (r *BootcNodePoolReconciler) driveRollout(ctx context.Context, pool *bootcv | |
|
|
||
| rs := buildRolloutState(log, ownedBootcNodes) | ||
|
|
||
| // Flag degraded nodes at the pool level. NodeConflict (set during | ||
| // membership sync) takes priority, so we only set NodeDegraded if | ||
| // the pool isn't already degraded for another reason. | ||
| syncNodeDegradedCondition(pool, rs.degraded) | ||
|
|
||
| // Free reboot slots for nodes that have successfully rebooted into | ||
| // the desired image. This runs before computing available slots so | ||
| // that freed capacity is immediately usable for new candidates. | ||
| if err := r.freeCompletedSlots(ctx, rs); err != nil { | ||
| return fmt.Errorf("freeing completed slots: %w", err) | ||
| } | ||
|
|
||
| // Check for unhealthy nodes on the target digest in reboot slots. If | ||
| // 2+ are unhealthy, halt the rollout. Note that freeCompletedSlots() | ||
| // already removed Ready nodes, so any upToDate node still in a slot is | ||
| // implied to be NotReady. | ||
| unhealthy := findUnhealthySlots(rs, pool.Status.TargetDigest) | ||
|
|
||
| rolloutHalted := len(unhealthy) >= unhealthySlotHaltThreshold | ||
| if rolloutHalted { | ||
| syncRolloutHaltedCondition(pool, unhealthy) | ||
| log.Info("Rollout halted: 2+ unhealthy nodes in reboot slots", | ||
| "unhealthyInSlots", len(unhealthy)) | ||
|
|
||
| // Return early to block new slot assignments and new/restarted | ||
| // drains. Note that collectDrainResults() already ran above, | ||
| // so drains that were in-flight and completed between | ||
| // reconciles will still have their results collected and | ||
| // desiredImageState set to Booted. Trying to "un-drain" and | ||
| // uncordon fully drained nodes is out of scope for now. | ||
| return nil | ||
| } | ||
|
|
||
| maxUnavail, err := resolveMaxUnavailable(pool, rs.nodeCount()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| availableSlots := max(0, maxUnavail-rs.occupiedSlots) | ||
| candidates := selectDrainCandidates(rs.staged, availableSlots) | ||
|
|
||
| log.V(1).Info("Rollout state", | ||
| "upToDate", len(rs.upToDate), | ||
|
|
@@ -84,7 +119,7 @@ func (r *BootcNodePoolReconciler) driveRollout(ctx context.Context, pool *bootcv | |
| "unclassified", nodeNames(rs.unclassified), | ||
| "occupiedSlots", rs.occupiedSlots, | ||
| "maxUnavailable", maxUnavail, | ||
| "availableSlots", availableSlots, | ||
| "candidates", nodeNames(candidates), | ||
| ) | ||
|
|
||
|
|
@@ -358,6 +393,83 @@ func buildRolloutState(log logr.Logger, ownedBootcNodes map[string]*bootcv1alpha | |
| return rs | ||
| } | ||
|
|
||
| // syncNodeDegradedCondition sets Degraded/NodeDegraded on the pool if any | ||
| // nodes are degraded. It respects priority: if the pool is already degraded | ||
| // for another reason (e.g. NodeConflict), it does not overwrite it. | ||
| // XXX(jl): Or... should this be a new condition type so it can be surfaced in | ||
| // parallel? Let's see how this approach feels and iterate. | ||
| func syncNodeDegradedCondition(pool *bootcv1alpha1.BootcNodePool, degraded []*bootcv1alpha1.BootcNode) { | ||
| if len(degraded) == 0 { | ||
| return | ||
| } | ||
| if apimeta.IsStatusConditionTrue(pool.Status.Conditions, bootcv1alpha1.PoolDegraded) { | ||
| return | ||
| } | ||
| names := nodeNames(degraded) | ||
| // Sort so the message is stable across reconciles. | ||
| slices.Sort(names) | ||
| apimeta.SetStatusCondition(&pool.Status.Conditions, metav1.Condition{ | ||
| Type: bootcv1alpha1.PoolDegraded, | ||
| Status: metav1.ConditionTrue, | ||
| Reason: bootcv1alpha1.PoolNodeDegraded, | ||
| Message: fmt.Sprintf("Degraded nodes: %s", strings.Join(names, ", ")), | ||
| }) | ||
| } | ||
|
|
||
| // unhealthySlot identifies a node in a reboot slot that is unhealthy. | ||
| type unhealthySlot struct { | ||
| name string | ||
| reason string // "Degraded" or "NotReady" | ||
| } | ||
|
|
||
| // findUnhealthySlots returns nodes occupying reboot slots that are unhealthy. | ||
| // Two categories qualify: | ||
| // - Degraded nodes (marked by the daemon itself) that booted the target | ||
| // digest (the image itself may be bad). | ||
| // - UpToDate nodes still holding a slot after freeCompletedSlots (not | ||
| // Ready). I.e. the daemon doesn't see anything wrong, but there's something | ||
| // wrong preventing the kubelet from working correctly. | ||
| func findUnhealthySlots(rs *rolloutState, targetDigest string) []unhealthySlot { | ||
| var result []unhealthySlot | ||
| for _, bn := range rs.degraded { | ||
| if !metav1.HasAnnotation(bn.ObjectMeta, bootcv1alpha1.AnnotationInRebootSlot) { | ||
| continue | ||
| } | ||
| // Note we count nil Booted as unhealthy here; this really | ||
| // shouldn't happen because clearly the daemon came up at least | ||
| // once in this node's history to be able to get to a reboot | ||
| // slot. And so Booted should always be set here. | ||
| if bn.Status.Booted == nil || bn.Status.Booted.ImageDigest == targetDigest { | ||
| result = append(result, unhealthySlot{name: bn.Name, reason: "Degraded"}) | ||
| } | ||
| } | ||
| for _, bn := range rs.upToDate { | ||
| if metav1.HasAnnotation(bn.ObjectMeta, bootcv1alpha1.AnnotationInRebootSlot) { | ||
| // Still has annotation after freeCompletedSlots → not Ready. | ||
| result = append(result, unhealthySlot{name: bn.Name, reason: "NotReady"}) | ||
| } | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| // syncRolloutHaltedCondition sets Degraded/RolloutHalted on the pool. It | ||
| // implicitly takes priority over NodeDegraded (which checks for existing | ||
|
Collaborator
There was a problem hiding this comment. What about if there is a NodeConflict, this will be silently wiped out? Also worth mentioning InvalidSpec and TagResolutionError even if in the code will return earlier in the reconcile
Collaborator
Author
There was a problem hiding this comment. OK took a much larger swing at this in a follow-up: #112 |
||
| // daemon-driven degraded status) by running later than | ||
| // syncNodeDegradedCondition. | ||
| func syncRolloutHaltedCondition(pool *bootcv1alpha1.BootcNodePool, unhealthy []unhealthySlot) { | ||
| details := make([]string, len(unhealthy)) | ||
| for i, u := range unhealthy { | ||
| details[i] = u.name + ": " + u.reason | ||
| } | ||
| slices.Sort(details) | ||
| apimeta.SetStatusCondition(&pool.Status.Conditions, metav1.Condition{ | ||
| Type: bootcv1alpha1.PoolDegraded, | ||
| Status: metav1.ConditionTrue, | ||
| Reason: bootcv1alpha1.PoolRolloutHalted, | ||
| Message: fmt.Sprintf("Rollout halted: 2+ unhealthy nodes in reboot slots (%s)", strings.Join(details, ", ")), | ||
| }) | ||
| } | ||
|
|
||
| // resolveMaxUnavailable computes the effective maxUnavailable value from the | ||
| // pool's rollout spec. Defaults to 1 when unset. A value of 0 is allowed and | ||
| // means no reboot slots are available (effectively paused). Returns an | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.