Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion internal/cli/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ func runClusterInfo(
// Discover the client's release — with the cluster-wide fallback scan
// when the namespace is just the kubeconfig default, so diagnostics find
// the client in its slug namespace instead of dead-ending on "default".
release, nsUsed, err := discoverRelease(ctx, p, cs, resolved.Namespace, binding.allowScan())
// leadRedirect=false: the "Kubeconfig" Section + context/server Fields above
// are already printed, so the multi-client redirect note stays inline between
// the server and namespace Fields — no mid-section blank (§380).
release, nsUsed, err := discoverRelease(ctx, p, cs, resolved.Namespace, binding.allowScan(), false)
if err != nil {
// 4 = "cluster reachable, but no tracebloc client here."
// Distinct from the kubeconfig error (3) so callers can
Expand Down
30 changes: 27 additions & 3 deletions internal/cli/clustertarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ type clusterTarget struct {
// `cluster doctor` is deliberately NOT a caller — it has a different exit
// contract (2/3 escalation, with discovery reported as a check Result rather
// than a hard error).
func resolveClusterTarget(ctx context.Context, p *ui.Printer, opts cluster.KubeconfigOptions, b activeClientBinding, needPVC bool) (*clusterTarget, error) {
// leadRedirect threads to discoverRelease: pass true when this resolve is the
// command's first output (resources show/set, data list, seal — the redirect
// note then self-leads its one clean leading blank), false when the command has
// already printed something before resolving (data ingest's "Connecting…", data
// delete's warning — the note stays inline, no mid-output blank). See §380.
func resolveClusterTarget(ctx context.Context, p *ui.Printer, opts cluster.KubeconfigOptions, b activeClientBinding, needPVC, leadRedirect bool) (*clusterTarget, error) {
resolved, err := loadClusterFn(opts)
if err != nil {
return nil, &exitError{code: exitLocalEnv, err: fmt.Errorf("loading kubeconfig: %w", err)}
Expand All @@ -76,7 +81,7 @@ func resolveClusterTarget(ctx context.Context, p *ui.Printer, opts cluster.Kubec
// --namespace/--context) and not the active-client binding. A binding miss
// must NOT silently redirect to some other client (§7.5 — that could be a
// different machine's client); it keeps the §7.3 "runs elsewhere" message.
release, nsUsed, err := discoverRelease(ctx, p, cs, resolved.Namespace, b.allowScan())
release, nsUsed, err := discoverRelease(ctx, p, cs, resolved.Namespace, b.allowScan(), leadRedirect)
if err != nil {
// Only a genuine "namespace has no release" maps to the §7.3
// "runs elsewhere" rewrite; an API/RBAC list failure or an
Expand Down Expand Up @@ -107,7 +112,16 @@ func resolveClusterTarget(ctx context.Context, p *ui.Printer, opts cluster.Kubec
// (never a silent redirect); several → name them and ask the user to pick;
// none, or a scan failure (e.g. RBAC forbids the cluster-wide list) → the
// original discovery error stands. Returns the namespace actually used.
func discoverRelease(ctx context.Context, p *ui.Printer, cs kubernetes.Interface, namespace string, allowScan bool) (*cluster.ParentRelease, string, error) {
//
// leadRedirect says the redirect note, when it fires, is the command's FIRST
// output — so it self-leads a single blank to separate it from the shell prompt
// (§380: gives resources show/set/data list/seal one clean leading blank on the
// multi-client path without a pre-resolve Newline() that would stack into a
// double). Commands that print BEFORE resolving — cluster info's "Kubeconfig"
// section, data ingest's "Connecting…" line, data delete's warning paragraph —
// pass false so the note stays inline and never splits their output with a
// mid-blank.
func discoverRelease(ctx context.Context, p *ui.Printer, cs kubernetes.Interface, namespace string, allowScan, leadRedirect bool) (*cluster.ParentRelease, string, error) {
release, err := cluster.DiscoverParentRelease(ctx, cs, namespace)
if err == nil || !allowScan || !errors.Is(err, cluster.ErrNoParentRelease) {
return release, namespace, err
Expand Down Expand Up @@ -140,6 +154,16 @@ func discoverRelease(ctx context.Context, p *ui.Printer, cs kubernetes.Interface
cluster.ErrNoParentRelease, namespace, strings.Join(found, ", "))
}
if p != nil {
// Self-lead with a single blank ONLY when this redirect is the command's
// opening line (leadRedirect) — resolve-first commands (resources
// show/set, data list, seal) then get exactly one leading blank on the
// multi-client path without a pre-resolve Newline() that would stack into
// a double (§380). Output-first commands (cluster info, data ingest, data
// delete) pass leadRedirect=false so the note stays inline and doesn't
// split their already-open output with a mid-blank.
if leadRedirect {
p.Newline()
}
p.Infof("No client in namespace %q — using the one in %q (override with --namespace).", namespace, found[0])
}
release, err = cluster.DiscoverParentRelease(ctx, cs, found[0])
Expand Down
22 changes: 15 additions & 7 deletions internal/cli/clustertarget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func withClusterSeams(t *testing.T, cs kubernetes.Interface) {
func TestResolveClusterTarget_NoClient_InstallerMessageExit4(t *testing.T) {
withClusterSeams(t, fake.NewSimpleClientset()) // empty cluster
_, err := resolveClusterTarget(context.Background(), nil,
cluster.KubeconfigOptions{}, activeClientBinding{}, true)
cluster.KubeconfigOptions{}, activeClientBinding{}, true, true)
if err == nil {
t.Fatal("expected an error when the cluster hosts no client")
}
Expand All @@ -67,7 +67,7 @@ func TestResolveClusterTarget_NoClient_InstallerMessageExit4(t *testing.T) {
func TestResolveClusterTarget_MultipleClients_PickOneExit4(t *testing.T) {
withClusterSeams(t, fake.NewSimpleClientset(jmDep("alpha"), jmDep("beta")))
_, err := resolveClusterTarget(context.Background(), nil,
cluster.KubeconfigOptions{}, activeClientBinding{}, true)
cluster.KubeconfigOptions{}, activeClientBinding{}, true, true)
if err == nil {
t.Fatal("expected an error when multiple clients are present")
}
Expand Down Expand Up @@ -214,7 +214,7 @@ func TestDiscoverRelease_ScanFindsSingleClientElsewhere(t *testing.T) {
cs := fake.NewSimpleClientset(jmDep("lukas-01"))
var buf bytes.Buffer
p := ui.New(&buf, ui.WithColor(false))
release, nsUsed, err := discoverRelease(context.Background(), p, cs, "default", true)
release, nsUsed, err := discoverRelease(context.Background(), p, cs, "default", true, true)
if err != nil {
t.Fatalf("expected scan to find the client, got: %v", err)
}
Expand All @@ -228,11 +228,19 @@ func TestDiscoverRelease_ScanFindsSingleClientElsewhere(t *testing.T) {
if !strings.Contains(buf.String(), "lukas-01") {
t.Errorf("expected a visible note about the redirect, got: %q", buf.String())
}
// §380: with leadRedirect=true (resolve-first callers) the redirect self-leads
// with exactly one blank, so a resolve-first command gets one leading blank
// without a pre-resolve Newline() that would stack into a double. The output
// must open with "\n " (one blank, then the indented note) — anything opening
// with "\n\n" already fails this check, so no separate "\n\n" guard is needed.
if !strings.HasPrefix(buf.String(), "\n ") {
t.Errorf("redirect must self-lead with exactly one blank line, got: %q", buf.String())
}
}

func TestDiscoverRelease_ScanMultipleNamespacesRefuses(t *testing.T) {
cs := fake.NewSimpleClientset(jmDep("alpha"), jmDep("beta"))
_, _, err := discoverRelease(context.Background(), nil, cs, "default", true)
_, _, err := discoverRelease(context.Background(), nil, cs, "default", true, false)
if err == nil {
t.Fatal("expected an error for multiple client namespaces")
}
Expand All @@ -250,7 +258,7 @@ func TestDiscoverRelease_NoScanWhenExplicit(t *testing.T) {
// The client exists in lukas-01, but the caller pinned the namespace —
// the scan must NOT engage and the plain discovery error stands.
cs := fake.NewSimpleClientset(jmDep("lukas-01"))
_, nsUsed, err := discoverRelease(context.Background(), nil, cs, "default", false)
_, nsUsed, err := discoverRelease(context.Background(), nil, cs, "default", false, false)
if err == nil {
t.Fatal("expected the namespace miss to stand when scan is disallowed")
}
Expand All @@ -268,7 +276,7 @@ func TestDiscoverRelease_NoScanWhenExplicit(t *testing.T) {
// ErrNoParentRelease so the exit-4 mapping holds.
func TestDiscoverRelease_ScanFindsNothing_InstallerGuidance(t *testing.T) {
cs := fake.NewSimpleClientset() // empty cluster — scan succeeds, finds nothing
_, _, err := discoverRelease(context.Background(), nil, cs, "default", true)
_, _, err := discoverRelease(context.Background(), nil, cs, "default", true, false)
if err == nil {
t.Fatal("expected an error on an empty cluster")
}
Expand Down Expand Up @@ -303,7 +311,7 @@ func TestDiscoverRelease_ScanUnavailable_KeepsOriginalError(t *testing.T) {
}
return false, nil, nil
})
_, _, err := discoverRelease(context.Background(), nil, cs, "default", true)
_, _, err := discoverRelease(context.Background(), nil, cs, "default", true, false)
if err == nil {
t.Fatal("expected an error when the scan can't run")
}
Expand Down
5 changes: 4 additions & 1 deletion internal/cli/data_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,10 @@ undone — re-ingesting the data is the only way back.`)
// running teardown against a cluster with no tracebloc install.
opts := cluster.KubeconfigOptions{Path: a.Kubeconfig, Context: a.Context, Namespace: a.Namespace}
binding := bindActiveClientNamespace(&opts)
target, err := resolveClusterTargetFn(ctx, a.Printer, opts, binding, true)
// leadRedirect=false: the warning paragraph above is already this command's
// opening output, so the multi-client redirect note stays inline — no
// mid-output blank between the warning and the note (§380).
target, err := resolveClusterTargetFn(ctx, a.Printer, opts, binding, true, false)
if err != nil {
return binding.explain(err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/data_delete_execute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestRunDataDelete_Execute(t *testing.T) {
resolveClusterTargetFn, listDatasetsFn, teardownFn = origRCT, origList, origTD
})

resolveClusterTargetFn = func(_ context.Context, _ *ui.Printer, _ cluster.KubeconfigOptions, _ activeClientBinding, _ bool) (*clusterTarget, error) {
resolveClusterTargetFn = func(_ context.Context, _ *ui.Printer, _ cluster.KubeconfigOptions, _ activeClientBinding, _, _ bool) (*clusterTarget, error) {
return &clusterTarget{
Resolved: &cluster.ResolvedConfig{Context: "ctx", Namespace: "tracebloc"},
Clientset: fake.NewSimpleClientset(),
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/data_delete_json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestRunDataDelete_OutputJSON(t *testing.T) {
resolveClusterTargetFn, listDatasetsFn, teardownFn = origRCT, origList, origTD
})

resolveClusterTargetFn = func(_ context.Context, _ *ui.Printer, _ cluster.KubeconfigOptions, _ activeClientBinding, _ bool) (*clusterTarget, error) {
resolveClusterTargetFn = func(_ context.Context, _ *ui.Printer, _ cluster.KubeconfigOptions, _ activeClientBinding, _, _ bool) (*clusterTarget, error) {
return &clusterTarget{
Resolved: &cluster.ResolvedConfig{Context: "ctx", Namespace: "tracebloc"},
Clientset: fake.NewSimpleClientset(),
Expand Down
5 changes: 4 additions & 1 deletion internal/cli/data_ingest_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ func connectIngestTarget(ctx context.Context, a *runDataIngestArgs) (target *clu
// Bound before we waste time provisioning a Pod that can't mount it.
opts := cluster.KubeconfigOptions{Path: a.Kubeconfig, Context: a.Context, Namespace: a.Namespace}
binding := bindActiveClientNamespace(&opts)
target, err = resolveClusterTarget(ctx, a.Printer, opts, binding, true)
// leadRedirect=false: the "Connecting…" line above is already this command's
// opening output, so the multi-client redirect note stays inline — no
// mid-output blank between "Connecting…" and the note (§380).
target, err = resolveClusterTarget(ctx, a.Printer, opts, binding, true, false)
if err != nil {
return nil, "", false, binding.explain(err)
}
Expand Down
4 changes: 3 additions & 1 deletion internal/cli/data_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ func runDataList(ctx context.Context, a runDataListArgs) (err error) {

opts := cluster.KubeconfigOptions{Path: a.Kubeconfig, Context: a.Context, Namespace: a.Namespace}
binding := bindActiveClientNamespace(&opts)
target, err := resolveClusterTarget(ctx, p, opts, binding, false)
// leadRedirect=true: data list prints nothing before resolving, so the
// multi-client redirect note is the opening line and self-leads its blank.
target, err := resolveClusterTarget(ctx, p, opts, binding, false, true)
if err != nil {
return binding.explain(err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ func realProbeEnv(ctx context.Context) envProbe {
return envProbe{local: localUnreachable}
}

release, nsUsed, err := discoverRelease(ctx, nil, cs, resolved.Namespace, binding.allowScan())
release, nsUsed, err := discoverRelease(ctx, nil, cs, resolved.Namespace, binding.allowScan(), false)
if err != nil {
if errors.Is(err, cluster.ErrNoParentRelease) {
// Cluster reachable, but this release isn't in the resolved context.
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/home_local_fallback.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func localEnvFallback(ctx context.Context) envProbe {
}
// Namespace-only discovery — never the cluster-wide scan, mirroring the
// gate's no-silent-retarget rule even on a local cluster.
release, nsUsed, err := discoverRelease(ctx, nil, cs, resolved.Namespace, false)
release, nsUsed, err := discoverRelease(ctx, nil, cs, resolved.Namespace, false, false)
if err != nil {
return envProbe{local: localNoRelease}
}
Expand Down
15 changes: 12 additions & 3 deletions internal/cli/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,14 @@ Exit codes:
// the jobs-manager env — the same source `cluster doctor` parses, so the two
// never disagree.
func runResourcesShow(ctx context.Context, p *ui.Printer, opts cluster.KubeconfigOptions) error {
p.Newline()

// The leading blank now lives in renderResources (before the first Stat), not
// here before resolve (§380): the resolve-time redirect in discoverRelease
// self-leads its own blank, so a pre-resolve Newline() here would stack a
// second one onto it in the multi-client case.
binding := bindActiveClientNamespace(&opts)
target, err := resolveClusterTargetFn(ctx, p, opts, binding, false)
// leadRedirect=true: this resolve is the command's first output, so the
// multi-client redirect note self-leads its one leading blank (§380).
target, err := resolveClusterTargetFn(ctx, p, opts, binding, false, true)
if err != nil {
return binding.explain(err)
}
Expand Down Expand Up @@ -125,6 +129,11 @@ func renderResources(ctx context.Context, p *ui.Printer, target *clusterTarget)
train.HasGPU = false
}

// One leading blank before the view. It lives here (not in runResourcesShow
// before resolve) so it doesn't stack on the now-self-leading resolve-time
// redirect in the multi-client case (§380). Stat does not self-lead, so this
// is the view's only opening blank.
p.Newline()
if nodeErr != nil {
p.Stat("Your secure environment is equipped with:", "unavailable")
p.Hintf(" couldn't read capacity: %v", nodeErr)
Expand Down
7 changes: 6 additions & 1 deletion internal/cli/resources_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,12 @@ func runResourcesSet(ctx context.Context, p *ui.Printer, pr prompter, opts clust
}

binding := bindActiveClientNamespace(&opts)
target, err := resolveClusterTarget(ctx, p, opts, binding, false)
// leadRedirect=true: resolve is the command's first output. On the
// multi-client path the redirect note self-leads its one blank; on the
// single-client path there's no note and the confirm/dry-run self-lead
// supplies the only leading blank — so `set` keeps NO pre-resolve Newline()
// and never regresses the #375 double-blank (§380).
target, err := resolveClusterTarget(ctx, p, opts, binding, false, true)
if err != nil {
return binding.explain(err)
}
Expand Down
42 changes: 42 additions & 0 deletions internal/cli/resources_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,3 +741,45 @@ func TestSet_ConfirmOpensWithSingleBlank(t *testing.T) {
t.Errorf("confirm path opens with a DOUBLE blank line (banner-removal regression): %q", head)
}
}

// TestSet_MultiClientRedirectOpensWithSingleBlank is the §380 multi-client mirror
// of TestSet_ConfirmOpensWithSingleBlank: it drives the OUTER runResourcesSet
// through the REAL resolveClusterTarget (loadClusterFn/newClientsetFn seams — NOT
// applyResourcesSet directly, the way runSet does), so discoverRelease's redirect
// actually fires. The kubeconfig default namespace hosts no client; the scan
// finds exactly one elsewhere; the redirect note (leadRedirect=true) self-leads
// its single blank, then the confirm PromptHint self-leads its own. The open must
// be exactly ONE leading blank then the note — never zero (the original
// "missing lead blank on set" bug) and never the #375 double.
func TestSet_MultiClientRedirectOpensWithSingleBlank(t *testing.T) {
t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // no active-client binding → scan allowed
fakeHelm(t)
// Ready node for the fit-check, plus a chart-labeled jobs-manager in a
// NON-default namespace so the scan retargets there.
cs := fake.NewClientset(resNode("n1", "8", "32Gi"), jmDep("lukas-01"))
withClusterSeams(t, cs)

var buf bytes.Buffer
// No --context/--namespace so allowScan stays true; a flag-driven change
// (cores 4, up from the chart-default 2) so it's not a no-op and reaches the
// confirm the proceedingPrompter accepts.
err := runResourcesSet(context.Background(), ui.New(&buf, ui.WithColor(false)), proceedingPrompter{},
cluster.KubeconfigOptions{}, setReq{cores: "4", coresSet: true})
if err != nil {
t.Fatalf("runResourcesSet (multi-client): %v\n%s", err, buf.String())
}
out := buf.String()
if !strings.Contains(out, "lukas-01") {
t.Fatalf("redirect must actually run (note naming the retargeted namespace), got:\n%s", out)
}
head := out
if len(head) > 64 {
head = head[:64]
}
if !strings.HasPrefix(out, "\n ") {
t.Errorf("multi-client set must open with a single blank then the redirect, got %q", head)
}
if strings.HasPrefix(out, "\n\n") {
t.Errorf("multi-client set opens with a DOUBLE blank line: %q", head)
}
}
Loading
Loading