feat(ingestor): resolve the spawned tag per environment (:dev / :stg) - #494
Conversation
dev and staging edges now spawn ingestion Jobs from the internal channels published by the matching data-ingestors branch (:dev / :stg) instead of the 0.7 release float, so an ingestor change can be validated on a real edge without a production release. Before this, the ingestor image existed only as a byproduct of a prod release -- on 2026-07-30 testing one change cost a prod PyPI publish plus an FR-gate override. - images.ingestor.tag becomes an explicit override, EMPTY by default. - images.ingestor.channelTags carries the per-environment floats (dev/stg/prod), keyed on the resolved CLIENT_ENV. - New tracebloc.ingestorTag helper mirrors tracebloc.ingestorDigest's precedence: explicit tag > channel for CLIENT_ENV > literal 0.7 (so a release predating these keys still renders under --reuse-values). - prod deliberately stays a semver float, NOT a :prod channel -- no such tag is published, and prod normally runs prodDigest anyway. - The ingestor-multiarch CI guard previously hard-failed on an empty tag; it now validates the explicit override when set plus every channelTags entry, since an edge resolves exactly one of them. Chart defaults propagate through the fleet auto-upgrade (--reset-then-reuse-values), and the installer does not pin the tag, so existing dev/staging edges pick up their channel on the next upgrade. An operator who set images.ingestor.tag explicitly keeps it. Verified: 307/307 helm unittest across 27 suites (9 new cases covering each environment, the override, an unknown CLIENT_ENV, and a channelTags-less replay); helm lint --strict clean on all four platform values files; rendered INGESTOR_IMAGE_TAG confirmed as dev/stg/0.7/0.7 for dev/stg/prod/unset; schema still rejects 'latest' in channelTags and now accepts the empty override.
|
👋 Heads-up — Code review queue is at 37 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
saadqbal
left a comment
saadqbal
left a comment
There was a problem hiding this comment.
Solid PR — verified the render myself (checked out the branch, helm template per env, 307/307 unittest, lint --strict clean on all four platform files). Prod is safe: digest pin still wins and its tag fallback is unchanged 0.7, so the behaviour change only touches dev/staging edges as intended. The tag never renders empty on any path I could build — including a --reuse-values replay predating the keys (channelTags: null → 0.7) and prodPin: false (→ 0.7), so no empty-repo: ImagePullBackOff hiding anywhere.
Keep it a draft until the ingestor-multiarch job goes green — merging before #422 publishes :dev/:stg multi-arch is the only real breakage here, and it's correctly gated.
Two nits, neither blocking:
- The
env.CLIENT_ENVschema description still says(dev, staging, prod), but resolution keys onstg— worth fixing now that the exact string is load-bearing for channel selection. - The
values.schema.jsonreformat (single-line → multi-line + unicode escapes) buries the ~30 real lines in a 576-line diff. Functionally inert, just harder to review.
Asad's first note landed on a real defect, not just a stale description. The schema documents CLIENT_ENV as (dev, staging, prod) while the channel keys are dev|stg|prod, so CLIENT_ENV=staging -- the documented value -- missed channelTags entirely and fell back to the prod float. Meanwhile client-runtime normalizes staging->stg at runtime, so that edge would have talked to the stg backend while spawning the 0.7 release ingestor: exactly the split-brain client-runtime#227 was filed for, reintroduced one layer up. tracebloc.ingestorTag now normalizes development/staging/production before the lookup, mirroring proxy_config.ENV_ALIASES, and the schema description states which values are canonical and that it is load-bearing for channel selection. Also addresses the second note: values.schema.json is edited surgically in the file's own style instead of being reformatted by a json round-trip (576-line diff -> 29). Verified: 310/310 helm unittest (3 new alias cases); lint --strict clean on all four platform files; rendered tag is dev/dev/stg/stg/0.7/0.7 for dev/development/stg/staging/prod/production, and an unknown value still falls back to the float rather than rendering empty.
|
@saadqbal thanks for checking out the branch and re-deriving the render — and your first note was not just a doc nit. It exposed a real defect, fixed in 6c207d6. The
|
| CLIENT_ENV | tag |
|---|---|
dev / development |
dev |
stg / staging |
stg |
prod / production |
0.7 |
anything else (produktion) |
0.7 — still falls back rather than rendering empty |
Second note: fixed, 576 → 29 lines
Fair complaint — I'd round-tripped the file through json.dumps(indent=2), which reformatted everything and buried the real change. Reverted to the original and edited it surgically in the file's own style (inline "not": { "const": "latest" }, 12-space indent). git diff --stat is now 26 insertions, 3 deletions.
Verification after the fix
- 310/310
helm unittestacross 27 suites — 3 new cases pinning each alias, including the one that would have caught this. helm lint --strictclean on all four platform values files.- Render re-derived for all seven inputs in the table above.
On your gating advice — agreed, and it has now cleared
Both channels are published and multi-arch:
:dev -> sha256:87fccee44d9695e9… linux/amd64 + linux/arm64
:stg -> sha256:ff92490567f368f7… linux/amd64 + linux/arm64
ingestor-multiarch was red only because it ran at 12:06, before the first publish at 12:19; re-run after :stg landed and it passes. So the gate you asked for is satisfied — this can come out of draft once you're happy with the alias fix.
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Both from Bugbot on #494, and both caused by my own half-applied alias fix. 1. The alias normalization went into tracebloc.ingestorTag only, while tracebloc.ingestorDigest still compared the RAW CLIENT_ENV to "prod". So CLIENT_ENV=production got the prod float tag but an EMPTY digest -- silently dropping the reproducibility pin (backend#1028/#1245) on an edge that looked correctly configured, which is worse than the bug the alias fix was for. Extracted tracebloc.clientEnv as the single normalizer and pointed both helpers at it, so they cannot drift again (the reason ENV_ALIASES lives once in client-runtime proxy_config). 2. scripts/resolve-ingestor-digest.sh read images.ingestor.tag, now empty by default, so the documented no-arg / --write path exited on an empty tag -- the exact command the chart comments and the ingestor-multiarch CI error tell operators to run. It now falls back to images.ingestor.channelTags.prod on both the yq and the yq-free path, with a matching sibling awk reader scoped the same way. Verified: 311/311 helm unittest (a new case pins the production-alias digest); lint --strict clean on all four platform files; rendered tag+digest correct for prod/production/stg/staging/dev/development; the script's no-arg path resolves 0.7 -> the multi-arch digest again, and the portable reader returns 0.7 with tag empty.
|
Both new findings were valid, and both were caused by my own half-applied alias fix — fixed in b87a4d4.
|
| CLIENT_ENV | tag | digest |
|---|---|---|
prod / production |
0.7 |
sha256:9098b3c9… |
stg / staging |
stg |
(empty — floats, intended) |
dev / development |
dev |
(empty — floats, intended) |
Empty tag breaks digest resolver (Medium) — also valid
scripts/resolve-ingestor-digest.sh read images.ingestor.tag, which this PR makes empty by default, so the documented no-arg / --write path exited with an empty-tag error. That is the exact command the chart comments and the ingestor-multiarch CI failure message tell operators to run to refresh prodDigest — so I had broken the prescribed remediation path.
It now falls back to images.ingestor.channelTags.prod, on both readers: the yq path and the portable yq-free awk parser, with a sibling reader scoped the same disciplined way (only the 6-space prod: leaf inside images: → ingestor: → channelTags: can match). Verified end to end:
$ scripts/resolve-ingestor-digest.sh
ghcr.io/tracebloc/ingestor@sha256:53b998893eba9c4c… (tag 0.7; platforms: linux/amd64 linux/arm64)
and the portable reader in isolation returns 0.7 with tag empty.
Verification
- 311/311
helm unittestacross 27 suites — a new case pins theproduction-alias digest specifically, so this regression can't come back silently. helm lint --strictclean on all four platform values files.bash -nclean on the script.shellcheckflags oneSC2295at line 135 — confirmed pre-existing ondevelop(4 occurrences there), not introduced here; left alone rather than mixed into this change.
Two review rounds, four real findings, three of them my own regressions — the helper duplication in particular was worth catching before it shipped to edges.
bugbot run
|
bugbot run |
There was a problem hiding this comment.
✅ 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 b87a4d4. Configure here.
|
👋 Heads-up — Code review queue is at 32 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
|
@saadqbal ready for review — the gate you asked for has cleared, and the diff has changed materially since you looked, so it's worth a fresh pass rather than a rubber stamp. Your gate is satisfiedBoth channels are published and multi-arch, and What changed since your review — three regressions, all mineYour first "nit" wasn't a doc issue. Following the documented Then Bugbot found my fix for that was half-applied, which was worse: I normalized in Third: I'd broken Your second nit is fixed too: the schema is edited surgically in its own style now — 576-line diff → 29. Current state
311/311 unittest (new cases pin both the One deployment note, not a code noteMerging to |
saadqbal
left a comment
saadqbal
left a comment
There was a problem hiding this comment.
Careful, well-scoped PR — the helper precedence, the single tracebloc.clientEnv normalizer shared by both ingestorTag and ingestorDigest, the CI multi-arch guard extension, and the resolver's channelTags.prod fallback all line up, and the unittest coverage is thorough (incl. the alias + reproducibility-pin cases). LGTM on the code.
Approving the code only — merge still waits on the sequencing gate you flagged: #422 has to publish :dev/:stg (both multi-arch) first, and ingestor-multiarch will legitimately stay red until then. One trivial comment inline.
The new read_ingestor_prod_channel landed between read_ingestor_tag's header comment and read_ingestor_tag itself, so that header described the wrong function and read_ingestor_tag sat comment-less ~30 lines below. Moved the new function below the original; each header is now directly above the function it documents. No behaviour change -- both readers verified still returning 0.7 / empty, and the no-arg resolver path still resolves the multi-arch digest.
|
Thanks for the approval — and the nit was a fair catch, fixed in 8a5b82d. You were exactly right: inserting No behaviour change — both readers re-verified ( One correction on the merge gate — it has already clearedYour review says #422 is merged, both channels published (a staging hop carried the workflow onto So the sequencing gate you set is satisfied and this is mergeable on the code you approved. The remaining call is deployment timing, not codeMerging to |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8a5b82d. Configure here.
… channel parser Two things, both caught by CI/review rather than by me: - The 'chart content ⇒ Chart.yaml version bump' guard was red: this PR edits client/templates and client/values.yaml, and a Helm repo publishes only on a version change, so without a bump the whole change would reach no install -- exactly how the perIngestionTables block shipped dark in #472. Chart 1.9.8 -> 1.9.9. - read_ingestor_prod_channel stripped quotes BEFORE removing an inline comment, the reverse of read_ingestor_tag. A channelTags.prod line with a trailing comment therefore parsed as 0.7" -- a stray quote -- and the no-arg/--write resolver would look up a nonexistent ref. Reordered to match: key, comment, trim, quotes. Verified across all four forms (double/single quoted with and without a trailing comment, and bare).
|
Two more, both fixed in fa74943 — and one of them would have made this whole PR a no-op. The chart-version guard was right to block it
That guard earned its keep here — it caught the exact failure mode it was written for. Bugbot:
|
| line | parsed |
|---|---|
prod: "0.7" # tracks the current line |
0.7 |
prod: '0.7' # single quoted |
0.7 |
prod: 0.7 |
0.7 |
prod: "0.7" |
0.7 |
Worth noting the shape of this one: I wrote a sibling parser and diverged from the sibling's ordering — the same class of "second copy drifts from the first" as the ingestorTag/ingestorDigest split you and Bugbot caught earlier. Third time that pattern showed up in this PR.
State
311/311 unittest, lint --strict clean on all four platform files, resolver's no-arg path still returning the multi-arch digest, gen-manifest no-op (no installer script changed). Thread resolved.
@saadqbal your approval predates these two commits — the chart bump and the parser reorder. Neither touches the logic you reviewed (the helper precedence, the shared normalizer, the guard extension, the channelTags.prod fallback), but flagging it so you can re-confirm rather than have your approval carried onto a diff you hadn't seen.
…path (#417 unblocked) Rebuilt against develop: #494 (backend#1360) landed the per-environment channel scheme while this PR sat on HOLD, which made the original one-line change WRONG, not merely conflicting. `images.ingestor.tag` is now an explicit force-override that is EMPTY by default; the effective tag comes from `channelTags[CLIENT_ENV]`. Setting `tag: "0.8"` as this PR originally did would have pinned 0.8 on EVERY edge and silently defeated the new :dev / :stg channel routing. The correct change on today's develop: - `channelTags.prod` 0.7 -> 0.8. dev/stg already track the internal :dev / :stg channels, so prod is the only entry that was still on a pre-D16 line. - `prodDigest` refreshed to the 0.8 multi-arch index, resolved with `scripts/resolve-ingestor-digest.sh --write` (never hand-typed, per the chart's own instruction) and independently cross-checked against registry-1/ghcr's `docker-content-digest`: sha256:d4af33420520d0fb01df3af4cf5259f664c89e95c7dff0681e8aa335a0c6e614 - `_helpers.tpl`'s last-resort literal 0.7 -> 0.8. It only fires for a release predating these keys under `--reuse-values`, but leaving it on 0.7 would spawn a pre-D16 ingestor on exactly those replays. - Chart.yaml 1.9.9 -> 1.9.10, version AND appVersion. develop had already published 1.9.9 (via #494), so the original bump to 1.9.9 would have collided. HOLD condition is now satisfied — this was the only thing blocking the PR: data-ingestors#419 merged 2026-07-30 10:52Z; v0.8.0 released 11:45Z. The CI multi-arch gate failed on this PR at 10:55Z, 50 minutes BEFORE the image was published, which is the whole of that red X. All four refs the gate now inspects verify multi-arch (linux/amd64 + linux/arm64) against ghcr.io today: :dev, :stg, :0.8, and the prodDigest index. Verification: helm unittest 320/320 across 27 suites; helm lint --strict clean for all four platform value files (aks/bm/eks/oc); gen-manifest --check and check-drift clean. Left as a DRAFT deliberately — flipping it ready is Lukas's call, and moving the prod pin reaches every prod edge on the next fleet auto-upgrade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
/fr-pass |
…path (#417 unblocked) Rebuilt against develop: #494 (backend#1360) landed the per-environment channel scheme while this PR sat on HOLD, which made the original one-line change WRONG, not merely conflicting. `images.ingestor.tag` is now an explicit force-override that is EMPTY by default; the effective tag comes from `channelTags[CLIENT_ENV]`. Setting `tag: "0.8"` as this PR originally did would have pinned 0.8 on EVERY edge and silently defeated the new :dev / :stg channel routing. The correct change on today's develop: - `channelTags.prod` 0.7 -> 0.8. dev/stg already track the internal :dev / :stg channels, so prod is the only entry that was still on a pre-D16 line. - `prodDigest` refreshed to the 0.8 multi-arch index, resolved with `scripts/resolve-ingestor-digest.sh --write` (never hand-typed, per the chart's own instruction) and independently cross-checked against registry-1/ghcr's `docker-content-digest`: sha256:d4af33420520d0fb01df3af4cf5259f664c89e95c7dff0681e8aa335a0c6e614 - `_helpers.tpl`'s last-resort literal 0.7 -> 0.8. It only fires for a release predating these keys under `--reuse-values`, but leaving it on 0.7 would spawn a pre-D16 ingestor on exactly those replays. - Chart.yaml 1.9.9 -> 1.9.10, version AND appVersion. develop had already published 1.9.9 (via #494), so the original bump to 1.9.9 would have collided. HOLD condition is now satisfied — this was the only thing blocking the PR: data-ingestors#419 merged 2026-07-30 10:52Z; v0.8.0 released 11:45Z. The CI multi-arch gate failed on this PR at 10:55Z, 50 minutes BEFORE the image was published, which is the whole of that red X. All four refs the gate now inspects verify multi-arch (linux/amd64 + linux/arm64) against ghcr.io today: :dev, :stg, :0.8, and the prodDigest index. Verification: helm unittest 320/320 across 27 suites; helm lint --strict clean for all four platform value files (aks/bm/eks/oc); gen-manifest --check and check-drift clean. Left as a DRAFT deliberately — flipping it ready is Lukas's call, and moving the prod pin reaches every prod edge on the next fleet auto-upgrade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…path (#417 unblocked) Rebuilt against develop: #494 (backend#1360) landed the per-environment channel scheme while this PR sat on HOLD, which made the original one-line change WRONG, not merely conflicting. `images.ingestor.tag` is now an explicit force-override that is EMPTY by default; the effective tag comes from `channelTags[CLIENT_ENV]`. Setting `tag: "0.8"` as this PR originally did would have pinned 0.8 on EVERY edge and silently defeated the new :dev / :stg channel routing. The correct change on today's develop: - `channelTags.prod` 0.7 -> 0.8. dev/stg already track the internal :dev / :stg channels, so prod is the only entry that was still on a pre-D16 line. - `prodDigest` refreshed to the 0.8 multi-arch index, resolved with `scripts/resolve-ingestor-digest.sh --write` (never hand-typed, per the chart's own instruction) and independently cross-checked against registry-1/ghcr's `docker-content-digest`: sha256:d4af33420520d0fb01df3af4cf5259f664c89e95c7dff0681e8aa335a0c6e614 - `_helpers.tpl`'s last-resort literal 0.7 -> 0.8. It only fires for a release predating these keys under `--reuse-values`, but leaving it on 0.7 would spawn a pre-D16 ingestor on exactly those replays. - Chart.yaml 1.9.9 -> 1.9.10, version AND appVersion. develop had already published 1.9.9 (via #494), so the original bump to 1.9.9 would have collided. HOLD condition is now satisfied — this was the only thing blocking the PR: data-ingestors#419 merged 2026-07-30 10:52Z; v0.8.0 released 11:45Z. The CI multi-arch gate failed on this PR at 10:55Z, 50 minutes BEFORE the image was published, which is the whole of that red X. All four refs the gate now inspects verify multi-arch (linux/amd64 + linux/arm64) against ghcr.io today: :dev, :stg, :0.8, and the prodDigest index. Verification: helm unittest 320/320 across 27 suites; helm lint --strict clean for all four platform value files (aks/bm/eks/oc); gen-manifest --check and check-drift clean. Left as a DRAFT deliberately — flipping it ready is Lukas's call, and moving the prod pin reaches every prod edge on the next fleet auto-upgrade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 participants
Part 2 of tracebloc/backend#1360. Draft on purpose — see the sequencing gate below.
Part 1 (tracebloc/data-ingestors#422) publishes
:dev/:stg. This is the half that makes them consumable: todayimages.ingestor.tagis a single value ("0.7") shared by dev and staging, so the two cannot sit on different channels. OnlyprodDigestis env-aware.What changes
images.ingestor.tag"0.7"(the only float)images.ingestor.channelTags{dev: dev, stg: stg, prod: "0.7"}, keyed on resolvedCLIENT_ENVNew
tracebloc.ingestorTaghelper, mirroringtracebloc.ingestorDigest's precedence: explicittag> channel forCLIENT_ENV> literal0.7. That last fallback matters — a release predating these keys still renders under a plain--reuse-valuesreplay.proddeliberately stays a semver float, not a:prodchannel — no such tag is published for the ingestor, and prod normally spawnsprodDigestanyway (the tag is only the fallback when pinning is disabled).Rendered result (verified, not asserted)
The CI guard had to move with it
ingestor-multiarchinhelm-ci.yamlhard-failed on an empty tag, so this change could not land without it. It now validates the explicit override when set plus everychannelTagsentry — an edge resolves exactly one of them, so a single-arch value in any would break ingestion on arm64 for whichever environment lands on it (client#186 / #160). Part 1's merge job already refuses to publish a single-arch index, so the channels satisfy it by construction.:devand:stgdo not exist yet. Merging this before Part 1 has published both would point dev/staging edges at a nonexistent tag → ImagePullBackOff on the next ingestion. Also, theingestor-multiarchjob on this PR will legitimately fail until then, because it inspects those tags.Order: merge #422 → let a
developand astagingpush publish:devand:stg(confirm both carry amd64+arm64) → mark this ready → merge.Behavioural change to be deliberate about
Chart defaults propagate through the fleet auto-upgrade (
--reset-then-reuse-valuesre-applies user-supplied values but reads new chart defaults), and the installer does not pin the tag — so existing dev/staging edges move onto their channel on the next upgrade. That is the intent (a staging edge should run staging code, as jobs-manager and the engine already do), but it is a real change in what those edges run. An operator who setimages.ingestor.tagexplicitly keeps it. Prod is untouched.These channels are unsigned internal images (Part 1 keeps one signed trust root at the prod tags) — hence prod is never pointed at them.
Verification
helm unittestacross 27 suites, including 9 new cases: each environment, the explicit override winning over a channel, an unknownCLIENT_ENVfalling back rather than rendering empty, and achannelTags: nullreplay.keeps the ingestor floating on devasserted tag0.7forCLIENT_ENV=dev; that is precisely the behaviour this PR changes, so it now assertsdevwith the rationale inline. The other two tag assertions were checked and remain correct (no-CLIENT_ENV→ prod float; the explicit0.4override case).helm lint --strictclean on all four platform values files (aks/bm/eks/oc).latestinchannelTags, now accepts the empty override.scripts/gen-manifest.shrun — no change (no installer script touched).Note
Medium Risk
Upgraded dev/staging edges will switch ingestor images on the next chart upgrade (intended but operationally significant), and merging before
:dev/:stgexist causes ImagePullBackOff; prod pinning logic was touched but alias handling is tightened.Overview
Spawns ingestion jobs with environment-specific ingestor tags instead of one shared
images.ingestor.tag(0.7).tagis now an optional override (empty by default); defaults live in newimages.ingestor.channelTags(dev→:dev,stg→:stg,prod→0.7). jobs-manager’sINGESTOR_IMAGE_TAGcomes fromtracebloc.ingestorTag(override → channel for resolved env →0.7fallback).Shared
tracebloc.clientEnvnormalizesCLIENT_ENValiases (staging/production/etc.) so tag selection and the prodprodDigestpin stay aligned—fixing the case whereproductiongot the prod float tag but lost the digest pin.CI and tooling follow the new model:
ingestor-multiarchvalidates every tag an edge might spawn (explicit override plus eachchannelTagsentry);resolve-ingestor-digest.shdefaults tochannelTags.prodwhentagis empty. Chart 1.9.9; helm unittest coverage for channels, overrides, aliases, and legacy--reuse-valueswithoutchannelTags.Reviewed by Cursor Bugbot for commit fa74943. Bugbot is set up for automated code reviews on this repo. Configure here.