Release v1.4.5: promote develop → main#199
Merged
Merged
Conversation
…rom clusterScope)
Under clusterScope: false the chart rendered only a namespace-scoped Role in
the release namespace. But the resource-monitor's code:
* calls core_v1_api.list_pod_for_all_namespaces(field_selector=spec.nodeName=...)
-- a CLUSTER-SCOPED list verb a namespaced Role can never satisfy; and
* read_namespaced_pod()s its OWN pod, which lives in
.Values.nodeAgents.namespace.name (NOT .Release.Namespace).
So with clusterScope: false the DaemonSet 403'd on startup and crashlooped
(70+ restarts observed on a live cluster). Per-node monitoring is intrinsically
cluster-scoped.
Always render the read-only ClusterRole + ClusterRoleBinding regardless of
clusterScope (get/list/watch on pods/nodes/namespaces + metrics; no write,
exec, or secret access). resourceMonitor: false still fully disables the
component. clusterScope continues to gate the training/jobs isolation footprint
elsewhere -- it must not leave the node monitor without permissions it cannot
run without.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…erScope=false Follow-up to the RBAC fix: node_agents_namespace_test.yaml still asserted the old behavior (namespaced Role + RoleBinding in the release namespace when clusterScope=false). Update that case to assert the corrected contract -- a ClusterRole + ClusterRoleBinding always render (with no metadata.namespace), while the subject SA still lives in the node-agents namespace. The clusterScope=false path stays under test; only the asserted behavior changes to match the fix. Verified with `helm unittest` (all resource-monitor suites pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The ingestion endpoint's orphan-resource verify path (client-runtime#52) and missing-row self-heal (client-runtime#54) read the existing ConfigMap/Secret on a create-409 to confirm content matches before reuse. The Role/ClusterRole only granted `create`, so those reads returned Forbidden and the endpoint 500'd instead of the intended 409/200-replay — verified live on the dev cluster. Add `get` alongside `create` in both the ClusterRole (clusterScope: true) and namespace Role (clusterScope: false) branches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sync main → develop after v1.4.4 release
…aps-secrets fix(rbac): grant `get` on configmaps/secrets to jobs-manager SA
fix(resource-monitor): always grant read-only ClusterRole (decouple from clusterScope)
…186) (#187) Add a helm-ci job (ingestor-multiarch) that parses images.ingestor.digest and fails the build unless it's a multi-arch index (linux/amd64 + linux/arm64). Greenfield installs spawn the ingestor Job from this PINNED digest before image-refresh first ticks, so an amd64-only pin breaks data ingestion on arm64 hosts (Apple Silicon, Graviton) with "no match for platform" / ImagePullBackOff. This would have caught #160 (the amd64-only v0.3.1 pin) before it shipped. ghcr.io/tracebloc/ingestor is public -> no secrets. Verified: passes on the current multi-arch baseline (sha256:d361fa77, v0.3.2 / #184), fails on the old amd64-only sha256:a0861ea9. Note: the digest is already multi-arch on develop as of v0.3.2 (#184 — the same d361fa77 index this PR previously bumped to), so #187 no longer touches values.yaml; it adds only the regression guard so an amd64-only pin can't slip back in. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…can't resolve (#191) image-refresh silently skipped every tick when get_latest_digest returned empty for the ghcr.io ingestor image (egress/proxy/firewall to ghcr.io, or a blocked token endpoint) — never reaching the registry-drift branch that sets the new digest. jobs-manager + pods-monitor pull from docker.io and refreshed fine, so the CronJob looked healthy while the ingestor digest stayed pinned on the install-time baseline. That's why the berlin-team arm64 install sat on the amd64-only v0.3.1 digest even after :0.3 went multi-arch (#186 follow-up #2). Now count consecutive ingestor-resolve failures on a deployment annotation: - below imageRefresh.ingestorResolveFailureThreshold (default 3, ~45 min at the 15-min schedule) -> WARN + skip, as before (tolerate transient blips); - at/above it -> ERROR with actionable guidance, a tracebloc.io/ingestor-refresh-last-error annotation, and a non-zero exit so the Job fails visibly in `kubectl get cronjob` / monitoring — the same surfacing idiom Pass 2's stuck-rollout check already relies on; - a successful resolve clears the streak. Threshold is nil-guarded (default 3) for --reuse-values upgrades and schema-validated (integer >= 1). The digest-resolution logic itself is unchanged (verified correct: it returns the multi-arch index digest). helm unittest 146/146, helm lint clean, shellcheck + sh -n clean. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…eployment (#194) requests-proxy-deployment.yaml was the only data-plane workload template without a unit test. This suite pins the properties most costly to regress: - security-context invariants (no SA-token automount, runAsNonRoot, seccomp RuntimeDefault, runAsUser 1001, no privilege escalation, drop ALL caps, read-only root filesystem) — see docs/SECURITY.md - the single-replica / single gunicorn worker constraint (the pod token registry is process-local; >1 worker silently shards token lookups) - the docker.io/tracebloc/jobs-manager image source and port 8888 - the nil-guarded resource defaults, plus an override case that exercises the default-through-dict fallthrough (guards the historic `readOnlyRootFilesystem: trueresources:` newline-eating regression) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The training-egress NetworkPolicy denies all pod-to-pod / ClusterIP egress (rule 2 excepts the cluster CIDRs) and re-permits only MySQL (rule 3). When the requests-proxy architecture shipped — training pods POST epoch results / FLOPs to requests-proxy-service:8888 instead of holding Service Bus credentials — this template was never updated to re-permit egress to the proxy. Result on every install with the policy enabled: pods hit "requests-proxy-service:8888 ... [Errno 111] Connection refused" at the first epoch finalize → CrashLoopBackOff → all experiments fail. Add rule 4 mirroring the MySQL rule: TCP/8888 to podSelector app=requests-proxy (same namespace). Service selector + port from templates/requests-proxy-service.yaml. Verified: `helm template -f ci/bm-values.yaml --show-only templates/network-policy-training.yaml` renders the new rule as valid YAML. Found live on a fresh client (tracebloc-amazon / k3d): jobs-manager reached the proxy (HTTP 401) while training pods got connection-refused — the only differentiator was this egress policy. Interim: live-patched the cluster + suspended its auto-upgrade CronJob (so reuse-values wouldn't revert the patch); re-enable once this lands + releases. Closes #196. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d, surface version (#192) * fix(chart): drop the data-plane PriorityClass by default The cluster-scoped, fixed-name `tracebloc-data-plane` PriorityClass was the only thing forcing one tracebloc client per cluster (a second release collided on it with a cryptic Helm error) and blocking multiple tracebloc namespaces in one BYO cluster. mysql doesn't need it: memory requests==limits (last evicted under memory pressure), data on a PVC (eviction = transient restart, not data loss), and a PDB guards voluntary disruptions. Its only unique benefit was letting the scheduler preempt training jobs to keep mysql scheduled on a packed node — a narrow case. Default priorityClass.create=false + name="" so new installs template no PriorityClass and mysql carries no priorityClassName. Opt back in (create:true + name) on contended clusters, or reference an out-of-band one (create:false + name:<existing>). helm-unittest updated; 144/144 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(installer): fixed namespace + one-per-machine guard; drop workspace prompt The "Choose a workspace name" prompt asked the user to invent a label that isn't their identity (the backend identifies a client by its credentials, not this string — it's just the local k8s namespace / Helm release name; the installer even discards the auth response body). It defaulted to a meaningless "default" and was the field that collided on a second install. - Drop the prompt; TB_NAMESPACE defaults to a fixed "tracebloc" (env-overridable for advanced/GitOps setups). - One-client-per-machine guard: after credentials verify, compare the entered Client ID against any client already installed here (helm get values). Same ID = a normal re-run/upgrade; a DIFFERENT ID hard-blocks with an explanation and options (repair / switch via `k3d cluster delete` / use another machine) instead of silently re-pointing the machine. This replaces the accidental PriorityClass collision (now dropped) with an intentional, explained guard. - Document the TB_NAMESPACE override; update bats (input sequences + 2 new guard tests). bats 26/27 — the 1 failure is a pre-existing macOS-bash-3.2 quirk in _extract_yaml_value, unrelated (CI bash 5 passes it). NOTE: install-k8s.ps1 + its Pester tests still need the same mirror (follow-up). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(installer-ps): mirror fixed namespace + one-per-machine guard (PowerShell) Mirrors the bash change in install-k8s.ps1: - drop the "Choose a workspace name" prompt; TB_NAMESPACE defaults to a fixed "tracebloc" (override via $env:TB_NAMESPACE). - one-client-per-machine guard: after credentials verify, compare the entered Client ID against any client already installed here (helm get values); a different ID hard-blocks with the same explanation/options as bash. - Pester: 2 new guard tests (block-different / allow-same). The existing Install-ClientHelm tests use dispatch-by-prompt Read-Host mocks, so the prompt removal doesn't disturb them. No pwsh locally -> verified via CI (Pester ubuntu+windows + PSScriptAnalyzer). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): scan all namespaces in the one-per-machine guard The guard checked only the `tracebloc` namespace, so a client installed by an older installer version (default namespace `default`, or a custom name) wasn't detected -- a re-run could create a second coexisting client. Now enumerate all client-chart releases (helm list -A) and compare each one's clientId, covering both fresh and migrated installs. bash uses jq (already a dependency; falls back to the tracebloc namespace if absent); PowerShell uses ConvertFrom-Json. The block message names the namespace. bats + Pester guard tests updated. Verified: bats green locally (jq path); PowerShell via CI Pester. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(installer): show client (chart) version in summary + --diagnose Users had no easy way to see which client version they're on (the CLI isn't shipped yet; `helm list` needs the namespace, and nothing surfaced it). Show the chart version where they already look: - install summary: a "Version" line next to Workspace. - --diagnose: as the first console line + recorded in the bundle header (the #1 thing support needs). Adds a best-effort `_chart_version` / Get-ChartVersion helper (greps helm's CHART column -> no jq). bash + PowerShell; bats + Pester coverage added. Verified: summary.bats + diagnose.bats green locally; ps1 via CI Pester. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…#198) 1.4.4 is already published on the tracebloc.github.io/client Pages channel and is what clusters run. The training-egress NetworkPolicy fix (#197, allow training → requests-proxy:8888) merged to develop without a version bump, so it is currently undeliverable: chart-releaser won't overwrite the existing 1.4.4 release, and clusters already on 1.4.4 would see no version change and pull nothing. Bump to 1.4.5 (lockstep version/appVersion, matching 1.4.3/1.4.4 history) so a v1.4.5 release publishes a new version that auto-upgrade actually pulls. Chart-only change; no image change. Ref #196 / #197. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
👋 Heads-up — Code review queue is at 15 / 8 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.) |
shujaatTracebloc
approved these changes
Jun 4, 2026
aptracebloc
approved these changes
Jun 4, 2026
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 ddb233a. Configure here.
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.

Release v1.4.5 — promote
develop→mainCuts the
v1.4.5release line. Merging this is not fleet-impacting — the fleet only auto-upgrades once av1.4.5GitHub Release is published (triggersrelease-helm-chart→ Pages). So this PR is the staging step; the release publish remains a separate, explicit go.Headline fix
requests-proxy:8888. Without it, training pods can't report epoch results (Connection refused→ CrashLoopBackOff) and all experiments fail. Found live on a fresh client; interim-patched + auto-upgrade suspended on that cluster pending this release.Also in this release (12 commits ahead of main)
geton configmaps/secrets) + sync mergesPost-merge release steps (release owner)
v1.4.5onmainand publish the GitHub Release →release-helm-chartpublishes the chart (:23).auto-upgradeCronJob so it converges onto the chart-managed NetworkPolicy rule:kubectl -n tracebloc-amazon patch cronjob tracebloc-amazon-auto-upgrade -p '{"spec":{"suspend":false}}'🤖 Generated with Claude Code
Note
High Risk
Training NetworkPolicy and RBAC changes affect every experiment and node telemetry on upgrade; publishing the v1.4.5 release triggers fleet-wide auto-upgrade, so the requests-proxy egress fix is broadly impactful but misconfiguration could still break training or monitoring.
Overview
Release v1.4.5 bumps the client Helm chart and bundles several fleet- and install-facing fixes.
The headline change adds an explicit training NetworkPolicy egress rule for in-namespace
requests-proxyon TCP 8888, so epoch reporting is not blocked by the “external HTTPS only” rule that was denying ClusterIP traffic to the proxy (#196).Image / ingestor: Pinned ingestor digest is documented as multi-arch and CI adds an
ingestor-multiarchjob that fails if the pin lackslinux/amd64+linux/arm64. Image-refresh now counts consecutive ghcr ingestor resolve failures, annotates the jobs-manager deployment, and fails the CronJob after configurable threshold (default 3) instead of skipping silently forever.RBAC / defaults: Jobs-manager ConfigMap/Secret rules gain
get(409 verify/replay paths). Resource-monitor always uses a ClusterRole (removed theclusterScope: falsenamespaced Role path). PriorityClass is off by default (create: false, emptyname) so mysql no longer references it unless opted in.Installers: Default namespace is fixed to
tracebloc(no workspace prompt);TB_NAMESPACEoverride documented. One-client-per-machine guard blocks install when a differentclientIdis already deployed cluster-wide. Success/diagnose flows surface chart version viahelm list.Tests/schema updated for image-refresh threshold, priority class defaults, resource-monitor RBAC, mysql priority, and new requests-proxy deployment unittest coverage.
Reviewed by Cursor Bugbot for commit ddb233a. Bugbot is set up for automated code reviews on this repo. Configure here.