etcd-multi-location 2.0.0 — deploy into an existing GVC - #508
Open
jacobecox wants to merge 3 commits into
Open
Conversation
… bootstrap on a bare majority
The chart no longer creates a GVC (createsGvc: false). Every resource lands in
.Values.global.cpln.gvc, and the location list moves from global.gvc.locations
to global.locations.
Why a major: an in-place `helm upgrade` from 1.x drops `kind: gvc` from the
manifest, and Helm prunes what a chart no longer declares — which deletes the
GVC and every workload, volumeset and identity in it. A render-time `fail` on
the legacy `global.gvc` key makes that upgrade impossible rather than merely
discouraged; migration prose is in both the README and the briefing.
The failure this closes is etcd-specific and is the worst one in the audit: a
GVC with fewer locations than the values list makes `--initial-cluster` declare
members that can never exist, and a `new` bootstrap only needs a majority of the
DECLARED set — so 3 declared / 2 present forms a healthy-looking cluster with
zero fault tolerance. Three layers now stop it:
- defaultOptions.minScale/maxScale: 0, with localOptions supplying the real
per-location count, so an undeclared GVC location runs nothing.
- Check A (values only, unconditional): a member whose CPLN_LOCATION is not in
the configured list exits with a named error instead of etcd's obscure one.
- Checks B and C (boot-time GVC read, via a new policy scoped `view` on the
one install GVC): B refuses a fresh bootstrap when quorum is arithmetically
impossible, C refuses one when any declared location is absent. Both warn
rather than fail on an already-initialised member, so a live cluster is never
crashed and the --force-new-cluster recovery path is never blocked. A failed
GVC read is a warning, never a failure.
Also: peer and self FQDNs now use ${CPLN_GVC} rather than a render-time GVC
name, and forceNewClusterInLocation gains a boot-time warning for the case
render cannot see — a recovery location present in the values but absent from
the GVC, which no member would ever apply.
Testing followed the README -- "Scope it with `workload-list` if the GVC holds workloads that should not have it" -- and every member died within ~2 minutes: http://127.0.0.1:2379 is unhealthy: failed to commit proposal: context deadline exceeded etcd: "peer became inactive (message send to peer failed)" / "stopped remote peer" The internal firewall list governs ALL inbound internal traffic, including raft peer traffic on 2380 between the members of this same workload. A list that omits the etcd workload blocks the members from each other and quorum is lost cluster-wide, while Helm prints `upgraded successfully`. Control proving the mechanism rather than a coincidence: re-running the identical upgrade with the etcd workload added to its own list brought the cluster back in 116 s. No legitimate configuration denies the members to each other, so the chart now appends itself instead of requiring the user to know. Rendered across four controls: absent on the default (same-gvc), first in the list when the user names only clients, present exactly once when the user also names etcd, and the previous behaviour untouched when workloads are set on a non-list type. Pre-existing in 1.x, unrelated to the GVC conversion. Also corrects two measured numbers the round refuted: the upgrade outage is a ~105 s rollout with ~20-30 s of lost quorum, not "about 66 seconds" -- and a NO-OP upgrade costs the same, so there is no safe upgrade to plan around. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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.
Review diff:
git diff b5a2bea..HEAD -- etcd-multi-location/versions/2.0.0/(
b5a2beais the verbatimcp -Rof 1.0.2; everything real is in the two commits after it.)Why a major version
The chart no longer creates a GVC — it deploys into the user's existing one. That is a breaking change and, more importantly, an upgrade that must never happen in place: a chart that stops declaring a GVC makes
helm upgradeprune it, and deleting a GVC takes every workload, volumeset and identity inside.Measured deliberately on a throwaway GVC during this round: an upgrade with no values deleted the GVC and all its contents in 5 seconds while printing
upgraded successfully.So 2.0.0 ships a render-time
failwhen the legacygvcvalues key is present. Testing confirmed the refusal happens before any resource is touched — afterwards the GVC was still at version 1 with its workload, volumeset and identity present and ready. One hole is not closable at render (an upgrade passing no values at all sees only the new defaults), so the migration path — stand up a NEW release against an existing GVC, move the data, remove the old release — is stated in both the README and the briefing.The three-layer defence, both arms of each
failon the legacygvckeydefaultOptions.minScale/maxScale: 0+localOptionsdesiredScale: 0, starts nothingtargetKind: gvcpolicy)The read returns 200 with only the scoped policy and 403 against another GVC. The fresh/initialised asymmetry is deliberate: the check can never crash a live cluster.
A pre-existing defect found while testing
internalAccess.type: workload-listsilently destroyed the cluster, reached by following the README's own advice. The internal firewall list also governs raft peer traffic on 2380 between the members of this workload, so a list naming only clients blocked the members from each other:Helm printed
upgraded successfullythroughout. The chart now appends its own workload link. Re-tested live: the identical upgrade that killed the cluster now holds — 0 failed writes across 14 samples out to 9 min 11 s, past the catalog's 305 s firewall-propagation high-water mark. The enforcement control still discriminates (listed client writes, unlisted client times out), so the fix widened the list by one entry rather than loosening the firewall.Present in 1.x and unrelated to the conversion.
Two corrected numbers
The README claimed a
helm upgradecosts "about 66 seconds". Measured: a ~105 s rollout with ~20-30 s of full quorum loss — and a no-op upgrade costs the same, so there is no safe upgrade to plan around. Nothing serializes the restart;rolloutOptions.maxUnavailableReplicasis dropped by the API on stateful workloads.Availability, unchanged by the conversion
leadership transfer.recovery.forceNewClusterInLocationrebuilt from WAL, with a pre-recovery control showing writes genuinely hung first.Testing
27 PASS · 0 FAIL. Drift gate clean — all six resources
Unchanged, spec byte-identical.test-gvcrestored and verified; janitor clean.Untested and stated as such:
same-orgwas proven permissive, not restrictive (no second org available). Auto-compaction reclamation and the quota read-only transition were exercised as flags only.Briefing updated in the same PR.
🤖 Generated with Claude Code