Skip to content

feat(#88): tracebloc cluster doctor — live-cluster health checks (WS3) - #89

Merged
saadqbal merged 6 commits into
developfrom
feat/88-cluster-doctor
Jun 18, 2026
Merged

feat(#88): tracebloc cluster doctor — live-cluster health checks (WS3)#89
saadqbal merged 6 commits into
developfrom
feat/88-cluster-doctor

Conversation

@saadqbal

@saadqbalsaadqbal commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

What

Adds tracebloc cluster doctor — a read-only health sweep of a running tracebloc client cluster that prints a ✔/⚠/✖ line per check with a remedy for anything that isn't green.

Closes#88. Parent epic: client-runtime#116 (WS3 — diagnose without tracebloc access).

Why

When a customer's experiment silently sits in Pending, today there's no self-serve way to ask "is my cluster healthy?" — the cluster info command answers "am I pointed at the right cluster?" but not "is it working?". doctor fills that gap, over the customer's normal kubeconfig.

It's the sibling cluster info's own doc comment already anticipated:

Future verbs (e.g. cluster doctor for diagnostics) hang off this parent in later phases.

The 6 checks (lean MVP)

CheckWhat it verifies
Cluster reachableAPI answers + parent client release discovered
Pod healthnothing crash-looping / stuck Pending (local complement to client-runtime#117)
Dataset volumeshared PVC exists + Bound (reuses cluster.DiscoverSharedPVC)
Proxy configurationin-cluster requests/egress proxy wiring (the corporate-proxy propagation class)
Backend egresstracebloc backend reachable (host-side, proxy-aware)
Service Bus egressrequests-proxy Ready — the broker for the experiments queue; while it's down, experiments stay Pending

Ends with a consolidated verdict + a pointer to ./install-k8s.sh --diagnose for a full support bundle.

How

  • internal/cli/doctor.go — thin cobra command, sibling of cluster info, same kubeconfig/context/namespace flags; renders results via ui.Printer and maps the worst status to an exit code.
  • internal/doctor/ — standalone package: Status/Result/Worst + the 6 checks. Reuses cluster.Load/NewClientset/DiscoverParentRelease/DiscoverSharedPVC. Network probes are injectable (Options.HTTPProbe) so it's fully testable without a real cluster or egress.
  • Every check is independent and best-effort — one failure never hides the others (mirrors the installer's preflight.sh contract).

Honest scope notes

  • Backend egress is probed from the machine the CLI runs on, not from inside the cluster (the cluster egresses via its egress-proxy). It still catches a customer network/proxy that can't reach the backend at all; a true in-cluster probe is the WS3 follow-up.
  • support-bundle already ships as the installer's install-k8s.sh --diagnose — not rebuilt here.
  • Deferred to a follow-up: node-resources-vs-spawned-job-request fit, and image pullability.

Exit codes

0 all passed (or warnings only) · 2 one or more checks failed · 3 kubeconfig couldn't be loaded.

Testing

make ci green: vet, go test -race -cover, errcheck, ineffassign, misspell, gofmt -s, schema-check. New internal/doctor package is 82.2% covered (table-driven, client-go fake clientset + injected probes). Manually verified tracebloc cluster doctor --help and command registration under cluster.

🤖 Generated with Claude Code


Note

Low Risk
Read-only Kubernetes inspection and optional outbound HTTP probes; no changes to auth, ingestion, or cluster mutation paths.

Overview
Adds tracebloc cluster doctor under the existing cluster command — a read-only post-install sweep that prints ✔/⚠/✖ per check with remedies, complementing cluster info (targeting vs health).

The CLI loads kubeconfig the same way as cluster info, runs internal/doctor, and maps the worst check to exit codes 0 (ok/warn), 2 (fail), or 3 (kubeconfig).

Six independent checks always run (failures don’t hide others): parent release reachable, pod crash-loop / long-Pending, shared PVC Bound, jobs-manager proxy env, backend HTTPS reachability from the user’s machine (injectable probe), and requests-proxy ready — with findDeployment tied to the discovered Helm release to avoid false greens across multiple installs.

Table-driven tests use the fake clientset and injected HTTP probes (~82% coverage on the new package).

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

Adds `tracebloc cluster doctor`, a read-only health sweep of a running
tracebloc client cluster that prints ✔/⚠/✖ per check with a remedy — so a
customer can diagnose "why isn't my experiment running?" without tracebloc
shelling into their cluster (epic client-runtime#116, WS3).
Sibling of `cluster info` (which the code's own comment anticipated); reuses
its kubeconfig/context/namespace flags + cluster.Load / NewClientset /
DiscoverParentRelease, the ui.Printer status vocabulary, and exitError.
Lean MVP — 6 checks:
- cluster reachable (parent client release discovered)
- pod health (crash-loops / long-Pending — local complement to #117)
- dataset volume (shared PVC Bound, via cluster.DiscoverSharedPVC)
- proxy configuration (in-cluster requests/egress proxy wiring)
- backend egress (host-side, proxy-aware probe; in-cluster probe = follow-up)
- Service Bus egress (requests-proxy readiness — the experiments-queue broker)
internal/doctor is a standalone package with injectable network probes,
82% covered via client-go's fake clientset. Every check is independent and
best-effort (one failure never hides the others); the worst status sets the
exit code (0 ok/warn, 2 failures, 3 kubeconfig).
Out of scope (already shipped / follow-up): support-bundle ships as the
installer's `--diagnose`; node-resources-vs-job-request and image-pullability
are the broader cut.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@saadqbalsaadqbal self-assigned this Jun 18, 2026
Comment threadinternal/doctor/doctor.go
podCrashLooping flagged any pod with RestartCount>=3 — including Succeeded
job pods that retried before completing, and Running pods that recovered
after past restarts — producing a false ✖ when nothing is actually unhealthy.
Guard terminal phases (Succeeded/Failed) and require the container to not be
currently running, mirroring the controller's recovered-container fix
(client-runtime#117). Adds regression tests for both false-positive cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment threadinternal/doctor/doctor.go
Comment threadinternal/doctor/doctor.go Outdated
…ploys (Bugbot)
Two Medium Bugbot findings on the previous commit:
- podCrashLooping ignored InitContainerStatuses, so an init container stuck in
CrashLoopBackOff read as a Pending warning instead of a failure even though
the pod cannot start. It now checks init + app containers, and detects only
active CrashLoopBackOff — dropping the RestartCount heuristic entirely, since
that was the source of the earlier Succeeded/recovered-pod false positives.
- requestsProxyNames/jobsManagerNames only probed unprefixed names when the
release was nil (e.g. DiscoverParentRelease errored on multiple releases),
falsely reporting missing wiring even though <release>-requests-proxy exists.
Added findDeployment: exact-name Get, then a namespace List + name-suffix
fallback that resolves the prefixed name without knowing the release.
Adds regression tests: init-crash-loop, nil-release-finds-prefixed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aptracebloc

Copy link
Copy Markdown
Contributor

Post-approval follow-up (non-blocking — already approved 👍): one small doc fix + two optional nits.

const vs var mismatch — internal/doctor/doctor.go:
The tunables comment says they're "exported-as-vars (not consts) so a future flag could override them," but they're declared as unexported const:

const (
pendingGrace=5*time.MinutehttpProbeTimeout=8*time.Second
)

A const can't be overridden at runtime (not addressable / can't be reassigned), so the rationale doesn't hold as written. Either reword the comment to "conservative package consts," or — if override is actually intended — make them Options fields (e.g. PendingGrace time.Duration) so it threads through the existing Run(opts) seam rather than package-level vars.

Minor (optional):

  • checkProxy reads only literal env from jobs-manager (jobsManagerEnv skips valueFrom), so a chart that sets REQUESTS_PROXY_URL via a configMap/secret ref would false-WARN. Worth a line in the remedy, or accept as a known edge.
  • httpProbe returns resp.Body.Close() as the probe result; defer resp.Body.Close(); return nil avoids reporting "unreachable" on a (rare) close error after a successful connect.

Really clean package otherwise — the injectable HTTPProbe + fake-clientset tests make it a solid template for future cluster <verb> siblings.

— drafted with Claude (Opus 4.8), sent by @aptracebloc

- Tunables comment: they're conservative package consts, not vars; point at
Options (like HTTPProbe) for any future runtime tuning.
- checkProxy WARN: note that a REQUESTS_PROXY_URL set via a configMap/secret
ref reads as empty here (jobsManagerEnv reads only literal env).
- httpProbe: a successful connection means reachable — discard the body-close
error rather than reporting it as "unreachable".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@saadqbal

Copy link
Copy Markdown
CollaboratorAuthor

Thanks @aptracebloc — all three addressed in 289f237 (non-blocking, but cheap + correct):

  1. const/var comment — reworded: they're conservative package consts; the comment now points at Options (like HTTPProbe) as the seam for any future runtime tuning, rather than implying these are overridable vars.
  2. checkProxyvalueFrom — the WARN detail now calls out that a REQUESTS_PROXY_URL set via a configMap/secret ref reads as empty here (we read only literal env), so a ref-based install isn't mistaken for missing wiring.
  3. httpProbe close error — a successful client.Do means the host is reachable; we now _ = resp.Body.Close(); return nil so a rare post-connect close error isn't reported as "unreachable" (kept the explicit discard to stay errcheck-clean rather than defer).

make ci still green. Appreciate the careful read 🙏

Comment threadinternal/doctor/doctor.go
…bot)
findDeployment's suffix fallback (added for the nil-release case) picked the
first suffix-matching deployment, so in a namespace running multiple parent
releases, jobsManagerEnv and checkRequestsProxy could resolve to different
releases in a single run — presenting mixed data as fact.
Resolve the fallback only when exactly one deployment carries the suffix;
with more than one (the multi-release case DiscoverParentRelease already
refuses to disambiguate) return nil and let the check report can't-determine.
The single-release nil-discovery case still resolves.
Adds TestCheckRequestsProxy_NilReleaseAmbiguous.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

Reviewed by Cursor Bugbot for commit 86787e2. Configure here.

Comment threadinternal/doctor/doctor.go
When a release was discovered, findDeployment's fallbacks could still match a
DIFFERENT release's component (or a stray bare one), so the Service Bus check
went green on the wrong requests-proxy while the discovered release's was
missing.
findDeployment now takes the release directly. When it's known, it accepts only
"<release>-<suffix>" or a bare "<suffix>" whose app.kubernetes.io/instance label
ties it to that release — never another release's, never an unattributable bare
one. The release-unknown path keeps the exactly-one-suffix-match rule (returns
nil on >1, so checks report can't-determine rather than guess).
Folds the jobsManagerNames/requestsProxyNames candidate builders into
findDeployment. Adds tests: other-release-ignored, bare-name-tied-by-label.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@saadqbal
saadqbal merged commit 6b259cd into developJun 18, 2026
15 checks passed
saadqbal added a commit that referenced this pull request Jun 19, 2026
…up) (#91)
* feat(#90): cluster doctor — node-fit + image-pull checks (WS3 follow-up)
Two read-only checks added to `tracebloc cluster doctor` (follow-up to #89):
- Node capacity: parses the resource requests jobs-manager stamps on spawned
training jobs (RESOURCE_REQUESTS / GPU_REQUESTS env) and checks at least one
Ready node can fit them — the "Pending forever, no node big enough" class.
GPU is soft: a hard ✖ only on cpu/mem, and a ⚠ when a GPU is requested but no
node exposes it (jobs-manager has a GPU->CPU fallback).
- Image pull secret: when jobs-manager references a registry pull secret,
verifies it exists and is a well-formed dockerconfigjson so private-image
pulls don't ImagePullBackOff.
Both read-only/best-effort, tested with client-go's fake clientset. The
in-cluster egress probe (the third deferred check on #90) is intentionally a
separate PR — it needs a port-forward/exec mechanism, not this read-only pattern.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(#90): node-fit must require cpu+mem+GPU on ONE node (Bugbot)
checkNodeFit set cpuMemFits and gpuFits independently, so they could come from
different nodes — reporting OK even when no single node had cpu+memory+GPU
together (a GPU job would then stay Pending). It now evaluates each node as a
whole: cpuMemFits (any node) drives the hard fail; fullFits (one node with
cpu+mem AND the GPU) drives the ok/warn split. Adds regression tests for the
cross-node and single-node-fits cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@saadqbal
saadqbal deleted the feat/88-cluster-doctor branch July 10, 2026 10:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@saadqbal@aptracebloc@LukasWodka