diff --git a/internal/cli/client.go b/internal/cli/client.go index 8511d187..100f620c 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "github.com/spf13/cobra" @@ -91,9 +92,12 @@ func newClientListCmd() *cobra.Command { func newClientUseCmd() *cobra.Command { return &cobra.Command{ - Use: "use ", - Short: "Enroll this machine as an existing client", - Args: cobra.ExactArgs(1), + Use: "use ", + Short: "Select the active client for this machine (by slug or id)", + Long: `Select which client this machine's data / cluster commands act on. +Accepts the client's namespace slug (e.g. tracebloc-amazon) — the handle shown +by ` + "`tracebloc client list`" + ` — or its numeric id.`, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runClientUse(cmd.Context(), printerFor(cmd), args[0]) }, @@ -573,6 +577,12 @@ func runClientList(ctx context.Context, p *ui.Printer) error { p.Hintf("No clients yet. Run `tracebloc client create`.") return nil } + // Backfill the active client's namespace cache if it's empty — e.g. a + // config written before the cache existed. `list` already has every + // client in hand, so this needs no extra request and lets `cluster info` + // / `data` commands bind (§7.3) without an explicit re-`use`. + backfillActiveClientCache(cfg, clients) + p.Section("Clients in your account") active := cfg.Current().ActiveClientID for _, c := range clients { @@ -613,7 +623,9 @@ func clientStateLabel(status int) string { } } -func runClientUse(ctx context.Context, p *ui.Printer, id string) error { +// runClientUse selects the active client by its handle — the namespace slug +// (the RFC-0001 §7.1/D3 handle) or, for backward compatibility, its numeric id. +func runClientUse(ctx context.Context, p *ui.Printer, handle string) error { client, cfg, err := authedClient() if err != nil { return &exitError{code: 1, err: err} @@ -622,18 +634,81 @@ func runClientUse(ctx context.Context, p *ui.Printer, id string) error { if err != nil { return &exitError{code: 1, err: err} } - for _, c := range clients { - if strconv.Itoa(c.ID) == id { - setActiveClient(cfg.Current(), &c) - if serr := cfg.Save(); serr != nil { - return &exitError{code: 1, err: serr} - } - p.Successf("This machine is now set to enroll as client %s (%s).", id, c.Name) - return nil + // Slug takes precedence over id. Namespaces are unique per account, so a + // slug match is unambiguous; matching id in the same pass would let a + // client whose slug is all-numeric collide with another client's id and + // pick whichever the API listed first. Fall back to id only if no slug + // matched (backward compatibility). + var byID *api.ProvisionedClient + for i := range clients { + if clients[i].Namespace == handle { + return selectClient(ctx, p, cfg, &clients[i]) + } + if byID == nil && strconv.Itoa(clients[i].ID) == handle { + byID = &clients[i] } } + if byID != nil { + return selectClient(ctx, p, cfg, byID) + } return &exitError{code: 1, err: fmt.Errorf( - "no client %s in your account — run `tracebloc client list` to see the ids", id)} + "no client %q in your account — run `tracebloc client list` to see the slugs and ids", handle)} +} + +// selectClient points this machine at c, persists the choice, then reports — +// non-fatally — whether c is actually usable from here, so `client use` never +// leaves the user to discover an unreachable/unconnected client via a later +// data-command failure (§7.3). +func selectClient(ctx context.Context, p *ui.Printer, cfg *config.Config, c *api.ProvisionedClient) error { + setActiveClient(cfg.Current(), c) + if err := cfg.Save(); err != nil { + return &exitError{code: 1, err: err} + } + p.Successf("Now using client %q (namespace %s).", c.Name, c.Namespace) + reportSelectedClientState(ctx, p, c) + return nil +} + +// reportSelectedClientState prints an honest, non-fatal note about the selected +// client's connectivity. Backend status (from `client list`) tells us online / +// offline / pending for free; for an online client we additionally probe the +// local cluster to distinguish "connected here" from "runs on another machine". +func reportSelectedClientState(ctx context.Context, p *ui.Printer, c *api.ProvisionedClient) { + switch c.Status { + case clientStatusOnline: + if probeClientReachableHere(ctx, c.Namespace) { + p.Infof("Connected on this machine — data commands will target namespace %s.", c.Namespace) + return + } + p.Warnf("This client is online but runs on another machine — data commands here can't reach its cluster.") + p.Hintf("Run data commands where it's installed, or `tracebloc client use` a client on this machine.") + case clientStatusPending: + p.Warnf("This client isn't connected to a cluster yet.") + p.Hintf("Install it on the target machine first — data commands won't work until it's online.") + case clientStatusOffline: + p.Warnf("This client is currently offline (no recent heartbeat).") + p.Hintf("Data commands will fail until it's back online — run `tracebloc cluster doctor` to diagnose.") + } +} + +// probeClientReachableHere reports, best-effort, whether the client's namespace +// hosts its tracebloc release on the cluster the default kubeconfig points at +// ("connected here"). Bounded so `client use` never hangs on an unreachable +// cluster; any failure (no kubeconfig, unreachable, no release) → false. A +// package var so tests can stub it without a real cluster. +var probeClientReachableHere = func(ctx context.Context, namespace string) bool { + ctx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + resolved, err := cluster.Load(cluster.KubeconfigOptions{Namespace: namespace}) + if err != nil { + return false + } + cs, err := cluster.NewClientset(resolved) + if err != nil { + return false + } + _, err = cluster.DiscoverParentRelease(ctx, cs, namespace) + return err == nil } // setActiveClient points this env's profile at c, caching its namespace and @@ -645,6 +720,26 @@ func setActiveClient(p *config.Profile, c *api.ProvisionedClient) { p.ActiveClientName = c.Name } +// backfillActiveClientCache fills the active client's namespace/name cache from +// an already-fetched client list when it's missing (a config written before the +// cache existed, or the id set some other way). Best-effort: if the active +// client isn't in the list, or the cache is already populated, it's a no-op; a +// Save failure is ignored (the next `client use` will persist it anyway). +func backfillActiveClientCache(cfg *config.Config, clients []api.ProvisionedClient) { + p := cfg.Current() + if p.ActiveClientID == "" || p.ActiveClientNamespace != "" { + return + } + for i := range clients { + if strconv.Itoa(clients[i].ID) == p.ActiveClientID { + p.ActiveClientNamespace = clients[i].Namespace + p.ActiveClientName = clients[i].Name + _ = cfg.Save() + return + } + } +} + // 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/client_test.go b/internal/cli/client_test.go index 5222510e..53093819 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -251,17 +251,82 @@ func TestClientList(t *testing.T) { func TestClientUse(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`[{"id":7,"first_name":"gamma","namespace":"gamma"}]`)) + _, _ = w.Write([]byte(`[{"id":7,"first_name":"gamma","namespace":"gamma-ns"}]`)) }) + + // By numeric id (backward compatible). if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "7"); err != nil { t.Fatal(err) } cfg, _ := config.Load() + if cfg.Current().ActiveClientID != "7" || cfg.Current().ActiveClientNamespace != "gamma-ns" { + t.Errorf("after use by id: %+v, want id=7 ns=gamma-ns", cfg.Current()) + } + + // By namespace slug — the RFC-0001 §7.1/D3 handle. + if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "gamma-ns"); err != nil { + t.Fatalf("use by slug: %v", err) + } + cfg, _ = config.Load() if cfg.Current().ActiveClientID != "7" { - t.Errorf("active = %q, want 7", cfg.Current().ActiveClientID) + t.Errorf("after use by slug: active = %q, want 7", cfg.Current().ActiveClientID) + } + + // Unknown handle → error. + if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "nope"); err == nil { + t.Error("expected an error for an unknown client handle") + } +} + +// client use reports the selected client's connectivity (§7.3): online-here vs +// online-elsewhere (via the local probe), pending, and offline. +func TestClientUse_ReportsConnectionState(t *testing.T) { + cases := []struct { + name string + status int + reachable bool + wantSubstr string + }{ + {"online_here", clientStatusOnline, true, "Connected on this machine"}, + {"online_elsewhere", clientStatusOnline, false, "runs on another machine"}, + {"pending", clientStatusPending, false, "isn't connected to a cluster yet"}, + {"offline", clientStatusOffline, false, "currently offline"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + body := fmt.Sprintf(`[{"id":7,"first_name":"box","namespace":"ns7","status":%d}]`, tc.status) + withClientBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(body)) + }) + orig := probeClientReachableHere + probeClientReachableHere = func(context.Context, string) bool { return tc.reachable } + t.Cleanup(func() { probeClientReachableHere = orig }) + + var out bytes.Buffer + if err := runClientUse(context.Background(), ui.New(&out), "ns7"); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), tc.wantSubstr) { + t.Errorf("want %q in output:\n%s", tc.wantSubstr, out.String()) + } + }) } - if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "99"); err == nil { - t.Error("expected an error for an unknown client id") +} + +// A numeric handle that is one client's slug AND another client's id must +// resolve deterministically to the slug owner (slug > id), regardless of list +// order — not "whichever the API returned first". +func TestClientUse_SlugBeatsNumericIDCollision(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, _ *http.Request) { + // id=1039 listed FIRST; the client whose slug is "1039" listed second. + _, _ = w.Write([]byte(`[{"id":1039,"first_name":"by-id","namespace":"other-ns"},{"id":42,"first_name":"by-slug","namespace":"1039"}]`)) + }) + if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "1039"); err != nil { + t.Fatal(err) + } + cfg, _ := config.Load() + if cfg.Current().ActiveClientID != "42" || cfg.Current().ActiveClientNamespace != "1039" { + t.Errorf("handle 1039 should select the slug owner (id 42), got %+v", cfg.Current()) } } diff --git a/internal/cli/cluster.go b/internal/cli/cluster.go index db2cd7ad..85316a10 100644 --- a/internal/cli/cluster.go +++ b/internal/cli/cluster.go @@ -122,11 +122,12 @@ func runClusterInfo( ) error { p.Banner("tracebloc", "cluster diagnostics") - resolved, err := cluster.Load(cluster.KubeconfigOptions{ - Path: kubeconfigPath, - Context: contextOverride, - Namespace: nsOverride, - }) + // Default the namespace to the active client's (RFC-0001 §7.3) when the user + // gave neither --namespace nor --context, so `cluster info` diagnoses the + // selected client's cluster rather than the kubeconfig's current-context ns. + opts := cluster.KubeconfigOptions{Path: kubeconfigPath, Context: contextOverride, Namespace: nsOverride} + binding := bindActiveClientNamespace(&opts) + resolved, err := cluster.Load(opts) if err != nil { // Kubeconfig errors are exit-code-3 territory (file/parse // problem, same conceptual class as `ingest validate`'s @@ -150,8 +151,10 @@ func runClusterInfo( // 4 = "cluster reachable, but no tracebloc release here." // Distinct from the kubeconfig error (3) so callers can // branch: 3 means "fix your kubeconfig", 4 means "install - // the parent chart first". - return &exitError{code: 4, err: err} + // the parent chart first". When the namespace came from the + // active-client binding, explain() rewrites this to the §7.3 + // "active client runs on another machine" guidance. + return binding.explain(&exitError{code: 4, err: err}) } // Apply the SA-name override here. Discovery doesn't read the diff --git a/internal/cli/clustertarget.go b/internal/cli/clustertarget.go index e24dc5da..fb9c75de 100644 --- a/internal/cli/clustertarget.go +++ b/internal/cli/clustertarget.go @@ -11,17 +11,6 @@ import ( "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. @@ -53,12 +42,10 @@ func resolveClusterTarget(ctx context.Context, opts cluster.KubeconfigOptions, n } 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}} - } + // The error carries cluster.ErrNoParentRelease for a genuine + // "namespace has no release" (which explain() rewrites as §7.3 + // "runs elsewhere"); an API/RBAC list failure or ambiguous + // multiple-release match wraps a different error and passes through. return nil, &exitError{code: 4, err: err} } t := &clusterTarget{Resolved: resolved, Clientset: cs, Release: release} @@ -106,14 +93,12 @@ func bindActiveClientNamespace(opts *cluster.KubeconfigOptions) activeClientBind // 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. +// doesn't host that client. Gated on cluster.ErrNoParentRelease so a +// PVC-missing failure (release was found), an API/RBAC error, or an ambiguous +// multiple-release match all pass through unchanged. Callable by any command +// that binds — data ingest/list/delete and cluster info. func (b activeClientBinding) explain(err error) error { - if !b.applied { - return err - } - var npr *noParentReleaseError - if !errors.As(err, &npr) { + if !b.applied || !errors.Is(err, cluster.ErrNoParentRelease) { return err } handle := b.name diff --git a/internal/cli/clustertarget_test.go b/internal/cli/clustertarget_test.go index d0deb0f3..34853fdf 100644 --- a/internal/cli/clustertarget_test.go +++ b/internal/cli/clustertarget_test.go @@ -2,6 +2,7 @@ package cli import ( "errors" + "fmt" "strings" "testing" @@ -18,6 +19,41 @@ func TestSetActiveClient_CachesNamespaceAndName(t *testing.T) { } } +func TestBackfillActiveClientCache(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + clients := []api.ProvisionedClient{ + {ID: 1039, Name: "aws-ubuntu", Namespace: "tracebloc-amazon"}, + {ID: 1053, Name: "asad-macbook", Namespace: "asad-macbook-2"}, + } + + // Pre-cache config (old binary): id set, namespace empty → backfilled. + cfg := &config.Config{CurrentEnv: "prod", Profiles: map[string]*config.Profile{ + "prod": {Token: "t", ActiveClientID: "1039"}, + }} + backfillActiveClientCache(cfg, clients) + if cfg.Current().ActiveClientNamespace != "tracebloc-amazon" || cfg.Current().ActiveClientName != "aws-ubuntu" { + t.Errorf("backfill = %+v, want ns=tracebloc-amazon name=aws-ubuntu", cfg.Current()) + } + + // Already cached → left untouched (no clobber). + cached := &config.Config{CurrentEnv: "prod", Profiles: map[string]*config.Profile{ + "prod": {Token: "t", ActiveClientID: "1039", ActiveClientNamespace: "custom-ns", ActiveClientName: "custom"}, + }} + backfillActiveClientCache(cached, clients) + if cached.Current().ActiveClientNamespace != "custom-ns" { + t.Errorf("backfill clobbered an existing cache: %+v", cached.Current()) + } + + // Active id not in the list → no-op (no crash). + orphan := &config.Config{CurrentEnv: "prod", Profiles: map[string]*config.Profile{ + "prod": {Token: "t", ActiveClientID: "9999"}, + }} + backfillActiveClientCache(orphan, clients) + if orphan.Current().ActiveClientNamespace != "" { + t.Errorf("orphan id should not backfill, got %+v", orphan.Current()) + } +} + func TestClientStateLabel(t *testing.T) { cases := map[int]string{ clientStatusOnline: "online", @@ -85,7 +121,9 @@ func TestBindActiveClientNamespace_NoActiveClient(t *testing.T) { } func TestActiveClientBinding_Explain(t *testing.T) { - noRelease := &exitError{code: 4, err: &noParentReleaseError{errors.New("no release")}} + // A discovery failure carrying the sentinel (as DiscoverParentRelease + // returns for a genuine not-found) — the case explain rewrites. + noRelease := &exitError{code: 4, err: fmt.Errorf("%w in namespace", cluster.ErrNoParentRelease)} pvcMissing := &exitError{code: 4, err: errors.New("shared PVC not bound")} // Applied + "no release here" → rewritten to the §7.3 guidance. diff --git a/internal/cli/root.go b/internal/cli/root.go index 2b7868dd..1af96bde 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -108,14 +108,20 @@ what's planned next.`, } p := printerFor(cmd) p.Banner("tracebloc", "interactive data ingestion for your cluster") - p.Section("Get started") + p.Section("Sign in & pick a client") + p.Infof("tracebloc login — sign in via your browser") + p.Infof("tracebloc client create — provision this machine as a client") + p.Infof("tracebloc client list — list your clients + their online state") + p.Infof("tracebloc client use — select the active client (by slug or id)") + p.Infof("tracebloc auth status — show who/where you're signed in") + p.Section("Work with data") p.Infof("tracebloc data ingest — stage + ingest a dataset interactively (or use --help to see flags)") p.Infof("tracebloc data list — list datasets ingested in the cluster") p.Infof("tracebloc data delete — delete an ingested dataset (its table + files)") - p.Infof("tracebloc cluster info — check the CLI can reach your cluster") + p.Infof("tracebloc cluster info — check the CLI can reach your active client's cluster") p.Infof("tracebloc ingest validate f.yaml — validate an ingest.yaml locally") p.Newline() - p.Hintf("Add --help to any command for the full flag list.") + p.Hintf("Run `tracebloc --help` for the full command list, or add --help to any command for its flags.") return nil }