feat: follow loki and tempo to their new home, and notice next time - #31
Merged
Conversation
Both observability charts moved out of grafana/helm-charts, matching the catalog
change in eks-gitops. One announced it; the other did not.
─── The two moves ───
tempo is flagged `deprecated: true` and its README names grafana-community as
the destination. The fork carries the full history, so it is the same chart with
the same single-binary topology, renumbered at the fork: 1.24.4 -> 2.2.3, app
2.9.0 -> 2.10.7. tempo-distributed is not the fallback the old comment in this
repo suggested — it was deprecated in the same move, and that comment is gone.
loki is the quieter one. The OSS chart moved to grafana-community, forked at
6.55.0; what stayed at grafana/helm-charts is the Grafana Enterprise Logs chart.
It sets no deprecation flag. The pin resolved, the chart installed, the render
gate passed, and Renovate kept offering 7.x patches for a chart that had changed
product. 7.2.0 -> 18.7.5, the OSS lineage.
─── The one real breakage, which rendering could not see ───
With persistence off, the new loki chart mounts its own emptyDir at /var/loki.
That is precisely the gap this repo's `extraVolumes` workaround existed to
fill — loki mkdir'ing storage and ruler dirs onto a read-only rootfs and
crashing. Keeping both gives the loki container two volumeMounts on one
mountPath, which Kubernetes rejects outright: the pod never starts.
`helm template` renders it and exits 0. render-check.sh only ever inspected the
exit status, so it would have shipped green.
So the workaround is removed, and the render gate now pipes each slice through
scripts/check-rendered-mounts.py instead of discarding it. That check has a
self-test covering the case that matters in both directions — two volumes on one
path in one container fails, the same path across two different containers is
legal — plus initContainers, nested CronJob pod specs, and a null volumeMounts.
It also refuses a stream that parses to zero manifests, so it cannot pass by
examining nothing.
It needs a constructor for YAML 1.1's `=` value tag: prometheus-operator-crds
ships a CRD containing one, and SafeLoader raises on it. Without that the choice
was failing the gate on an unrelated chart or swallowing the document whole.
─── The check ───
scripts/check-chart-deprecation.py, the kx side of the same check now in
eks-gitops. Split by what is and is not a function of this commit:
offline, in ci.yml, BLOCKING — every pinned chart has a provenance record and
every record names a chart still pinned.
--live, weekly in chart-provenance.yml — fetches all 32 pins and fails on a
`deprecated: true` or a description that no longer matches its record.
Recording the description is what catches the loki class, where nothing else
moved. Recording the repository also closes a gap in mirror-check, which
compares chart to version and never looks at where a chart came from: the two
repos could pull one version from two different repositories and it would pass.
Changing where a chart comes from now has to be re-recorded deliberately.
That matters most for the seven kx-only slices mirror-check does not cover at
all. Nothing upstream watches those.
─── Two things the pin move surfaced ───
`mirror-check.py sync` wrote its manifest with json.dump defaults, so every
em-dash in the divergence reasons came back as a — escape and moving a
one-line ref rewrote prose across the file. Fixed with ensure_ascii=False.
Moving the pin to the merged catalog commit also surfaced a divergence that had
been invisible: eks-gitops began pinning the eks-agent-platform operator chart
after kx's last sync. Both sides run the operator; only eks-gitops pins a
version, because kx installs the chart from the sibling checkout and kind-loads
an image built from that tree. Declared in stack/upstream.json rather than
papered over — an omission and a decision should not look the same.
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.
Mirrors the catalog change in eks-gitops#190.
Two moves, one announced
tempo is
deprecated: trueand its README namesgrafana-communityas the destination. The fork carries the full chart history, so it's the same chart with the same single-binary topology, renumbered at the fork:1.24.4→2.2.3, app2.9.0→2.10.7.tempo-distributedis not the fallback the old comment in this repo suggested — it was deprecated in the same move. That comment is gone.loki is the quiet one. The OSS chart moved to the same place, forked at 6.55.0; what stayed behind is the Grafana Enterprise Logs chart, and it sets no deprecation flag. The pin resolved, the chart installed, the render gate passed, Renovate kept offering 7.x patches.
7.2.0→18.7.5.The breakage rendering could not see
With persistence off, the new loki chart mounts its own emptyDir at
/var/loki— precisely the gap this repo'sextraVolumesworkaround existed to fill. Keeping both gives the loki container two volumeMounts on one path:Kubernetes rejects that outright; the pod never starts.
helm templaterenders it and exits 0, andrender-check.shonly ever inspected the exit status — so this would have shipped green.The workaround is removed, and the render gate now pipes each slice through
scripts/check-rendered-mounts.pyinstead of discarding the output. Its self-test covers the case in both directions:It also refuses a stream that parses to zero manifests, so it can't pass by examining nothing. And it needs a constructor for YAML 1.1's
=value tag —prometheus-operator-crdsships a CRD containing one andSafeLoaderraises on it; without that the choice was failing the gate on an unrelated chart or swallowing the document whole.The check
scripts/check-chart-deprecation.py, the kx side of the check in eks-gitops. Split by what is and isn't a function of this commit:ci.yml, blocking — every pinned chart has a provenance record, every record names a chart still pinned.--live, weekly inchart-provenance.yml— fetches all 32 pins, fails ondeprecated: trueor a description that no longer matches.Recording the description is what catches the loki class, where nothing else moved. Recording the repository also closes a gap in
mirror-check, which compares chart→version and never looks at where a chart came from — the two repos could pull one version from two different repositories and it would pass. That matters most for the seven kx-only slicesmirror-checkdoesn't cover at all; nothing upstream watches those.Separate files from the eks-gitops copy rather than a shared module, because the two repos state their pins in different languages (install.sh vs ApplicationSet YAML), and a repo that can't gate itself without cloning another one isn't really gated.
Two things the pin move surfaced
mirror-check.py syncmangled its own manifest. It wrote JSON withjson.dumpdefaults, so every em-dash in the divergence reasons came back as a—escape — moving a one-line ref rewrote prose across the whole file. Fixed withensure_ascii=False.An undeclared divergence. Moving the pin to the merged catalog commit surfaced that eks-gitops began pinning the eks-agent-platform
operatorchart after kx's last sync. Both sides run the operator; only eks-gitops pins a version, because kx installs the chart from the sibling checkout and kind-loads an image built from that tree. Declared instack/upstream.jsonrather than papered over — an omission and a decision shouldn't look the same. Pre-existing, not caused here.Verification
Config carry-over was checked by rendering old and new with the same values: receivers, storage backend, schema v13, retention and replication all identical. tempo's whole manifest diff is the image bump, one newly-explicit default, and upstream dropping the OpenCensus receiver port nothing here enabled.