Repository navigation
feat(role): judge voters by etcd's own vote and let a holder renew the regrade Lease - #210
Merged
Merged
Conversation
The core acquires the Lease once for a step sized at a few operation timeouts, so a holder that works for longer, such as the role controller, had no way to keep it. renew_lease takes the holder's identity, writes a later renew time and keeps everything else (acquire time, duration, annotations). It fails, leaving the Lease as it was, when another holder has it, when it was released or is absent, or when it changed while being renewed.
k3s labels a server as an etcd node when it joins as a learner and never removes the label, so a
learner counted as a voter and a removed member still did.
A voter is now a node with the etcd label and the EtcdIsVoter condition True. A labelled node
without the condition is reported as a block ("member but not a voter") and is no longer among the
voters, so it also starts the membership cooldown when it is promoted.
The decision takes the voter count to expect as config.expected_voters, the target or one more
while a swap holds a new voter. A pass of the controller expects the target.
Mearman
marked this pull request as ready for review
November 3, 2026 22:10
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins. |
|
🎉 This PR is included in version 2.33.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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.
Refs #183
PR E1 of the implementation plan in #183: a voter is judged by etcd's own vote, and the holder of the regrade Lease can renew it.
Lease.
renew_lease HOLDERinfleet-regrade-core.shreplaces the Lease with a laterrenewTimeand keeps the holder,acquireTime, duration and annotations. It fails and leaves the Lease alone when another holder has it, when it was released or is absent, when it cannot be read, or when it changed while being renewed (the replacement carries theresourceVersionit read).Voters.
scripts/fleet-role.jqreads a voter as a node with k3s's etcd label and theEtcdIsVoter=Truecondition. A node with the label and no condition is a member but not a voter: it is left out of the voters, so it also starts the membership cooldown when it is promoted, and it is a block ("member but not a voter"). The count the gate checks is$config.expected_voters, the target or the target plus one while a swap holds a new voter;scripts/fleet-role.shpasses the target, since this controller still changes no membership.Decisions where the plan and the code differ:
renew_leasetakes the holder as an argument and notNODE_NAME: the controller's holder is its own identity, which is no node. A holder whose Lease has run out may renew it if nobody took it since; the replacement'sresourceVersioncheck is what keeps that safe.fleet-role.shis touched beyond the plan's file list: withoutexpected_votersin its config the jq would read null and every pass would block on the count. The README's Role controller paragraph names the new voter rule.node(voter=True)) and a learner only the label (node(learner=True)); no existing test was edited.fleet-role.jqdirectly, since nothing in the controller yet raises the count above the target; the swap that does is PR E2.Tests (each seen failing against the old code first): renew keeps the holder, moves the renew time to now, keeps
acquireTimeand the duration, keeps annotations, extends past the earlier expiry, is refused for another holder, a released Lease, no Lease, a conflicting write and an unreadable Lease; a learner by label is not a voter and blocks; a node whose condition is False is not a voter; the expected count is the target, or the target plus one while a swap holds a new voter, and a count short of it blocks. Each rule was broken once in the code to see its tests fail.