From 6ede3a2246c141ac4a89716664fb0c4361a588a1 Mon Sep 17 00:00:00 2001 From: shujaat hasan Date: Thu, 20 Aug 2026 16:47:51 +0200 Subject: [PATCH 1/3] feat(doctor): surface the machine beneath the nodes (backend#2221) Every other check in this package reads the cluster and believes it. On a local k3d install that belief is misplaced: the node containers are created with `NanoCpus=0 CpuQuota=0 Memory=0`, so each one honestly reports the WHOLE Docker VM and a default server+agent cluster tells Kubernetes the machine is twice its real size. Measured on k3d v5.9.0 / k3s v1.35.5 / Docker 29.5.2: host 10 cpu / 16.00 GiB Docker VM 10 cpu / 7.75 GiB <- the real machine 2 node containers 15.50 GiB <- what Kubernetes believes The node memory was byte-identical to the VM's MemTotal on BOTH nodes. So checkNodeFit can truthfully say "a Ready node can schedule this job" for two jobs that cannot both exist, and its drift nudge advertises half the VM twice. Adds "Machine capacity": host -> VM -> node capacity -> unrequested, plus the one invariant that makes the lie legible, sum(node capacity) <= VM capacity. Uncapped k3d violates it by exactly the node count. Verified against a LIVE 2-node edge, not just fakes: WARN Machine capacity -- host 16.00 GiB -> Docker VM 7.75 GiB (10 cpu) -> 2 nodes claiming 15.50 GiB -> 4.55 GiB unrequested -- Kubernetes believes 2.00x the memory this machine has [...] That live run also found a bug in this check that fakes could not: the "unrequested" level was computed from the inflated SUM, so it reported 12.30 GiB free on a 7.75 GiB VM -- the fourth level inheriting the third's error, this check repeating the very lie it exists to expose. Now measured against min(sum, VM), a no-op on an honest cluster. Regression-tested. Refuses to assert what it cannot know, in four ways: a non-k3d or mixed cluster gets StatusUnknown (on EKS the nodes ARE machines and `docker info` on this laptop describes something unrelated); so does an unreadable VM, no Ready node, and nodes reporting no capacity at all -- "0 GiB claimed, all good" would be a green with nothing behind it. StatusUnknown carries no signal, so none of these move the verdict or the exit code. The tolerance is measured too. A node capped at 3 GiB (3221225472 B) reported capacity 3221225Ki, 2.4% ABOVE the limit it came from, so a strict `sum > vm` would call correct capping an over-commit. 1.05 is far below the 2.00x the real bug produces and far above that rounding. Probes are injectable (Options.VMProbe / HostProbe), so the chain is fully exercisable without Docker. The host level is best-effort and OMITTED rather than zero-filled when unreadable -- the VM is the constraint, the host is context. Mutation-tested: widening the tolerance, dropping the k3d guard, and counting terminal pods as holding memory each redden their own tests. Does not touch the installers, so it cannot conflict with the client-side work on this ticket. Co-Authored-By: Claude Opus 5 --- .../cli/testdata/golden/zz-all-strings.golden | 20 ++ internal/doctor/doctor.go | 15 + internal/doctor/doctor_test.go | 8 +- internal/doctor/machine.go | 290 ++++++++++++++++ internal/doctor/machine_test.go | 326 ++++++++++++++++++ 5 files changed, 657 insertions(+), 2 deletions(-) create mode 100644 internal/doctor/machine.go create mode 100644 internal/doctor/machine_test.go diff --git a/internal/cli/testdata/golden/zz-all-strings.golden b/internal/cli/testdata/golden/zz-all-strings.golden index 69adf1b..c666d64 100644 --- a/internal/cli/testdata/golden/zz-all-strings.golden +++ b/internal/cli/testdata/golden/zz-all-strings.golden @@ -76,6 +76,7 @@ screen. %s/%d are runtime placeholders. "%s · starting up, not ready yet — run %s" "%s ×%d" "%s — %s" +"%s — Kubernetes believes %.2f× the memory this machine has, because the k3d node containers are uncapped and each reports the whole VM" "%s, … and %d more" "%s/%s" "%s: %v" @@ -138,6 +139,7 @@ screen. %s/%d are runtime placeholders. "Cancelled — nothing was removed." "Cancelled — the name didn't match. Nothing was removed." "Chart uninstall reported: %v" +"Check Docker is running: docker info" "Check on it later with: kubectl logs -f -n %s job/%s" "Check your data" "Check your network / HTTP(S)_PROXY, then run `%s doctor` again." @@ -189,10 +191,12 @@ screen. %s/%d are runtime placeholders. "Diagnose auth / cluster problems with: tracebloc doctor" "Do you want to change the allocation? Run `%s resources set` (guided walkthrough on a terminal)." "Do you want to ingest training or test data?" +"Docker VM %s (%d cpu) → %d node%s claiming %s → %s unrequested" "Docker and related tools — remove them yourself if you no longer need them" "Dry run — nothing was changed" "Dry-run complete — your data and secure environment check out; nothing was created." "Dry-run — nothing was deleted." +"Each node container reports the whole VM, so %d of them double-count it. Run a single-node environment, or cap the nodes (k3d --servers-memory/--agents-memory). Until then a job that fits a node may still OOM the VM." "Each training run already uses up to %s — nothing to change." "Each training run may now use up to %s." "Email it to support@tracebloc.io." @@ -485,6 +489,9 @@ screen. %s/%d are runtime placeholders. "decoding submit response (got body %q): %w" "deleting stage Pod %s/%s: %w" "destination" +"docker info reported cores=%d mem=%d" +"docker info returned %q, want two fields" +"docker info: %w" "dropping %s.%s: %w%s" "e.g. %s or %s" "e.g. 17 for COCO pose" @@ -508,10 +515,13 @@ screen. %s/%d are runtime placeholders. "generating idempotency key: %w" "generating staging-dir suffix: %w" "github releases: HTTP %d" +"host %s →" +"host memory unsupported on %s" "how many CPU cores a single training run may use" "how much memory a single training run may use, in GiB" "http://%s.%s.svc.cluster.local:%d" "http://localhost:%d" +"hw.memsize" "image pull secret %q not found" "images" "images/ and annotations/ don't pair up: %s. Every image needs a same-named .xml annotation (and vice versa) — the cluster rejects mismatches after the upload." @@ -579,9 +589,11 @@ screen. %s/%d are runtime placeholders. "no .txt files found in %q. Text categories expect /%s/*.txt." "no .xml annotation files found in %q. object_detection expects /annotations/*.xml (Pascal VOC)." "no CLI-supported tasks for %s data yet" +"no MemTotal in /proc/meminfo" "no PersistentVolumeClaim named %q found in namespace %q. The chart's _helpers.tpl pins this name; if your install renamed it out-of-band, the CLI doesn't yet support that (read-name-from-jobs-manager is a v0.2 follow-up). Verify with: kubectl get pvc -n %s" "no Ready node can fit a training job (needs %s)" "no Ready node on this machine to size a training run against" +"no Ready node to measure" "no Running Pod backing service %s/%s (found %d Pod(s); check `kubectl get pods -n %s -l %s`)" "no Running pod with name containing %q in namespace %q" "no Secret of type kubernetes.io/service-account-token bound to ServiceAccount %s found in namespace %s" @@ -596,7 +608,9 @@ screen. %s/%d are runtime placeholders. "no tracebloc client found" "no usable image files in %q — found %s, but the ingestor accepts only .jpg, .jpeg, or .png. Convert the images and re-run." "no usable ingestor token. TokenRequest failed: %v. Fallback to static secret also failed: %w. Remediation: either grant your user the `create` verb on `serviceaccounts/token` (RBAC), or have an admin create a long-lived Secret of type kubernetes.io/service-account-token that references the %s ServiceAccount in namespace %s." +"nodes report no memory capacity to compare against the VM" "none detected" +"not a local k3d cluster — the host/VM chain applies only to a cluster running on this machine's Docker VM" "not signed in — run `tracebloc login` first" "outcome: early exit before the cluster was probed" "outcome: early exit — no roll-up verdict (granular checks below)" @@ -617,6 +631,7 @@ screen. %s/%d are runtime placeholders. "push.StagedPrefix: unsafe table name %q — caller must ValidateTableName before constructing a PVC path" "pvc path" "querying datasets: %w%s" +"read /proc/meminfo: %w" "reading %q: %w" "reading %s header: %w" "reading %s/: %w" @@ -700,6 +715,8 @@ screen. %s/%d are runtime placeholders. "submit response missing job_name (got body %q)" "submit response missing namespace (got body %q)" "synthesized spec failed schema validation; check the flag values above" +"sysctl" +"sysctl hw.memsize: %w" "table %q already exists in this secure environment. Re-ingesting the same table doesn't merge or replace — the run would fail after uploading everything. Re-run with --overwrite to replace it, or pick a different --name. (`tracebloc data delete %s` also removes it.)" "table name is %d characters; the max is %d (matches both the MySQL identifier limit and the Kubernetes label-value limit, which the stage Pod's tracebloc.io/table label is bound by). Use a shorter name." "tabular = a CSV table; image = labels.csv + images/; text = labels.csv + texts/" @@ -745,6 +762,9 @@ screen. %s/%d are runtime placeholders. "unavailable" "unknown backend environment %q — valid values are dev, stg, prod (default). Check --env / $CLIENT_ENV" "unknown command %q for %q" +"unparseable MemTotal %q: %w" +"unparseable NCPU %q: %w" +"unparseable hw.memsize: %w" "upgrade didn't complete (%w). You can run the installer directly:\n %s" "values:" "verifying removal of %s: %w" diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index 83ffa66..f65608b 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -137,6 +137,13 @@ type Options struct { // HTTPProbe reports whether a URL is reachable from where the CLI runs. // nil => httpProbe (proxy-aware, short timeout). Injected in tests. HTTPProbe func(ctx context.Context, url string) error + + // VMProbe reports the container runtime's VM size, and HostProbe the + // physical machine's. nil => dockerVMProbe / hostProbe. Both injected in + // tests so the four-level chain is exercisable without Docker + // (backend#2221). + VMProbe VMProbe + HostProbe HostProbe } // Run executes every check in display order and returns their results. It @@ -147,6 +154,12 @@ func Run(ctx context.Context, cs kubernetes.Interface, opts Options) []Result { if opts.HTTPProbe == nil { opts.HTTPProbe = httpProbe } + if opts.VMProbe == nil { + opts.VMProbe = dockerVMProbe + } + if opts.HostProbe == nil { + opts.HostProbe = hostProbe + } ns := opts.Namespace // Discovered once: the first API call. Its error is the reachability signal @@ -168,6 +181,7 @@ func Run(ctx context.Context, cs kubernetes.Interface, opts Options) []Result { unknownCheck("Restart history"), unknownCheck("Dataset volume (PVC)"), unknownCheck("Node capacity"), + unknownCheck("Machine capacity"), unknownCheck("Image pull secret"), unknownCheck("Proxy configuration"), checkBackendEgress(ctx, nil, opts.HTTPProbe), @@ -184,6 +198,7 @@ func Run(ctx context.Context, cs kubernetes.Interface, opts Options) []Result { checkRestartHistory(ctx, cs, ns), checkPVC(ctx, cs, ns), checkNodeFit(ctx, cs, jmEnv), + checkMachineChain(ctx, cs, opts.VMProbe, opts.HostProbe), checkImagePull(ctx, cs, ns, release), checkProxy(jmEnv), checkBackendEgress(ctx, jmEnv, opts.HTTPProbe), diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go index 531ac8f..c59a7e6 100644 --- a/internal/doctor/doctor_test.go +++ b/internal/doctor/doctor_test.go @@ -547,8 +547,12 @@ func TestRun_HealthyCluster(t *testing.T) { HTTPProbe: func(context.Context, string) error { return nil }, }) - if len(results) != 9 { - t.Fatalf("want 9 checks, got %d", len(results)) + // 10 since backend#2221 added "Machine capacity". These nodes are not + // k3d-named, so that check reports StatusUnknown — which the rollup ignores, + // so the healthy verdict below is unaffected. That is the intended + // behaviour on a non-local cluster, not an accident of the fixture. + if len(results) != 10 { + t.Fatalf("want 10 checks, got %d", len(results)) } if w := worstStatus(results); w != StatusOK { for _, r := range results { diff --git a/internal/doctor/machine.go b/internal/doctor/machine.go new file mode 100644 index 0000000..30208a5 --- /dev/null +++ b/internal/doctor/machine.go @@ -0,0 +1,290 @@ +package doctor + +import ( + "context" + "fmt" + "os" + "os/exec" + "runtime" + "strconv" + "strings" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +// The four-level chain: host RAM -> VM RAM -> node allocatable -> free +// (backend#2221, RFC-BACKEND-664 §P4). +// +// Every other check in this package reads the cluster and believes it. On a +// local k3d install that belief is misplaced, because the node containers are +// created with `NanoCpus=0 CpuQuota=0 Memory=0`: each one honestly reports the +// WHOLE Docker VM, so a default server+agent cluster tells Kubernetes the +// machine is twice its real size. Measured on k3d v5.9.0 / k3s v1.35.5 / +// Docker 29.5.2: +// +// host 10 cpu / 16.00 GiB +// Docker VM 10 cpu / 7.75 GiB <- the real machine +// 2 node containers 15.50 GiB <- what Kubernetes believes +// +// The node memory was byte-identical to the VM's MemTotal on BOTH nodes. So +// checkNodeFit can truthfully say "a Ready node can schedule this job" for two +// jobs that cannot both exist, and its own drift nudge ("this machine could +// give a run up to cpu=9,memory=4Gi") advertises half the VM twice. +// +// The gap between the levels IS the customer's problem, and today no check +// shows it: doctor reads level 3 only, the installer's preflight gates on +// level 1, and nothing looks at level 2 at all. This check shows all four and +// names the one invariant that matters: +// +// sum(node capacity) <= VM capacity +// +// Uncapped k3d violates it by exactly the node count. + +// VMProbe reports the container runtime's VM size — `docker info` NCPU and +// MemTotal. Injected in tests; nil means dockerVMProbe. +// +// This is deliberately NOT the host and NOT the sum of node allocatable. On +// macOS and Windows the host is much larger than the VM (measured: a 16 GiB +// host behind a 7.75 GiB VM), which is why gating on host RAM passes machines +// whose cluster cannot hold what it was sized for. +type VMProbe func(ctx context.Context) (cores int64, memBytes int64, err error) + +// HostProbe reports the physical machine's cores and RAM. Injected in tests; +// nil means hostProbe. Best-effort: the chain drops this level rather than +// failing when it cannot be read, because the VM is the binding constraint and +// the host is context. +type HostProbe func() (cores int64, memBytes int64, err error) + +// dockerVMProbe asks the container runtime how big its VM is. +func dockerVMProbe(ctx context.Context) (int64, int64, error) { + // #nosec G204 -- argv is compile-time constant: literal "docker" with a + // fixed --format template. No user input reaches this command line. + out, err := exec.CommandContext(ctx, "docker", "info", "--format", "{{.NCPU}} {{.MemTotal}}").Output() + if err != nil { + return 0, 0, fmt.Errorf("docker info: %w", err) + } + fields := strings.Fields(string(out)) + if len(fields) != 2 { + return 0, 0, fmt.Errorf("docker info returned %q, want two fields", strings.TrimSpace(string(out))) + } + cores, err := strconv.ParseInt(fields[0], 10, 64) + if err != nil { + return 0, 0, fmt.Errorf("unparseable NCPU %q: %w", fields[0], err) + } + mem, err := strconv.ParseInt(fields[1], 10, 64) + if err != nil { + return 0, 0, fmt.Errorf("unparseable MemTotal %q: %w", fields[1], err) + } + if cores <= 0 || mem <= 0 { + return 0, 0, fmt.Errorf("docker info reported cores=%d mem=%d", cores, mem) + } + return cores, mem, nil +} + +// hostProbe reads physical RAM for the platforms the local install supports. +// runtime.NumCPU() is the host's logical CPU count on both — the CLI runs on +// the host, not inside the VM. +func hostProbe() (int64, int64, error) { + cores := int64(runtime.NumCPU()) + switch runtime.GOOS { + case "darwin": + // #nosec G204 -- argv is compile-time constant. + out, err := exec.Command("sysctl", "-n", "hw.memsize").Output() + if err != nil { + return cores, 0, fmt.Errorf("sysctl hw.memsize: %w", err) + } + mem, err := strconv.ParseInt(strings.TrimSpace(string(out)), 10, 64) + if err != nil { + return cores, 0, fmt.Errorf("unparseable hw.memsize: %w", err) + } + return cores, mem, nil + case "linux": + raw, err := os.ReadFile("/proc/meminfo") + if err != nil { + return cores, 0, fmt.Errorf("read /proc/meminfo: %w", err) + } + for _, line := range strings.Split(string(raw), "\n") { + if !strings.HasPrefix(line, "MemTotal:") { + continue + } + f := strings.Fields(line) + if len(f) < 2 { + break + } + kb, err := strconv.ParseInt(f[1], 10, 64) + if err != nil { + break + } + return cores, kb * 1024, nil + } + return cores, 0, fmt.Errorf("no MemTotal in /proc/meminfo") + default: + // Windows: the VM level is what binds there too, and .wslconfig is the + // thing a user actually changes. Skip the host level rather than shell + // out to WMI for a number this check only prints. + return cores, 0, fmt.Errorf("host memory unsupported on %s", runtime.GOOS) + } +} + +// overCommitTolerance is how far sum(node capacity) may exceed the VM before +// this check calls it a double-count. +// +// Not slack for the real bug — that is 2.00x, and anything above ~1.05 is +// already unambiguous. It exists because node capacity is derived from the +// container's cgroup limit with a rounding that does not exactly invert: on a +// node capped at 3 GiB (3221225472 B) kubelet reported 3221225Ki, which is +// 2.4% ABOVE the limit it came from. A strict `sum > vm` would report that +// rounding as over-commit on a correctly capped cluster. +const overCommitTolerance = 1.05 + +// checkMachineChain surfaces host -> VM -> node allocatable -> free, and warns +// when the nodes claim more of the machine than the VM has. +func checkMachineChain(ctx context.Context, cs kubernetes.Interface, vm VMProbe, host HostProbe) Result { + const name = "Machine capacity" + + nodes, err := cs.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + if err != nil { + return Result{ + Name: name, + Status: StatusUnknown, + Detail: "could not list nodes: " + err.Error(), + } + } + + var ready []corev1.Node + for i := range nodes.Items { + if nodeReady(nodes.Items[i]) { + ready = append(ready, nodes.Items[i]) + } + } + if len(ready) == 0 { + return Result{Name: name, Status: StatusUnknown, Detail: "no Ready node to measure"} + } + + // Only a k3d cluster has a Docker VM beneath it. On EKS the nodes ARE + // machines and `docker info` on this laptop describes something unrelated, + // so asserting anything from it would be worse than staying quiet. + if !allK3d(ready) { + return Result{ + Name: name, + Status: StatusUnknown, + Detail: "not a local k3d cluster — the host/VM chain applies only to a cluster running on this machine's Docker VM", + } + } + + // CAPACITY, not allocatable: the invariant this check exists to state is + // about what the nodes claim the machine IS. Making `allocatable` honest is + // a separate question (kubelet reservations, RFC §P3) and k3s sets none, so + // on the clusters this check applies to the two are byte-identical anyway. + var sumCPU, sumMem int64 + for i := range ready { + capacity := ready[i].Status.Capacity + sumCPU += capacity.Cpu().MilliValue() + sumMem += capacity.Memory().Value() + } + if sumMem <= 0 { + // No capacity reported at all. Saying "0 GiB claimed, all good" would be + // a green this check cannot back. + return Result{Name: name, Status: StatusUnknown, Detail: "nodes report no memory capacity to compare against the VM"} + } + + vmCores, vmMem, err := vm(ctx) + if err != nil { + // The VM is the level this check exists to add. Without it there is no + // chain, and guessing is what the bug already does. + return Result{ + Name: name, + Status: StatusUnknown, + Detail: "could not read the Docker VM's size: " + err.Error(), + Remedy: "Check Docker is running: docker info", + } + } + + // Level 4: what is actually left to request. + // + // Measured against min(sum, VM), NOT against the sum. On a double-counted + // cluster the sum is a fiction, and "12.30 GiB unrequested" on a 7.75 GiB + // VM would be this check repeating the very lie it is here to expose — the + // fourth level inheriting the error from the third. min() is a no-op on an + // honest cluster, where the sum never exceeds the VM. + trueCeiling := sumMem + if vmMem < trueCeiling { + trueCeiling = vmMem + } + freeMem := trueCeiling - requestedMemory(ctx, cs) + if freeMem < 0 { + freeMem = 0 + } + + chain := "" + if _, hostMem, herr := host(); herr == nil && hostMem > 0 { + chain = fmt.Sprintf("host %s → ", gib(hostMem)) + } + chain += fmt.Sprintf("Docker VM %s (%d cpu) → %d node%s claiming %s → %s unrequested", + gib(vmMem), vmCores, len(ready), plural(len(ready)), gib(sumMem), gib(freeMem)) + + if float64(sumMem) > float64(vmMem)*overCommitTolerance { + ratio := float64(sumMem) / float64(vmMem) + return Result{ + Name: name, + Status: StatusWarn, + Detail: fmt.Sprintf("%s — Kubernetes believes %.2f× the memory this machine has, because the k3d node containers are uncapped and each reports the whole VM", chain, ratio), + Remedy: fmt.Sprintf("Each node container reports the whole VM, so %d of them double-count it. Run a single-node environment, or cap the nodes (k3d --servers-memory/--agents-memory). Until then a job that fits a node may still OOM the VM.", len(ready)), + } + } + + return Result{Name: name, Status: StatusOK, Detail: chain} +} + +// allK3d reports whether every Ready node is a k3d node container. k3d names +// them "k3d--server-N" / "-agent-N"; a mixed cluster is not a local +// install and gets no verdict. +func allK3d(nodes []corev1.Node) bool { + for i := range nodes { + if !strings.HasPrefix(nodes[i].Name, "k3d-") { + return false + } + } + return true +} + +// requestedMemory sums memory requests across pods that still hold resources. +// Best-effort: on a read failure it returns 0, so the "unrequested" level +// degrades to the full claim rather than reporting a negative remainder. +func requestedMemory(ctx context.Context, cs kubernetes.Interface) int64 { + pods, err := cs.CoreV1().Pods("").List(ctx, metav1.ListOptions{}) + if err != nil { + return 0 + } + var total int64 + for i := range pods.Items { + p := pods.Items[i] + // Succeeded/Failed pods hold no resources; counting them would + // understate what is actually free. + if p.Status.Phase == corev1.PodSucceeded || p.Status.Phase == corev1.PodFailed { + continue + } + for j := range p.Spec.Containers { + if q, ok := p.Spec.Containers[j].Resources.Requests[corev1.ResourceMemory]; ok { + total += q.Value() + } + } + } + return total +} + +func gib(b int64) string { + if b <= 0 { + return "0 GiB" + } + return fmt.Sprintf("%.2f GiB", float64(b)/float64(1<<30)) +} + +func plural(n int) string { + if n == 1 { + return "" + } + return "s" +} diff --git a/internal/doctor/machine_test.go b/internal/doctor/machine_test.go new file mode 100644 index 0000000..43eaa62 --- /dev/null +++ b/internal/doctor/machine_test.go @@ -0,0 +1,326 @@ +package doctor + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +// The numbers this ticket was measured on: k3d v5.9.0 / k3s v1.35.5 / Docker +// 29.5.2, macOS aarch64. `docker info` reported NCPU=10 MemTotal=8321712128 +// and BOTH uncapped node containers reported 8126672Ki — byte-identical to the +// VM's MemTotal (8126672 * 1024 == 8321712128). +const ( + measuredVMMem = int64(8321712128) + measuredVMCores = int64(10) + measuredNodeMem = "8126672Ki" +) + +// k3dNode builds a Ready k3d node container with CAPACITY set (the field the +// chain compares against the VM), mirroring what kubelet actually reports. +func k3dNode(name, cpu, mem string) *corev1.Node { + rl := corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpu), + corev1.ResourceMemory: resource.MustParse(mem), + } + return &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: corev1.NodeStatus{ + Capacity: rl, + Allocatable: rl.DeepCopy(), + Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionTrue}}, + }, + } +} + +func fixedVM(cores, mem int64) VMProbe { + return func(context.Context) (int64, int64, error) { return cores, mem, nil } +} + +func failingVM(msg string) VMProbe { + return func(context.Context) (int64, int64, error) { return 0, 0, errors.New(msg) } +} + +func fixedHost(cores, mem int64) HostProbe { + return func() (int64, int64, error) { return cores, mem, nil } +} + +func noHost() HostProbe { + return func() (int64, int64, error) { return 0, 0, errors.New("unsupported") } +} + +// ── the bug, at the numbers it was measured at ─────────────────────────────── + +func TestMachineChain_TwoUncappedNodesDoubleCountTheVM(t *testing.T) { + // The shipped default topology: SERVERS=1 AGENTS=1, both uncapped. + cs := fake.NewClientset( + k3dNode("k3d-tracebloc-server-0", "10", measuredNodeMem), + k3dNode("k3d-tracebloc-agent-0", "10", measuredNodeMem), + ) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) + + if got.Status != StatusWarn { + t.Fatalf("two uncapped nodes on one VM => %v (%q), want warn", got.Status, got.Detail) + } + // The ratio is the number that makes the lie legible, so pin it. + if !strings.Contains(got.Detail, "2.00×") { + t.Errorf("detail should name the 2.00x over-count, got %q", got.Detail) + } + // All four levels present. + for _, want := range []string{"host 16.00 GiB", "Docker VM 7.75 GiB", "2 nodes claiming 15.50 GiB", "unrequested"} { + if !strings.Contains(got.Detail, want) { + t.Errorf("chain missing %q, got %q", want, got.Detail) + } + } + if got.Remedy == "" { + t.Error("a warn must carry a remedy") + } +} + +func TestMachineChain_SingleNodeIsHonest(t *testing.T) { + // One node container reporting the whole VM is not a lie — it IS the VM. + cs := fake.NewClientset(k3dNode("k3d-tracebloc-server-0", "10", measuredNodeMem)) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) + if got.Status != StatusOK { + t.Fatalf("single node claiming exactly the VM => %v (%q), want ok", got.Status, got.Detail) + } + if strings.Contains(got.Detail, "×") { + t.Errorf("an honest cluster should not report a ratio, got %q", got.Detail) + } +} + +func TestMachineChain_CappedNodesPassDespiteRounding(t *testing.T) { + // MEASURED: a node capped at 3 GiB (3221225472 B) reported capacity + // 3221225Ki, which is 2.4% ABOVE the limit it came from. Two of those sum + // to 0.79x of this VM, so the invariant holds — but a strict `sum > vm` + // would still have to survive that rounding, which is what the tolerance + // is for. This is the regression guard for capping correctly. + cs := fake.NewClientset( + k3dNode("k3d-cap-server-0", "10", "3221225Ki"), + k3dNode("k3d-cap-agent-0", "10", "3221225Ki"), + ) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + if got.Status != StatusOK { + t.Fatalf("correctly capped nodes => %v (%q), want ok", got.Status, got.Detail) + } +} + +func TestMachineChain_RoundingAloneNeverWarns(t *testing.T) { + // A single node whose reported capacity sits just above the VM by the same + // rounding must not be called a double-count. + cs := fake.NewClientset(k3dNode("k3d-x-server-0", "10", fmt.Sprintf("%d", measuredVMMem+measuredVMMem/50))) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + if got.Status != StatusOK { + t.Fatalf("2%% over the VM is rounding, not over-commit: got %v (%q)", got.Status, got.Detail) + } +} + +func TestMachineChain_ThreeNodesReportTheirRatio(t *testing.T) { + cs := fake.NewClientset( + k3dNode("k3d-t-server-0", "10", measuredNodeMem), + k3dNode("k3d-t-agent-0", "10", measuredNodeMem), + k3dNode("k3d-t-agent-1", "10", measuredNodeMem), + ) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + if got.Status != StatusWarn { + t.Fatalf("three uncapped nodes => %v, want warn", got.Status) + } + if !strings.Contains(got.Detail, "3.00×") { + t.Errorf("want 3.00x for three nodes, got %q", got.Detail) + } +} + +// ── the fourth level ──────────────────────────────────────────────────────── + +func TestMachineChain_UnrequestedSubtractsLiveRequests(t *testing.T) { + pod := func(name string, mem string, phase corev1.PodPhase) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "tracebloc"}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "c", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse(mem)}, + }, + }}}, + Status: corev1.PodStatus{Phase: phase}, + } + } + cs := fake.NewClientset( + k3dNode("k3d-t-server-0", "10", "8Gi"), + pod("live", "2Gi", corev1.PodRunning), + // Terminal pods hold nothing; counting them would understate free. + pod("done", "4Gi", corev1.PodSucceeded), + pod("dead", "4Gi", corev1.PodFailed), + ) + got := checkMachineChain(bg(), cs, fixedVM(10, 8*(1<<30)), noHost()) + if !strings.Contains(got.Detail, "6.00 GiB unrequested") { + t.Fatalf("8Gi capacity - 2Gi live request should leave 6.00 GiB, got %q", got.Detail) + } +} + +// ── refusing to assert what it cannot know ────────────────────────────────── + +func TestMachineChain_NonK3dClusterGetsNoVerdict(t *testing.T) { + // On EKS the nodes ARE machines and `docker info` on this laptop describes + // something unrelated. Asserting from it would be worse than silence. + cs := fake.NewClientset( + k3dNode("ip-10-0-1-23.eu-central-1.compute.internal", "8", "32Gi"), + k3dNode("ip-10-0-1-24.eu-central-1.compute.internal", "8", "32Gi"), + ) + got := checkMachineChain(bg(), cs, fixedVM(2, 4*(1<<30)), noHost()) + if got.Status != StatusUnknown { + t.Fatalf("non-k3d cluster => %v (%q), want unknown", got.Status, got.Detail) + } +} + +func TestMachineChain_MixedClusterGetsNoVerdict(t *testing.T) { + cs := fake.NewClientset( + k3dNode("k3d-t-server-0", "10", measuredNodeMem), + k3dNode("ip-10-0-1-24.eu-central-1.compute.internal", "8", "32Gi"), + ) + if got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { + t.Fatalf("mixed cluster => %v, want unknown", got.Status) + } +} + +func TestMachineChain_UnreadableVMGetsNoVerdict(t *testing.T) { + cs := fake.NewClientset(k3dNode("k3d-t-server-0", "10", measuredNodeMem)) + got := checkMachineChain(bg(), cs, failingVM("docker daemon not running"), noHost()) + if got.Status != StatusUnknown { + t.Fatalf("unreadable VM => %v, want unknown (the VM is the level this check adds)", got.Status) + } + if got.Remedy == "" { + t.Error("an unreadable VM should say how to make it readable") + } +} + +func TestMachineChain_NoCapacityReportedGetsNoVerdict(t *testing.T) { + // Allocatable set, Capacity absent — "0 GiB claimed, all good" would be a + // green with nothing behind it. + n := k3dNode("k3d-t-server-0", "10", measuredNodeMem) + n.Status.Capacity = nil + cs := fake.NewClientset(n) + if got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { + t.Fatalf("no capacity => %v (%q), want unknown", got.Status, got.Detail) + } +} + +func TestMachineChain_NotReadyNodesAreNotCounted(t *testing.T) { + notReady := k3dNode("k3d-t-agent-0", "10", measuredNodeMem) + notReady.Status.Conditions = []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionFalse}} + cs := fake.NewClientset(k3dNode("k3d-t-server-0", "10", measuredNodeMem), notReady) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) + // One Ready node claiming the VM is honest; the not-Ready one must not add + // a phantom second claim. + if got.Status != StatusOK { + t.Fatalf("one Ready + one NotReady => %v (%q), want ok", got.Status, got.Detail) + } + if !strings.Contains(got.Detail, "1 node claiming") { + t.Errorf("want singular '1 node claiming', got %q", got.Detail) + } +} + +func TestMachineChain_NoReadyNodesGetsNoVerdict(t *testing.T) { + n := k3dNode("k3d-t-server-0", "10", measuredNodeMem) + n.Status.Conditions = []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionFalse}} + cs := fake.NewClientset(n) + if got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { + t.Fatalf("no Ready node => %v, want unknown", got.Status) + } +} + +func TestMachineChain_MissingHostLevelStillReportsTheRest(t *testing.T) { + // The host is context; the VM is the constraint. Losing the host level + // must not cost the chain. + cs := fake.NewClientset(k3dNode("k3d-t-server-0", "10", measuredNodeMem)) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + if got.Status != StatusOK { + t.Fatalf("unreadable host => %v, want ok", got.Status) + } + if strings.Contains(got.Detail, "host ") { + t.Errorf("host level should be omitted, not zero-filled: %q", got.Detail) + } + if !strings.Contains(got.Detail, "Docker VM") { + t.Errorf("VM level must survive: %q", got.Detail) + } +} + +// ── the probes' own parsing ───────────────────────────────────────────────── + +func TestGib(t *testing.T) { + for _, tc := range []struct { + in int64 + want string + }{ + {0, "0 GiB"}, + {-1, "0 GiB"}, + {1 << 30, "1.00 GiB"}, + {measuredVMMem, "7.75 GiB"}, + {17179869184, "16.00 GiB"}, + } { + if got := gib(tc.in); got != tc.want { + t.Errorf("gib(%d) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestPlural(t *testing.T) { + if plural(1) != "" || plural(0) != "s" || plural(2) != "s" { + t.Error("plural should be empty only for 1") + } +} + +func TestMachineChain_UnrequestedIsMeasuredAgainstTheVMNotTheInflatedSum(t *testing.T) { + // Regression guard for the fourth level inheriting the third's error. + // Two uncapped nodes claim 15.50 GiB on a 7.75 GiB VM; with 3 GiB + // requested, the honest remainder is 4.75 GiB (VM - requests), NOT + // 12.50 GiB (sum - requests). Caught by running this against a live edge. + cs := fake.NewClientset( + k3dNode("k3d-t-server-0", "10", measuredNodeMem), + k3dNode("k3d-t-agent-0", "10", measuredNodeMem), + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "hog", Namespace: "tracebloc"}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "c", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + }, + }}}, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + }, + ) + got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + if !strings.Contains(got.Detail, "4.75 GiB unrequested") { + t.Fatalf("unrequested must be VM-bounded (7.75 - 3 = 4.75 GiB), got %q", got.Detail) + } + if strings.Contains(got.Detail, "12.50 GiB unrequested") { + t.Error("unrequested was computed from the inflated node sum") + } +} + +func TestMachineChain_OverRequestedNeverGoesNegative(t *testing.T) { + cs := fake.NewClientset( + k3dNode("k3d-t-server-0", "10", "8Gi"), + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "hog", Namespace: "tracebloc"}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "c", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("20Gi")}, + }, + }}}, + Status: corev1.PodStatus{Phase: corev1.PodRunning}, + }, + ) + got := checkMachineChain(bg(), cs, fixedVM(10, 8*(1<<30)), noHost()) + if !strings.Contains(got.Detail, "0 GiB unrequested") { + t.Fatalf("over-requested should floor at 0, got %q", got.Detail) + } +} From d0460e30f891479f794bf984eb902e58a001a0bd Mon Sep 17 00:00:00 2001 From: shujaat hasan Date: Thu, 20 Aug 2026 16:53:46 +0200 Subject: [PATCH 2/3] docs(doctor): name the real capping mechanism, measured (backend#2221) The tolerance comment said node capacity came from "the cgroup limit with a rounding that does not exactly invert". That is wrong about the mechanism, and the mechanism matters for the rest of this ticket. Measured: k3d --servers-memory 3g -> cgroup memory.max = 3221225472 k3d's fake /proc/meminfo -> MemTotal: 3221225 kB kubelet capacity -> 3221225Ki == 3298534400 B, +2.4% k3d caps a node by bind-mounting a SYNTHETIC /proc/meminfo into the node container (a "fakeowner" mount), not by the cgroup -- kubelet never reads the cgroup for capacity. k3d writes MemTotal as bytes/1000 labelled kB, but kB there means 1024 bytes, so advertised capacity overstates the real cgroup limit by 2.4%. The tolerance was right; the reason recorded beside it was not. Two consequences worth having written down, both measured: * capping is a CREATE-TIME operation. `docker update --memory=4g` on a running node container set memory.max=4294967296 and left /proc/meminfo at the VM's 8126672 kB, so capacity stayed 8126672Ki even after a restart. Existing edges cannot be capped in place; they have to be recreated. * a pod sized to the advertised capacity can exceed the node's real cgroup limit by 2.4%. The 3 GiB platform overhead absorbs that at any realistic cap, but it is a reason not to shrink the overhead casually. Comment-only; no behaviour change. Co-Authored-By: Claude Opus 5 --- internal/doctor/machine.go | 22 +++++++++++++++++----- internal/doctor/machine_test.go | 11 ++++++----- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/internal/doctor/machine.go b/internal/doctor/machine.go index 30208a5..b76c04a 100644 --- a/internal/doctor/machine.go +++ b/internal/doctor/machine.go @@ -132,11 +132,23 @@ func hostProbe() (int64, int64, error) { // this check calls it a double-count. // // Not slack for the real bug — that is 2.00x, and anything above ~1.05 is -// already unambiguous. It exists because node capacity is derived from the -// container's cgroup limit with a rounding that does not exactly invert: on a -// node capped at 3 GiB (3221225472 B) kubelet reported 3221225Ki, which is -// 2.4% ABOVE the limit it came from. A strict `sum > vm` would report that -// rounding as over-commit on a correctly capped cluster. +// already unambiguous. It exists because a CORRECTLY capped k3d node reports +// slightly MORE than its own limit, which was measured: +// +// k3d --servers-memory 3g -> cgroup memory.max = 3221225472 +// k3d's fake /proc/meminfo -> MemTotal: 3221225 kB +// kubelet capacity -> 3221225Ki == 3298534400 B, +2.4% +// +// k3d caps a node by bind-mounting a synthetic /proc/meminfo into the node +// container (a "fakeowner" mount) — NOT by the cgroup, which kubelet never +// reads. It writes MemTotal by dividing the byte limit by 1000 and labelling +// the result kB, but kB there means 1024 bytes, so the advertised capacity +// overstates the real limit by 2.4%. A strict `sum > vm` would therefore +// report correct capping as over-commit. +// +// The same measurement is why capping is a CREATE-TIME operation: `docker +// update --memory` on a running node container moves the cgroup and leaves +// /proc/meminfo alone, so node capacity does not budge even across a restart. const overCommitTolerance = 1.05 // checkMachineChain surfaces host -> VM -> node allocatable -> free, and warns diff --git a/internal/doctor/machine_test.go b/internal/doctor/machine_test.go index 43eaa62..1b7f026 100644 --- a/internal/doctor/machine_test.go +++ b/internal/doctor/machine_test.go @@ -97,11 +97,12 @@ func TestMachineChain_SingleNodeIsHonest(t *testing.T) { } func TestMachineChain_CappedNodesPassDespiteRounding(t *testing.T) { - // MEASURED: a node capped at 3 GiB (3221225472 B) reported capacity - // 3221225Ki, which is 2.4% ABOVE the limit it came from. Two of those sum - // to 0.79x of this VM, so the invariant holds — but a strict `sum > vm` - // would still have to survive that rounding, which is what the tolerance - // is for. This is the regression guard for capping correctly. + // MEASURED: `k3d --servers-memory 3g` gives cgroup memory.max 3221225472 + // and a synthetic /proc/meminfo reading "MemTotal: 3221225 kB", so kubelet + // reports 3221225Ki — 2.4% ABOVE the limit it came from, because k3d + // divides the byte limit by 1000 and labels it kB. Two of those sum to + // 0.79x of this VM, so the invariant holds, but a strict `sum > vm` would + // have to survive that overstatement. Regression guard for capping right. cs := fake.NewClientset( k3dNode("k3d-cap-server-0", "10", "3221225Ki"), k3dNode("k3d-cap-agent-0", "10", "3221225Ki"), From 6ebe9ae8982b653766b428097d29bfa1690b4aef Mon Sep 17 00:00:00 2001 From: shujaat hasan Date: Thu, 20 Aug 2026 16:58:55 +0200 Subject: [PATCH 3/3] fix(doctor): bound the machine probes, and refuse a remote k3d cluster (Bugbot #541) Two real defects from Bugbot, both reproduced with a failing test first. 1. The probes could hang the sweep. hostProbe ran `sysctl` through a bare exec.Command with no context at all, and dockerVMProbe took the signal ctx with no deadline of its own -- unlike httpProbeTimeout (8s) on the other injectable probe in this package. The sharp edge is that the remedy for an unreadable VM is `docker info`, which is exactly what hangs against a WEDGED daemon as opposed to a stopped one, so the probe for that case could hang `tracebloc doctor` with no verdict and no exit code. Both are now bounded by machineProbeTimeout, mirroring httpProbeTimeout; the installer's own _docker_answers bounds `docker info` for the same reason. HostProbe takes a context now, which is also the honest signature for something that shells out. 2. k3d node NAMES are not proof the cluster is on this machine. A kubeconfig pointing at another host's k3d cluster -- a LAN address, an SSH tunnel, a copied config -- has the same k3d-* names while `docker info` here describes an unrelated VM. That is the same error the EKS guard already refuses, seen from a direction I missed. Now a PRESENT, non-loopback API endpoint is a disqualifier and the check returns StatusUnknown, reusing the isLoopback this package already had. An EMPTY ServerURL deliberately still measures. It is documented as optional throughout this package, so treating absence as "remote" would let a missing Option silence a check that has node-name evidence to go on. Pinned by its own test, and mutation-testing confirms tightening the guard to `!isLoopback(serverURL)` reddens exactly that test. Re-verified on the live 2-node edge afterwards: served on loopback, so it is still measured and still reports 2.00x. Mutation-tested: dropping the remote guard reddens the remote test and nothing else; tightening it to reject an empty URL reddens the empty-URL test. One new string catalogued in zz-all-strings.golden. Co-Authored-By: Claude Opus 5 --- .../cli/testdata/golden/zz-all-strings.golden | 3 +- internal/doctor/doctor.go | 2 +- internal/doctor/machine.go | 43 +++++++- internal/doctor/machine_test.go | 103 +++++++++++++++--- 4 files changed, 126 insertions(+), 25 deletions(-) diff --git a/internal/cli/testdata/golden/zz-all-strings.golden b/internal/cli/testdata/golden/zz-all-strings.golden index c666d64..4878ceb 100644 --- a/internal/cli/testdata/golden/zz-all-strings.golden +++ b/internal/cli/testdata/golden/zz-all-strings.golden @@ -521,7 +521,6 @@ screen. %s/%d are runtime placeholders. "how much memory a single training run may use, in GiB" "http://%s.%s.svc.cluster.local:%d" "http://localhost:%d" -"hw.memsize" "image pull secret %q not found" "images" "images/ and annotations/ don't pair up: %s. Every image needs a same-named .xml annotation (and vice versa) — the cluster rejects mismatches after the upload." @@ -715,7 +714,6 @@ screen. %s/%d are runtime placeholders. "submit response missing job_name (got body %q)" "submit response missing namespace (got body %q)" "synthesized spec failed schema validation; check the flag values above" -"sysctl" "sysctl hw.memsize: %w" "table %q already exists in this secure environment. Re-ingesting the same table doesn't merge or replace — the run would fail after uploading everything. Re-run with --overwrite to replace it, or pick a different --name. (`tracebloc data delete %s` also removes it.)" "table name is %d characters; the max is %d (matches both the MySQL identifier limit and the Kubernetes label-value limit, which the stage Pod's tracebloc.io/table label is bound by). Use a shorter name." @@ -728,6 +726,7 @@ screen. %s/%d are runtime placeholders. "text category %q has no primary_subdir in the vendored layout contract — the Go registry has drifted from layout.v1.json; re-run scripts/sync-schema.sh" "that doesn't fit. This machine has %s · %s, and tracebloc keeps about %s and %s for itself, so one run can use at most %d cores and %d GiB. Try --cores %d --memory %d." "the client running in this namespace is anchored to a different cluster (%s) than --kubeconfig/--context points at (%s) — check you're targeting the right cluster" +"the cluster API is at %s, not this machine — the host/VM chain describes the Docker VM the CLI runs on, so it cannot speak for a remote k3d cluster" "the cluster API server at %s isn't answering — is the cluster running?" "the duration/time column name" "the label column %q has %d distinct value(s) — a classification dataset needs at least 2 classes. The cluster rejects this after the upload; check the labels and re-run." diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index f65608b..2fd9e32 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -198,7 +198,7 @@ func Run(ctx context.Context, cs kubernetes.Interface, opts Options) []Result { checkRestartHistory(ctx, cs, ns), checkPVC(ctx, cs, ns), checkNodeFit(ctx, cs, jmEnv), - checkMachineChain(ctx, cs, opts.VMProbe, opts.HostProbe), + checkMachineChain(ctx, cs, opts.ServerURL, opts.VMProbe, opts.HostProbe), checkImagePull(ctx, cs, ns, release), checkProxy(jmEnv), checkBackendEgress(ctx, jmEnv, opts.HTTPProbe), diff --git a/internal/doctor/machine.go b/internal/doctor/machine.go index b76c04a..7a2584f 100644 --- a/internal/doctor/machine.go +++ b/internal/doctor/machine.go @@ -8,6 +8,7 @@ import ( "runtime" "strconv" "strings" + "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -51,14 +52,26 @@ import ( // whose cluster cannot hold what it was sized for. type VMProbe func(ctx context.Context) (cores int64, memBytes int64, err error) +// machineProbeTimeout bounds the two subprocess probes below, mirroring +// httpProbeTimeout on the other injectable probe in this package. +// +// Bugbot on #541: without it, the remedy for an unreadable VM ("docker info") +// is the exact command that hangs against a WEDGED daemon — as opposed to a +// stopped one, which fails fast — so the probe for that case could hang the +// whole sweep with no verdict and no exit code. The installer's own +// _docker_answers bounds `docker info` for the same reason. +const machineProbeTimeout = 8 * time.Second + // HostProbe reports the physical machine's cores and RAM. Injected in tests; // nil means hostProbe. Best-effort: the chain drops this level rather than // failing when it cannot be read, because the VM is the binding constraint and // the host is context. -type HostProbe func() (cores int64, memBytes int64, err error) +type HostProbe func(ctx context.Context) (cores int64, memBytes int64, err error) // dockerVMProbe asks the container runtime how big its VM is. func dockerVMProbe(ctx context.Context) (int64, int64, error) { + ctx, cancel := context.WithTimeout(ctx, machineProbeTimeout) + defer cancel() // #nosec G204 -- argv is compile-time constant: literal "docker" with a // fixed --format template. No user input reaches this command line. out, err := exec.CommandContext(ctx, "docker", "info", "--format", "{{.NCPU}} {{.MemTotal}}").Output() @@ -86,12 +99,14 @@ func dockerVMProbe(ctx context.Context) (int64, int64, error) { // hostProbe reads physical RAM for the platforms the local install supports. // runtime.NumCPU() is the host's logical CPU count on both — the CLI runs on // the host, not inside the VM. -func hostProbe() (int64, int64, error) { +func hostProbe(ctx context.Context) (int64, int64, error) { cores := int64(runtime.NumCPU()) switch runtime.GOOS { case "darwin": + ctx, cancel := context.WithTimeout(ctx, machineProbeTimeout) + defer cancel() // #nosec G204 -- argv is compile-time constant. - out, err := exec.Command("sysctl", "-n", "hw.memsize").Output() + out, err := exec.CommandContext(ctx, "sysctl", "-n", "hw.memsize").Output() if err != nil { return cores, 0, fmt.Errorf("sysctl hw.memsize: %w", err) } @@ -153,7 +168,7 @@ const overCommitTolerance = 1.05 // checkMachineChain surfaces host -> VM -> node allocatable -> free, and warns // when the nodes claim more of the machine than the VM has. -func checkMachineChain(ctx context.Context, cs kubernetes.Interface, vm VMProbe, host HostProbe) Result { +func checkMachineChain(ctx context.Context, cs kubernetes.Interface, serverURL string, vm VMProbe, host HostProbe) Result { const name = "Machine capacity" nodes, err := cs.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) @@ -186,6 +201,24 @@ func checkMachineChain(ctx context.Context, cs kubernetes.Interface, vm VMProbe, } } + // k3d node NAMES are not proof the cluster is on THIS machine (Bugbot on + // #541): a kubeconfig pointing at another host's k3d cluster — a LAN + // address, an SSH tunnel, a copied config — has the same names while + // `docker info` here describes an unrelated VM. Comparing the two is the + // same error the EKS guard above refuses, so refuse it too. + // + // A PRESENT, non-loopback endpoint is the disqualifier. An empty ServerURL + // is documented as fine throughout this package, so it falls through and is + // measured on the node-name evidence — a missing Option must not silence a + // check that has something to go on. + if serverURL != "" && !isLoopback(serverURL) { + return Result{ + Name: name, + Status: StatusUnknown, + Detail: fmt.Sprintf("the cluster API is at %s, not this machine — the host/VM chain describes the Docker VM the CLI runs on, so it cannot speak for a remote k3d cluster", serverURL), + } + } + // CAPACITY, not allocatable: the invariant this check exists to state is // about what the nodes claim the machine IS. Making `allocatable` honest is // a separate question (kubelet reservations, RFC §P3) and k3s sets none, so @@ -231,7 +264,7 @@ func checkMachineChain(ctx context.Context, cs kubernetes.Interface, vm VMProbe, } chain := "" - if _, hostMem, herr := host(); herr == nil && hostMem > 0 { + if _, hostMem, herr := host(ctx); herr == nil && hostMem > 0 { chain = fmt.Sprintf("host %s → ", gib(hostMem)) } chain += fmt.Sprintf("Docker VM %s (%d cpu) → %d node%s claiming %s → %s unrequested", diff --git a/internal/doctor/machine_test.go b/internal/doctor/machine_test.go index 1b7f026..be96cef 100644 --- a/internal/doctor/machine_test.go +++ b/internal/doctor/machine_test.go @@ -21,6 +21,8 @@ const ( measuredVMMem = int64(8321712128) measuredVMCores = int64(10) measuredNodeMem = "8126672Ki" + // Every installer-created k3d cluster is served on loopback. + localAPI = "https://127.0.0.1:6550" ) // k3dNode builds a Ready k3d node container with CAPACITY set (the field the @@ -49,11 +51,11 @@ func failingVM(msg string) VMProbe { } func fixedHost(cores, mem int64) HostProbe { - return func() (int64, int64, error) { return cores, mem, nil } + return func(context.Context) (int64, int64, error) { return cores, mem, nil } } func noHost() HostProbe { - return func() (int64, int64, error) { return 0, 0, errors.New("unsupported") } + return func(context.Context) (int64, int64, error) { return 0, 0, errors.New("unsupported") } } // ── the bug, at the numbers it was measured at ─────────────────────────────── @@ -64,7 +66,7 @@ func TestMachineChain_TwoUncappedNodesDoubleCountTheVM(t *testing.T) { k3dNode("k3d-tracebloc-server-0", "10", measuredNodeMem), k3dNode("k3d-tracebloc-agent-0", "10", measuredNodeMem), ) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) if got.Status != StatusWarn { t.Fatalf("two uncapped nodes on one VM => %v (%q), want warn", got.Status, got.Detail) @@ -87,7 +89,7 @@ func TestMachineChain_TwoUncappedNodesDoubleCountTheVM(t *testing.T) { func TestMachineChain_SingleNodeIsHonest(t *testing.T) { // One node container reporting the whole VM is not a lie — it IS the VM. cs := fake.NewClientset(k3dNode("k3d-tracebloc-server-0", "10", measuredNodeMem)) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) if got.Status != StatusOK { t.Fatalf("single node claiming exactly the VM => %v (%q), want ok", got.Status, got.Detail) } @@ -107,7 +109,7 @@ func TestMachineChain_CappedNodesPassDespiteRounding(t *testing.T) { k3dNode("k3d-cap-server-0", "10", "3221225Ki"), k3dNode("k3d-cap-agent-0", "10", "3221225Ki"), ) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()) if got.Status != StatusOK { t.Fatalf("correctly capped nodes => %v (%q), want ok", got.Status, got.Detail) } @@ -117,7 +119,7 @@ func TestMachineChain_RoundingAloneNeverWarns(t *testing.T) { // A single node whose reported capacity sits just above the VM by the same // rounding must not be called a double-count. cs := fake.NewClientset(k3dNode("k3d-x-server-0", "10", fmt.Sprintf("%d", measuredVMMem+measuredVMMem/50))) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()) if got.Status != StatusOK { t.Fatalf("2%% over the VM is rounding, not over-commit: got %v (%q)", got.Status, got.Detail) } @@ -129,7 +131,7 @@ func TestMachineChain_ThreeNodesReportTheirRatio(t *testing.T) { k3dNode("k3d-t-agent-0", "10", measuredNodeMem), k3dNode("k3d-t-agent-1", "10", measuredNodeMem), ) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()) if got.Status != StatusWarn { t.Fatalf("three uncapped nodes => %v, want warn", got.Status) } @@ -160,7 +162,7 @@ func TestMachineChain_UnrequestedSubtractsLiveRequests(t *testing.T) { pod("done", "4Gi", corev1.PodSucceeded), pod("dead", "4Gi", corev1.PodFailed), ) - got := checkMachineChain(bg(), cs, fixedVM(10, 8*(1<<30)), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(10, 8*(1<<30)), noHost()) if !strings.Contains(got.Detail, "6.00 GiB unrequested") { t.Fatalf("8Gi capacity - 2Gi live request should leave 6.00 GiB, got %q", got.Detail) } @@ -175,7 +177,7 @@ func TestMachineChain_NonK3dClusterGetsNoVerdict(t *testing.T) { k3dNode("ip-10-0-1-23.eu-central-1.compute.internal", "8", "32Gi"), k3dNode("ip-10-0-1-24.eu-central-1.compute.internal", "8", "32Gi"), ) - got := checkMachineChain(bg(), cs, fixedVM(2, 4*(1<<30)), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(2, 4*(1<<30)), noHost()) if got.Status != StatusUnknown { t.Fatalf("non-k3d cluster => %v (%q), want unknown", got.Status, got.Detail) } @@ -186,14 +188,14 @@ func TestMachineChain_MixedClusterGetsNoVerdict(t *testing.T) { k3dNode("k3d-t-server-0", "10", measuredNodeMem), k3dNode("ip-10-0-1-24.eu-central-1.compute.internal", "8", "32Gi"), ) - if got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { + if got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { t.Fatalf("mixed cluster => %v, want unknown", got.Status) } } func TestMachineChain_UnreadableVMGetsNoVerdict(t *testing.T) { cs := fake.NewClientset(k3dNode("k3d-t-server-0", "10", measuredNodeMem)) - got := checkMachineChain(bg(), cs, failingVM("docker daemon not running"), noHost()) + got := checkMachineChain(bg(), cs, localAPI, failingVM("docker daemon not running"), noHost()) if got.Status != StatusUnknown { t.Fatalf("unreadable VM => %v, want unknown (the VM is the level this check adds)", got.Status) } @@ -208,7 +210,7 @@ func TestMachineChain_NoCapacityReportedGetsNoVerdict(t *testing.T) { n := k3dNode("k3d-t-server-0", "10", measuredNodeMem) n.Status.Capacity = nil cs := fake.NewClientset(n) - if got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { + if got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { t.Fatalf("no capacity => %v (%q), want unknown", got.Status, got.Detail) } } @@ -217,7 +219,7 @@ func TestMachineChain_NotReadyNodesAreNotCounted(t *testing.T) { notReady := k3dNode("k3d-t-agent-0", "10", measuredNodeMem) notReady.Status.Conditions = []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionFalse}} cs := fake.NewClientset(k3dNode("k3d-t-server-0", "10", measuredNodeMem), notReady) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), fixedHost(10, 17179869184)) // One Ready node claiming the VM is honest; the not-Ready one must not add // a phantom second claim. if got.Status != StatusOK { @@ -232,7 +234,7 @@ func TestMachineChain_NoReadyNodesGetsNoVerdict(t *testing.T) { n := k3dNode("k3d-t-server-0", "10", measuredNodeMem) n.Status.Conditions = []corev1.NodeCondition{{Type: corev1.NodeReady, Status: corev1.ConditionFalse}} cs := fake.NewClientset(n) - if got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { + if got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()); got.Status != StatusUnknown { t.Fatalf("no Ready node => %v, want unknown", got.Status) } } @@ -241,7 +243,7 @@ func TestMachineChain_MissingHostLevelStillReportsTheRest(t *testing.T) { // The host is context; the VM is the constraint. Losing the host level // must not cost the chain. cs := fake.NewClientset(k3dNode("k3d-t-server-0", "10", measuredNodeMem)) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()) if got.Status != StatusOK { t.Fatalf("unreadable host => %v, want ok", got.Status) } @@ -297,7 +299,7 @@ func TestMachineChain_UnrequestedIsMeasuredAgainstTheVMNotTheInflatedSum(t *test Status: corev1.PodStatus{Phase: corev1.PodRunning}, }, ) - got := checkMachineChain(bg(), cs, fixedVM(measuredVMCores, measuredVMMem), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(measuredVMCores, measuredVMMem), noHost()) if !strings.Contains(got.Detail, "4.75 GiB unrequested") { t.Fatalf("unrequested must be VM-bounded (7.75 - 3 = 4.75 GiB), got %q", got.Detail) } @@ -320,8 +322,75 @@ func TestMachineChain_OverRequestedNeverGoesNegative(t *testing.T) { Status: corev1.PodStatus{Phase: corev1.PodRunning}, }, ) - got := checkMachineChain(bg(), cs, fixedVM(10, 8*(1<<30)), noHost()) + got := checkMachineChain(bg(), cs, localAPI, fixedVM(10, 8*(1<<30)), noHost()) if !strings.Contains(got.Detail, "0 GiB unrequested") { t.Fatalf("over-requested should floor at 0, got %q", got.Detail) } } + +// ── Bugbot on #541: a remote k3d cluster is not this machine ───────────────── + +func TestMachineChain_RemoteK3dClusterGetsNoVerdict(t *testing.T) { + // A kubeconfig pointing at ANOTHER host's k3d cluster (LAN IP, SSH tunnel, + // a copied config) still has k3d-* node names, but `docker info` here + // describes an unrelated VM. That is the same error this check already + // refuses on EKS, so it must refuse here too rather than compare a remote + // cluster's nodes to a local VM. + cs := fake.NewClientset( + k3dNode("k3d-tracebloc-server-0", "10", measuredNodeMem), + k3dNode("k3d-tracebloc-agent-0", "10", measuredNodeMem), + ) + got := checkMachineChain(bg(), cs, "https://192.168.1.50:6443", fixedVM(measuredVMCores, measuredVMMem), noHost()) + if got.Status != StatusUnknown { + t.Fatalf("remote k3d cluster => %v (%q), want unknown", got.Status, got.Detail) + } +} + +func TestMachineChain_LoopbackK3dIsStillMeasured(t *testing.T) { + // The local case must keep working — every k3d cluster the installer + // creates is served on 127.0.0.1. + cs := fake.NewClientset( + k3dNode("k3d-tracebloc-server-0", "10", measuredNodeMem), + k3dNode("k3d-tracebloc-agent-0", "10", measuredNodeMem), + ) + got := checkMachineChain(bg(), cs, "https://127.0.0.1:6550", fixedVM(measuredVMCores, measuredVMMem), noHost()) + if got.Status != StatusWarn { + t.Fatalf("loopback k3d => %v (%q), want the warn", got.Status, got.Detail) + } +} + +func TestMachineChain_EmptyServerURLStillMeasures(t *testing.T) { + // ServerURL is documented as optional throughout this package. An absent + // one must not silence a check that has k3d node names to go on — that + // would turn a missing Option into a lost verdict. + cs := fake.NewClientset(k3dNode("k3d-t-server-0", "10", measuredNodeMem)) + got := checkMachineChain(bg(), cs, "", fixedVM(measuredVMCores, measuredVMMem), noHost()) + if got.Status != StatusOK { + t.Fatalf("empty ServerURL => %v (%q), want ok", got.Status, got.Detail) + } +} + +// ── Bugbot on #541: the probes must not hang doctor ───────────────────────── + +func TestDockerVMProbe_RespectsADeadline(t *testing.T) { + // The remedy for an unreadable VM is "docker info" — the very command that + // hangs against a wedged daemon. An unbounded probe there would hang the + // whole sweep with no verdict and no exit code. + ctx, cancel := context.WithCancel(bg()) + cancel() + if _, _, err := dockerVMProbe(ctx); err == nil { + t.Fatal("a cancelled context must fail the probe, not block") + } +} + +func TestHostProbe_TakesAContext(t *testing.T) { + ctx, cancel := context.WithCancel(bg()) + cancel() + // Cores come from the Go runtime and are always available; memory needs a + // subprocess on darwin, which must honour the context. Either way this must + // return promptly rather than block. + cores, _, _ := hostProbe(ctx) + if cores <= 0 { + t.Errorf("cores should come from the runtime regardless of ctx, got %d", cores) + } +}