cockroach 2.0.0 — deploy into an existing GVC - #510
Merged
Conversation
…ailure loud The chart no longer creates a GVC. `gvc.name`/`gvc.locations` are gone and `locations` is top-level; a render-time `fail` refuses any values file still carrying a `gvc:` key, because `helm upgrade`ing a 1.x release onto this version drops `kind: gvc` from the manifest and Helm deletes what a chart stops declaring — taking the GVC and every workload, volumeset and identity in it. The audit classified cockroach a Redesign because the location list drove five different things and each of them failed silently when it was wrong: - `defaultOptions.minScale` was `locations[0].replicas`, so a GVC location the values did not list silently started three more nodes that DID join the cluster, advertising a region the database was never told about and that PgBouncer never routed to. Both workloads now pin `defaultOptions` to 0/0 with `localOptions` supplying the real per-location counts, which closes that by construction, plus a startup guard that exits non-zero if a node finds itself in an undeclared location. - The opposite direction is not validated by the platform at all, so each node now reads its own GVC at boot (`view` scoped to that one GVC) and hard-fails on a node with no data, warning on one that already has data so a live cluster is never crashed. - PgBouncer's backend list was rendered by Helm from its own copy of the location list and could disagree with what the nodes joined. It is now built in PgBouncer's startup script from the same CRDB_* env the nodes use. - `backup.location` pointing outside `locations` meant the cron never ran anywhere, with no run, no log and no alert. Now refused at render. - `SURVIVE REGION FAILURE` ran bare under `set -e` in a backgrounded subshell, so a failure killed the subshell without printing an error or the completion line while the install reported success. Now wrapped and reported. Also: `replicas: 0` is refused rather than silently suspending a location that still counts as a region; duplicate and non-list `locations` are refused; the default is a single location rather than 3x3; and the README's restore procedure is corrected — `backup.sh` takes a full-cluster backup, and the documented bare `RESTORE FROM LATEST IN` cannot run against a cluster this template deployed because it always creates `mydb`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ncer is off The self-inclusion existed only in the `pgbouncer.enabled` branch. With pgbouncer DISABLED, `internal_access.type: workload-list` fell through to a branch that rendered the user's list verbatim -- and that list governs node-to-node gossip and range replication between this workload's own replicas. Measured 2026-08-27: node status -> 2,false,false / 3,false,false while every replica still reported `ready: true`. Adding the workload to its own list brought all three back. Fourth instance of this class in one batch (etcd-multi-location, clickhouse, pgedge, cockroach). A catalog audit found 74 templates expose `workload-list` and only 6 include their own workloads. Three render controls: the pgbouncer branch is untouched, the pgbouncer-off workload-list arm now puts the cockroach workload ahead of the client, and the non-list arm is unchanged. Not yet live-tested -- needs a round with an enforcement control, since a render cannot show whether the nodes actually traverse the firewall. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Maintainer rulings on the two pre-existing FAILs testing surfaced.
A18 -- a one-node deployment can never start, and now fails at render. Each node
builds its --join list from every replica EXCEPT itself, so a single node gets an
empty list and CockroachDB exits immediately:
Starting CockroachDB on ... with join []
ERROR: invalid argument "" for "-j, --join" flag (exitCode 4, crash loop)
Nothing caught it, and `validateReplicas` ("must have at least 1 replica")
actively read as endorsing the shape. Three render controls: one replica in one
location is refused; the default (3 in one location) still renders; and 3
locations x 1 replica, which is also 3 nodes, renders.
A19 -- `multiZone: true` wedges silently in some locations. Three controlled
probes isolated it to *stateful + block volumeset* in `aws-us-west-2`; the same
location is fine for standard workloads and for volume-less stateful ones, and
east-1/east-2 are unaffected. That is a platform limitation rather than a chart
defect, so per the maintainer's ruling the chart keeps the knob and tells the
user to confirm their locations support multi-zone before enabling it. Recorded
in values.yaml, the README config block, Important Notes and the briefing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
My first pass at this was incomplete in the same way pgedge's was, and for the
same reason -- I hand-listed the members instead of enumerating them.
Two gaps it left:
- the cockroach workload's else branch added only the cockroach workload, so a
`workload-list` would have denied the BACKUP cron, which connects to
{cockroach}.{gvc}.cpln.local:26257. That is exactly what broke pgedge's
backups: ten consecutive `pg_dumpall: error ... server closed the connection
unexpectedly` (measured 2026-08-28).
- the pgbouncer workload had NO self-inclusion at all -- it rendered the user's
list verbatim. The backup reaches PgBouncer too, and nothing allowed it.
Both now come from `cockroach.ownWorkloadLinks`, defined once and used at all
three call sites, with each member gated on the toggle that creates it. Three
call sites each hand-listing the set is precisely how the first fix drifted, so
the helper is the actual fix; the added links are the symptom.
Four render controls: all three workloads listed when pgbouncer and backup are
enabled; only cockroach plus the client when both are off, so the list never
names a workload that does not exist; pgbouncer's own list self-includes; and no
duplicates when the user names an own workload too. Default render and the
single-node guard both unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`schedule: {{ .Values.backup.schedule }}` was unquoted, and a leading `*` is a
YAML alias indicator -- so every `*/N * * * *` schedule failed at render:
Error: YAML parse error on cockroach/templates/workload-backup.yaml:
error converting YAML to JSON: yaml: line 64: did not find expected
alphabetic or numeric character
Quoting it in the user's own values file does not help; the template has to do
it. Controls isolate it to the leading character: the shipped default
`0 2 * * *` and a comma form both rendered fine before the fix, and `*/15` and
`@daily` both render after it.
Present identically in 1.3.0 through 1.5.0, so pre-existing -- but it is one
line in a version that has not merged, and every user reaching for `*/15` hits
it. The test-runner had to work around it to run round 3.
Note for the sweep: `kafka` renders its volumeset snapshot schedules the same
unquoted way, in every version. 85 other call sites in the catalog quote it.
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
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 4a83634..HEAD -- cockroach/versions/2.0.0/(
4a83634is the verbatimcp -Rof 1.5.0; everything real is in the three commits after it.)Why a major version
The chart no longer creates a GVC — it deploys into the user's existing one. The upgrade across that boundary is destructive: a chart that stops declaring a GVC makes
helm upgradeprune it, taking every workload, volumeset and identity inside.2.0.0 ships a render-time
failon the legacygvckey. Tested against a live 1.5.0 release in its own chart-created GVC: the in-place upgrade with the old values file was refused at render (identity.yaml:2:4), the release stayed at revision 1, and the GVC, workloads, volumeset and identity all still existed afterwards.The three-layer defence passed with both arms of each. Both previously-unproven assumptions are now measured:
CPLN_GVCis present in the PgBouncer container underinheritEnv: false(backend list correctly formed — 3 hosts single-location, 9 multi-location), and the boot GVC read returns 200 with only the new scopedtargetKind: gvcpolicy.Three pre-existing defects found while testing
1. The documented restore could not work.
backup.shrunsBACKUP INTOwith no target — a full-cluster backup — and 1.x told users to restore with a bareRESTORE FROM LATEST IN. CockroachDB refuses a full-cluster restore onto a cluster that already has user databases, and this template always createsmydb/myuser. Reproduced on a real cloud backup:Both documented paths now restore real rows end to end — from S3 (
500 / 3 / 1, md5 payloads intact) and from GCS (120). New caveat documented: the full-cluster path returnsERROR: … node liveness error: restarting in backgroundand bounces a node for ~2 min while nevertheless succeeding, which reads as failure.2.
SURVIVE REGION FAILUREfailed silently — bare underset -ein a backgrounded subshell, printing neither an error nor a completion line while the install reported success, leaving the database at the default zone survival goal. Now verified on 9 nodes across 3 regions:SHOW SURVIVAL GOAL→region, and stopping all threeaws-us-east-2replicas produced zero failed reads or writes across a 2-minute region outage — with a control (two regions down) producing real unavailability, so the probe discriminates.3.
replicas: 0silently suspended a location that still counted as a region no node ever joined. Refused at render, alongside 8 other negatives and a positive control.Two more found in round 1, fixed on maintainer ruling
internal_access.type: workload-listdenied the nodes each other when PgBouncer is off. The self-inclusion existed only in thepgbouncer.enabledbranch. Measured:node status→2,false,false / 3,false,falsewhile every replica reportedready: true. Fourth instance of this class in one batch (etcd-multi-location, clickhouse, pgedge, cockroach).Re-tested with an enforcement control that isolates the firewall as the only variable — identical containers, identical
psql, identical host, only the list differs. Listed client:INSERT 0 1then 3 rows. Unlisted:rc=2on 5/5 attempts with real connection timeouts.node statusfrom each of the three replicas →1,true,true / 2,true,true / 3,true,true.A one-node deployment could never start. Each node builds its
--joinlist from every replica except itself, so a single node gets an empty list:ERROR: invalid argument "" for "-j, --join" flag, exit 4, permanent crash loop.validateReplicas("must have at least 1 replica") actively read as endorsing it. Now refused at render; the smallest supported shape (3 in one location) forms in 119 s and recovers to 3/3 after three rolling restarts.multiZone: truewedges silently in some locations — isolated by three controlled probes to stateful + block volumeset inaws-us-west-2; the same location is fine for standard workloads and volume-less stateful ones, and east-1/east-2 are unaffected. Per maintainer ruling this is a platform limitation, so the knob stays and the user is told to confirm their locations support multi-zone first (values.yaml, README Configuration, Important Notes, briefing).Testing
29 PASS · 0 FAIL across two rounds. Drift gate clean — both no-op upgrades fully
Unchanged, stored specs byte-identical. The rendered-vs-stored ordering difference ininboundAllowWorkload(chart renders self-first, API stores sorted) causes no churn, matching etcd and clickhouse.Two platform findings recorded in CLAUDE.md: a firewall change took 559 s here (new high-water mark — and 286 s for the same knob in the same location next round, so propagation is not location-attributable), and
cpln gvc update --set locationLinks+=…prints success while applying nothing.One open question, non-blocking
A two-node deployment still renders — the new guard fires only on a total of exactly 1, matching the ruling as given. Two nodes start but lose quorum if either dies, so the floor arguably belongs at 3 (CockroachDB's default replication factor). One line to change if you want it.
Briefing updated in the same PR.
🤖 Generated with Claude Code