From a2a08012c43bdd0f76b688c2a6f1964355dec645 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 24 Jul 2026 14:58:34 +0200 Subject: [PATCH] fix(resources): clamp wizard defaults into their own valid range (#398) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a machine that shrank under the configured ceiling (WSL2 field case: node ~6.7 GiB under a lingering chart-default 8Gi RESOURCE_LIMITS), the 'Choose an amount' prompts offered the current ceiling as the default — outside their own validator range — so pressing Enter on '(8)' answered 'must be between 2 and 3'. Defaults are now clamped into [floor, max], and the header annotates an over-ceiling budget instead of printing a bare '8 of 6.7 GiB'. Co-Authored-By: Claude Opus 4.8 --- internal/cli/resources_set.go | 44 ++++++++++++++++++++++++++---- internal/cli/resources_set_test.go | 33 ++++++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/internal/cli/resources_set.go b/internal/cli/resources_set.go index 2df0733..34010af 100644 --- a/internal/cli/resources_set.go +++ b/internal/cli/resources_set.go @@ -364,11 +364,27 @@ func desiredFromFlags(req setReq, current resources.Training, gpuName corev1.Res func runResourcesWizard(p *ui.Printer, pr prompter, node resources.Machine, current resources.Training, gpuName corev1.ResourceName, gpuCount int64, machineHasGPU bool) (resources.Training, error) { maxCores := resources.MaxRunCores(node) maxGiB := resources.MaxRunGiB(node) - - // (1) Current per-run budget vs the machine. + // The configured budget floored to the wizard's whole-unit domain — used to + // clamp prompt defaults and annotate the header (#398). `current` is the + // cluster-wide ceiling (chart default 8Gi), which a machine can shrink UNDER + // (the WSL2 field case: node ~6.7 GiB under a lingering 8Gi ceiling). + curCores := int(current.CPU.MilliValue() / 1000) + curGiB := int(current.Mem.Value() >> 30) + + // (1) Current per-run budget vs the machine. When the configured budget + // exceeds what this machine can still give one run, say so instead of + // printing a bare ">100%" line like "8 of 6.7 GiB" (#398). p.Section("How much of this machine a training run may use") - p.Field("CPU", fmt.Sprintf("%s of %s cores", coresNum(current.CPU), coresNum(node.CPU))) - p.Field("Memory", fmt.Sprintf("%s of %s GiB", gibNum(current.Mem), gibNum(node.Mem))) + cpuLine := fmt.Sprintf("%s of %s cores", coresNum(current.CPU), coresNum(node.CPU)) + if curCores > maxCores { + cpuLine += " — more than this machine can give one run now" + } + p.Field("CPU", cpuLine) + memLine := fmt.Sprintf("%s of %s GiB", gibNum(current.Mem), gibNum(node.Mem)) + if curGiB > maxGiB { + memLine += " — more than this machine can give one run now" + } + p.Field("Memory", memLine) if machineHasGPU { p.Field("GPU", fmt.Sprintf("%d of %d", currentGPUCount(current), gpuCount)) } @@ -411,17 +427,21 @@ func runResourcesWizard(p *ui.Printer, pr prompter, node resources.Machine, curr "overhead it can offer a training run at most %d core(s) and %d GiB. Free up "+ "resources or use a larger machine.", maxCores, maxGiB)} } + // The offered defaults are the current budget CLAMPED into each prompt's own + // valid range (#398): unclamped, a machine that shrank under the configured + // ceiling offered a default its own validator then rejected — pressing Enter + // on "(8)" answered "must be between 2 and 3". coresAns, err := pr.Input( fmt.Sprintf("CPU cores for one run (1–%d)", maxCores), "how many CPU cores a single training run may use", - coresNum(current.CPU), boundedInt(1, maxCores)) + strconv.Itoa(clampInt(curCores, 1, maxCores)), boundedInt(1, maxCores)) if err != nil { return resources.Training{}, err } memAns, err := pr.Input( fmt.Sprintf("Memory for one run in GiB (2–%d)", maxGiB), "how much memory a single training run may use, in GiB", - gibNum(current.Mem), boundedInt(2, maxGiB)) + strconv.Itoa(clampInt(curGiB, 2, maxGiB)), boundedInt(2, maxGiB)) if err != nil { return resources.Training{}, err } @@ -602,6 +622,18 @@ func sameCeiling(a, b resources.Training) bool { func coresNum(q resource.Quantity) string { return strings.TrimSuffix(resources.FormatCPU(q), " CPU") } func gibNum(q resource.Quantity) string { return strings.TrimSuffix(resources.FormatMem(q), " GiB") } +// clampInt limits v to [lo, hi]. Prompt defaults must sit inside their own valid +// range, or accepting the offer is rejected by its own validator (#398). +func clampInt(v, lo, hi int) int { + if v < lo { + return lo + } + if v > hi { + return hi + } + return v +} + func currentGPUCount(t resources.Training) int64 { if t.HasGPU { return t.GPU.Value() diff --git a/internal/cli/resources_set_test.go b/internal/cli/resources_set_test.go index 75103c6..20407e4 100644 --- a/internal/cli/resources_set_test.go +++ b/internal/cli/resources_set_test.go @@ -333,6 +333,39 @@ func TestWizard_ChooseAnAmount(t *testing.T) { } } +// TestWizard_DefaultsClampedToShrunkMachine (#398): on a machine that shrank +// under the configured ceiling (the WSL2 field case: node smaller than the +// chart-default 8Gi RESOURCE_LIMITS), pressing Enter on every prompt must +// succeed — the offered defaults are clamped into their own valid ranges. +// Before the clamp, accepting the memory default "(8)" was rejected by its own +// validator ("must be between 2 and N"). +func TestWizard_DefaultsClampedToShrunkMachine(t *testing.T) { + pr := &fakePrompter{ + answers: map[string]string{ + "How much may one training run use?": "Choose an amount", + // CPU + memory prompts deliberately NOT scripted: the fake returns + // each prompt's DEFAULT — exactly "the user pressed Enter". + }, + confirm: boolPtr(true), + } + // 12-CPU / 7-GiB node → maxCores 11, maxGiB 4; current cpu=2 fits, memory + // 8Gi exceeds → default must clamp 8 → 4. + cs := csWith("12", "7Gi", map[string]string{"RESOURCE_LIMITS": "cpu=2,memory=8Gi"}) + var buf bytes.Buffer + err := applyResourcesSet(context.Background(), ui.New(&buf, ui.WithColor(false)), pr, + setTarget(cs), cluster.KubeconfigOptions{}, setReq{dryRun: true}) + if err != nil { + t.Fatalf("Enter-on-defaults must succeed on a shrunk machine: %v", err) + } + if !strings.Contains(buf.String(), "cpu=2,memory=4Gi") { + t.Errorf("clamped defaults should apply cpu=2,memory=4Gi:\n%s", buf.String()) + } + // The header must not present the stale ceiling as a bare ">100%" fact. + if !strings.Contains(buf.String(), "more than this machine can give one run now") { + t.Errorf("header should annotate the over-ceiling budget:\n%s", buf.String()) + } +} + // TestWizard_ChooseAnAmountTooSmallMachine: on a machine too small to give a run // even the 1-core / 2-GiB minimum after tracebloc's overhead, the "Choose an // amount" path must NOT prompt an impossible range (e.g. "1–0") that rejects