diff --git a/internal/cli/client.go b/internal/cli/client.go index 74ad73a..e7f32bb 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -265,7 +265,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien return &exitError{code: 1, err: err} } - cfg.Current().ActiveClientID = strconv.Itoa(pc.ID) + setActiveClient(cfg.Current(), pc) p.Newline() if adopted { @@ -449,17 +449,45 @@ func runClientList(ctx context.Context, p *ui.Printer) error { return nil } p.Section("Clients in your account") + active := cfg.Current().ActiveClientID for _, c := range clients { marker := "" - if strconv.Itoa(c.ID) == cfg.Current().ActiveClientID { - marker = " (active)" + if strconv.Itoa(c.ID) == active { + marker = " (active — this machine)" } p.Field(strconv.Itoa(c.ID)+marker, - fmt.Sprintf("%s namespace=%s location=%s", c.Name, c.Namespace, c.Location)) + fmt.Sprintf("%s state=%s namespace=%s location=%s", + c.Name, clientStateLabel(c.Status), c.Namespace, c.Location)) } + // §7.3: separate "selected" (this machine's local pointer) from "connected" + // (the backend's last-heartbeat state) so a stale pointer is visible. + p.Hintf("\"active\" is this machine's selected client; state is its last reported status to tracebloc.") return nil } +// EdgeDevice.status codes mirrored from the backend (metaApi User.py). +const ( + clientStatusOffline = 0 + clientStatusOnline = 1 + clientStatusPending = 2 +) + +// clientStateLabel maps the backend status code to a TTY/CI-safe word. Plain +// text (not an emoji glyph) on purpose — flag/emoji glyphs mojibake in CI logs +// and Windows consoles (RFC-0001 §12 watch-item). +func clientStateLabel(status int) string { + switch status { + case clientStatusOnline: + return "online" + case clientStatusOffline: + return "offline" + case clientStatusPending: + return "pending" + default: + return "unknown" + } +} + func runClientUse(ctx context.Context, p *ui.Printer, id string) error { client, cfg, err := authedClient() if err != nil { @@ -471,7 +499,7 @@ func runClientUse(ctx context.Context, p *ui.Printer, id string) error { } for _, c := range clients { if strconv.Itoa(c.ID) == id { - cfg.Current().ActiveClientID = id + setActiveClient(cfg.Current(), &c) if serr := cfg.Save(); serr != nil { return &exitError{code: 1, err: serr} } @@ -483,6 +511,15 @@ func runClientUse(ctx context.Context, p *ui.Printer, id string) error { "no client %s in your account — run `tracebloc client list` to see the ids", id)} } +// setActiveClient points this env's profile at c, caching its namespace and +// display name alongside the id so the data commands can bind to the active +// client's cluster (§7.3) without a backend round-trip. Callers Save() after. +func setActiveClient(p *config.Profile, c *api.ProvisionedClient) { + p.ActiveClientID = strconv.Itoa(c.ID) + p.ActiveClientNamespace = c.Namespace + p.ActiveClientName = c.Name +} + // renderClientReview shows the assembled inputs before the confirm prompt, so // the user sees the derived namespace and location before anything is created. func renderClientReview(p *ui.Printer, name, namespace, location, clusterID string) { diff --git a/internal/cli/clustertarget.go b/internal/cli/clustertarget.go new file mode 100644 index 0000000..e24dc5d --- /dev/null +++ b/internal/cli/clustertarget.go @@ -0,0 +1,127 @@ +package cli + +import ( + "context" + "errors" + "fmt" + + "k8s.io/client-go/kubernetes" + + "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/config" +) + +// noParentReleaseError marks the exit-4 case where the reached cluster +// genuinely hosts no tracebloc release in the target namespace +// (cluster.ErrNoParentRelease) — as opposed to a present-but-PVC-missing +// release, an API/RBAC list failure, or an ambiguous multiple-release match. +// §7.3 uses it to turn an active-client binding miss into a clear "runs on +// another machine" message; the other failures keep their own diagnostics. +type noParentReleaseError struct{ err error } + +func (e *noParentReleaseError) Error() string { return e.err.Error() } +func (e *noParentReleaseError) Unwrap() error { return e.err } + +// clusterTarget bundles the cluster handles the data commands resolve from a +// kubeconfig before doing any work: the resolved config, a clientset, the +// parent tracebloc release, and — when asked — the shared data PVC. +type clusterTarget struct { + Resolved *cluster.ResolvedConfig + Clientset kubernetes.Interface + Release *cluster.ParentRelease + PVC *cluster.SharedPVC // nil unless needPVC was requested +} + +// resolveClusterTarget centralizes the identical Load → NewClientset → +// DiscoverParentRelease (→ DiscoverSharedPVC) sequence that `data ingest`, +// `data list`, and `data delete` each repeated, together with its exit-code +// contract: exit 3 for kubeconfig / clientset failures (can't reach a cluster +// at all), exit 4 for a missing tracebloc release or shared PVC (reached a +// cluster, but it isn't a tracebloc workspace). +// +// `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, opts cluster.KubeconfigOptions, needPVC bool) (*clusterTarget, error) { + resolved, err := cluster.Load(opts) + if err != nil { + return nil, &exitError{code: 3, err: fmt.Errorf("loading kubeconfig: %w", err)} + } + cs, err := cluster.NewClientset(resolved) + if err != nil { + return nil, &exitError{code: 3, err: err} + } + release, err := cluster.DiscoverParentRelease(ctx, cs, resolved.Namespace) + 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 + // ambiguous multiple-release match keeps its own message. + if errors.Is(err, cluster.ErrNoParentRelease) { + return nil, &exitError{code: 4, err: &noParentReleaseError{err}} + } + return nil, &exitError{code: 4, err: err} + } + t := &clusterTarget{Resolved: resolved, Clientset: cs, Release: release} + if needPVC { + pvc, err := cluster.DiscoverSharedPVC(ctx, cs, resolved.Namespace) + if err != nil { + return nil, &exitError{code: 4, err: err} + } + t.PVC = pvc + } + return t, nil +} + +// activeClientBinding records that a data command defaulted its target +// namespace to the active client's cached namespace (§7.3), so a subsequent +// "no release here" failure can be explained as "the active client runs +// elsewhere" rather than a bare discovery error. +type activeClientBinding struct { + applied bool + name string + namespace string +} + +// bindActiveClientNamespace defaults opts.Namespace to the active client's +// cached namespace when the user overrode neither --namespace nor --context. +// It never fails: no config, no active client, or no cached namespace all +// leave opts untouched (unchanged current-context behavior), so this is +// backward compatible for anyone who hasn't run `client use`/`create`. +func bindActiveClientNamespace(opts *cluster.KubeconfigOptions) activeClientBinding { + if opts.Namespace != "" || opts.Context != "" { + return activeClientBinding{} // user was explicit — don't second-guess + } + cfg, err := config.Load() + if err != nil { + return activeClientBinding{} + } + p := cfg.Current() + if p.ActiveClientNamespace == "" { + return activeClientBinding{} + } + opts.Namespace = p.ActiveClientNamespace + return activeClientBinding{applied: true, name: p.ActiveClientName, namespace: p.ActiveClientNamespace} +} + +// explain rewrites a "no tracebloc release in namespace" failure (exit 4) into +// §7.3's "client runs on another machine" guidance when the target namespace +// came from the active-client binding: the cluster the kubeconfig reaches +// doesn't host that client. Non-binding errors (and PVC-missing, where the +// release *was* found) pass through unchanged. +func (b activeClientBinding) explain(err error) error { + if !b.applied { + return err + } + var npr *noParentReleaseError + if !errors.As(err, &npr) { + return err + } + handle := b.name + if handle == "" { + handle = b.namespace + } + return &exitError{code: 4, err: fmt.Errorf( + "active client %q runs on another machine — namespace %q isn't on the cluster your kubeconfig points at; "+ + "run this command there, `tracebloc client use` a client on this cluster, or override with --namespace/--context", + handle, b.namespace)} +} diff --git a/internal/cli/clustertarget_test.go b/internal/cli/clustertarget_test.go new file mode 100644 index 0000000..d0deb0f --- /dev/null +++ b/internal/cli/clustertarget_test.go @@ -0,0 +1,114 @@ +package cli + +import ( + "errors" + "strings" + "testing" + + "github.com/tracebloc/cli/internal/api" + "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/config" +) + +func TestSetActiveClient_CachesNamespaceAndName(t *testing.T) { + p := &config.Profile{} + setActiveClient(p, &api.ProvisionedClient{ID: 7, Name: "Lab A", Namespace: "lab-a", Location: "FR"}) + if p.ActiveClientID != "7" || p.ActiveClientNamespace != "lab-a" || p.ActiveClientName != "Lab A" { + t.Errorf("profile after setActiveClient = %+v", p) + } +} + +func TestClientStateLabel(t *testing.T) { + cases := map[int]string{ + clientStatusOnline: "online", + clientStatusOffline: "offline", + clientStatusPending: "pending", + 99: "unknown", + } + for status, want := range cases { + if got := clientStateLabel(status); got != want { + t.Errorf("clientStateLabel(%d) = %q, want %q", status, got, want) + } + } +} + +// writeActiveClientConfig writes a signed-in dev profile whose active client +// carries the given namespace + name, into a temp config dir. +func writeActiveClientConfig(t *testing.T, namespace, name string) { + t.Helper() + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + cfg := &config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok", ActiveClientID: "5", ActiveClientNamespace: namespace, ActiveClientName: name}, + }} + if err := cfg.Save(); err != nil { + t.Fatal(err) + } +} + +func TestBindActiveClientNamespace_DefaultsFromActiveClient(t *testing.T) { + writeActiveClientConfig(t, "munich-radiology", "Munich Radiology") + opts := cluster.KubeconfigOptions{} + b := bindActiveClientNamespace(&opts) + if !b.applied { + t.Fatal("binding not applied") + } + if opts.Namespace != "munich-radiology" { + t.Errorf("opts.Namespace = %q, want munich-radiology", opts.Namespace) + } + if b.name != "Munich Radiology" || b.namespace != "munich-radiology" { + t.Errorf("binding = %+v", b) + } +} + +func TestBindActiveClientNamespace_ExplicitOverrideWins(t *testing.T) { + writeActiveClientConfig(t, "munich-radiology", "Munich Radiology") + + // --namespace set → don't touch it. + optsNS := cluster.KubeconfigOptions{Namespace: "chosen"} + if b := bindActiveClientNamespace(&optsNS); b.applied || optsNS.Namespace != "chosen" { + t.Errorf("with --namespace: applied=%v ns=%q, want false/chosen", b.applied, optsNS.Namespace) + } + + // --context set → user is steering the cluster; don't bind. + optsCtx := cluster.KubeconfigOptions{Context: "some-ctx"} + if b := bindActiveClientNamespace(&optsCtx); b.applied || optsCtx.Namespace != "" { + t.Errorf("with --context: applied=%v ns=%q, want false/empty", b.applied, optsCtx.Namespace) + } +} + +func TestBindActiveClientNamespace_NoActiveClient(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // no config written → nothing cached + opts := cluster.KubeconfigOptions{} + if b := bindActiveClientNamespace(&opts); b.applied || opts.Namespace != "" { + t.Errorf("no active client: applied=%v ns=%q, want false/empty", b.applied, opts.Namespace) + } +} + +func TestActiveClientBinding_Explain(t *testing.T) { + noRelease := &exitError{code: 4, err: &noParentReleaseError{errors.New("no release")}} + pvcMissing := &exitError{code: 4, err: errors.New("shared PVC not bound")} + + // Applied + "no release here" → rewritten to the §7.3 guidance. + bound := activeClientBinding{applied: true, name: "gpu-box-01", namespace: "gpu-box-01"} + got := bound.explain(noRelease) + if got == noRelease { + t.Fatal("expected a rewritten error") + } + if !strings.Contains(got.Error(), "another machine") || !strings.Contains(got.Error(), "gpu-box-01") { + t.Errorf("rewritten error = %q", got.Error()) + } + var ee *exitError + if !errors.As(got, &ee) || ee.Code() != 4 { + t.Errorf("rewritten error should stay exit 4, got %v", got) + } + + // Applied but a PVC failure (release WAS found) → pass through untouched. + if bound.explain(pvcMissing) != pvcMissing { + t.Error("PVC-missing error should not be rewritten") + } + + // Not applied → always pass through. + if (activeClientBinding{}).explain(noRelease) != noRelease { + t.Error("unbound explain should pass the error through") + } +} diff --git a/internal/cli/data.go b/internal/cli/data.go index b91c8bb..e7e7ec2 100644 --- a/internal/cli/data.go +++ b/internal/cli/data.go @@ -591,34 +591,19 @@ contributors train against it without ever seeing the raw files.`)) // consistent across pre-flight commands. a.Printer.Step(2, 4, "Connect to your workspace's cluster") a.Printer.Hintf("Using your kubeconfig to find the tracebloc release in your workspace and the shared storage your dataset will live on.") - resolved, err := cluster.Load(cluster.KubeconfigOptions{ - Path: a.Kubeconfig, - Context: a.Context, - Namespace: a.Namespace, - }) + // 6. PVC discovery (needPVC) confirms the chart's shared-data PVC is + // 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, opts, true) if err != nil { - return &exitError{code: 3, err: fmt.Errorf("loading kubeconfig: %w", err)} - } - cs, err := cluster.NewClientset(resolved) - if err != nil { - return &exitError{code: 3, err: err} - } - release, err := cluster.DiscoverParentRelease(ctx, cs, resolved.Namespace) - if err != nil { - return &exitError{code: 4, err: err} + return binding.explain(err) } + resolved, cs, release, pvc := target.Resolved, target.Clientset, target.Release, target.PVC if a.IngestorSAName != "" { release.IngestorSAName = a.IngestorSAName } - // 6. PVC discovery — confirms the chart's shared-data PVC is - // Bound before we waste time provisioning a Pod that can't - // mount it. - pvc, err := cluster.DiscoverSharedPVC(ctx, cs, resolved.Namespace) - if err != nil { - return &exitError{code: 4, err: err} - } - // 7. Show what we found on the cluster — the customer's last look // before any bytes move. printClusterSummary(a.Printer, release, pvc) diff --git a/internal/cli/data_delete.go b/internal/cli/data_delete.go index 974630a..a0d94ac 100644 --- a/internal/cli/data_delete.go +++ b/internal/cli/data_delete.go @@ -110,31 +110,17 @@ undone — re-ingesting the data is the only way back.`) return &exitError{code: 2, err: fmt.Errorf("invalid table name %q: %w", a.Table, err)} } - // 2. Resolve cluster + clientset (kubeconfig errors = exit 3). - resolved, err := cluster.Load(cluster.KubeconfigOptions{ - Path: a.Kubeconfig, - Context: a.Context, - Namespace: a.Namespace, - }) - if err != nil { - return &exitError{code: 3, err: fmt.Errorf("loading kubeconfig: %w", err)} - } - cs, err := cluster.NewClientset(resolved) - if err != nil { - return &exitError{code: 3, err: err} - } - - // 3. Confirm the parent release + shared PVC exist (exit 4 if not) — + // 2. Resolve cluster + clientset (kubeconfig errors = exit 3), then + // confirm the parent release + shared PVC exist (exit 4 if not) — // both "is this the right cluster?" context and a guard against // running teardown against a cluster with no tracebloc install. - release, err := cluster.DiscoverParentRelease(ctx, cs, resolved.Namespace) - if err != nil { - return &exitError{code: 4, err: err} - } - pvc, err := cluster.DiscoverSharedPVC(ctx, cs, resolved.Namespace) + opts := cluster.KubeconfigOptions{Path: a.Kubeconfig, Context: a.Context, Namespace: a.Namespace} + binding := bindActiveClientNamespace(&opts) + target, err := resolveClusterTarget(ctx, opts, true) if err != nil { - return &exitError{code: 4, err: err} + return binding.explain(err) } + resolved, cs, release, pvc := target.Resolved, target.Clientset, target.Release, target.PVC // 4. Show exactly what will be deleted — the customer's last look // before destructive, unrecoverable work. diff --git a/internal/cli/data_list.go b/internal/cli/data_list.go index 0d6029a..92c299c 100644 --- a/internal/cli/data_list.go +++ b/internal/cli/data_list.go @@ -111,22 +111,13 @@ func runDataList(ctx context.Context, a runDataListArgs) (err error) { p := a.Printer p.Banner("tracebloc", "datasets in the cluster") - resolved, err := cluster.Load(cluster.KubeconfigOptions{ - Path: a.Kubeconfig, - Context: a.Context, - Namespace: a.Namespace, - }) + opts := cluster.KubeconfigOptions{Path: a.Kubeconfig, Context: a.Context, Namespace: a.Namespace} + binding := bindActiveClientNamespace(&opts) + target, err := resolveClusterTarget(ctx, opts, false) if err != nil { - return &exitError{code: 3, err: fmt.Errorf("loading kubeconfig: %w", err)} - } - cs, err := cluster.NewClientset(resolved) - if err != nil { - return &exitError{code: 3, err: err} - } - release, err := cluster.DiscoverParentRelease(ctx, cs, resolved.Namespace) - if err != nil { - return &exitError{code: 4, err: err} + return binding.explain(err) } + resolved, cs, release := target.Resolved, target.Clientset, target.Release tables, err := push.ListDatasets(ctx, cs, resolved.RestConfig, resolved.Namespace) if err != nil { diff --git a/internal/cluster/discover.go b/internal/cluster/discover.go index b1718f9..5945dad 100644 --- a/internal/cluster/discover.go +++ b/internal/cluster/discover.go @@ -2,6 +2,7 @@ package cluster import ( "context" + "errors" "fmt" "strings" @@ -10,6 +11,12 @@ import ( "k8s.io/client-go/kubernetes" ) +// ErrNoParentRelease is the sentinel for DiscoverParentRelease's "the namespace +// has no tracebloc release" case — distinct from an API/RBAC list failure or an +// ambiguous multiple-release match. Callers use errors.Is to tell "this cluster +// doesn't host the release" apart from "couldn't determine the release." +var ErrNoParentRelease = errors.New("no tracebloc parent client release found") + // ParentRelease describes the tracebloc parent client chart release // discovered in the customer's cluster. The information comes from // the helm-managed Deployment's labels (and a few of its env vars), @@ -114,11 +121,11 @@ func DiscoverParentRelease(ctx context.Context, cs kubernetes.Interface, namespa switch len(jmDeps) { case 0: return nil, fmt.Errorf( - "no tracebloc parent client release found in namespace %q "+ + "%w in namespace %q "+ "(no chart-managed Deployment named *-jobs-manager). "+ "Install with `helm install tracebloc/client --namespace %s` first, "+ "or pass --namespace to point at the namespace where it's running.", - namespace, namespace, + ErrNoParentRelease, namespace, namespace, ) case 1: // happy path diff --git a/internal/cluster/discover_test.go b/internal/cluster/discover_test.go index 5c0921c..230aecd 100644 --- a/internal/cluster/discover_test.go +++ b/internal/cluster/discover_test.go @@ -2,6 +2,7 @@ package cluster import ( "context" + "errors" "strings" "testing" @@ -105,6 +106,11 @@ func TestDiscoverParentRelease_NoReleaseFound(t *testing.T) { t.Errorf("expected error to mention %q, got: %s", want, err) } } + // The sentinel gates the §7.3 "runs elsewhere" rewrite (cli#128) — a + // genuine not-found must be errors.Is-identifiable. + if !errors.Is(err, ErrNoParentRelease) { + t.Errorf("not-found error should match ErrNoParentRelease, got: %v", err) + } } func TestDiscoverParentRelease_MultipleReleases(t *testing.T) { @@ -125,6 +131,11 @@ func TestDiscoverParentRelease_MultipleReleases(t *testing.T) { t.Errorf("expected error to mention %q, got: %s", want, err) } } + // An ambiguous match is NOT "no release" — it must not trip the §7.3 + // rewrite (the cluster does host tracebloc, just more than one). + if errors.Is(err, ErrNoParentRelease) { + t.Error("multiple-release error must not match ErrNoParentRelease") + } } // Regression for the real-cluster discovery bug. Pre-fix, the diff --git a/internal/config/config.go b/internal/config/config.go index da62557..ca43568 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -42,6 +42,14 @@ type Profile struct { Token string `json:"token,omitempty"` // user token from device login ExpiresAt string `json:"expires_at,omitempty"` // token expiry (RFC 3339), when known ActiveClientID string `json:"active_client_id,omitempty"` // client this machine enrolls as, for THIS env + + // ActiveClientNamespace + ActiveClientName cache the active client's k8s + // namespace and display name at `client use`/`create` time, so the data + // commands can bind to the active client's cluster (RFC-0001 §7.3) without + // a backend round-trip — they run cluster-local and may be offline. Empty + // when no client is active or for pre-v2 configs that predate the cache. + ActiveClientNamespace string `json:"active_client_namespace,omitempty"` + ActiveClientName string `json:"active_client_name,omitempty"` } // Config is the on-disk CLI state: env-scoped profiles plus the current env.