fix(doctor): requests-proxy check — result-egress framing + honest ✔ (cli#351 Part 1)#356
Merged
Merged
Conversation
When the cluster API is unreachable (Docker/k3d stopped, wrong server,
network), doctor ran all 8 downstream checks anyway and each invented a
domain-specific cause with the wrong fix: PVC "usable StorageClass", proxy
"chart too old", requests-proxy "not found -> reinstall the chart", pods/RBAC
"kubeconfig user can list pods". The one true cause (cluster stopped) was
buried under ~6 false verdicts. Root mechanism: findDeployment/jobsManagerEnv
collapse an API transport error and genuine absence into the same nil/empty.
- Add StatusUnknown ("could not check") — Worst() ignores it, so it never
affects the verdict/exit code.
- isUnreachable(err): a transport/connectivity error (connection refused,
timeout, DNS, TLS; net.Error/url.Error) that is NOT ErrNoParentRelease and
NOT an RBAC/NotFound status (those mean the API answered).
- Run() short-circuits on isUnreachable(relErr): emit the one honest
"Cluster reachable" ✖ and mark the 7 cluster checks StatusUnknown.
checkBackendEgress still runs (probes the backend from this machine, not the
cluster API). Display order unchanged. When the API IS reachable, every check
runs and reports exactly as before (isolated-failure messaging preserved).
- checkReachable names the endpoint ("the cluster API server at <url> isn't
answering — is the cluster running?") and, for a loopback server, gives the
local-cluster remedy ("start Docker Desktop, then `k3d cluster start`")
instead of the kubeconfig/chart remedy.
- doctor renders StatusUnknown as a neutral · line (no ✖/⚠, no remedy).
Verified by TestRun_UnreachableCascade (fake clientset, connection-refused
reactor): one ✖ naming 127.0.0.1:6550 + start-cluster remedy, 7 StatusUnknown
with no remedy, backend egress still ok, Worst()=Fail (exit code unchanged).
Fixes #350. Part of epic tracebloc/backend#1142 (Theme A).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(Part 1) The "Service Bus egress (requests-proxy)" check had the direction backwards and overclaimed its ✔: - It said a down requests-proxy makes experiments "stay Pending". Wrong path: requests-proxy is the OUTBOUND relay (training pods POST epoch results/FLOPs → it forwards to Service Bus). Scheduling is a separate inbound path jobs-manager consumes directly. A down proxy stalls result/weights egress MID-RUN; it does not block scheduling. The old remedy misroutes triage. - The ✔ is inferred from Deployment ReadyReplicas, but requests-proxy has no readiness probe (backend#1143), so "Ready" only means the container started, not that Service Bus egress works. Reword the doc comment + all three lines: missing/not-ready remedies now say "result egress stalls mid-run (scheduling unaffected)"; the OK detail says "deployment-ready only; egress not directly probed". Status stays OK on a healthy install (no new ⚠, exit code unchanged). Name const kept for now. TestCheckRequestsProxy_Wording locks it: no line says "Pending", and the ✔ admits egress isn't probed. Part 2 (a real in-cluster Service Bus-egress probe) is deferred — it needs the requests-proxy /healthz + readiness probe (backend#1143 + a client-runtime health endpoint) that don't exist yet. Fixes #351 (Part 1). Part of epic tracebloc/backend#1142. Stacked on #354 (fix/doctor-reachability-gate) — merge that first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@BugBot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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 c4494cb. Configure here.
This was referenced Jul 21, 2026
aptracebloc
approved these changes
Jul 21, 2026
This was referenced Jul 21, 2026
Closed
Merged
Contributor
Author
|
/fr-pass — verified live on dev (secure env |
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
3 participants
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.
Fixes #351 (Part 1) · epic backend#1142 · stacked on #354 (merge #354 first; until then this PR's diff shows both commits).
Problem
The requests-proxy check had the direction backwards and overclaimed its ✔:
ReadyReplicasbut there's no readiness probe (backend#1143), so "Ready" ≠ egress works.Fix (Part 1 — CLI reword, S)
Reworded the doc comment + all three lines: remedies now say "result egress stalls mid-run (scheduling unaffected)"; the OK detail says "deployment-ready only; egress not directly probed." Status stays OK on a healthy install (no new ⚠, exit code unchanged).
TestCheckRequestsProxy_Wordinglocks it (no "Pending"; honest ✔).Deferred (Part 2)
A real in-cluster Service-Bus-egress probe needs the requests-proxy
/healthz+ readiness probe (backend#1143 + a client-runtime health endpoint) that don't exist yet — carved into a follow-up.gofmt/vet clean; doctor + cli packages green.
Note
Low Risk
CLI-only diagnostic UX and messaging; no auth, data, or cluster mutation paths; exit codes unchanged for healthy clusters and tests cover the unreachable short-circuit.
Overview
Unreachable cluster:
tracebloc doctornow passesServerURLfrom kubeconfig intodoctor.Run. If discovery fails with a transport error (isUnreachable), the sweep short-circuits: Cluster reachable is the single ✖ (names the API URL; loopback clusters get a start-Docker/k3d remedy), cluster-dependent checks returnStatusUnknown(neutral info lines, no remedies),Worst()ignores unknowns, and Backend egress still runs from the local machine.requests-proxy (cli#351 Part 1): Doc comment, fail remedies, and OK detail for Service Bus egress (requests-proxy) no longer claim experiments "stay Pending"; they describe mid-run result/FLOPs egress and note that Ready ≠ egress probed.
TestCheckRequestsProxy_WordingandTestRun_UnreachableCascadelock behavior.Reviewed by Cursor Bugbot for commit c4494cb. Bugbot is set up for automated code reviews on this repo. Configure here.