Skip to content

ci: chart-invariant gate — render tracebloc/client @ pinned ref, assert the 8 CLI-assumed invariants#311

Merged
saadqbal merged 2 commits into
developfrom
feat/290-chart-drift-gate
Jul 14, 2026
Merged

ci: chart-invariant gate — render tracebloc/client @ pinned ref, assert the 8 CLI-assumed invariants#311
saadqbal merged 2 commits into
developfrom
feat/290-chart-drift-gate

Conversation

@LukasWodka

@LukasWodka LukasWodka commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #290 (WS-D.1 of tracebloc/backend#1106 — cli side; the client-side half is a separate PR extending tracebloc/client's scripts/tests/check-drift.sh).

Summary

The CLI hardcodes 8 invariants about the tracebloc/client chart. Today 3 are guarded at the source (the two Deployment names + the helm-standard labels ride on the client repo's own drift check); 5 are unguarded — a chart rename ships green in both repos and breaks discovery/doctor/ingest in the field. This adds a cli-side render gate that asserts all 8 against the chart at a pinned ref.

What changed

  • scripts/.client-ref (new) — pins the tracebloc/client commit the gate renders, same format + rationale as scripts/.data-ingestors-ref (pin, don't float, so an unrelated chart commit can't red every open CLI PR; adopting upstream is a deliberate SHA bump). Pinned to client develop HEAD at gate introduction: ca9a3352f3b1a1a9135c196dcbb694ce239be1fe.
  • scripts/chart-invariants/ (new, Go, no new deps — gopkg.in/yaml.v3 is already direct) — reads a helm template render, asserts the 8 invariants, each mapped to the CLI constant it protects:
    1. discovery labels (app.kubernetes.io/name=client + managed-by=Helm, instance/chart/version reads) — internal/cluster/discover.go DiscoverParentRelease
    2. jobs-manager Deployment name, both tolerated forms (<release>-jobs-manager / jobs-manager)
    3. jobs-manager Service (either probed name) with port 8080pickJobsManagerService + jobsManagerPort
    4. PVC client-pvc mounted at /data/shared (claim-backed, not just any volume) — pvc.go SharedPVCClaimName/SharedPVCMountPath
    5. <release>-ingestion-authz ConfigMap, key ingestion-authz.yaml, allowed[]{service_account, namespace} shape, unambiguous for the release namespace, and the default SA staying ingestor with the SA object rendered — discoverIngestorSAName + the IngestorSAName fallback
    6. INGESTOR_IMAGE_DIGEST env on the first container (the exact read the CLI does)
    7. requests-proxy Deployment under a name doctor's findDeployment accepts — checkRequestsProxy
    8. submit path: containerPort 8080 + the render still referencing POST /internal/submit-ingestion-runinternal/submit SubmitPath. (The path is comment-carried in the render — a weak but deliberate tripwire; a client-runtime endpoint move updates those comments and must then confront the CLI constant.)
  • .github/workflows/chart-drift.yml (new, dedicated file — nothing appended to build.yml) — checks out the pin (ref shape validated first), helm template with client/ci/bm-values.yaml, runs the checker. Triggers: PRs touching internal/cluster/** / internal/doctor/** / the gate's own files, weekly cron (Mon 05:00 UTC, offset from e2e + path-persist), workflow_dispatch.
  • Unit tests: 16-mutation table pinning that each invariant actually trips, plus the bare-requests-proxy instance-label acceptance path.

Deviations from the ticket (verified against reality)

  • "Same private-repo secret approach as the goldens job" — there is no secret to reuse: tracebloc/client is public, and the PR ci: cross-repo fresh-shell PATH-persistence guard for install.sh #62 precedent checks it out with the default token (no secrets.*). No goldens workflow with a secret exists yet either (sync-validator-goldens.sh's CI job is still pending). So there is nothing to guard-skip on; the workflow notes inline that a read token becomes necessary only if the client repo ever goes private. @LukasWodka flagging this per the ticket — if you'd rather pre-provision an org read token now, say the word and a guarded fallback goes in.
  • "helm template with the chart defaults" — bare defaults refuse to render by design (values.schema.json requires non-empty clientId/clientPassword), so the gate renders with client/ci/bm-values.yaml — the same values file the client repo's own check-drift.sh CI half picks, dummy creds + platform bits only; every invariant object is unaffected.
  • The weekly cron runs the default branch's tree, so it goes live once this promotes develop → main on the normal cadence; PR-path triggers are active immediately.

Verified locally (green)

  • Full end-to-end simulation: pin parsed from scripts/.client-ref → matches client develop HEAD → helm template (local helm v4.1.1; CI pins v3.15.4, same as the client repo's drift job) → checker passes 8/8 against the real render.
  • Negative paths: chart mutations (service port off 8080, PVC rename) trip the client-side sibling check; the Go checker's 16-mutation test table covers each invariant here.
  • go build ./..., go vet ./..., full go test ./... suite, go test -race ./scripts/..., gofmt -s, errcheck@v1.20.0, ineffassign@v0.2.0, misspell@v0.3.4 — all clean.
  • actionlint on chart-drift.yml — clean.

Needs CI

  • The workflow's real cross-repo run on GitHub runners — this PR touches .github/workflows/chart-drift.yml + scripts/chart-invariants/**, which are in its own path filter, so it exercises itself here.

🤖 Generated with Claude Code


Note

Low Risk
Adds CI and a validation script only; no runtime CLI behavior changes. Failures surface contract drift before release rather than altering production paths.

Overview
Adds a cross-repo chart drift gate so CLI assumptions about the tracebloc/client Helm chart cannot silently diverge from what ships in the field.

scripts/.client-ref pins the client commit CI renders (same pin-don’t-float model as scripts/.data-ingestors-ref). scripts/chart-invariants is a small Go tool that parses helm template output and fails if any of 8 invariants break—discovery labels, jobs-manager Deployment/Service (8080, unprefixed Service precedence), client-pvc at /data/shared, ingestion-authz + ingestor SA, INGESTOR_IMAGE_DIGEST on the first container, requests-proxy naming, and submit path/port markers tied to internal/cluster, internal/doctor, and internal/submit.

.github/workflows/chart-drift.yml checks out the pin, renders with client/ci/bm-values.yaml and Helm v3.15.4, runs the checker, and triggers on relevant PR paths, weekly schedule, and workflow_dispatch. Unit tests cover each invariant via fixture mutations.

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

The CLI hardcodes 8 invariants about the tracebloc/client chart —
discovery labels + jobs-manager Deployment name forms, the jobs-manager
Service on 8080, the client-pvc mount at /data/shared, the
ingestion-authz ConfigMap + "ingestor" SA fallback, the
INGESTOR_IMAGE_DIGEST env, the requests-proxy Deployment, and the
POST /internal/submit-ingestion-run port. A chart rename ships green in
both repos and breaks discovery/doctor/ingest in the field.

New scripts/.client-ref pins the client commit the gate renders (the
scripts/.data-ingestors-ref pattern, backend#1009): chart-drift.yml
checks out tracebloc/client at that ref, helm-templates it with the same
CI values the client repo's own drift job uses, and
scripts/chart-invariants asserts every invariant with a mapping back to
the CLI constant it protects. Triggers: PRs touching internal/cluster/**
or internal/doctor/** (or the gate's own files, weekly cron, manual
dispatch. The client-side half extends that repo's
scripts/tests/check-drift.sh so renaming chart PRs fail at the source.

Refs #290 (WS-D.1, backend#1106)

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

Copy link
Copy Markdown
Contributor Author

@BugBot run

@LukasWodka

Copy link
Copy Markdown
Contributor Author

👋 Heads-up — Code review queue is at 52 / 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):

  • averaging-service#181 — feat(weights): normalize-on-read — mixed cycles average instead of rejecting (SEC-03 §8 step 2) · author: @shujaatTracebloc · no reviewer assigned
  • averaging-service#182 — feat(weights): averaging writes SafeTensors (TF + PyTorch) — SEC-03 §8 step 4 · author: @shujaatTracebloc · no reviewer assigned
  • backend#1079 — feat(global_meta): edge dataset_meta exposure + attributes contract at ingest (#924 G4a) · author: @divyasinghds · no reviewer assigned
  • backend#1086 — docs(rfc): SafeTensors weight-format migration — SEC-03 Phase 1 (RFC 0004) · author: @shujaatTracebloc · no reviewer assigned
  • backend#1093 — chore(deps): bump django from 5.2.14 to 5.2.15 · author: @dependabot · no reviewer assigned
  • backend#1095 — feat(experiment): configurable preprocessing knobs incl. tabular scaler — RFC 0003 L1 + L1b (#1094) · author: @LukasWodka · no reviewer assigned
  • backend#1100 — fix(boot): pin SDK install to tracebloc==0.11.2, drop 404 dev line (#1098) · author: @LukasWodka · reviewer: @saqlainsyed007
  • backend#1105 — perf(api): query micro-fixes — notifications N+1, cached data_scientist, composite index, sampling (#975) · author: @aptracebloc · no reviewer assigned
  • backend#1107 — perf(experiments): prefetch ExperimentListSerializer relations + cache carbon intensity (#977) · author: @aptracebloc · reviewer: @saadqbal
  • backend#1108 — test(api): CLI response-shape contract fixtures for the 9 CLI endpoints (WS-D.2) · author: @LukasWodka · reviewer: @saadqbal

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 470441b. Configure here.

Comment thread scripts/chart-invariants/main.go Outdated
@LukasWodka LukasWodka self-assigned this Jul 14, 2026
saadqbal
saadqbal previously approved these changes Jul 14, 2026

@saadqbal saadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍 Excellent gate — the 8 invariants trace to real CLI assumptions (the label-selector + -jobs-manager suffix match DiscoverParentRelease exactly), it's pinned to .client-ref (ref-shape validated), least-privilege, and the whole render→assert flow ran green here plus the 266-line unit test. The drift message pointing at the CLI-side constants to bump is a nice touch.

@saadqbal

Copy link
Copy Markdown
Collaborator

Status: approved, but 1 open Cursor Bugbot finding blocks merge: Service check ignores probe order (chart-invariants/main.go:277). Resolve/fix and it's ready.

Invariant 3 passed if any jobs-manager/<release>-jobs-manager Service
exposed 8080, but pickJobsManagerService (internal/cluster/discover.go)
selects the unprefixed "jobs-manager" first by existence, ignoring ports.
With both Services rendered and only the non-selected one on 8080, the gate
passed while the CLI would port-forward to the wrong target. Now the gate
mirrors that precedence and checks the selected Service. Adds a regression
case covering the both-Services-only-sibling-has-8080 scenario.

Fixes a Cursor Bugbot finding on #311.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@saadqbal saadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-approving — Bugbot fix verified: the gate now mirrors pickJobsManagerService's precedence (unprefixed 'jobs-manager' first) and asserts the selected Service exposes 8080, not merely 'some' Service — with a test for the selected-lacks-8080-but-sibling-has-it case. Correct. 👍

@saadqbal
saadqbal merged commit 4d9c78e into develop Jul 14, 2026
20 checks passed
@saadqbal
saadqbal deleted the feat/290-chart-drift-gate branch July 14, 2026 16:10
Sign up for free to 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.

2 participants