Complete milestone 3b#121
Open
jlebon wants to merge 3 commits into
Open
Conversation
Convert all gomega-based tests to comply with the project conventions documented in REVIEW_GOLANG.md added in bootc-dev/infra#231. Assisted-by: Pi (Claude Opus 4.6) Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
When a node leaves the pool while a drain is in progress, removeBootcNode now cancels the drain goroutine and removes it from the drains map before proceeding with cleanup (uncordon, delete BootcNode, remove managed label). While we're here, store the drain goroutine's context in drainStatus. collectDrainResults can then use it to know if the drain was cancelled or truly errored out. Assisted-by: Pi (Claude Opus 4.6) Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
Assisted-by: Pi (Claude Opus 4.6) Signed-off-by: Jonathan Lebon <jonathan@jlebon.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
As usual, see individual commits. But the main commit here is drain cancellation, and adding a test that this works as expected.
I actually thought on paper about some other drain-related error-handling but it turns out our state machine is quite good already and still converges anyway. There are some things there that are somewhat tricky to balance. E.g. let's say we've already drained a node, and then the target image changes... what should we do? Currently the code just leaves the node cordoned while it stages the new commit and then eventually reboots into the new target image. That might not make sense if your connection is extremely throttled because meanwhile you're not servicing pods. Whereas probably a better approach in that case would be to uncordon on the old node, and stage in the background as usual. But anyway, at least we converge in the end. Handling this kind of "backing out" is surprisingly tricky and it didn't feel worth paying for that complexity just yet until we really hit it in the wild.