Skip to content

Implement rollout halting and degraded node surfacing#96

Merged
alicefr merged 6 commits into
bootc-dev:mainfrom
jlebon:pr/more-3b
Jul 16, 2026
Merged

Implement rollout halting and degraded node surfacing#96
alicefr merged 6 commits into
bootc-dev:mainfrom
jlebon:pr/more-3b

Conversation

@jlebon

@jlebon jlebon commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Various patches that add more error-handling as described in the 3b implementation plan.

Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
Comment thread internal/controller/rollout_envtest_test.go Outdated
@alicefr

alicefr commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Overall looks good, I only had a question and small nits. Thanks for the work! Love the test cases!

@jlebon
jlebon force-pushed the pr/more-3b branch 2 times, most recently from 2114f0e to 676ee01 Compare July 15, 2026 03:16
@jlebon

jlebon commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the review! Updated for comments.

}

// syncRolloutHaltedCondition sets Degraded/RolloutHalted on the pool. It
// implicitly takes priority over NodeDegraded (which checks for existing

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK took a much larger swing at this in a follow-up: #112

Comment thread internal/controller/rollout.go Outdated
Comment thread internal/controller/rollout.go Outdated
@alicefr

alicefr commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Some minors nits and a question about the NodeConflict condition, but I think it is done on purpose, and it should only be integrated in the function comment.

alicefr
alicefr previously approved these changes Jul 15, 2026
jlebon added 6 commits July 15, 2026 12:42
When a daemon reports Degraded=True on a BootcNode, the controller now
sets the pool's Degraded condition with reason NodeDegraded and a message
listing the affected node names.

Assisted-by: Pi (Claude Opus 4.6)
Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
Add a new Degraded condition reason for when the controller stops
assigning reboot slots because 2 or more nodes in reboot slots are
unhealthy (Degraded or not Ready after rebooting into the target
image).

No behavioral changes yet; the constant is wired up in a follow-up
commit.

Assisted-by: Pi (Claude Opus 4.6)
Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
When 2 or more nodes occupying reboot slots are unhealthy (BootcNode
Degraded after booting the target digest, or K8s Node not Ready after
update), the controller stops assigning new reboot slots and sets the
pool Degraded with reason RolloutHalted. A single unhealthy node
might be a hardware issue, but two suggest a bad image.

The rollout resumes automatically once the unhealthy count drops
below the threshold (e.g. nodes recover or are manually fixed).

Assisted-by: Pi (Claude Opus 4.6)
Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
The controller concurrently patches spec.unschedulable on the same
Node, which bumps the resourceVersion. A Status().Update fails in
that case because it checks the top-level resourceVersion. Switch
to Status().Patch which avoids the conflict entirely.

Assisted-by: Pi (Claude Opus 4.6)
Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
To match the other simulation code in that file and also because it
seems weird to wipe out the `NodeDegraded` type condition type entirely.

Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
This was done in 21c3169 ("daemon: add StatusWatcher for fsnotify +
polling").

Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
@alicefr
alicefr merged commit f976390 into bootc-dev:main Jul 16, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants