grafana-multi-location 2.0.0 — deploy into an existing GVC - #520
Merged
Conversation
MAJOR. 1.x created its own GVC; 2.0.0 creates none and deploys into `global.cpln.gvc`. A `helm upgrade` across that boundary drops `kind: gvc` from the manifest and Helm prunes it, destroying the GVC and every workload, volumeset and identity in it — here both Grafana tiers, the whole Patroni cluster and its data volumes, etcd, and the Redis tier. Last of the eleven conversions, and the only one vendoring two converted subcharts: postgres-multi-location 1.0.3 -> 2.0.0 and redis-multi-location 2.1.0 -> 3.0.0. `global.gvc.locations` -> `global.locations`, one shared list still serving all three charts. Three-layer defence: - render-time `fail` on the legacy `global.gvc` key (verified with `-f` against the shipped 1.1.1 values file, not just `--set`) - `defaultOptions.minScale/maxScale: 0` on the UI tier with complete per-location `localOptions` - a boot-time GVC read in both Grafana wrappers, behind a `view` policy scoped by `targetLinks` to the one install GVC The boot check uses curl (the pinned image has curl + bash + timeout and no perl/python3), pins HTTP/1.1, and bounds both the connect phase and an accept-never-respond server — both measured in the image against genuinely slow failures. Guards keyed only off this replica's own location are FATAL; findings from the GVC read are WARNINGs, because both tiers are stateless and there is no fresh/initialised discriminator to key an asymmetry off. Also fixed in this version: - `grafana-ml.ownWorkloadLinks` + `grafana-ml.internalFirewall`, so a `workload-list` cannot cut the release off from itself and exactly one `inboundAllowWorkload` key is emitted - `grafana-ml.validateSubchartFirewall` refuses to render when a subchart is set to `workload-list` without the Grafana workloads, which would otherwise silently cut Grafana off from its database or its alerting coordinator - `redisML.engine` exposed (default `redis`), closing the Valkey gap the dependency bump opens README gains a `Migrating from 1.x` section; briefing updated and reconciled against the shipped values. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rkload-list On the DEFAULT `same-gvc` type both Grafana workloads drifted from their own manifest from creation: the chart sent a partial `internal` block and the API completed it with `inboundAllowWorkload: []`. The three subchart tiers already ship the key unconditionally and never drifted, which is what isolated it. The helper's own comment justified the omission by citing 1.1.1's clean drift gate. That reasoning was wrong, and it is worth recording WHY, because the same argument would justify the same bug in any chart: **a `helm upgrade` drift gate cannot see this class.** Upgrade #2 reported `Unchanged` for these two workloads while the difference was present the whole time -- the field is backfilled once at creation and then stable, so nothing ever churns. Only comparing the render against the stored spec finds it. Comment rewritten to say so. Both arms verified: `same-gvc` now emits `[]`, and `workload-list` still emits this release's own workload links with user duplicates de-duped. All five workloads across the parent and both subcharts now carry exactly one key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tall The README described the silent shape -- evaluator holding zero replicas, dashboards perfectly healthy, nothing being evaluated -- without saying when it applies. Testing established that it applies only AFTER initialisation. On a fresh install the user finds out immediately: `alerting.location` must also be in `global.locations`, and etcd and Patroni refuse to bootstrap on a fresh data directory for any location the GVC lacks, so the whole stack crash-loops with a named error. Only a location REMOVED from the GVC after initialisation produces the quiet version. That distinction is what settles whether the guard needs to be fatal: it does not. The loud case is already loud, and making the quiet case fatal would take the UI down over an alerting misconfiguration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sured The README stated 9 m 0 s - 9 m 30 s. The final round measured 10 m 15 s, past the stated ceiling. A user who time-boxes on the old figure concludes the rollout is stuck and starts editing policies -- which is exactly what the note exists to prevent -- so the range now reflects all three rounds and says to keep polling rather than time-box. 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 919c605..HEAD -- grafana-multi-location/versions/2.0.0/(
919c605is the verbatimcp -Rof 1.1.1; everything real is in the commits after it.)The tenth and last GVC conversion, and the only one vendoring two converted subcharts:
postgres-multi-location1.0.3 → 2.0.0 andredis-multi-location2.1.0 → 3.0.0, with one sharedglobal.locationsstill driving all three charts.The gate holds. 1.1.1 installed into a throwaway chart-created GVC; the in-place upgrade exited 1 with empty stdout, and re-reading showed the GVC still at
version: 1with all 5 workloads, 2 volumesets and 3 identities intact.The defect worth reading, because it changes what we trust
On the default
internalAccess.type: same-gvc, both Grafana workloads drifted from their own manifest from creation: the chart sent a partialinternalblock and the API completed it withinboundAllowWorkload: [].The helper's own comment justified omitting the key by citing 1.1.1's clean drift gate — and that reasoning would have justified the same bug in any chart. A
helm upgradedrift gate structurally cannot see this class. Upgrade #2 reportedUnchangedfor those two workloads while the render and the stored spec disagreed the entire time, because the field is backfilled once at creation and then never churns.Unchangedwas the honest answer from that gate throughout.This is the first time
render-vs-storedhas caught something the older gate declared clean, which is exactly the gap it was added to close. The comment is rewritten to say so rather than deleted.Fixed and re-verified on a fresh install with
internalAccessleft entirely unset, so the previously-broken arm is what got installed:0 unexplained difference(s)across all 19 resources, confirmed independently of the script by reading the stored spec back. Theworkload-listarm was re-tested with a list that deliberately repeats a workload the chart auto-adds, exercising the de-dup; also 0. Drift gate: upgrade #1's only difference anywhere is the platform revision tag, and the tier carrying the fix reportedUnchangedoutright; upgrade #2 all 19Unchanged. Cold start 4 m 11 s against 4 m 12 s pre-fix, so the change costs nothing at boot.The problem unique to this chart
Grafana must reach both children (HAProxy 5432/8404, Sentinel 26379, Redis 6379), but the children never initiate to Grafana — so Grafana belongs in their lists, not the reverse. A parent cannot inject a rendered name into a subchart's values, so the chart refuses to render when either child is set to
workload-listwithout the Grafana links, printing the exact strings to add. Enforcement verified live with a temporal control that shows the unlisted client provably could reach the URL moments before the knob landed:The severity split, and why
.Release.IsInstallwas rejectedBoth Grafana tiers are stateless — no volumeset, all state in Patroni — so there is no fresh-vs-initialised discriminator.
.Release.IsInstallwas considered and rejected: it renders a different container argument on install than on upgrade, which is permanent drift on the first no-op upgrade. Severity is split by cost instead: FATAL for guards keyed off a replica's own location, WARNING for anything derived from the GVC read, because crash-looping every UI instance over a topology mismatch turns a degraded deployment into an outage with no data at risk.alerting.location— the open question, answeredConfirmed and partly refuted. The evaluator does hold zero replicas with nothing visible in the UI — but on a fresh install the user never reaches that state:
alerting.locationmust also be inglobal.locations, and etcd and Patroni refuse to bootstrap on a fresh data directory for any location the GVC lacks, so the whole stack crash-loops with a named error. The silent shape is real only for a location removed after initialisation.So the guard stays a warning — the loud case is already loud, and making the quiet case fatal would take the UI down over an alerting misconfiguration. The README now states the distinction.
Other results
Boot GVC read returns 200 from the chart's own curl+
sedpipeline insidegrafana/grafana:13.1.3against the real$CPLN_ENDPOINT, with the 403 control giving all three retry attempts plus the WARNING branch while that replica kept serving. (The image has no perl and no python3, hencesed; HTTP/1.1 was confirmed on the wire, so redis-ml's 426 trap does not apply here.)minScale: 0confinement holds including storage — the undeclared location appears nowhere in either volumeset'sstatus.locations.Two findings that refine CLAUDE.md's ~4-minute auth cache: revocation was visible to a running replica in ~11 s, but replicas booted 27-92 s after the policy delete still read 200 — so the cache is per-replica-boot rather than global. And the 3-attempt retry loop is load-bearing: two replicas hit a transient 403 with the policy fully in place and recovered ~2 s later.
Known and stated
grafana1.2.0 is the single-location template.etcd-multi-location. Helm renders the deepest subchart first and all four charts guard the same key, so a sibling's copy aborts first. Protection intact, wording is not ours; not fixable from the parent.smtp.*andsame-org(render-only), Redis/Sentinel passwords,replicas > 1, failover behaviour.cpln/marketplace-template: postgresML.Briefing updated and reconciled against the shipped values.
🤖 Generated with Claude Code