feat: add node join reverter#6
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an in-memory reverter mechanism for cleaning up membership state when a node join operation fails. The implementation provides automatic rollback of three key components: k8s-dqlite state, etcd membership, and Kubernetes Node objects.
Changes:
- Added three reverter registration functions that clean up datastore state, etcd membership, and Kubernetes node objects on join failure
- Integrated reverters into the
onPostJoinhook with proper ordering after each setup step - Included quorum protection for etcd cleanup (only removes member if cluster has 3+ nodes)
- Added comprehensive unit tests covering success and failure scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/k8sd/app/hooks_join.go | Implements three reverter registration functions and integrates them into the onPostJoin flow at appropriate stages |
| pkg/k8sd/app/hooks_join_reverter_test.go | Adds unit tests for all three reverter types, covering both success and failure scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6 tasks
Contributor
|
@louiseschmidtgen I've opened a new pull request, #7, to work on those changes. Once the pull request is ready, I'll request review from you. |
Signed-off-by: Louise K. Schmidtgen <louise.schmidtgen@canonical.com>
Signed-off-by: Louise K. Schmidtgen <louise.schmidtgen@canonical.com>
3eb5b61 to
ba7f80d
Compare
bschimke95
reviewed
Jan 28, 2026
bschimke95
reviewed
Jan 28, 2026
bschimke95
approved these changes
Jan 28, 2026
6 tasks
ktsakalozos-canonical
pushed a commit
that referenced
this pull request
May 28, 2026
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
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.
Description
This PR adds an in memory reverter for membership state.
If a node fails anywhere in the process membership get's reverted based on where we failed in the join process:
Limitation:
Clean-up of etcd is only desirable if we have less than 3 nodes in the cluster. If we were to remove 1 etcd node from a 2 node configuration we would loose quorum. Admins will need to manually need to clean-up the broken configuration.
The test case does not include a case for reverting etcd as there is no suitable etcd client mock available. Creating one goes beyond the scope of this PR. We would need a full integration test with failure injection.
Backport
1.32, 1.33, 1.34, 1.35
Reference
Closes canonical/k8s-snap#2309 since k8sd was moved out of the k8s-snap.
Checklist
type: title