Skip to content

feat(chart): stop injecting the fixed training-job envelope; let jobs-manager size it (backend#664) - #733

Merged
shujaatTracebloc merged 2 commits into
developfrom
feat/664-pod-envelope-chart
Aug 17, 2026
Merged

feat(chart): stop injecting the fixed training-job envelope; let jobs-manager size it (backend#664)#733
shujaatTracebloc merged 2 commits into
developfrom
feat/664-pod-envelope-chart

Conversation

@shujaatTracebloc

@shujaatTraceblocshujaatTracebloc commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Utilization Ladder Step 01 (L0) — chart half (plan PR 1.21)

Part of the org Utilization Ladder review for tracebloc/backend#664; delivery plan PR 1.21 on tracebloc-engine PR #647. Companion to tracebloc/client-runtime#339, which makes jobs-manager derive training-job resources from node allocatable.

Problem

The fixed cpu=2, memory=8Gi envelope exists in two places kept in lockstep on purpose (backend#745): the in-code fallback in client-runtime's jobs_manager.py, and this chart — which always injectedRESOURCE_REQUESTS/RESOURCE_LIMITS, defaulting to the literal. Nothing reads node capacity: a 4-core NUC and a 128-core Xeon produce the same Job spec; on a 16-core/32GB machine 14 cores and 24GB are allocated to nothing.

What this does

With env.RESOURCE_REQUESTS/env.RESOURCE_LIMITSunset, the jobs-manager deployment now omits the vars instead of injecting the literal, so jobs-manager sizes the envelope from node allocatable (75% with cpu=2/8Gi floors, requests == limits / Guaranteed QoS — see client-runtime#339). Set values render exactly as before (explicit override, unchanged semantics).

No behavior flip ahead of the code (readers-before-writers)

Upgrade order between chart and image does not matter:

  • New chart + old image: var omitted → old jobs-manager falls back to its built-in cpu=2,memory=8Gi literal — the exact value this chart used to inject. Identical behavior.
  • Old chart + new image: literal still injected → env-override path → literal. Identical behavior.
  • New chart + new image: derivation activates.

Installer-provisioned clusters are unaffected either way: the installer writes explicit RESOURCE_REQUESTS/RESOURCE_LIMITS sized at install time (backend#1236), which remains an explicit override. Guaranteed QoS is preserved on every path; burst (requests < limits) is an explicitly open product decision (ladder L0/L4) and out of scope. GPU vars untouched.

Changes

  • templates/jobs-manager-deployment.yaml: render RESOURCE_REQUESTS/RESOURCE_LIMITS only when set (both api + pods-monitor containers)
  • values.yaml + values.schema.json: document unset → derived-from-node, set → explicit override
  • tests/jobs_manager_test.yaml: default case now asserts the vars are omitted; override test extended to both containers
  • docs/SECURITY.md §8.8: limits wording updated (limits are still always applied)
  • Chart.yaml: version + appVersion → 1.9.45 (lockstep)

Evidence

make check green (incl. all 28 chart-env-vocabulary checks); helm unittest — Jobs Manager Deployment suite passes; the 4 failing suites (auto-upgrade, image-refresh, network-policy, priority-class) fail identically on clean develop with local helm v4 (pre-existing negative-schema-test quirk, not introduced here). No .bats files touched.

🤖 Generated with Claude Code


Note

Medium Risk
Changes default training-job CPU/memory scheduling for chart-direct installs once paired with a new jobs-manager image; upgrade order is designed to stay behavior-identical until both chart and runtime ship.

Overview
Utilization Ladder L0 (chart half): When env.RESOURCE_REQUESTS and env.RESOURCE_LIMITS are both unset, the jobs-manager deployment no longer injects the fixed cpu=2,memory=8Gi pair on the api and pods-monitor containers—omission lets jobs-manager derive training-job envelopes from node allocatable (with companion client-runtime behavior). Explicit values still render the same pair as before; if only one key is set, both env vars are emitted and the missing side keeps the historic literal so a lone var is never published.

Docs and schema drop the implied chart default and describe unset-vs-set semantics; helm unittest now asserts default omission, full override, and the half-set pairing case. Chart version 1.9.45; SECURITY.md §8.8 notes limits are always applied (derived or pinned).

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

…-manager size it (backend#664)
Utilization Ladder Step 01 (L0), plan PR 1.21 — chart half of
tracebloc/client-runtime#339. With env.RESOURCE_REQUESTS/RESOURCE_LIMITS
unset, the jobs-manager deployment no longer injects the cpu=2,memory=8Gi
literal: the vars are omitted so client-runtime derives the envelope from
node allocatable. Older images fall back to their built-in identical
literal, so chart and image upgrade in either order without a behavior
flip. Set values remain an exact override. Installer installs are
unaffected (they write explicit values, backend#1236).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@shujaatTraceblocshujaatTracebloc self-assigned this Aug 17, 2026
@shujaatTracebloc
shujaatTracebloc marked this pull request as ready for review August 17, 2026 09:49

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

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.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3a01931. Configure here.

Comment threadclient/templates/jobs-manager-deployment.yaml
…et config renders both, as before (backend#664)
Bugbot: separate hasKey gates emitted a lone env var when only one value
was set; the old template always rendered the pair. Both vars now share
one gate (like the GPU block): neither set -> both omitted (jobs-manager
derives), either set -> both rendered with the literal filling the gap,
byte-identical to the pre-#664 output. Adds a unittest pinning the pair.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean, careful PR 👍 The one-gate-for-the-pair fix is right — verified by rendering: default omits both vars (jobs-manager sizes), an explicit override passes through, and a half-set config still renders the pair with the missing side falling back to the historic cpu=2,memory=8Gi on both containers. Contract with the runtime side holds: older images fall back to that same literal, so chart and image can upgrade in either order, and there's always a floor (no unbounded job). Schema/values/SECURITY.md docs all track the new behavior.

@shujaatTracebloc
shujaatTracebloc merged commit bf98c6d into developAug 17, 2026
22 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the feat/664-pod-envelope-chart branch August 17, 2026 12:31
@LukasWodka

Copy link
Copy Markdown
Contributor

/fr-pass

Best-effort functional review passed (triage: merged clean, non-interactive; behavioral evidence limited while e2e journey is red — backend#2206). Advancing to Ready for prod.

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

@shujaatTracebloc@LukasWodka@saadqbal