From 1b0e8717f90b23210ea319f35df4264b769af875 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Mon, 13 Jul 2026 18:31:29 +0200 Subject: [PATCH] test(data delete): cover the destructive teardown path (execute + partial-fail) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runDataDelete was 22% — every test stopped at cluster discovery, so the actual DROP TABLE + file removal (unrecoverable) had no coverage. Add two fn-var test seams (default to the real funcs, zero behavior change), mirroring loadClusterFn/listDatasetsFn: - resolveClusterTargetFn: inject a canned target (fake clientset + release + PVC) without seeding the k8s objects discoverRelease/DiscoverSharedPVC look for. - teardownFn: over push.Teardown. TestRunDataDelete_Execute drives past discovery and exercises the three teardown outcomes: clean -> exit 0; table dropped but file removal fails -> exit 7 + recovery hint; fails before the drop -> exit 7 "teardown failed". The mixed-case "Churn" input also pins the case-insensitive resolveDeleteTarget match (backend#1027). make ci green. Follow-ups: loadClusterFn routing for cluster-info/doctor/ home + stageFn for ingest --overwrite; resolveClusterTarget's own success tail (needs a k8s-object fixture). Co-Authored-By: Claude Opus 4.8 --- internal/cli/clustertarget.go | 6 ++ internal/cli/data.go | 6 ++ internal/cli/data_delete.go | 4 +- internal/cli/data_delete_execute_test.go | 99 ++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 internal/cli/data_delete_execute_test.go diff --git a/internal/cli/clustertarget.go b/internal/cli/clustertarget.go index 2ebe2f80..8b8173d1 100644 --- a/internal/cli/clustertarget.go +++ b/internal/cli/clustertarget.go @@ -35,6 +35,12 @@ var ( newClientsetFn = cluster.NewClientset ) +// resolveClusterTargetFn is a test seam over resolveClusterTarget so a command +// test can inject a fully-resolved target (fake clientset + release + PVC) +// without seeding the k8s objects discoverRelease / DiscoverSharedPVC look for. +// Same fn-var seam pattern as loadClusterFn / listDatasetsFn. +var resolveClusterTargetFn = resolveClusterTarget + // 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. diff --git a/internal/cli/data.go b/internal/cli/data.go index 31051444..53dcef63 100644 --- a/internal/cli/data.go +++ b/internal/cli/data.go @@ -1437,6 +1437,12 @@ func writePushErrorJSON(w io.Writer, sp push.SpecArgs, e error, code int) { // listDatasetsFn is a test seam over push.ListDatasets. var listDatasetsFn = push.ListDatasets +// teardownFn is a test seam over push.Teardown (the destructive DROP TABLE + +// file removal). Production points at the real Teardown; a test overrides it to +// drive the clean and the partial-failure (table dropped, files remain → exit +// 7) paths without a cluster. +var teardownFn = push.Teardown + // Test seams over the cluster-touching steps of runIngestionRun (#1009). // Production wires them to the real functions; a table test overrides them to // drive the classify → exit-code → JSON → reclaim matrix without a cluster diff --git a/internal/cli/data_delete.go b/internal/cli/data_delete.go index 27d1812e..d306157f 100644 --- a/internal/cli/data_delete.go +++ b/internal/cli/data_delete.go @@ -126,7 +126,7 @@ 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 := resolveClusterTarget(ctx, a.Printer, opts, binding, true) + target, err := resolveClusterTargetFn(ctx, a.Printer, opts, binding, true) if err != nil { return binding.explain(err) } @@ -198,7 +198,7 @@ undone — re-ingesting the data is the only way back.`) // files on any volume type — including hostPath, where fsGroup is // a no-op (tracebloc/client#259). p.Infof("Removing in-cluster artifacts…") - res, err := push.Teardown(ctx, cs, &push.SPDYExecutor{Config: resolved.RestConfig, Client: cs}, resolved.Namespace, plan, push.PodSpecOptions{ + res, err := teardownFn(ctx, cs, &push.SPDYExecutor{Config: resolved.RestConfig, Client: cs}, resolved.Namespace, plan, push.PodSpecOptions{ Namespace: resolved.Namespace, PVCClaimName: pvc.ClaimName, PVCMountPath: pvc.MountPath, diff --git a/internal/cli/data_delete_execute_test.go b/internal/cli/data_delete_execute_test.go new file mode 100644 index 00000000..afd4f096 --- /dev/null +++ b/internal/cli/data_delete_execute_test.go @@ -0,0 +1,99 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/rest" + + "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/push" + "github.com/tracebloc/cli/internal/ui" +) + +// TestRunDataDelete_Execute covers the destructive teardown path — the P0 the +// coverage audit flagged (runDataDelete was 22%; every existing test stopped at +// cluster discovery). It drives the command past discovery via the +// resolveClusterTargetFn seam (a canned target — no k8s fixture needed) and the +// listDatasetsFn seam (target resolution), then exercises the three teardown +// outcomes through the new teardownFn seam: +// - clean success -> exit 0 + a "Deleted" line +// - table dropped but file removal fails -> exit 7 + the recovery hint +// (the idempotent-DROP re-run guidance, backend#1027's sibling) +// - teardown fails before the drop -> exit 7 "teardown failed" +// +// The mixed-case "Churn" case also pins the case-insensitive resolveDeleteTarget +// match (backend#1027: a mis-cased name used to DROP nothing and still exit 0). +func TestRunDataDelete_Execute(t *testing.T) { + origRCT, origList, origTD := resolveClusterTargetFn, listDatasetsFn, teardownFn + t.Cleanup(func() { + resolveClusterTargetFn, listDatasetsFn, teardownFn = origRCT, origList, origTD + }) + + resolveClusterTargetFn = func(_ context.Context, _ *ui.Printer, _ cluster.KubeconfigOptions, _ activeClientBinding, _ bool) (*clusterTarget, error) { + return &clusterTarget{ + Resolved: &cluster.ResolvedConfig{Context: "ctx", Namespace: "tracebloc"}, + Clientset: fake.NewSimpleClientset(), + Release: &cluster.ParentRelease{ReleaseName: "tracebloc", IngestorSAName: "tracebloc-ingestor"}, + PVC: &cluster.SharedPVC{ClaimName: "client-pvc", MountPath: "/data/shared"}, + }, nil + } + listDatasetsFn = func(_ context.Context, _ kubernetes.Interface, _ *rest.Config, _ string) ([]string, error) { + return []string{"churn"}, nil + } + + run := func(table string) (string, error) { + var buf bytes.Buffer + err := runDataDelete(context.Background(), runDataDeleteArgs{ + Table: table, Yes: true, Printer: ui.New(&buf), + }) + return buf.String(), err + } + + t.Run("clean teardown -> success", func(t *testing.T) { + teardownFn = func(_ context.Context, _ kubernetes.Interface, _ push.Executor, _ string, _ push.TeardownPlan, _ push.PodSpecOptions) (push.TeardownResult, error) { + return push.TeardownResult{RemovedPaths: []string{"/data/shared/churn"}}, nil + } + out, err := run("churn") + if err != nil { + t.Fatalf("clean teardown: want nil error, got %v", err) + } + if !strings.Contains(out, "Deleted") { + t.Errorf("want a success line, got:\n%s", out) + } + }) + + t.Run("table dropped but file removal fails -> exit 7 + recovery hint", func(t *testing.T) { + teardownFn = func(_ context.Context, _ kubernetes.Interface, _ push.Executor, _ string, _ push.TeardownPlan, _ push.PodSpecOptions) (push.TeardownResult, error) { + return push.TeardownResult{DroppedTable: true}, errors.New("pod exec failed") + } + // Mixed case also exercises the case-insensitive resolveDeleteTarget match. + _, err := run("Churn") + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 7 { + t.Fatalf("partial failure: want exit 7, got %v", err) + } + if !strings.Contains(err.Error(), "was dropped, but removing its files failed") { + t.Errorf("want the dropped-but-files-remain recovery message, got: %v", err) + } + }) + + t.Run("teardown fails before the drop -> exit 7 teardown failed", func(t *testing.T) { + teardownFn = func(_ context.Context, _ kubernetes.Interface, _ push.Executor, _ string, _ push.TeardownPlan, _ push.PodSpecOptions) (push.TeardownResult, error) { + return push.TeardownResult{}, errors.New("could not reach the mysql pod") + } + _, err := run("churn") + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 7 { + t.Fatalf("pre-drop failure: want exit 7, got %v", err) + } + if !strings.Contains(err.Error(), "teardown failed") { + t.Errorf("want a plain \"teardown failed\" message, got: %v", err) + } + }) +}