Skip to content

fix(chart): wait for MySQL before starting jobs-manager (backend#2913) - #941

Closed
aptracebloc wants to merge 5 commits into
developfrom
fix/2913-jobs-manager-wait-for-mysql
Closed

fix(chart): wait for MySQL before starting jobs-manager (backend#2913)#941
aptracebloc wants to merge 5 commits into
developfrom
fix/2913-jobs-manager-wait-for-mysql

Conversation

@aptracebloc

@aptracebloc aptracebloc commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What

jobs-manager's api container (jobs_manager.py) makes an initial, un-retried MySQL connect at startup. On an install where mysqld is not listening yet — errno 111, connection refused, not a credential/permission problem — that connect raises and the process exits. The kubelet then crashloops the pod and the e2e leg fails before it ever reaches training. The pod recovers minutes later and looks healthy by the time anyone inspects, so the only evidence is the previous container's log (measured on staging run 33393213667, leg staging · amd64 · none · time_series_classification).

The chart already sequences its other MySQL clients (requests-proxy retries its bootstrap in-image); the api container had no such gate.

How

Add an always-on wait-for-mysql initContainer to the jobs-manager Deployment that bounded-retries a TCP connect to mysql-client:3306 (busybox nc -z, 80 attempts 3s apart) and then hands off — or exits non-zero past the cap so a genuinely-down MySQL surfaces as a loud init failure rather than an infinite Pending or a silent crashloop.

  • Chart-level, because the retry does not live in the jobs-manager image — this also protects every non-e2e install.
  • A TCP connect is the right signal: errno 111 is exactly "not listening yet", and mysqld binds :3306 only once it is ready to accept connections. The cap (~240s of sleeping when refused; up to ~480s if a connect hangs) sits above MySQL's own ~130s worst-case startup (its startupProbe: initialDelaySeconds 10 + failureThreshold 24 * periodSeconds 5).
  • Appended after the hostPath-only init-writable-data, so that perm fix keeps index 0. On CSI (hostPath disabled) wait-for-mysql is the sole init container — previously there were none, so busybox is now referenced on every install (already an enumerated mirror image; the mirror-enumeration guard stays green).
  • Non-root (runAsUser: 1000, drop ALL caps, read-only rootfs) — consistent with requests-proxy/image-refresh/auto-upgrade/storage-assertions, which already pin a UID.

Testing

  • helm unittest — asserts the gate is present, always-on (CSI + hostPath), non-root, bounded, and fails loudly; updates the CSI test (init list is no longer empty).
  • New behavioural bats scripts/tests/jobs-manager-waits-for-mysql.bats — extracts the real rendered loop and runs it against a fake nc, covering the not-ready-then-ready handoff and the bounded loud-failure path (the acceptance criterion).
  • helm lint, mirror-enumeration-complete.sh, openshift-scc-coverage.sh, automount-token-explicit.sh, bats-hygiene, chart-version-guard — all green. Chart version/appVersion bumped 1.9.90 → 1.9.91.

Out of scope

Issue "Do" item 2 — whether a leg that dies before observing an axis should record framework=unknown vs record nothing in the coverage ledger — is an e2e-test-agent / ledger decision (Lukas's domain), not this crashloop fix. Flagged for a separate ticket.

Closes tracebloc/backend#2913


Note

Low Risk
Chart-only startup ordering for jobs-manager; no application or credential logic changes, with broad test coverage for the new init gate.

Overview
Adds an always-on wait-for-mysql init container to the jobs-manager Deployment so the api container does not start until mysql-client:3306 accepts TCP connections, fixing startup crashloops when jobs_manager.py hits errno 111 before mysqld is ready.

The init runs busybox nc -z in a bounded loop (80 × 3s), exits non-zero with a FATAL log if MySQL never comes up, and is non-root with matched 10m/16Mi requests and limits so CSI installs keep Guaranteed QoS. On hostPath installs it runs after init-writable-data; on CSI it is the sole init container.

Helm unittest cases cover presence, ordering, security, resources, and failure behavior; new bats jobs-manager-waits-for-mysql.bats runs the rendered wait script against a fake nc. Chart version bumps to 1.9.92.

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

jobs-manager's api container (jobs_manager.py) makes an INITIAL, un-retried
MySQL connect at startup. On an install where mysqld is not listening yet
(errno 111, connection refused -- not a credential problem) that connect
raises and the process exits, so the kubelet crashloops the pod and an e2e
leg fails before it ever reaches training. The pod recovers minutes later and
looks healthy by the time anyone inspects, so the only evidence is the
PREVIOUS container's log -- which is what made this expensive to diagnose
(measured on staging run 33393213667).

The chart already sequences its other MySQL clients (requests-proxy retries
its bootstrap in-image); the api container had no such gate. Add an always-on
`wait-for-mysql` initContainer that bounded-retries a TCP connect to
mysql-client:3306 (busybox `nc -z`, 80 attempts 3s apart) and then hands off,
or exits non-zero past the cap so a genuinely-down MySQL surfaces as a LOUD
init failure rather than an infinite Pending or a silent crashloop. It is a
chart-level wait because the retry does not live in the jobs-manager image;
putting it here also protects every non-e2e install.

It is appended AFTER the hostPath-only init-writable-data, so that perm fix
keeps index 0; on CSI (hostPath disabled) wait-for-mysql is the sole init
container -- previously there were none, so busybox is now referenced on every
install (already an enumerated mirror image; mirror-enumeration guard stays
green).

Coverage: helm-unittest asserts the gate is present, non-root, bounded and
fails loudly; a new behavioural bats (jobs-manager-waits-for-mysql.bats)
extracts the REAL rendered loop and runs it against a fake nc for the
not-ready-then-ready handoff and the bounded loud-failure path.

Out of scope (issue "Do" item 2): whether a leg that dies before observing an
axis should record framework=unknown vs record nothing is an
e2e-test-agent/ledger decision, split out for Lukas.

Closes tracebloc/backend#2913

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@aptracebloc
aptracebloc requested a review from saadqbal as a code owner August 31, 2026 15:47
@aptracebloc aptracebloc self-assigned this Aug 31, 2026

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

Good fix, and the bats that extracts the rendered loop and drives it against a fake nc
is the right shape — mutating exit 1 -> exit 0 and the cap to 100000 catches both, so
it asserts the handoff and the bounded loud failure rather than that an initContainer
exists. The init gate is also the correct mechanism: it genuinely blocks process start,
which a readiness probe would not, so it reaches the un-retried connect. And no new
values.yaml key, so the --reuse-values class #939 just fixed isn't reintroduced.

Holding on one thing, which is six lines: wait-for-mysql sets no resources at all —
neither dimension, neither requests nor limits.

Nothing shipped changes class today; jobs-manager is Burstable either way because api's
cpu is 250m/1000m. But it becomes the first unresourced container on a CSI install,
where there were none, and that measurably removes a capability. On eks-values.yaml with
resources.jobsManager and resources.podsMonitor cpu equalised, develop renders
Guaranteed and this branch renders Burstable — silently, no error, no log line. That is
the route #919 landed and #922 documents, and losing it invisibly is precisely the failure
mode #922 exists to prevent.

It also collides with #922 concretely rather than hypothetically. Its five
pod-qos-expect.*.txt files assert the unresourced-init set by equality, so
wait-for-mysql appearing on every profile reddens all five — and csi.txt states in as
many words that jobs-manager "drops off this list entirely — that IS the difference between
the two modes", which this makes false. Whichever of #941/#922 merges second has to
reconcile; if it's #922, it goes red on develop.

requests == limits on both dimensions at cpu 10m / memory 16Mi fixes all of it: Guaranteed
comes back, defaults stay Burstable, the bats stay green, and #922 needs no edit because an
unresourced init never enters its asserted set. Cost is nil — Kubernetes takes the max of
init requests against the sum of app containers, and 10m sits well under api's 250m, so
the pod's effective request doesn't move. (#922's comment rejects "resource every init
container" on cost grounds; that objection doesn't survive at 10m/16Mi.) Same six lines also
cover a namespace with a ResourceQuota on requests.cpu, which would otherwise reject the
pod outright on CSI — a new exposure, since hostPath already had init-writable-data.

Minor while you're in there: the comment justifies the TCP probe with "mysqld binds :3306
only once it is ready to accept connections", which is an unchecked property of a custom
image. The stronger and structural reason is that mysql-client is a plain selector Service
with no publishNotReadyAddresses, so its EndpointSlice carries only Ready addresses — a
successful connect therefore proves mysql passed its mysqladmin ping readinessProbe, not
just that a socket is open. Worth saying that instead, because it's the thing actually
carrying the argument, and it tells the next person why adding publishNotReadyAddresses
later would quietly downgrade this gate to a bare TCP probe.

Timeout disposition is right, for the record: past the cap it exits 1 with a diagnostic
naming host, port, attempts and the kubectl get pods next step, and restartPolicy: Always
turns that into a visible Init:CrashLoopBackOff that self-heals when MySQL appears. Not
configurable, which I think is the right trade at zero nil-guard surface.

…iness rationale (backend#2913)

Review (Asad on client#941):

- wait-for-mysql is the FIRST init container on a CSI install (init-writable-data
  is hostPath-only), so leaving it unresourced silently forbade Guaranteed QoS on
  a profile whose api/pods-monitor requests==limits are equalised (eks-values),
  and collided with client#922's pod-qos goldens. Set requests == limits on both
  dimensions (cpu 10m / memory 16Mi): Guaranteed stays reachable, the default
  stays Burstable (api cpu is 250m/1000m), the pod stays admissible under a
  ResourceQuota on requests.cpu, and the cost is nil (k8s takes max init request
  vs sum of app containers; 10m << api's 250m). Guarded by an equality assertion
  in the unit test.

- Rewrote the TCP-connect justification: the load-bearing reason is structural,
  not the custom image's bind timing. mysql-client is a plain selector Service
  with no publishNotReadyAddresses, so its EndpointSlice carries only Ready
  addresses -- a successful connect proves mysqld passed its mysqladmin-ping
  readinessProbe, and adding publishNotReadyAddresses later would downgrade the
  gate to a bare TCP probe.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@aptracebloc

Copy link
Copy Markdown
Contributor Author

Thanks @saadqbal — both addressed in 0533ec4:

Resources (blocking). wait-for-mysql now sets requests == limits on both dimensions (cpu 10m / memory 16Mi). You're right that as the first init on a CSI install it was silently forbidding Guaranteed on the equalised profiles and colliding with #922's goldens by equality; equalising it keeps Guaranteed reachable, leaves the default Burstable (api cpu is 250m/1000m), keeps the pod admissible under a requests.cpu ResourceQuota, and costs nothing (max-init vs sum-of-app; 10m ≪ 250m). An unresourced init never enters #922's asserted set, so #922 needs no edit. Locked in with an equality assertion in jobs_manager_test.yaml.

Readiness rationale (minor). Rewrote the comment to carry the structural argument instead of the custom-image bind timing: mysql-client is a plain selector Service with no publishNotReadyAddresses, so its EndpointSlice holds only Ready addresses — a successful connect proves mysqld passed its mysqladmin ping readinessProbe, and the comment now warns that adding publishNotReadyAddresses later would downgrade the gate to a bare TCP probe.

Timeout/crashloop disposition unchanged, as you noted it was right.

@aptracebloc
aptracebloc requested a review from saadqbal September 1, 2026 07:14
@aptracebloc

Copy link
Copy Markdown
Contributor Author

bugbot run

…ger-wait-for-mysql

# Conflicts:
#	client/templates/jobs-manager-deployment.yaml
@aptracebloc

Copy link
Copy Markdown
Contributor Author

bugbot run

Comment thread ingestor/templates/job.yaml Outdated
…ge (backend#2913)

A two-line stub (kind: Job / new: true) with no apiVersion/metadata/spec was
pulled into the conflict-resolution merge commit by git add -A from an untracked
file in the shared worktree. Helm renders every non-_ template, so it broke the
ingestor chart (Helm lint / drift / bugbot) and tripped the version-bump gate on
an unbumped ingestor Chart.yaml. It has nothing to do with this PR. Remove it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread ingestor/templates/job.yaml Outdated
@aptracebloc

Copy link
Copy Markdown
Contributor Author

bugbot run

@LukasWodka

Copy link
Copy Markdown
Contributor

Flagging a duplicate and one defect, since I hit both today.

These are the same fix

I opened client#942 for backend#2913 this morning without seeing this — yours predates mine by 14 hours (2026-08-31 15:47 vs 2026-09-01 05:53), so mine is the duplicate, not yours. Same diagnosis, same approach: an init container that waits for mysql-client:3306, and lifting initContainers: out of if .Values.hostPath.enabled so an edge without hostPath gets one at all.

@LukasWodka to decide which to keep — I am not closing anything.

The defect, which is in both of ours

wait-for-mysql sets runAsUser: 1000 (line ~190 here). Bugbot flagged it on mine as a High and it is right:

OpenShift's restricted SCC assigns a uid from the project range… A single container pinned to 1000 makes the whole pod fail SCC admission, so jobs-manager stays Pending on OpenShift even after MySQL is up.

Confirmed against the chart rather than taken on trust — neither api nor pods-monitor-container pins a uid, and the pod-level securityContext comment says it outright:

the spawned training pods (UID 1001 / OpenShift arbitrary-UID, GID 0)

So the pod is deliberately built for an assigned uid, and a pinned container turns a startup race into a permanent stop on one of the four supported platforms — strictly worse than the bug being fixed.

Your comment's reasoning is sound and still points the wrong way: "busybox has no baked non-root user, so runAsUser is set". True, but runAsNonRoot: true is already set at pod level and inherited, and OpenShift supplies the uid. Dropping the line loosens nothing — the container still cannot run as root, it simply stops caring which non-root uid it gets. Opening a TCP socket needs no particular identity.

On the platforms that do not assign one, the pod-level runAsNonRoot: true means the kubelet refuses to start a container whose image would run as root — which is the check that matters, and it is unaffected.

A guard, if you want it

I added one to my branch that renders with hostPath.enabled=false — the configuration where every remaining container must accept an assigned uid — and fails if any pins one:

the uid pin comes back  ->  FAIL a jobs-manager container pins a uid on the arbitrary-UID path
restored                ->  OK

Rendering with hostPath off is what makes it checkable: init-writable-data legitimately needs runAsUser: 0 to chown a hostPath volume, and with hostPath on it would mask the check.

Happy to port that onto this branch instead, or to close mine — say which.

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

Both my items are cleared, and you've already dropped the stray ingestor commit — so from
my side this is done; I'm only holding the approval because the gate isn't there yet.

I measured the QoS rather than reading the values. wait-for-mysql now carries requests == limits on cpu and memory (10m / 16Mi), and on eks-values.yaml with
resources.jobsManager/resources.podsMonitor cpu equalised the class is back to
Guaranteed — develop was Guaranteed, the commit I blocked rendered Burstable, and this
head renders Guaranteed again. Defaults stay Burstable on both EKS and bare-metal, which
is correct. #922's csi.txt also stays true: the unresourced-init set on CSI is
mysql-client alone, so jobs-manager still drops off that list and its goldens don't
redden. Hardcoding the literals in the template rather than plumbing a values key is the
right call — nothing new to nil-guard.

The readiness rationale is better than what I asked for. You replaced the image-property
claim with the structural one, and added the note that adding publishNotReadyAddresses to
mysql-service.yaml later would silently downgrade the gate to a bare TCP probe. That
second half is the part that will save someone.

What's left is just CI: 13 checks still pending, and bugbot / review is red but stale — it
completed in 5s while Cursor Bugbot is still running, and both of its High threads (the
ingestor stub) are already resolved against the commit you dropped. It should clear on the
re-run without a push. I'll pick it up next pass and approve once the checks land.

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

✅ 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 a0914f8. Configure here.

@aptracebloc

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #942, which merged first for the same issue (tracebloc/backend#2913, now closed/completed).

#942's approach is the better one and I'm happy it landed: it runs the wait as an init container on the jobs-manager app image (USER 1001) with no pinned runAsUser, so it works on OpenShift (arbitrary-UID SCC) and plain Kubernetes — avoiding the exact pitfall this PR carried (busybox has an empty Config.User, so runAsNonRoot: true needs a pinned uid, and runAsUser: 1000 then breaks OpenShift). It also ships equivalent behavioural coverage (scripts/tests/jobs-manager-waits-for-mysql.sh).

Nothing here to salvage over #942. Thanks @LukasWodka.

@aptracebloc aptracebloc closed this Sep 1, 2026
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.

3 participants