diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 16c0482d..61bd0b00 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -1,9 +1,14 @@ package cli import ( + "bytes" "context" "errors" + "fmt" "net/http" + "os" + "strings" + "time" "github.com/spf13/cobra" @@ -14,67 +19,59 @@ import ( "github.com/tracebloc/cli/internal/ui" ) -// doctorRunFn is a test seam over doctor.Run (the cluster-side health sweep). -// Production runs the real checks; tests inject a fixed []doctor.Result so -// runClusterDoctor's render loop (the ✓/⚠/✖ switch + remedy hints) and its -// overall-verdict switch can be exercised with a controlled OK/Warn/Fail mix, -// without standing up a fake cluster that happens to produce that exact mix -// (the per-check logic has its own tests in internal/doctor). +// installCmd is the one-line installer we point people at when there's no +// secure environment on this machine, or a component needs reinstalling. Kept in +// one place so every remedy says the same thing. +const installCmd = "bash <(curl -fsSL https://tracebloc.io/i.sh)" + +// doctorRunFn is a test seam over doctor.Run (the cluster-side probe). Tests +// inject a fixed []doctor.Result so the roll-up + render can be exercised with a +// controlled mix without standing up a fake cluster. var doctorRunFn = doctor.Run -// newDoctorCmd builds the `doctor` command. The SAME command is registered two -// ways: as the top-level `tracebloc doctor` (visible — the home screen and its -// env-status lines point here), and, hidden, as `tracebloc cluster doctor` (its -// original path, kept working so existing docs, muscle memory, and scripts don't -// break). Pass hidden=true for the cluster-subtree alias. Both entry points -// share one RunE (runClusterDoctor), so there is a single diagnostic code path. -// -// It answers "is this running cluster healthy enough to run an experiment, and -// if not, what do I fix?" — a read-only, post-install health sweep with remedies -// (epic client-runtime#116, WS3). The three kubeconfig flags match `cluster -// info` exactly so muscle memory carries over; all are zero-value-safe. +// newDoctorCmd builds `doctor`, registered as top-level `tracebloc doctor` and +// (hidden) `tracebloc cluster doctor`. It answers, in plain language: is my +// secure environment connected to tracebloc and ready to run training — and if +// not, exactly what do I do. The Kubernetes detail lives behind --verbose; +// --diagnose writes a redacted bundle for support. func newDoctorCmd(hidden bool) *cobra.Command { var ( kubeconfigPath string contextOverride string nsOverride string + diagnose bool ) cmd := &cobra.Command{ Use: "doctor", Hidden: hidden, - Short: "Diagnose a running tracebloc client cluster (✔/⚠/✖ health checks + remedies)", - Long: `Runs a read-only health sweep over the tracebloc client release in the -configured cluster + namespace and prints a ✔/⚠/✖ line per check with a -remedy for anything that isn't green: - - • Cluster reachable — the API answers and the client chart is installed - • Pod health — nothing crash-looping or stuck Pending - • Dataset volume — the shared PVC exists and is Bound - • Proxy configuration — the in-cluster requests/egress proxy wiring - • Backend egress — the tracebloc backend is reachable (from this machine) - • Service Bus egress — the requests-proxy that brokers experiment egress is Ready + Short: "Check your secure environment is connected and ready to run training", + Long: `Checks, in plain terms, whether your secure environment is connected to +tracebloc and ready to run training — and if something's wrong, exactly what to +do about it. -For a full redacted support bundle to send to tracebloc, use the installer's -` + "`./install-k8s.sh --diagnose`" + ` instead. + --verbose the full technical breakdown (for support) + --diagnose write a redacted support bundle to email to tracebloc Exit codes: - 0 all checks passed (or warnings only) - 2 one or more checks failed - 3 kubeconfig could not be loaded`, + 0 healthy + 2 a problem was found + 3 couldn't read your local config`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { return runClusterDoctor( cmd.Context(), printerFor(cmd), - kubeconfigPath, contextOverride, nsOverride, + kubeconfigPath, contextOverride, nsOverride, diagnose, ) }, } addKubeconfigFlags(cmd, &kubeconfigPath, &contextOverride, kubeconfigFlagUsage, contextFlagUsage) addNamespaceFlag(cmd, &nsOverride, - "namespace where your tracebloc client is installed (default: your active client's namespace, else the context's)") + "namespace where your secure environment is installed (default: your active client's)") + cmd.Flags().BoolVar(&diagnose, "diagnose", false, + "write a redacted support bundle for tracebloc support and exit") return cmd } @@ -83,167 +80,432 @@ func runClusterDoctor( ctx context.Context, p *ui.Printer, kubeconfigPath, contextOverride, nsOverride string, -) error { - p.Banner("tracebloc", "doctor") - - // Auth / config checks run FIRST and don't need a cluster — so `doctor` can - // diagnose a failed provision (bad/expired token, wrong env, no active - // client) even before any cluster is reachable (RFC-0001 §8.5). - authStatus := runAuthChecks(ctx, p) - - // Target the active client's namespace exactly like `cluster info`, the data - // commands, and the home screen: bind opts.Namespace to the cached active - // client when the user overrode neither --namespace nor --context. Without - // this, doctor checked only the kubeconfig default namespace — so a typical - // install whose client lives in its slug namespace could show one state on the - // home screen and a conflicting "no client here" from the on-screen `doctor` - // hint (Bugbot / review). - opts := cluster.KubeconfigOptions{ - Path: kubeconfigPath, - Context: contextOverride, - Namespace: nsOverride, - } - bindActiveClientNamespace(&opts) // side-effect: defaults opts.Namespace to the active client's - // Through the loadClusterFn/newClientsetFn seams (like cluster info and the - // data commands) so the cluster half — the Checks render loop and the verdict - // switch — is testable with a fake clientset instead of only a real kubeconfig. - resolved, err := loadClusterFn(opts) + diagnose bool, +) (rerr error) { + // 1. Who you are — a local config read (instant), so we can answer even + // before any cluster is reachable (RFC-0001 §8.5). + cfg, err := config.Load() if err != nil { - // 3 = kubeconfig file/parse problem (same class as cluster info). The - // auth section above already ran; if IT also failed, escalate to 2 so - // automation doesn't read a real auth failure as a kubeconfig-only one. - p.Section("Cluster") - p.Errorf("Kubeconfig — couldn't load it: %v", err) - p.Hintf(" point --kubeconfig / --context at your cluster, or fix ~/.kube/config") - return &exitError{code: kubeconfigExitCode(authStatus), err: nil} + p.Errorf("Couldn't read your tracebloc config — run `%s login` to recreate it.", launcher()) + return &exitError{code: exitLocalEnv, err: nil} + } + if !cfg.SignedIn() { + p.Errorf("Not signed in — run `%s login`.", launcher()) + return &exitError{code: exitChecksFailed, err: nil} + } + if email := cfg.Current().Email; email != "" { + p.Para("Signed in as " + email) + } else { + p.Para("Signed in") } + // State we accumulate so `--diagnose` can leave a bundle from wherever we + // exit. tok is declared here because the session probe below sets it. + var ( + tok = tokenOK + resolved *cluster.ResolvedConfig + results []doctor.Result + connected, ready healthLine + ) + + // 2. Is the session still good with tracebloc? A 401 (expired/revoked) or a + // 426 (CLI too old) is a hard stop with a one-command fix. Anything else + // folds into the Connected line below — but a backend that ANSWERED with + // an error (5xx/403/decode) is a tracebloc-side problem, distinct from a + // network failure to reach it at all. Conflating the two would blame the + // user's network (and hand them a proxy remedy) for tracebloc's own error. + apiClient := newAPIClient(cfg.CurrentEnv) + apiClient.Token = cfg.Current().Token + if _, werr := apiClient.WhoAmI(ctx); werr != nil { + var ae *api.APIError + var ue *api.UpgradeRequiredError + switch { + case errors.As(werr, &ae) && ae.StatusCode == http.StatusUnauthorized: + p.Newline() + p.Errorf("Your session expired — run `%s login`.", launcher()) + return &exitError{code: exitChecksFailed, err: nil} + case errors.As(werr, &ue): + p.Newline() + p.Errorf("This CLI is out of date — update it: %s", installCmd) + return &exitError{code: exitChecksFailed, err: nil} + case errors.As(werr, &ae): + tok = tokenServerErr // tracebloc answered, just not with 200 + default: + tok = tokenUnreachable // couldn't reach tracebloc at all (network/proxy) + } + } + + // Register the --diagnose bundle writer now that the session outcome is known. + // Placed AFTER the 401/426 hard stops (which return above): a bundle is + // pointless for those one-command fixes (login / update), and — crucially — + // writing one there would record "session: confirmed" for an expired or + // upgrade-required session, misleading triage. For every path past here tok is + // accurate, so `--diagnose` always leaves a truthful bundle (even on the + // no-environment / clientset-error exits, exactly where a remedy sends the + // user). The write only sets the exit code when nothing worse already did, so + // a bundle hiccup never masks a real Fail verdict. + if diagnose { + defer func() { + p.Newline() + if werr := writeDiagnoseBundle(p, resolved, results, tok, connected, ready); werr != nil && rerr == nil { + rerr = werr + } + }() + } + + // 3. Find the secure environment (local kubeconfig read). If it isn't here, + // that's the headline — but surface a session/backend fault we already + // found first, so a reinstall is never recommended while a live session + // problem goes unexplained. (A 401/426 is a hard stop earlier; only the + // soft tokenUnreachable/tokenServerErr states reach here.) + opts := cluster.KubeconfigOptions{Path: kubeconfigPath, Context: contextOverride, Namespace: nsOverride} + bindActiveClientNamespace(&opts) + resolved, err = loadClusterFn(opts) + if err != nil { + p.Newline() + noteSessionProblem(p, tok) + p.Errorf("No secure environment on this machine yet.") + p.Hintf(" Set one up: %s", installCmd) + return &exitError{code: earlyExitCode(tok), err: nil} + } cs, err := newClientsetFn(resolved) if err != nil { - p.Section("Cluster") - p.Errorf("Kubeconfig — %v", err) - return &exitError{code: kubeconfigExitCode(authStatus), err: nil} + p.Newline() + noteSessionProblem(p, tok) + p.Errorf("Couldn't connect to your secure environment — check your kubeconfig/context.") + renderDetailsIfVerbose(p, resolved, results) + return &exitError{code: earlyExitCode(tok), err: nil} + } + // 4. Probe the cluster. + results = doctorRunFn(ctx, cs, doctor.Options{Namespace: resolved.Namespace, ServerURL: resolved.ServerURL}) + + // A reachable cluster with no tracebloc chart installed is the same "no secure + // environment here" state as a missing kubeconfig — route it through the same + // message (which also surfaces any session fault) rather than naming an + // environment that isn't installed. This unifies all three no-environment + // exits: missing kubeconfig, clientset error, and no chart. + if reachStateOf(results) == doctor.ReachNoEnv { + p.Newline() + noteSessionProblem(p, tok) + p.Errorf("No secure environment on this machine yet.") + p.Hintf(" Set one up: %s", installCmd) + renderDetailsIfVerbose(p, resolved, results) + return &exitError{code: earlyExitCode(tok), err: nil} + } + + // An environment is installed here — name it (nothing prints between this and + // "Signed in" above, so the two context lines read as a pair), then roll up. + p.Para(fmt.Sprintf("Secure environment %q", envDisplayName(resolved))) + connected, ready = summarizeDoctor(results, tok) + + p.Newline() + renderHealth(p, connected) + renderHealth(p, ready) + renderDetailsIfVerbose(p, resolved, results) + // --diagnose writes the support bundle via the deferred writer registered + // above, so it fires on this path and on every early exit alike. + + // 6. Verdict + exit code (0 healthy/partial, 2 a problem). + p.Newline() + fail, allGood := doctorVerdict(connected.status, ready.status) + switch { + case fail: + if !diagnose { // they just wrote a bundle — don't send them to write it again + p.Hintf("Still stuck? Email support@tracebloc.io with the output of `%s doctor --diagnose`.", launcher()) + } + return &exitError{code: exitChecksFailed, err: nil} + case allGood: + p.Successf("Everything looks good — you're ready to run training.") + default: + // Connected and nothing failed, but a check couldn't complete (e.g. pod + // health unreadable — RBAC → ready is StatusUnknown). Don't overclaim + // "everything looks good"; say so honestly. Not a failure → exit 0 (Bugbot). + p.Infof("No problems found, but some checks couldn't finish — re-run with --verbose for detail.") } + return nil +} + +// healthLine is one rolled-up, user-facing health signal: a status, the plain +// line to show, and (on failure) one concrete action. +type healthLine struct { + status doctor.Status + text string + remedy string +} + +// tokenState classifies the WhoAmI probe for the Connected rollup: the session +// confirmed (tokenOK), the backend unreachable from this machine (tokenUnreachable +// — network/proxy), or the backend reachable but answering with an error +// (tokenServerErr — 5xx/403/decode, a tracebloc-side problem, not the network). +type tokenState int - p.Section("Kubeconfig") +const ( + tokenOK tokenState = iota + tokenUnreachable + tokenServerErr +) + +// noteSessionProblem prints a soft WhoAmI/session fault (unreachable backend or a +// server-side error) as a standalone ✖ + fix. Used on the no-local-environment +// path so a fault detected before the kubeconfig read isn't silently dropped when +// that read fails. It's a no-op when the session is fine (tokenOK). +func noteSessionProblem(p *ui.Printer, tok tokenState) { + switch tok { + case tokenUnreachable: + p.Errorf("Can't reach tracebloc from here.") + p.Hintf(" Check your network / HTTP(S)_PROXY, then run `%s doctor` again.", launcher()) + case tokenServerErr: + p.Errorf("tracebloc didn't confirm your session (server error).") + p.Hintf(" Try again shortly; if it persists, email support@tracebloc.io with `%s doctor --diagnose`.", launcher()) + } +} + +// earlyExitCode picks the exit code for a local-environment early exit (no +// kubeconfig / clientset error). A connectivity or session fault we already +// detected (tok != tokenOK) is a checks failure and takes precedence over the +// local-env code, so those states exit 2 here just as they do on the full probe +// path — a script keying on "exit 2 = a problem was found" never misses a +// session fault just because local config also failed. +func earlyExitCode(tok tokenState) int { + if tok != tokenOK { + return exitChecksFailed + } + return exitLocalEnv +} + +// tokenLabel is the one-line session status recorded in the support bundle. +func tokenLabel(tok tokenState) string { + switch tok { + case tokenUnreachable: + return "backend unreachable from this machine (network/proxy)" + case tokenServerErr: + return "backend answered with an error (5xx/403/decode)" + default: + return "confirmed" + } +} + +// summarizeDoctor collapses the granular checks into the two lines the owner +// reads: "Connected to tracebloc" and "Ready to run training". Each expands to +// the specific plain-language problem + fix on failure; the Kubernetes detail +// stays in --verbose. When the owner isn't connected — for ANY reason — a +// healthy local cluster still can't run training, so readiness degrades honestly +// to "can't check" rather than showing a reassuring ✔ next to a Connected ✖. +func summarizeDoctor(results []doctor.Result, tok tokenState) (connected, ready healthLine) { + by := make(map[string]doctor.Result, len(results)) + for _, r := range results { + by[r.Name] = r + } + reach := by["Cluster reachable"] + + switch { + case reach.Status == doctor.StatusFail: + // ReachNoEnv (reachable, no chart) is short-circuited in runClusterDoctor, + // so it never reaches here. The two remaining fails are worded from the + // classification, never a kubectl; an unclassified fail (e.g. a hand-built + // result) defaults to "isn't answering" — the safe interpretation. + switch reach.Reach { + case doctor.ReachError: + connected = healthLine{doctor.StatusFail, + "Not connected — couldn't read your secure environment.", + fmt.Sprintf("Email support@tracebloc.io with the output of `%s doctor --diagnose`.", launcher())} + default: // ReachUnreachable + connected = healthLine{doctor.StatusFail, + "Not connected — your secure environment isn't answering.", + reach.Remedy} + } + case tok == tokenUnreachable: + // WhoAmI is the definitive "can this machine reach tracebloc" signal, so it + // alone drives this line. The "Backend egress (from this machine)" check is + // explicitly indicative-not-definitive (it probes the cluster's configured + // host from here, not the cluster's real egress path), so it must NOT flip + // Connected to a network error after WhoAmI already succeeded — that would + // contradict the successful session probe. It stays a --verbose diagnostic. + connected = healthLine{doctor.StatusFail, + "Not connected — can't reach tracebloc from here.", + fmt.Sprintf("Check your network / HTTP(S)_PROXY, then run `%s doctor` again.", launcher())} + case tok == tokenServerErr: + connected = healthLine{doctor.StatusFail, + "Not connected — tracebloc didn't confirm your session (server error).", + fmt.Sprintf("Try again shortly; if it persists, email support@tracebloc.io with `%s doctor --diagnose`.", launcher())} + case by["Service Bus egress (requests-proxy)"].Status == doctor.StatusFail: + connected = healthLine{doctor.StatusFail, + "Training results can't reach tracebloc — experiments will stall.", + fmt.Sprintf("Email support@tracebloc.io with the output of `%s doctor --diagnose`.", launcher())} + default: + connected = healthLine{doctor.StatusOK, "Connected to tracebloc", ""} + } + + // Readiness only means something once we're connected: a healthy local + // cluster still can't run training while it can't reach tracebloc. If + // Connected is anything but OK, degrade honestly — never a green ✔ under a ✖. + if connected.status != doctor.StatusOK { + ready = healthLine{doctor.StatusUnknown, "Ready to run training — can't check yet", ""} + return connected, ready + } + switch { + case by["Pod health"].Status == doctor.StatusFail: + ready = healthLine{doctor.StatusFail, + "Not ready — part of your secure environment isn't running.", + fmt.Sprintf("Reinstall with `%s`, or email support@tracebloc.io with `%s doctor --diagnose`.", installCmd, launcher())} + case by["Pod health"].Status == doctor.StatusWarn && strings.HasPrefix(by["Pod health"].Detail, "could not list pods"): + // checkPods returns StatusWarn for TWO different situations: pods stuck + // Pending (below) AND a failure to list pods at all (e.g. RBAC, doctor.go + // checkPods). For the latter we simply can't tell whether training can run, + // so report an honest can't-check — never the stuck-pending/compute remedy, + // which would misdiagnose a permissions problem (Bugbot). + ready = healthLine{doctor.StatusUnknown, + "Ready to run training — couldn't check your workloads (run with --verbose)", ""} + case by["Pod health"].Status == doctor.StatusWarn: + // Pods stuck Pending past the grace window (unschedulable / image can't + // pull) mean training can't actually schedule — so this is NOT ready, even + // though the granular Pod-health check rates it a softer ⚠. Without this, + // a stuck-pending environment rolled up to ✔ "Ready to run training" and + // the "Everything looks good" verdict (Bugbot). + ready = healthLine{doctor.StatusFail, + "Not ready — part of your secure environment can't start yet.", + fmt.Sprintf("Some pods are stuck starting — usually not enough free compute, or a training image that can't be pulled. Free some up in Docker Desktop → Resources, then re-run `%s doctor`; if it persists, email support@tracebloc.io with `%s doctor --diagnose`.", launcher(), launcher())} + case by["Image pull secret"].Status == doctor.StatusFail: + ready = healthLine{doctor.StatusFail, + "Not ready — the training images can't be pulled.", + fmt.Sprintf("Email support@tracebloc.io with the output of `%s doctor --diagnose`.", launcher())} + case by["Dataset volume (PVC)"].Status == doctor.StatusFail: + ready = healthLine{doctor.StatusFail, + "Not ready — dataset storage isn't available.", + fmt.Sprintf("Email support@tracebloc.io with the output of `%s doctor --diagnose`.", launcher())} + case by["Node capacity"].Status == doctor.StatusFail: + ready = healthLine{doctor.StatusFail, + "Not ready — not enough free compute to start a training.", + "Free some up, or raise the machine's allocation in Docker Desktop → Resources."} + default: + ready = healthLine{doctor.StatusOK, "Ready to run training", ""} + } + return connected, ready +} + +// renderHealth prints one rolled-up line: ✔ for OK, ✖ + remedy for a problem, +// and a neutral · "can't check" for StatusUnknown (no false green, no alarm). +func renderHealth(p *ui.Printer, h healthLine) { + switch h.status { + case doctor.StatusOK: + p.Successf("%s", h.text) + case doctor.StatusUnknown: + p.Infof("%s", h.text) + default: + p.Errorf("%s", h.text) + if h.remedy != "" { + p.Hintf(" %s", h.remedy) + } + } +} + +// renderDoctorDetails is the --verbose (and --diagnose) technical breakdown: +// kubeconfig + every granular check. This is the only place Kubernetes +// vocabulary appears. +// renderDetailsIfVerbose prints the --verbose technical breakdown when the flag +// is set and a cluster config was resolved. Called at every exit that has a +// resolved config — including the no-environment / clientset-error early exits — +// so `tb doctor --verbose` still yields the support-facing detail on the failure +// paths that most need it, not only on the healthy full run. +func renderDetailsIfVerbose(p *ui.Printer, resolved *cluster.ResolvedConfig, results []doctor.Result) { + if p.Verbose() && resolved != nil { + renderDoctorDetails(p, resolved, results) + } +} + +func renderDoctorDetails(p *ui.Printer, resolved *cluster.ResolvedConfig, results []doctor.Result) { + p.Newline() + p.Section("Details (for support)") p.Field("context", resolved.Context) p.Field("server", resolved.ServerURL) p.Field("namespace", resolved.Namespace) - - results := doctorRunFn(ctx, cs, doctor.Options{ - Namespace: resolved.Namespace, - ServerURL: resolved.ServerURL, - }) - - p.Section("Checks") for _, r := range results { + mark := "·" switch r.Status { case doctor.StatusOK: - p.Successf("%s — %s", r.Name, r.Detail) + mark = "OK " case doctor.StatusWarn: - p.Warnf("%s — %s", r.Name, r.Detail) - if r.Remedy != "" { - p.Hintf(" %s", r.Remedy) - } + mark = "WARN" case doctor.StatusFail: - p.Errorf("%s — %s", r.Name, r.Detail) - if r.Remedy != "" { - p.Hintf(" %s", r.Remedy) - } - case doctor.StatusUnknown: - // No signal: a neutral · line, no ✖/⚠ and no remedy — the one honest - // "Cluster reachable" ✖ above already carries the cause and the fix. - p.Infof("%s — %s", r.Name, r.Detail) + mark = "FAIL" + } + p.Detailf("%s %s — %s", mark, r.Name, r.Detail) + if r.Remedy != "" { + p.Detailf(" %s", r.Remedy) } } +} - p.Newline() - // Overall verdict folds in the auth section, so an auth ✖/⚠ counts even when - // the cluster itself is healthy. - switch worseStatus(authStatus, doctor.Worst(results)) { - case doctor.StatusFail: - p.Errorf("Problems found — fix the ✖ items above.") - p.Hintf("For deeper triage, send tracebloc a support bundle: ./install-k8s.sh --diagnose") - // Silent (err == nil): the per-check lines above already explained it, - // so main() shouldn't print a redundant "Error:" line. - return &exitError{code: exitChecksFailed, err: nil} - case doctor.StatusWarn: - p.Warnf("Completed with warnings — review the ⚠ items above.") - return nil +// writeDiagnoseBundle owns the support bundle (moved out of install-k8s.sh, which +// the user may not have on disk). It writes the redacted technical breakdown to a +// file the owner emails to support. +func writeDiagnoseBundle(p *ui.Printer, resolved *cluster.ResolvedConfig, results []doctor.Result, tok tokenState, connected, ready healthLine) error { + var buf bytes.Buffer + fmt.Fprintf(&buf, "tracebloc doctor — support bundle (%s)\n\n", time.Now().Format(time.RFC3339)) + bp := ui.New(&buf, ui.WithColor(false), ui.WithVerbose(true)) + // Session + rolled-up verdict first — the state support reads before the raw + // Kubernetes detail, and the reason each remedy sends them here. On an early + // exit (no environment, clientset error) the roll-up never ran, so record the + // session state and note the short-circuit instead of empty verdict lines. + bp.Detailf("session: %s", tokenLabel(tok)) + switch { + case connected.text != "": + bp.Detailf("connected: %s — %s", connected.status, connected.text) + bp.Detailf("ready: %s — %s", ready.status, ready.text) + case len(results) > 0: + // Probed, but exited before the two-line roll-up (e.g. no chart installed). + // The granular checks are written below, so don't claim we never probed. + bp.Detailf("outcome: early exit — no roll-up verdict (granular checks below)") default: - p.Successf("All checks passed — auth and cluster look healthy.") - return nil + bp.Detailf("outcome: early exit before the cluster was probed") } -} - -// runAuthChecks reports on the CLI's own auth/config state (~/.tracebloc): are -// we signed in, to which env, is an active client selected, and does the backend -// still accept the token. It's the half of `cluster doctor` that diagnoses a -// failed *provision* rather than a sick cluster (RFC-0001 §8.5). Returns the -// worst status seen so the caller can fold it into the overall verdict. -func runAuthChecks(ctx context.Context, p *ui.Printer) doctor.Status { - p.Section("Auth & config") - - cfg, err := config.Load() - if err != nil { - p.Errorf("Config — couldn't read the CLI config: %v", err) - p.Hintf(" check ~/.tracebloc/config.json, or run `tracebloc login` to recreate it") - return doctor.StatusFail + if resolved != nil { + renderDoctorDetails(bp, resolved, results) } - if !cfg.SignedIn() { - p.Errorf("Sign-in — not signed in") - p.Hintf(" run `tracebloc login` (add --env dev|stg|prod for a non-prod backend)") - return doctor.StatusFail + + name := fmt.Sprintf("tracebloc-doctor-%s.txt", time.Now().Format("20060102-150405")) + if werr := os.WriteFile(name, buf.Bytes(), 0o600); werr != nil { + p.Errorf("Couldn't write the support bundle: %v", werr) + return &exitError{code: exitLocalEnv, err: nil} } + p.Successf("Wrote a support bundle to ./%s", name) + p.Hintf(" Email it to support@tracebloc.io.") + return nil +} - env := cfg.CurrentEnv - prof := cfg.Current() - if prof.Email != "" { - p.Successf("Sign-in — signed in to %s as %s", env, prof.Email) - } else { - p.Successf("Sign-in — signed in to %s", env) +// launcher resolves the command name to print in remedies: "tb" on a real +// install (the alias is beside the CLI), else the invoked name — same rule the +// home screen uses, so copy-paste always works. +func launcher() string { + if tbAliasAvailable() { + return binTB } + return invokedName() +} - worst := doctor.StatusOK - if prof.ActiveClientID == "" { - p.Warnf("Active client — none selected for %s", env) - p.Hintf(" run `tracebloc client create` (or re-run the installer) to set the client this machine enrolls as") - worst = doctor.StatusWarn - } else { - p.Successf("Active client — %s", prof.ActiveClientID) +// envDisplayName is the secure environment's user-facing handle — the namespace +// slug (RFC-0001 §7: the slug IS the handle). +func envDisplayName(r *cluster.ResolvedConfig) string { + if r != nil && r.Namespace != "" { + return r.Namespace } + return "your secure environment" +} - // Live token check. Best-effort: an explicit 401 is a failure (expired / - // revoked → must re-login); a network/proxy error is only a warning, since - // we can't conclude the token itself is bad. - p.Detailf("verifying the token against %s …", api.BaseURL(env)) - client := newAPIClient(env) - client.Token = prof.Token - if _, werr := client.WhoAmI(ctx); werr != nil { - var ae *api.APIError - var ue *api.UpgradeRequiredError - switch { - case errors.As(werr, &ae) && ae.StatusCode == http.StatusUnauthorized: - p.Errorf("Backend auth — %s rejected the token (401)", api.BaseURL(env)) - p.Hintf(" your session expired or was revoked — run `tracebloc login`") - return doctor.StatusFail - case errors.As(werr, &ue): - // 426: the server enforces a newer CLI. That's a hard, actionable - // failure ("upgrade"), not a transient "couldn't verify" warning. - p.Errorf("Backend auth — this CLI is too old for %s (HTTP 426)", api.BaseURL(env)) - p.Hintf(" %s", ue.Error()) - return doctor.StatusFail - default: - p.Warnf("Backend auth — couldn't verify the token: %v", werr) - p.Hintf(" the backend may be unreachable from here — check your network / HTTP(S)_PROXY") - return worseStatus(worst, doctor.StatusWarn) +// reachStateOf returns the "Cluster reachable" check's classification, so the +// caller can tell a reachable-but-uninstalled cluster (ReachNoEnv) apart from one +// that simply isn't answering. ReachOK when the check is absent. +func reachStateOf(results []doctor.Result) doctor.ReachState { + for _, r := range results { + if r.Name == "Cluster reachable" { + return r.Reach } } - p.Successf("Backend auth — token valid at %s", api.BaseURL(env)) - return worst + return doctor.ReachOK } // worseStatus returns the more severe of two doctor statuses (Fail > Warn > OK). +// StatusUnknown carries no signal, so it never worsens the verdict. func worseStatus(a, b doctor.Status) doctor.Status { if a == doctor.StatusFail || b == doctor.StatusFail { return doctor.StatusFail @@ -254,12 +516,16 @@ func worseStatus(a, b doctor.Status) doctor.Status { return doctor.StatusOK } -// kubeconfigExitCode is 3 ("kubeconfig could not be loaded") unless the auth -// section also failed — then it escalates to 2 ("a check failed"), so a bad -// token isn't masked behind a kubeconfig-only exit code (Bugbot). -func kubeconfigExitCode(authStatus doctor.Status) int { - if authStatus == doctor.StatusFail { - return exitChecksFailed +// doctorVerdict decides the closing line from the two rolled-up health lines: +// fail (a real problem → exit 2), or allGood (BOTH genuinely OK → "everything +// looks good"). The key subtlety: allGood requires both to be StatusOK, NOT +// merely "not Fail" — a readiness we couldn't determine (StatusUnknown, e.g. a +// pod-list RBAC failure) must not be reported as good, even though worseStatus +// treats Unknown as non-worsening (Bugbot). When neither holds, the caller +// reports a partial "couldn't finish some checks" result (still exit 0). +func doctorVerdict(connected, ready doctor.Status) (fail, allGood bool) { + if worseStatus(connected, ready) == doctor.StatusFail { + return true, false } - return exitLocalEnv + return false, connected == doctor.StatusOK && ready == doctor.StatusOK } diff --git a/internal/cli/doctor_cluster_test.go b/internal/cli/doctor_cluster_test.go index 931e9656..d6d108cc 100644 --- a/internal/cli/doctor_cluster_test.go +++ b/internal/cli/doctor_cluster_test.go @@ -5,21 +5,25 @@ import ( "context" "errors" "net/http" + "os" + "path/filepath" "strings" "testing" + "time" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" + "github.com/tracebloc/cli/internal/api" "github.com/tracebloc/cli/internal/cluster" "github.com/tracebloc/cli/internal/config" "github.com/tracebloc/cli/internal/doctor" "github.com/tracebloc/cli/internal/ui" ) -// withDoctorRun seams doctorRunFn to return a fixed set of check results, so -// runClusterDoctor's render loop + verdict switch can be exercised with a -// controlled OK/Warn/Fail mix (the per-check logic is tested in internal/doctor). +// withDoctorRun seams doctorRunFn to return a fixed set of granular check +// results, so runClusterDoctor's roll-up + verdict can be exercised with a +// controlled mix (the per-check logic is tested in internal/doctor). func withDoctorRun(t *testing.T, results []doctor.Result) { t.Helper() orig := doctorRunFn @@ -29,13 +33,9 @@ func withDoctorRun(t *testing.T, results []doctor.Result) { } } -// runDoctorClusterHalf drives runClusterDoctor with the auth half stubbed HEALTHY -// (signed in + active client + a 200 WhoAmI), so the overall verdict reflects the -// cluster checks rather than the auth section, and the cluster reached through the -// seams with doctor.Run returning `results`. Before this PR the cluster half — the -// ✓/⚠/✖ render loop and the verdict switch — was reachable only via a real -// kubeconfig pointing at an unroutable server (dial failure), never with a -// controlled result mix. +// runDoctorClusterHalf drives runClusterDoctor with the identity half stubbed +// HEALTHY (signed in + a 200 WhoAmI) and the cluster reached through the seams +// with doctor.Run returning `results`, into a non-verbose printer. func runDoctorClusterHalf(t *testing.T, results []doctor.Result) (string, error) { t.Helper() t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) @@ -45,80 +45,116 @@ func runDoctorClusterHalf(t *testing.T, results []doctor.Result) (string, error) t.Fatal(err) } stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok → auth OK + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok → session healthy }) withClusterSeams(t, fake.NewSimpleClientset()) // cs only flows to doctorRunFn, which ignores it withDoctorRun(t, results) var buf bytes.Buffer - err := runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false)), "", "", "") + err := runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false)), "", "", "", false) return buf.String(), err } -// All three render arms fire (✓ detail, ⚠ + remedy hint, ✖ + remedy hint) and a -// single failing check drives the overall verdict to Fail → exit 2 with a silent -// error (the per-check lines already explained it) + the support-bundle hint. -func TestRunClusterDoctor_RendersAllStatusesAndFails(t *testing.T) { +// All healthy → the two plain lines + the "ready to run training" verdict, exit 0. +// The resolved namespace is printed as the secure-environment name (the seam +// resolves it to "default"). +func TestRunClusterDoctor_AllHealthy(t *testing.T) { + out, err := runDoctorClusterHalf(t, []doctor.Result{ + {Name: "Cluster reachable", Status: doctor.StatusOK, Detail: "reachable"}, + {Name: "Backend egress (from this machine)", Status: doctor.StatusOK, Detail: "reachable"}, + }) + if err != nil { + t.Fatalf("all healthy → want nil error (exit 0), got %v", err) + } + for _, want := range []string{ + "Signed in as a@b.io", + `Secure environment "default"`, + "Connected to tracebloc", + "Ready to run training", + "Everything looks good", + } { + if !strings.Contains(out, want) { + t.Errorf("healthy view missing %q:\n%s", want, out) + } + } + // No Kubernetes vocabulary leaks into the default view. + for _, banned := range []string{"Kubeconfig", "context", "PVC", "kubectl", "requests-proxy", "Pending"} { + if strings.Contains(out, banned) { + t.Errorf("leaked k8s term %q in default view:\n%s", banned, out) + } + } +} + +// A failing cluster check rolls up into a plain ✖ line + concrete remedy, drives +// the verdict to Fail (exit 2, silent error), and points at the support bundle. +func TestRunClusterDoctor_FailRollsUp(t *testing.T) { out, err := runDoctorClusterHalf(t, []doctor.Result{ - {Name: "Cluster reachable", Status: doctor.StatusOK, Detail: "api answered"}, - {Name: "Pod health", Status: doctor.StatusWarn, Detail: "1 pod restarted", Remedy: "check pod logs"}, - {Name: "Dataset volume", Status: doctor.StatusFail, Detail: "PVC not bound", Remedy: "provision the shared PVC"}, + {Name: "Cluster reachable", Status: doctor.StatusOK, Detail: "reachable"}, + {Name: "Dataset volume (PVC)", Status: doctor.StatusFail, Detail: "PVC not bound"}, }) var ee *exitError if !errors.As(err, &ee) || ee.Code() != 2 { t.Fatalf("a failing check → want exit 2, got %v", err) } if ee.err != nil { - t.Errorf("the Fail verdict must be a silent exitError (per-check lines already printed), got err=%v", ee.err) + t.Errorf("Fail verdict must be a silent exitError (lines already printed), got err=%v", ee.err) } - for _, want := range []string{ - "Cluster reachable", "api answered", // OK arm - "Pod health", "1 pod restarted", "check pod logs", // Warn arm + remedy hint - "Dataset volume", "PVC not bound", "provision the shared PVC", // Fail arm + remedy hint - "Problems found", "support bundle", // Fail verdict + its hint - } { + for _, want := range []string{"Not ready", "dataset storage", "--diagnose"} { if !strings.Contains(out, want) { - t.Errorf("output missing %q:\n%s", want, out) + t.Errorf("failed view missing %q:\n%s", want, out) } } + // The raw k8s detail ("PVC not bound") must NOT appear in the default view. + if strings.Contains(out, "PVC not bound") { + t.Errorf("raw k8s detail leaked into the default view:\n%s", out) + } } -// Warnings but no failure → overall Warn → exit 0 (nil error) with the -// "completed with warnings" verdict. -func TestRunClusterDoctor_WarnVerdict(t *testing.T) { +// A Warn-level granular check (e.g. a proxy note) is not user-actionable and must +// NOT fail the verdict — it degrades to --verbose. Default view stays healthy. +func TestRunClusterDoctor_WarnsDontFail(t *testing.T) { out, err := runDoctorClusterHalf(t, []doctor.Result{ - {Name: "Cluster reachable", Status: doctor.StatusOK, Detail: "ok"}, - {Name: "Proxy configuration", Status: doctor.StatusWarn, Detail: "not fully wired", Remedy: "set the proxy env"}, + {Name: "Cluster reachable", Status: doctor.StatusOK, Detail: "reachable"}, + {Name: "Proxy configuration", Status: doctor.StatusWarn, Detail: "not fully wired"}, }) if err != nil { t.Fatalf("warnings-only → want nil error (exit 0), got %v", err) } - if !strings.Contains(out, "Completed with warnings") { - t.Errorf("want the warnings verdict:\n%s", out) + if !strings.Contains(out, "Everything looks good") { + t.Errorf("want the healthy verdict (warns are verbose-only):\n%s", out) } } -// All checks pass (and auth is healthy) → overall OK → exit 0 with the all-healthy -// verdict. Also pins that the resolved namespace is printed in the Kubeconfig -// section (the seam resolves it to "default"). -func TestRunClusterDoctor_AllHealthy(t *testing.T) { - out, err := runDoctorClusterHalf(t, []doctor.Result{ - {Name: "Cluster reachable", Status: doctor.StatusOK, Detail: "api answered, client installed"}, - {Name: "Backend egress", Status: doctor.StatusOK, Detail: "reachable"}, - }) - if err != nil { - t.Fatalf("all healthy → want nil error (exit 0), got %v", err) +// --verbose surfaces the Kubernetes detail (kubeconfig + granular checks) under +// the plain summary — the one place that vocabulary is allowed. +func TestRunClusterDoctor_VerboseShowsDetails(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) } - if !strings.Contains(out, "All checks passed") { - t.Errorf("want the all-healthy verdict:\n%s", out) + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) + }) + withClusterSeams(t, fake.NewSimpleClientset()) + withDoctorRun(t, []doctor.Result{ + {Name: "Cluster reachable", Status: doctor.StatusOK, Detail: "reachable"}, + {Name: "Service Bus egress (requests-proxy)", Status: doctor.StatusOK, Detail: "ready"}, + }) + var buf bytes.Buffer + if err := runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false), ui.WithVerbose(true)), "", "", "", false); err != nil { + t.Fatalf("verbose healthy → want nil error, got %v", err) } - if !strings.Contains(out, "default") { - t.Errorf("want the resolved namespace printed in the Kubeconfig section:\n%s", out) + for _, want := range []string{"Details (for support)", "Service Bus egress (requests-proxy)"} { + if !strings.Contains(buf.String(), want) { + t.Errorf("verbose view missing %q:\n%s", want, buf.String()) + } } } // The clientset-build arm: loadClusterFn succeeds but newClientsetFn fails. With -// the auth half healthy, this keeps the documented exit-3 contract (a kubeconfig- -// class failure) and surfaces the underlying error. +// the session healthy, this is a local-environment problem (exit 3) framed +// plainly (no raw Kubernetes error in the default view). func TestRunClusterDoctor_ClientsetError(t *testing.T) { t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ @@ -127,7 +163,7 @@ func TestRunClusterDoctor_ClientsetError(t *testing.T) { t.Fatal(err) } stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // auth OK + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) }) origLoad, origCS := loadClusterFn, newClientsetFn t.Cleanup(func() { loadClusterFn, newClientsetFn = origLoad, origCS }) @@ -138,12 +174,220 @@ func TestRunClusterDoctor_ClientsetError(t *testing.T) { return nil, errors.New("bad rest config") } var buf bytes.Buffer - err := runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false)), "", "", "") + err := runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false)), "", "", "", false) var ee *exitError if !errors.As(err, &ee) || ee.Code() != 3 { - t.Fatalf("clientset build error + auth-OK → want exit 3, got %v", err) + t.Fatalf("clientset build error + healthy session → want exit 3, got %v", err) + } + if !strings.Contains(buf.String(), "Couldn't connect to your secure environment") { + t.Errorf("want the plain connect-failure message:\n%s", buf.String()) + } +} + +// --diagnose writes a support bundle that records the session + rolled-up verdict +// (not just k8s detail) and still exits on the REAL verdict — never a misleading +// 0 when checks failed (Bugbot #365). +func TestRunClusterDoctor_DiagnoseBundle(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // session healthy + }) + withClusterSeams(t, fake.NewSimpleClientset()) + // A failing check → the verdict is a problem, so exit must be 2, not a bare 0. + withDoctorRun(t, []doctor.Result{ + {Name: "Cluster reachable", Status: doctor.StatusOK}, + {Name: "Pod health", Status: doctor.StatusFail, Detail: "a pod is CrashLoopBackOff"}, + }) + + // writeDiagnoseBundle writes into the CWD — run it inside a temp dir. + tmp := t.TempDir() + orig, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(tmp); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(orig) }) + + var buf bytes.Buffer + err = runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false)), "", "", "", true) + + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("diagnose with a failing check → want exit 2 (real verdict), got %v", err) + } + if !strings.Contains(buf.String(), "Wrote a support bundle") { + t.Errorf("want a 'wrote bundle' confirmation, got:\n%s", buf.String()) + } + + // The bundle exists and records the session + verdict, not just the k8s detail. + entries, _ := os.ReadDir(tmp) + var bundle string + for _, e := range entries { + if strings.HasPrefix(e.Name(), "tracebloc-doctor-") { + bundle = filepath.Join(tmp, e.Name()) + } + } + if bundle == "" { + t.Fatalf("no support bundle written to %s (entries: %v)", tmp, entries) + } + b, err := os.ReadFile(bundle) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"session:", "connected:", "ready:", "Pod health"} { + if !strings.Contains(string(b), want) { + t.Errorf("bundle missing %q; got:\n%s", want, string(b)) + } + } +} + +// --diagnose must still leave a bundle when the run exits EARLY (here a clientset +// failure) — that's exactly when a remedy tells the user to run it. The early-exit +// code (3) is preserved: the bundle write doesn't mask it (Bugbot #365). +func TestRunClusterDoctor_DiagnoseOnEarlyExit(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // session healthy + }) + origLoad, origCS := loadClusterFn, newClientsetFn + t.Cleanup(func() { loadClusterFn, newClientsetFn = origLoad, origCS }) + loadClusterFn = func(cluster.KubeconfigOptions) (*cluster.ResolvedConfig, error) { + return &cluster.ResolvedConfig{Namespace: "default", Context: "test-ctx"}, nil + } + newClientsetFn = func(*cluster.ResolvedConfig) (kubernetes.Interface, error) { + return nil, errors.New("bad rest config") + } + + tmp := t.TempDir() + orig, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(tmp); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(orig) }) + + var buf bytes.Buffer + err = runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false)), "", "", "", true) + + // The early-exit code survives the bundle write (not masked to 0 or the write code). + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 3 { + t.Fatalf("clientset-fail + --diagnose → want exit 3 preserved, got %v", err) + } + if !strings.Contains(buf.String(), "Couldn't connect to your secure environment") { + t.Errorf("want the connect-failure message, got:\n%s", buf.String()) + } + if !strings.Contains(buf.String(), "Wrote a support bundle") { + t.Errorf("--diagnose on an early exit should still write a bundle, got:\n%s", buf.String()) + } + entries, _ := os.ReadDir(tmp) + found := false + for _, e := range entries { + if strings.HasPrefix(e.Name(), "tracebloc-doctor-") { + found = true + } + } + if !found { + t.Errorf("no support bundle written on early exit (entries: %v)", entries) + } +} + +// A reachable cluster with no tracebloc chart (ReachNoEnv) must not print +// `Secure environment ""` — the Connected line says none is installed, so +// naming one would assert it both exists and doesn't (Bugbot #365). +func TestRunClusterDoctor_NoEnvSuppressesEnvHeader(t *testing.T) { + out, err := runDoctorClusterHalf(t, []doctor.Result{ + {Name: "Cluster reachable", Status: doctor.StatusFail, Reach: doctor.ReachNoEnv}, + }) + if !strings.Contains(out, "No secure environment on this machine yet") { + t.Errorf("want the unified no-environment message, got:\n%s", out) + } + if strings.Contains(out, `Secure environment "`) { + t.Errorf("must not name a secure environment when none is installed, got:\n%s", out) + } + if strings.Contains(out, "kubectl") { + t.Errorf("no-env output must not leak kubectl, got:\n%s", out) + } + // Healthy session → a local-env setup problem (exit 3), consistent with a + // missing kubeconfig. + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 3 { + t.Fatalf("ReachNoEnv (healthy session) → want exit 3, got %v", err) + } +} + +// A session fault alongside no-environment must be surfaced (not hidden) and must +// dominate the exit code (2), matching the other no-env exits (Bugbot #365). +func TestRunClusterDoctor_NoEnvSurfacesSessionFault(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + // WhoAmI at a closed port → transport error → tokenUnreachable. + origAPI := newAPIClient + t.Cleanup(func() { newAPIClient = origAPI }) + newAPIClient = func(string) *api.Client { + return &api.Client{BaseURL: "http://127.0.0.1:1", HTTP: &http.Client{Timeout: 2 * time.Second}} + } + withClusterSeams(t, fake.NewSimpleClientset()) + withDoctorRun(t, []doctor.Result{ + {Name: "Cluster reachable", Status: doctor.StatusFail, Reach: doctor.ReachNoEnv}, + }) + + var buf bytes.Buffer + err := runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false)), "", "", "", false) + if !strings.Contains(buf.String(), "Can't reach tracebloc from here") { + t.Errorf("session fault must be surfaced on no-env, got:\n%s", buf.String()) + } + if !strings.Contains(buf.String(), "No secure environment on this machine yet") { + t.Errorf("want the no-environment message too, got:\n%s", buf.String()) + } + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("session fault + no-env → want exit 2 (fault dominates), got %v", err) + } +} + +// --verbose must still print the support-facing Details on failure paths that +// have a resolved config (here ReachNoEnv), not only on a healthy full run — the +// early exit returns before the main render (Bugbot #365). +func TestRunClusterDoctor_VerboseDetailsOnNoEnv(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // session healthy + }) + withClusterSeams(t, fake.NewSimpleClientset()) + withDoctorRun(t, []doctor.Result{ + {Name: "Cluster reachable", Status: doctor.StatusFail, Reach: doctor.ReachNoEnv, Detail: "no tracebloc client found"}, + }) + + var buf bytes.Buffer + _ = runClusterDoctor(context.Background(), ui.New(&buf, ui.WithColor(false), ui.WithVerbose(true)), "", "", "", false) + if !strings.Contains(buf.String(), "Details (for support)") { + t.Errorf("--verbose on ReachNoEnv should still print the Details section, got:\n%s", buf.String()) } - if !strings.Contains(buf.String(), "bad rest config") { - t.Errorf("want the clientset error surfaced in the Cluster section:\n%s", buf.String()) + if !strings.Contains(buf.String(), "Cluster reachable") { + t.Errorf("--verbose Details should include the granular checks, got:\n%s", buf.String()) } } diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go index 880ec93a..2ef1917f 100644 --- a/internal/cli/doctor_test.go +++ b/internal/cli/doctor_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/tracebloc/cli/internal/api" + "github.com/tracebloc/cli/internal/cluster" "github.com/tracebloc/cli/internal/config" "github.com/tracebloc/cli/internal/doctor" "github.com/tracebloc/cli/internal/ui" @@ -28,149 +29,165 @@ func stubBackend(t *testing.T, h http.HandlerFunc) { t.Cleanup(func() { newAPIClient = orig }) } -// cli#101: `cluster doctor` auth/config/token checks (RFC-0001 §8.5). These pin -// runAuthChecks — the half of doctor that diagnoses a failed *provision*. - -func TestRunAuthChecks_NotSignedIn(t *testing.T) { - t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - var out bytes.Buffer - if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { - t.Errorf("not signed in → want Fail, got %v", st) - } - if !strings.Contains(out.String(), "Auth & config") || !strings.Contains(out.String(), "not signed in") { - t.Errorf("missing auth section / not-signed-in line:\n%s", out.String()) - } -} - -func TestRunAuthChecks_TokenValid(t *testing.T) { +// signedInConfig writes a signed-in config with an active client for the tests +// that need to get past the identity gate. +func signedInConfig(t *testing.T) { + t.Helper() t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, }}).Save(); err != nil { t.Fatal(err) } - stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) - }) +} + +// ── Identity / session gate (runs before any cluster I/O) ── + +func TestDoctor_NotSignedIn(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) var out bytes.Buffer - if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusOK { - t.Errorf("valid token + active client → want OK, got %v;\n%s", st, out.String()) + err := runClusterDoctor(context.Background(), ui.New(&out, ui.WithColor(false)), "", "", "", false) + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("not signed in → want exit 2, got %v", err) } - if !strings.Contains(out.String(), "token valid") { - t.Errorf("missing token-valid line:\n%s", out.String()) + if !strings.Contains(out.String(), "Not signed in") || !strings.Contains(out.String(), "login") { + t.Errorf("want a plain 'Not signed in — run ... login', got:\n%s", out.String()) } } -func TestRunAuthChecks_TokenRejected401(t *testing.T) { - t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ - "dev": {Token: "x", ActiveClientID: "5"}, - }}).Save(); err != nil { - t.Fatal(err) - } +func TestDoctor_SessionExpired401(t *testing.T) { + signedInConfig(t) stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusUnauthorized) _, _ = w.Write([]byte(`{"detail":"Invalid token."}`)) }) var out bytes.Buffer - if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { - t.Errorf("token rejected (401) → want Fail, got %v", st) + err := runClusterDoctor(context.Background(), ui.New(&out, ui.WithColor(false)), "", "", "", false) + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("401 → want exit 2, got %v", err) } - if !strings.Contains(out.String(), "rejected the token (401)") { - t.Errorf("missing 401 line:\n%s", out.String()) + if !strings.Contains(out.String(), "session expired") || !strings.Contains(out.String(), "login") { + t.Errorf("want 'Your session expired — run ... login', got:\n%s", out.String()) } } -func TestRunAuthChecks_NoActiveClientWarns(t *testing.T) { - t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ - "dev": {Token: "x"}, // signed in, but no active client selected - }}).Save(); err != nil { - t.Fatal(err) - } +func TestDoctor_OutOfDate426(t *testing.T) { + signedInConfig(t) stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) + w.WriteHeader(http.StatusUpgradeRequired) // 426 + _, _ = w.Write([]byte(`{"error":"upgrade_required","min_version":"0.9.0"}`)) }) var out bytes.Buffer - if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusWarn { - t.Errorf("valid token but no active client → want Warn, got %v;\n%s", st, out.String()) + err := runClusterDoctor(context.Background(), ui.New(&out, ui.WithColor(false)), "", "", "", false) + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("426 → want exit 2, got %v", err) } - if !strings.Contains(out.String(), "Active client — none") { - t.Errorf("missing no-active-client warning:\n%s", out.String()) + if !strings.Contains(out.String(), "out of date") || !strings.Contains(out.String(), "tracebloc.io/i.sh") { + t.Errorf("want 'This CLI is out of date — update it: ', got:\n%s", out.String()) } } -// TestClusterDoctor_KubeconfigFailEscalatesWhenAuthFails pins the Bugbot fix: a -// kubeconfig load failure normally exits 3, but if the auth section ALSO failed -// (here: not signed in) it escalates to 2 so a bad token isn't masked as a -// kubeconfig-only problem. -func TestClusterDoctor_KubeconfigFailEscalatesWhenAuthFails(t *testing.T) { - t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // not signed in → auth Fail +// A 401 hard stop with --diagnose must NOT write a bundle: the fix is `login` +// (no bundle needed), and one written here would falsely record "session: +// confirmed" for an expired session. The defer is registered after the session +// probe precisely so 401/426 return first (Bugbot #365). +func TestDoctor_DiagnoseNotWrittenOnExpiredSession(t *testing.T) { + signedInConfig(t) + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"detail":"Invalid token."}`)) + }) + tmp := t.TempDir() + orig, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(tmp); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(orig) }) + var out bytes.Buffer - err := runClusterDoctor(context.Background(), ui.New(&out), "/nonexistent-kubeconfig-xyz", "", "") + err = runClusterDoctor(context.Background(), ui.New(&out, ui.WithColor(false)), "", "", "", true) var ee *exitError if !errors.As(err, &ee) || ee.Code() != 2 { - t.Fatalf("kubeconfig-fail + auth-fail → want exit 2, got %v", err) + t.Fatalf("401 + --diagnose → want exit 2, got %v", err) + } + if strings.Contains(out.String(), "Wrote a support bundle") { + t.Errorf("must not write a bundle on an expired session, got:\n%s", out.String()) + } + entries, _ := os.ReadDir(tmp) + for _, e := range entries { + if strings.HasPrefix(e.Name(), "tracebloc-doctor-") { + t.Errorf("a bundle file was written on 401 (%s) — should be none", e.Name()) + } } } -// TestClusterDoctor_KubeconfigFailStays3WhenAuthOK: with auth healthy, a -// kubeconfig failure keeps the documented exit-3 contract. -func TestClusterDoctor_KubeconfigFailStays3WhenAuthOK(t *testing.T) { - t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ - "dev": {Token: "x", ActiveClientID: "5"}, - }}).Save(); err != nil { - t.Fatal(err) +// A session fault (here a transport error → tokenUnreachable) that coincides with +// a local-env failure must surface as exit 2 ("a problem was found"), matching the +// full-probe path — not be masked as the local-env exit 3 (Bugbot #365). +func TestDoctor_SessionFaultDominatesEarlyExit(t *testing.T) { + signedInConfig(t) + // Point the API client at a closed port so WhoAmI is a transport error → + // tokenUnreachable (not an APIError). + origAPI := newAPIClient + t.Cleanup(func() { newAPIClient = origAPI }) + newAPIClient = func(string) *api.Client { + return &api.Client{BaseURL: "http://127.0.0.1:1", HTTP: &http.Client{Timeout: 2 * time.Second}} } - stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok → auth OK - }) + // No environment on this machine. + origLoad := loadClusterFn + t.Cleanup(func() { loadClusterFn = origLoad }) + loadClusterFn = func(cluster.KubeconfigOptions) (*cluster.ResolvedConfig, error) { + return nil, errors.New("no kubeconfig here") + } + var out bytes.Buffer - err := runClusterDoctor(context.Background(), ui.New(&out), "/nonexistent-kubeconfig-xyz", "", "") + err := runClusterDoctor(context.Background(), ui.New(&out, ui.WithColor(false)), "", "", "", false) var ee *exitError - if !errors.As(err, &ee) || ee.Code() != 3 { - t.Fatalf("kubeconfig-fail + auth-OK → want exit 3 (contract), got %v", err) + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("session fault + no-env → want exit 2 (problem found), got %v", err) + } + if !strings.Contains(out.String(), "No secure environment") { + t.Errorf("want the no-environment line, got:\n%s", out.String()) + } + if !strings.Contains(out.String(), "Can't reach tracebloc from here") && + !strings.Contains(out.String(), "didn't confirm your session") { + t.Errorf("want the session fault surfaced alongside no-environment, got:\n%s", out.String()) } } -// TestRunAuthChecks_426IsHardFailure pins the Bugbot fix: a 426 (server enforces -// a newer CLI) from the live token check is a hard "upgrade" failure, not a -// transient "couldn't verify — check your network" warning. -func TestRunAuthChecks_426IsHardFailure(t *testing.T) { - t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) - if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ - "dev": {Token: "x", ActiveClientID: "5"}, - }}).Save(); err != nil { - t.Fatal(err) - } +// A bad kubeconfig, with auth healthy, is a local-environment problem (exit 3) +// framed as "no secure environment here yet" — not a Kubernetes error dump. +func TestDoctor_KubeconfigFailIsLocalEnv(t *testing.T) { + signedInConfig(t) stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusUpgradeRequired) // 426 - _, _ = w.Write([]byte(`{"error":"upgrade_required","min_version":"0.9.0"}`)) + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok }) var out bytes.Buffer - if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { - t.Errorf("426 from the token check → want Fail (not a transient Warn), got %v", st) + err := runClusterDoctor(context.Background(), ui.New(&out, ui.WithColor(false)), "/nonexistent-kubeconfig-xyz", "", "", false) + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 3 { + t.Fatalf("kubeconfig-fail + auth-OK → want exit 3, got %v", err) } - if !strings.Contains(out.String(), "too old") || !strings.Contains(out.String(), "426") { - t.Errorf("426 should report a clear 'too old / upgrade' failure, got:\n%s", out.String()) + if !strings.Contains(out.String(), "No secure environment") { + t.Errorf("want the plain no-environment message, got:\n%s", out.String()) } } -// TestClusterDoctor_BindsActiveClientNamespace pins the review fix: with no -// --namespace/--context, doctor must target the active client's cached namespace -// (like `cluster info` and the home screen), not the kubeconfig default — else -// the home screen and doctor can disagree about the same install. The server is -// unroutable, so discovery falls back to the bound namespace; we assert doctor -// reports THAT namespace. Mutation-proven: drop bindActiveClientNamespace and the -// namespace becomes the kubeconfig default, failing this assertion. -func TestClusterDoctor_BindsActiveClientNamespace(t *testing.T) { +// With no --namespace/--context, doctor targets the active client's cached +// namespace (like `cluster info` + the home screen) and prints it as the +// secure-environment name. Mutation-proven: drop bindActiveClientNamespace and +// the printed name becomes the kubeconfig default. +func TestDoctor_BindsActiveClientNamespace(t *testing.T) { writeActiveClientConfig(t, "munich-radiology", "Munich Radiology") stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok → auth healthy + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok }) - // Valid kubeconfig at an unroutable TEST-NET address: loads fine, so the - // namespace resolves from the binding; the later cluster dial just fails. const kubeconfig = `apiVersion: v1 kind: Config clusters: @@ -188,14 +205,215 @@ users: if err := os.WriteFile(kc, []byte(kubeconfig), 0o600); err != nil { t.Fatal(err) } - // Bound the context so doctor.Run's dial to the unroutable server can't hang - // the test (and CI) — we only assert the namespace resolved from the binding, - // which is printed before any cluster I/O. - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() var out bytes.Buffer - _ = runClusterDoctor(ctx, ui.New(&out), kc, "", "") // no ns/context override + _ = runClusterDoctor(ctx, ui.New(&out, ui.WithColor(false)), kc, "", "", false) if !strings.Contains(out.String(), "munich-radiology") { - t.Fatalf("doctor should target the active client's namespace (munich-radiology), got:\n%s", out.String()) + t.Fatalf("doctor should name the active client's environment (munich-radiology), got:\n%s", out.String()) + } +} + +// ── summarizeDoctor: the roll-up of the granular checks into two plain lines ── + +func TestSummarizeDoctor(t *testing.T) { + res := func(name string, s doctor.Status) doctor.Result { return doctor.Result{Name: name, Status: s} } + allOK := []doctor.Result{ + res("Cluster reachable", doctor.StatusOK), + res("Backend egress (from this machine)", doctor.StatusOK), + res("Service Bus egress (requests-proxy)", doctor.StatusOK), + res("Pod health", doctor.StatusOK), + res("Dataset volume (PVC)", doctor.StatusOK), + res("Node capacity", doctor.StatusOK), + } + with := func(base []doctor.Result, name string, s doctor.Status) []doctor.Result { + out := make([]doctor.Result, len(base)) + copy(out, base) + for i := range out { + if out[i].Name == name { + out[i].Status = s + } + } + return out + } + + t.Run("all healthy → both OK", func(t *testing.T) { + c, r := summarizeDoctor(allOK, tokenOK) + if c.status != doctor.StatusOK || r.status != doctor.StatusOK { + t.Fatalf("want both OK, got connected=%v ready=%v", c.status, r.status) + } + if c.text != "Connected to tracebloc" || r.text != "Ready to run training" { + t.Errorf("unexpected healthy text: %q / %q", c.text, r.text) + } + }) + + t.Run("unreachable → connected Fail, ready can't-check", func(t *testing.T) { + c, r := summarizeDoctor(with(allOK, "Cluster reachable", doctor.StatusFail), tokenOK) + if c.status != doctor.StatusFail { + t.Errorf("connected should Fail when unreachable, got %v", c.status) + } + if r.status != doctor.StatusUnknown { + t.Errorf("ready should be Unknown (can't check) when unreachable, got %v", r.status) + } + if !strings.Contains(r.text, "can't check") { + t.Errorf("ready text should say can't check, got %q", r.text) + } + }) + + t.Run("token unreachable → connected Fail", func(t *testing.T) { + c, _ := summarizeDoctor(allOK, tokenUnreachable) + if c.status != doctor.StatusFail || !strings.Contains(c.text, "can't reach tracebloc") { + t.Errorf("token-unreachable → want connected Fail 'can't reach tracebloc', got %v %q", c.status, c.text) + } + }) + + t.Run("results egress down → connected Fail (experiments stall)", func(t *testing.T) { + c, _ := summarizeDoctor(with(allOK, "Service Bus egress (requests-proxy)", doctor.StatusFail), tokenOK) + if c.status != doctor.StatusFail || !strings.Contains(c.text, "results can't reach") { + t.Errorf("want connected Fail on results-egress down, got %v %q", c.status, c.text) + } + }) + + t.Run("no compute → ready Fail", func(t *testing.T) { + _, r := summarizeDoctor(with(allOK, "Node capacity", doctor.StatusFail), tokenOK) + if r.status != doctor.StatusFail || !strings.Contains(r.remedy, "Docker Desktop") { + t.Errorf("want ready Fail with a raise-allocation remedy, got %v remedy=%q", r.status, r.remedy) + } + }) + + t.Run("component down → ready Fail (reinstall/support)", func(t *testing.T) { + _, r := summarizeDoctor(with(allOK, "Pod health", doctor.StatusFail), tokenOK) + if r.status != doctor.StatusFail || !strings.Contains(r.remedy, "tracebloc.io/i.sh") { + t.Errorf("want ready Fail with a reinstall remedy, got %v remedy=%q", r.status, r.remedy) + } + }) + + // Pod health has TWO StatusWarn sources (checkPods): pods stuck Pending, and a + // failure to list pods at all (e.g. RBAC). They must roll up differently. + warnPods := func(detail string) []doctor.Result { + out := with(allOK, "Pod health", doctor.StatusWarn) + for i := range out { + if out[i].Name == "Pod health" { + out[i].Detail = detail + } + } + return out + } + + // Pods stuck Pending past the grace window: training can't schedule, so the + // rollup must NOT report ✔ "Ready to run training" — that false green was the + // original Bugbot finding. + t.Run("pods stuck pending (warn) → ready Fail, not a false green", func(t *testing.T) { + _, r := summarizeDoctor(warnPods("Pending > 5m0s: [trainer-x]"), tokenOK) + if r.status != doctor.StatusFail { + t.Fatalf("stuck-pending pods must roll up to not-ready, got %v %q", r.status, r.text) + } + if !strings.Contains(r.text, "Not ready") { + t.Errorf("want a Not-ready readiness line for stuck-pending pods, got %q", r.text) + } + }) + + // The other Pod-health Warn source, "could not list pods" (RBAC), is a + // can't-check — it must NOT get the stuck-pending/compute (Docker Desktop) + // remedy (Bugbot follow-up). + t.Run("pod-health warn = could not list pods (RBAC) → can't-check, not stuck-pending", func(t *testing.T) { + _, r := summarizeDoctor(warnPods("could not list pods: pods is forbidden"), tokenOK) + if r.status == doctor.StatusFail { + t.Errorf("a can't-list-pods warn must not be a hard not-ready, got %v %q", r.status, r.text) + } + if strings.Contains(r.remedy, "Docker Desktop") { + t.Errorf("must not give the stuck-pending/compute remedy for a read failure, got remedy=%q", r.remedy) + } + }) + + // A reachable cluster with no tracebloc installed must NOT be reported as + // "isn't answering" with a kubectl remedy — it's a reinstall (Bugbot #365). + // A failing image-pull check means training images can't be fetched — that + // is not-ready, and must not be silently dropped from the rollup (Bugbot #365). + t.Run("images can't be pulled → ready Fail", func(t *testing.T) { + withPull := append(append([]doctor.Result{}, allOK...), res("Image pull secret", doctor.StatusFail)) + _, r := summarizeDoctor(withPull, tokenOK) + if r.status != doctor.StatusFail || !strings.Contains(r.text, "images can't be pulled") { + t.Errorf("image-pull down → want ready Fail 'images can't be pulled', got %v %q", r.status, r.text) + } + if !strings.Contains(r.remedy, "--diagnose") { + t.Errorf("image-pull remedy should point at --diagnose, got %q", r.remedy) + } + }) + + // A backend that ANSWERED with an error (5xx/403/decode) is a tracebloc-side + // problem — it must not be blamed on the user's network with a proxy remedy + // (Bugbot #365). + t.Run("backend answered with an error → connected Fail (support, not network)", func(t *testing.T) { + c, r := summarizeDoctor(allOK, tokenServerErr) + if c.status != doctor.StatusFail || !strings.Contains(c.text, "server error") { + t.Errorf("server-err → want connected Fail 'server error', got %v %q", c.status, c.text) + } + if strings.Contains(c.remedy, "PROXY") || strings.Contains(c.remedy, "network") { + t.Errorf("server-err remedy must not blame the network, got %q", c.remedy) + } + if !strings.Contains(c.remedy, "--diagnose") { + t.Errorf("server-err remedy should point at support/--diagnose, got %q", c.remedy) + } + if r.status != doctor.StatusUnknown { + t.Errorf("server-err → ready should be can't-check (Unknown), got %v", r.status) + } + }) + + // Disconnected but the local cluster is healthy: Ready must NOT show a green + // ✔ next to a Connected ✖ — training can't complete while disconnected + // (Bugbot #365). + t.Run("disconnected but cluster healthy → ready not a false check", func(t *testing.T) { + c, r := summarizeDoctor(with(allOK, "Service Bus egress (requests-proxy)", doctor.StatusFail), tokenOK) + if c.status != doctor.StatusFail { + t.Fatalf("precondition: want connected Fail (service bus down), got %v", c.status) + } + if r.status == doctor.StatusOK { + t.Errorf("ready must not be a green check while disconnected, got OK %q", r.text) + } + }) + + // The "Backend egress (from this machine)" probe is indicative-not-definitive; + // a miss must NOT contradict a successful WhoAmI by claiming the network is + // down. With a healthy session it stays a --verbose diagnostic (Bugbot #365). + t.Run("backend-egress miss + healthy session → connected stays OK", func(t *testing.T) { + c, _ := summarizeDoctor(with(allOK, "Backend egress (from this machine)", doctor.StatusFail), tokenOK) + if c.status != doctor.StatusOK { + t.Errorf("indicative backend-egress miss + healthy WhoAmI → want connected OK, got %v %q", c.status, c.text) + } + if strings.Contains(c.text, "can't reach tracebloc from here") { + t.Errorf("must not blame the network after a successful WhoAmI, got %q", c.text) + } + }) +} + +// ── doctorVerdict: the closing "everything looks good" / problem / partial call ── + +func TestDoctorVerdict(t *testing.T) { + ok, warn, fail, unknown := doctor.StatusOK, doctor.StatusWarn, doctor.StatusFail, doctor.StatusUnknown + cases := []struct { + name string + connected, ready doctor.Status + wantFail bool + wantAllGood bool + }{ + {"both OK → everything good", ok, ok, false, true}, + {"ready Fail → problem", ok, fail, true, false}, + {"connected Fail → problem", fail, ok, true, false}, + // The Bugbot case: connected but readiness couldn't be checked (RBAC → + // Unknown). Not a hard failure, but NOT "everything looks good". + {"connected + ready can't-check → neither", ok, unknown, false, false}, + // Not-connected already Fails via connected, regardless of ready=Unknown. + {"disconnected + ready unknown → problem", fail, unknown, true, false}, + {"a warn that isn't Fail → not everything-good", ok, warn, false, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gotFail, gotAllGood := doctorVerdict(tc.connected, tc.ready) + if gotFail != tc.wantFail || gotAllGood != tc.wantAllGood { + t.Errorf("doctorVerdict(%v,%v) = fail=%v allGood=%v, want fail=%v allGood=%v", + tc.connected, tc.ready, gotFail, gotAllGood, tc.wantFail, tc.wantAllGood) + } + }) } } diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go index 9cf2228e..f5949f82 100644 --- a/internal/cli/home_test.go +++ b/internal/cli/home_test.go @@ -891,8 +891,8 @@ func TestDoctor_TopLevelSharesClusterDoctor(t *testing.T) { t.Errorf("`doctor` exit = %d, want 2 (auth fail + kubeconfig fail escalates)", topErr.Code()) } for label, out := range map[string]string{"doctor": topOut, "cluster doctor": clOut} { - if !strings.Contains(out, "Auth & config") || !strings.Contains(out, "not signed in") { - t.Errorf("%s output missing the shared diagnostic (auth section + not-signed-in):\n%s", label, out) + if !strings.Contains(out, "Not signed in") { + t.Errorf("%s output missing the shared diagnostic (not-signed-in):\n%s", label, out) } } } diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index 25e15677..ed134196 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -33,7 +33,7 @@ import ( ) // Status is a single check's severity. Ordered so the numerically-greatest -// status is the worst — see Worst. +// status is the worst; the cli layer rolls the checks up into the verdict. type Status int const ( @@ -44,9 +44,10 @@ const ( // StatusUnknown marks a check that could not run because a prerequisite was // unavailable — today, the cluster API being unreachable. It carries NO signal: -// Worst() ignores it, so it never affects the overall verdict or exit code. It -// exists so a single root cause (a stopped cluster) renders as one honest ✖ plus -// neutral "couldn't check" lines, instead of every check inventing a false cause. +// the verdict rollup ignores it, so it never affects the overall result or exit +// code. It exists so a single root cause (a stopped cluster) renders as one +// honest ✖ plus neutral "couldn't check" lines, instead of every check inventing +// a false cause. const StatusUnknown Status = -1 func (s Status) String() string { @@ -71,23 +72,27 @@ type Result struct { Status Status Detail string Remedy string -} -// Worst returns the most severe status across results (StatusOK for none). -// The doctor command maps a non-OK worst to a non-zero exit code. -func Worst(results []Result) Status { - worst := StatusOK - for _, r := range results { - if r.Status == StatusUnknown { - continue // no signal — a "couldn't check" never sets the verdict - } - if r.Status > worst { - worst = r.Status - } - } - return worst + // Reach classifies WHY the "Cluster reachable" check landed where it did, so + // the cli summary can word the remedy correctly — an unreachable API (stopped + // container runtime / network) needs a different fix than a reachable cluster + // with no tracebloc installed. Zero (ReachOK) on every other check. + Reach ReachState } +// ReachState classifies the "Cluster reachable" outcome so the cli summary can +// word its remedy correctly (see Result.Reach). A stopped API, a reachable +// cluster with no tracebloc installed, and an RBAC/other error each need a +// different plain-language fix — never the same "isn't answering" line. +type ReachState int + +const ( + ReachOK ReachState = iota // API answered and the tracebloc chart is here + ReachUnreachable // API never answered (container runtime/network down) + ReachNoEnv // API answered, but no tracebloc secure environment here + ReachError // API answered with some other error (RBAC/NotFound/…) +) + // Conservative tunables, kept as package consts to avoid false positives on a // busy cluster. If a check ever needs runtime tuning, thread it through Options // (like HTTPProbe) rather than making these package vars. @@ -127,8 +132,8 @@ type Options struct { // Run executes every check in display order and returns their results. It // never returns an error: an unreachable cluster or a failing probe is a -// Result, not a Go error — the command renders all of them and derives the -// exit code from Worst. +// Result, not a Go error — the cli layer rolls them up into the two health +// lines the owner reads and derives the exit code from that verdict. func Run(ctx context.Context, cs kubernetes.Interface, opts Options) []Result { if opts.HTTPProbe == nil { opts.HTTPProbe = httpProbe @@ -191,22 +196,30 @@ func checkReachable(release *cluster.ParentRelease, err error, ns, serverURL str if serverURL != "" { detail = fmt.Sprintf("the cluster API server at %s isn't answering — is the cluster running?", serverURL) } - remedy := "Check the cluster is running and that your kubeconfig/context points at it." + remedy := "Check your secure environment is running." if isLoopback(serverURL) { - remedy = "This is a local cluster — start Docker Desktop (or your container runtime), then `k3d cluster start` (or your cluster's start command)." + remedy = "Start Docker Desktop (`open -a Docker`) — your secure environment restarts with it — then run this again." } - return Result{Name: name, Status: StatusFail, Detail: detail, Remedy: remedy} + return Result{Name: name, Status: StatusFail, Detail: detail, Remedy: remedy, Reach: ReachUnreachable} } // API answered but no chart here (ErrNoParentRelease) or another error. // The discovery error's remediation tail points at doctor — which is // what's running. Strip it so doctor never tells the user to run doctor. // (Must match the exact suffix cluster.discover appends.) detail := strings.TrimSuffix(strings.TrimSpace(err.Error()), "Diagnose with `tracebloc doctor`.") + // The API answered: a missing chart means "no environment installed here" + // (fix: reinstall), anything else is an RBAC/NotFound-class error (fix: + // support). The cli summary words each differently — never a kubectl. + reach := ReachError + if errors.Is(err, cluster.ErrNoParentRelease) { + reach = ReachNoEnv + } return Result{ Name: name, Status: StatusFail, Detail: strings.TrimSpace(detail), Remedy: "Check your kubeconfig/context and that the tracebloc client chart is installed here: kubectl get deploy -n " + ns, + Reach: reach, } } return Result{ @@ -255,9 +268,14 @@ func isUnreachable(err error) bool { return false } -// isLoopback reports whether serverURL points at the local machine (127.0.0.1 / -// localhost / ::1) — a k3d/kind/Docker-Desktop cluster, whose "isn't answering" -// almost always means the container runtime or the cluster is simply stopped. +// isLoopback reports whether serverURL points at the local machine — a +// k3d/kind/Docker-Desktop cluster, whose "isn't answering" almost always means +// the container runtime or the cluster is simply stopped, so the remedy is +// "start Docker Desktop". It covers the loopback addresses (127.0.0.0/8, ::1, +// localhost), the unspecified/wildcard bind addresses k3d writes into a +// kubeconfig when no explicit host is pinned (0.0.0.0, ::), and Docker Desktop's +// host alias (host.docker.internal). No genuinely-remote cluster is ever reached +// through any of these, so treating them as local carries no false-positive risk. func isLoopback(serverURL string) bool { if serverURL == "" { return false @@ -266,19 +284,19 @@ func isLoopback(serverURL string) bool { if err != nil { return false } - host := u.Hostname() - if host == "localhost" { + switch u.Hostname() { + case "localhost", "host.docker.internal": return true } - if ip := net.ParseIP(host); ip != nil { - return ip.IsLoopback() + if ip := net.ParseIP(u.Hostname()); ip != nil { + return ip.IsLoopback() || ip.IsUnspecified() } return false } // unknownCheck is the placeholder for a cluster check that could not run because -// the API is unreachable — no signal, so Worst() ignores it and the verdict -// comes from "Cluster reachable" alone. +// the API is unreachable — no signal, so the verdict rollup ignores it and the +// verdict comes from "Cluster reachable" alone. func unknownCheck(name string) Result { return Result{ Name: name, diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index b1f6b34b..be6ee50e 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -157,20 +157,30 @@ func initCrashPod(name string) *corev1.Pod { } } -func TestWorst(t *testing.T) { - if got := Worst(nil); got != StatusOK { - t.Fatalf("Worst(nil) = %v, want ok", got) - } - rs := []Result{{Status: StatusOK}, {Status: StatusFail}, {Status: StatusWarn}} - if got := Worst(rs); got != StatusFail { - t.Fatalf("Worst = %v, want fail", got) +// worstStatus mirrors the overall-verdict rollup for the Run() tests below: the +// most severe status across results, StatusUnknown ("couldn't check") ignored. +// Production derives the exit code in the cli layer from the two rolled-up health +// lines; this keeps the package tests' verdict assertions concise. +func worstStatus(results []Result) Status { + worst := StatusOK + for _, r := range results { + if r.Status != StatusUnknown && r.Status > worst { + worst = r.Status + } } + return worst } func TestCheckReachable(t *testing.T) { - // A non-transport error (e.g. no chart / RBAC) keeps the kubeconfig+chart remedy. - if r := checkReachable(nil, errors.New("boom"), ns, ""); r.Status != StatusFail { - t.Fatalf("error => %v, want fail", r.Status) + // A non-transport, non-chart error (e.g. RBAC) keeps the kubeconfig+chart + // remedy and classifies as ReachError. + if r := checkReachable(nil, errors.New("boom"), ns, ""); r.Status != StatusFail || r.Reach != ReachError { + t.Fatalf("other error => status %v reach %v, want fail/ReachError", r.Status, r.Reach) + } + // No chart installed (the API answered) classifies as ReachNoEnv so the + // summary points at the installer, never a kubectl (Bugbot #365). + if r := checkReachable(nil, cluster.ErrNoParentRelease, ns, ""); r.Reach != ReachNoEnv { + t.Fatalf("no-chart => reach %v, want ReachNoEnv", r.Reach) } rel := &cluster.ParentRelease{ReleaseName: "tb", ChartVersion: "1.3.5", AppVersion: "1.3.5"} r := checkReachable(rel, nil, ns, "") @@ -190,6 +200,45 @@ func TestCheckReachable(t *testing.T) { if !strings.Contains(tr.Remedy, "start") || strings.Contains(tr.Remedy, "kubectl get deploy") { t.Fatalf("transport remedy = %q, want a start-the-cluster hint, not the chart remedy", tr.Remedy) } + + // A stopped k3d cluster advertised as 0.0.0.0 (the kubeconfig host when the + // installer pins no explicit --api-port host) must still get the start-Docker + // remedy, not the generic "check it's running" line (Bugbot #365). + wc := checkReachable(nil, errors.New(`Get "https://0.0.0.0:6550/api": dial tcp 0.0.0.0:6550: connect: connection refused`), ns, "https://0.0.0.0:6550") + if wc.Reach != ReachUnreachable { + t.Fatalf("0.0.0.0 transport => reach %v, want ReachUnreachable", wc.Reach) + } + if !strings.Contains(wc.Remedy, "Docker Desktop") { + t.Fatalf("0.0.0.0 remedy = %q, want the start-Docker-Desktop hint", wc.Remedy) + } +} + +// TestIsLoopback covers every kubeconfig host that means "the cluster is local, +// so the fix is start Docker Desktop": the loopback addresses, the wildcard bind +// addresses k3d writes when no host is pinned (0.0.0.0, ::), and Docker Desktop's +// host alias — but never a genuinely-remote endpoint (Bugbot #365). +func TestIsLoopback(t *testing.T) { + for _, s := range []string{ + "https://127.0.0.1:6550", + "https://localhost:6550", + "https://[::1]:6550", + "https://0.0.0.0:6550", + "https://[::]:6550", + "https://host.docker.internal:6550", + } { + if !isLoopback(s) { + t.Errorf("isLoopback(%q) = false, want true (local cluster)", s) + } + } + for _, s := range []string{ + "https://api.k8s.example.com:6443", + "https://10.1.2.3:6443", + "", + } { + if isLoopback(s) { + t.Errorf("isLoopback(%q) = true, want false (not a local cluster)", s) + } + } } // TestRun_UnreachableCascade mimics the reported failure: the cluster API is @@ -243,8 +292,8 @@ func TestRun_UnreachableCascade(t *testing.T) { if r := byName["Backend egress (from this machine)"]; r.Status != StatusOK { t.Errorf("Backend egress = %v, want ok (probed from this machine, independent of the cluster API)", r.Status) } - if w := Worst(results); w != StatusFail { - t.Fatalf("Worst = %v, want fail (verdict from the one real ✖, StatusUnknown ignored)", w) + if w := worstStatus(results); w != StatusFail { + t.Fatalf("worst = %v, want fail (verdict from the one real ✖, StatusUnknown ignored)", w) } } @@ -491,7 +540,7 @@ func TestRun_HealthyCluster(t *testing.T) { if len(results) != 9 { t.Fatalf("want 9 checks, got %d", len(results)) } - if w := Worst(results); w != StatusOK { + if w := worstStatus(results); w != StatusOK { for _, r := range results { t.Logf("%-32s %-4s %s", r.Name, r.Status, r.Detail) }