Skip to content

fix(chart): pin mysql-client by digest — keep fleets on the 5.7-lineage prod image - #454

Merged
LukasWodka merged 5 commits into
developfrom
fix/pin-mysql-client-digest
Jul 28, 2026
Merged

fix(chart): pin mysql-client by digest — keep fleets on the 5.7-lineage prod image#454
LukasWodka merged 5 commits into
developfrom
fix/pin-mysql-client-digest

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

images.mysqlClient now defaults to a digest pin on the current tracebloc/mysql-client:prod image (5.7-lineage, pushed 2026-04-24) instead of following the floating :prod tag.

Why: the repo's Dockerfile.mysql_client has moved ahead to MySQL 8.4 (backend#723), but every existing workspace datadir is 5.7-format, and MySQL supports only staged upgrades (5.7 → 8.0 → 8.4) — an 8.4 server refuses to start on a 5.7 datadir. With a floating tag, any republish of :prod would reach fleets implicitly: fresh installs immediately, running workspaces on their next fresh pull (cache eviction / node replacement). Pinning by digest makes the MySQL engine upgrade an explicit, reviewed chart change — exactly what the surrounding comment already recommends ("pin via digest instead").

Decided 2026-07-28 (Lukas + Saqlain) as part of the mysql-client migration sequencing on backend#723. The companion guard on the manual build path is in tracebloc/client-runtime#213.

Type

  • Fix

Test plan

  • helm template (with required install values set) renders the mysql container as docker.io/tracebloc/mysql-client@sha256:f546e47f… — the digest path of the tracebloc.image helper. Verified locally.
  • Digest verified equal to the current Docker Hub :prod manifest digest (tag_last_pushed 2026-04-24).
  • values.schema.json digest pattern (^(sha256:[a-f0-9]{64})?$) satisfied — chart schema validation passes.

Rollout

No image change is triggered by this PR — it pins what fleets already run. Takes effect per fleet at its next helm upgrade. Remove/replace the pin only as an explicit step of the staged migration in backend#723.

🤖 Generated with Claude Code


Note

Medium Risk
Pins the data-plane MySQL image digest (correctness-sensitive for 5.7 datadirs); rollout is no-op until helm upgrade, with low blast radius because the pin matches the current tag.

Overview
Pins images.mysqlClient.digest in chart defaults to the current MySQL 5.7-lineage tracebloc/mysql-client:prod image so fleets no longer follow the floating :prod tag. A republish of :prod (e.g. toward 8.4 per backend#723) would otherwise reach installs implicitly while existing workspace datadirs still require staged upgrades; behavior is unchanged because the digest matches what the tag already resolved to.

Adds a Helm unittest on mysql-deployment.yaml that the mysql container image renders as docker.io/tracebloc/mysql-client@sha256:….

The e2e auto-upgrade script now reads the baseline release’s images.ingestor.prodDigest via helm get values + jq (with a jq preflight) and asserts that helm upgrade --reuse-values either leaves the ingestor pin absent (pre-pin baselines) or replays the baseline digest verbatim—not the new chart default—with notes on path-1/path-2 interaction when pins diverge (client#459).

Reviewed by Cursor Bugbot for commit c118c61. Bugbot is set up for automated code reviews on this repo. Configure here.

…ge prod image (backend#723)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Jul 28, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8408cd3. Configure here.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

The "Fleet auto-upgrade E2E (k3d)" failure is unrelated to this diff — it's a baseline shift, and it will hit every chart-touching PR until fixed:

  • The failing assertion (scripts/tests/e2e-auto-upgrade.sh:139-145) encodes: "plain --reuse-values replays the old release's COMPUTED values, which predate images.ingestor.prodDigest, so the prod pin does NOT arrive on this path."
  • Yesterday's develop → main promotion (Release develop → main #383, 2026-07-27) published a chart release that now includes the prod ingestor pin (fix(chart): move the prod ingestor pin into chart defaults so it reaches edges #398). So the "last published release" the test upgrades from now carries prodDigest in its stored values, --reuse-values replays it, jm_ingestor_digest is non-empty, and the assertion fails — exactly as its own comment predicts for this era boundary.
  • Helm Chart CI was green on develop all of yesterday (pre-promotion baseline); this PR is simply the first chart PR to run against the new baseline. Nothing in this diff touches the ingestor pin (it changes one value: images.mysqlClient.digest; Bugbot, helm lint/unit, and all template renders pass).

Fix (separate small PR): make the expectation era-aware — derive it from whether the baseline release's computed values contain images.ingestor.prodDigest, instead of hardcoding the pre-#398 era.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

The E2E baseline fix is up as #455 — once it merges, I'll re-run this PR's failed check (the merge ref will then include the era-aware assertion).

…ase now carries the prod pin
The path-1 assertion hardcoded the pre-#398 era: it expected NO ingestor
pin after a --reuse-values upgrade because the published release's
computed values predated images.ingestor.prodDigest. The #383 promotion
(2026-07-27) published a release that includes the pin, so replayed
computed values now carry it and the assertion fails on every
chart-touching PR (first hit: #454).
The expectation is now read from the baseline release itself
(helm get values --all): pin absent => must not arrive (old behavior);
pin present => the SAME digest must be replayed verbatim. Both eras keep
asserting the actual limitation: --reuse-values never injects new chart
defaults. Path-2's era-stale comment corrected, with a note on the
replay-contamination signal a future pin bump will surface.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Cherry-picked the era-aware E2E assertion fix (#455, commit 8879713b77d95d here) onto this branch so this PR's CI is self-contained — the failing "Fleet auto-upgrade E2E (k3d)" check re-runs now with the corrected assertion in the merge ref.

The fix is already proven: #455's own E2E leg went green against the real post-#383 baseline (the pin-carrying release). Same commit content in both PRs, so either merge order is clean:

bugbot run

…CI runners)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@saqlainsyed007

Copy link
Copy Markdown
Contributor

Code Review

Overview

Two defensive changes:

  1. client/values.yaml — sets images.mysqlClient.digest to a digest pin instead of "" (floating :prod). Freezes fleets on the current MySQL 5.7-lineage image so a future :prod republish (8.4, backend#723) can't reach a 5.7 datadir implicitly and brick workspaces.
  2. scripts/tests/e2e-auto-upgrade.sh — makes path 1 (--reuse-values) era-aware: it reads the baseline release's computed images.ingestor.prodDigest and asserts either "no pin" (pre-pin baseline) or "verbatim replay of the baseline pin" (post-pin baseline), rather than always assuming the pin is absent.

Correctness — verified ✅

  • Digest matches the live tag. Docker Hub :prod currently resolves to sha256:f546e47f…163354 — byte-for-byte the pinned value. The core claim ("pins what fleets already run, no image change") holds.
  • Schema passes. Value satisfies ^(sha256:[a-f0-9]{64})?$.
  • Render path is correct.mysql-deployment.yaml:65 calls tracebloc.image with the mysqlClient digest; the helper emits docker.io/tracebloc/mysql-client@sha256:… when digest is set. imagePullPolicy: IfNotPresent + a digest ref actually strengthens the guard — a node that cached an 8.4 :prod won't satisfy the 5.7 digest, so kubelet pulls the correct image.
  • e2e branch logic is sound.helm get values --all -o json | jq -r '.images.ingestor.prodDigest // ""' is null-safe, reads the published chart's effective default right after install, and both branches are consistent with the existing assumption that prodPin resolves true in CI.

Issues / risks

  • [Minor — doc inconsistency] The new comment block (~L131) cites two different PRs for the same era boundary: "published before client#398 have no prodDigest" vs "releases since client#383 carry it". Since #383 < #398, one number is wrong. In a change whose value is these load-bearing comments, the boundary PR should be stated once, correctly.

  • [Accepted / documented — flag for tracking] The "ERA NOTE" (~L171) correctly identifies that path 1's --reuse-values promotes the baseline's full computed values into the release's override set, contaminating path 2's --reset-then-reuse-values. Against a post-pin baseline this is harmless only while local_prod_digest == baseline pin. The first time someone bumps the ingestor pin, path 2's GOT == WANT_DIGEST assertion will fail in CI even though nothing is broken. Deferring the path isolation is a reasonable call, but it plants a future CI failure a maintainer must recognize as expected — worth an issue/TODO reference.

  • [Info — no regression] The pinned digest is a single-archmanifest.v2 (not a multi-arch index), unlike images.ingestor.prodDigest, whose schema mandates a multi-arch index (Ingestor pinned to an amd64-only digest — arm64 data ingestion fails (ImagePullBackOff) #186). Not a regression: the floating :prod tag already resolved to this same single-arch manifest, so fleet arch behavior is unchanged. Just note there's no multi-arch safety net here for a future re-pin.

Test coverage

No automated assertion that the chart renders the mysqlClient digest path — relies on the manual helm template check in the test plan. A one-line helm template … | grep mysql-client@sha256: guard in the existing harness would cheaply lock this in. Low priority.

Security

Pinning by digest is a supply-chain improvement (immutability/reproducibility over a mutable tag). Net positive.

Verdict

Approve with minor comments. Well-scoped, the pin is verified correct against the live registry, and the e2e update genuinely hardens the gate. Only the #398/#383 comment mismatch is worth fixing before merge; the path-1/path-2 contamination and single-arch notes are tracking items, not blockers.

🤖 Reviewed with Claude Code

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d76af0a. Configure here.

LukasWodkaand others added 2 commits July 28, 2026 16:58
…the path-2 tripwire in #459 (review)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…est path (review)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

All four points addressed — thanks for the precise review:

  1. Era-boundary inconsistency (the must-fix) — reworded to state one boundary, once: the Release develop → main #383 promotion (2026-07-27), the first published release to carry the images.ingestor.prodDigest default that fix(chart): move the prod ingestor pin into chart defaults so it reaches edges #398 added to the chart source. Both numbers keep their correct roles (source-change vs published-baseline), and it's now unambiguous which one the test's expectation hangs on. Applied to both copies of the script (5e582dc on fix(e2e): era-aware --reuse-values pin assertion — baseline release now carries the prod pin #455, cherry-picked here as 2bbe536).

  2. Path-2 tripwire tracking — filed as Chart-default pin updates never reach an edge once upgraded with --reuse-values (replay contamination) — e2e path 2 trips on next prodDigest bump #459, framed around the real defect (an edge once upgraded with --reuse-values stops receiving chart-default pin updates), with the expected CI failure signature spelled out so the maintainer who hits it on the first pin bump lands on the decision (isolate path 2 vs fix the contamination) instead of debugging. The ERA NOTE now references it.

  3. Single-arch digest — noted in the values.yaml pin comment (c118c61): this digest is the amd64 image manifest, identical to what the floating tag always resolved to (arm64 stays emulated, no behavior change), and a future re-pin (the 8.4 multi-arch image, backend#723) should pin the manifest-index digest like images.ingestor.prodDigest does.

  4. Render-path coverage — added a matchRegex assertion to client/tests/mysql_test.yaml: the rendered mysql container image must match ^docker\.io/tracebloc/mysql-client@sha256:[a-f0-9]{64}$, so a tag ref can never silently sneak back. Regex (structural) rather than the exact digest, so the eventual reviewed re-pin doesn't need a test edit. 296/296 helm unit tests pass locally.

bugbot run

@LukasWodka
LukasWodka merged commit ee36cb3 into developJul 28, 2026
39 checks passed
LukasWodka added a commit that referenced this pull request Jul 28, 2026
…ow carries the prod pin (#455)
* fix(e2e): era-aware --reuse-values pin assertion — the published release now carries the prod pin
The path-1 assertion hardcoded the pre-#398 era: it expected NO ingestor
pin after a --reuse-values upgrade because the published release's
computed values predated images.ingestor.prodDigest. The #383 promotion
(2026-07-27) published a release that includes the pin, so replayed
computed values now carry it and the assertion fails on every
chart-touching PR (first hit: #454).
The expectation is now read from the baseline release itself
(helm get values --all): pin absent => must not arrive (old behavior);
pin present => the SAME digest must be replayed verbatim. Both eras keep
asserting the actual limitation: --reuse-values never injects new chart
defaults. Path-2's era-stale comment corrected, with a note on the
replay-contamination signal a future pin bump will surface.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(e2e): fail fast when jq is missing (review: local runs, not just CI runners)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(e2e): one era boundary, stated once (the #383 promotion); track the path-2 tripwire in #459 (review)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@LukasWodka
LukasWodka deleted the fix/pin-mysql-client-digest branch August 14, 2026 13:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LukasWodka@saqlainsyed007@divyasinghds