fix(check): tear down a kind:local bed in place, never with the container verb

candy/plugin-check/bed_debug_hint_test.go

@@ -43,3 +43,27 @@ func TestDebugRetentionNotice_VMHintsUseDeployDomain(t *testing.T) {
 		t.Errorf("destroy hint is missing the --domain the cleanup path uses:\n%s", got)
 	}
 }
+
+// TestDebugRetentionNotice_LocalHintUsesDeployDel pins the local half of the same
+// operator-facing guarantee (opencharly/plugin-check#129): a kind:local apply is torn
+// down with the deploy-tree verb `charly deploy del`, exactly what the cleanup path
+// issues, never the container verb `charly remove` — which dies with `no container
+// engine found` on the host venues a local bed actually runs on.
+func TestDebugRetentionNotice_LocalHintUsesDeployDel(t *testing.T) {
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("pipe: %v", err)
+	}
+	printDebugRetentionNotice(w, "check-local-bed", spec.CheckBedReply{IsLocal: true}, "")
+	_ = w.Close()
+	buf := make([]byte, 4096)
+	n, _ := r.Read(buf)
+	got := string(buf[:n])
+
+	if !strings.Contains(got, "charly deploy del check-local-bed") {
+		t.Errorf("local hint does not print the deploy verb the cleanup path issues:\n%s", got)
+	}
+	if strings.Contains(got, "charly remove") {
+		t.Errorf("local hint still prints the container verb `charly remove`:\n%s", got)
+	}
+}

candy/plugin-check/bed_local_teardown_test.go

@@ -0,0 +1,84 @@
+package check
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+// TestLocalBedTeardownNeverUsesTheContainerVerb is the regression guard for
+// opencharly/plugin-check#129. A kind:local (`host: local`) bed was torn down with the
+// CONTAINER verb `charly remove --purge` in BOTH cleanup switches, because each switch
+// tested d.IsExternal alone and let d.IsLocal fall through into the pod arm. The arm is
+// reached only after every authored assertion has passed, so a host venue with no
+// container engine (a kind:local venue's normal shape) could never pass a bed — the
+// run ended FAIL (steps=N) with `no container engine found (install podman or docker)`
+// at the LAST step.
+//
+// The test reads bed_run.go and requires the ONE teardown decision to own both halves:
+//   - no switch may test d.IsExternal alone (the fall-through this defect is made of);
+//   - the pre-run switch and the final cleanup switch must both call
+//     bedRootTeardownArgs(d, name, isKubeVirt);
+//   - the failure-path retention hint for a local bed must print `charly deploy del`,
+//     the verb the cleanup path itself issues, never `charly remove`.
+//
+// It is a source-structure test on purpose: the two switches live inside the bed-run
+// closure and are only reachable after a full deploy, so a unit test cannot walk them
+// without a venue. The behavioural half of the same predicate is pinned in
+// bed_root_teardown_test.go.
+func TestLocalBedTeardownNeverUsesTheContainerVerb(t *testing.T) {
+	src, err := os.ReadFile("bed_run.go")
+	if err != nil {
+		t.Fatalf("reading bed_run.go: %v", err)
+	}
+	text := string(src)
+
+	preRunCalls, cleanupCalls := 0, 0
+	for i, line := range strings.Split(text, "\n") {
+		// The defect's signature: an in-place arm guarded by IsExternal ONLY, so a
+		// local bed falls through to the container arm below it.
+		if strings.Contains(line, "if d.IsExternal {") {
+			t.Errorf("bed_run.go:%d tests IsExternal alone — a local bed falls through to the container arm: %s",
+				i+1, strings.TrimSpace(line))
+		}
+		if strings.Contains(line, "bestEffort(bedRootTeardownArgs(d, name, isKubeVirt)...)") {
+			preRunCalls++
+		}
+		if strings.Contains(line, `step("cleanup", bedRootTeardownArgs(d, name, isKubeVirt)...)`) {
+			cleanupCalls++
+		}
+	}
+	if preRunCalls != 1 {
+		t.Errorf("pre-run cleanup routes through bedRootTeardownArgs at %d sites, want 1 — the pre-run and final teardown decisions must be the SAME helper", preRunCalls)
+	}
+	if cleanupCalls != 1 {
+		t.Errorf("final cleanup routes through bedRootTeardownArgs at %d sites, want 1 — the pre-run and final teardown decisions must be the SAME helper", cleanupCalls)
+	}
+
+	hint := localRetentionHint(text)
+	if hint == "" {
+		t.Fatal("could not locate the local retention hint in bed_run.go")
+	}
+	if !strings.Contains(hint, "charly deploy del") {
+		t.Errorf("local retention hint does not print the deploy verb:\n%s", hint)
+	}
+	if strings.Contains(hint, "charly remove") {
+		t.Errorf("local retention hint still prints the container verb `charly remove`:\n%s", hint)
+	}
+}
+
+// localRetentionHint returns the `case d.IsLocal:` block of printDebugRetentionNotice —
+// the operator-facing hint printed when a FAILED local bed's apply is left in place.
+func localRetentionHint(src string) string {
+	const marker = "local apply left in place for debugging"
+	i := strings.Index(src, marker)
+	if i < 0 {
+		return ""
+	}
+	start := strings.LastIndex(src[:i], "case d.IsLocal:")
+	end := strings.Index(src[i:], "case d.IsExternal:")
+	if start < 0 || end < 0 {
+		return ""
+	}
+	return src[start : i+end]
+}

candy/plugin-check/bed_root_teardown_test.go

@@ -0,0 +1,67 @@
+package check
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/opencharly/spec/spec"
+)
+
+// TestBedRootTeardownArgsForEveryVenue pins the ONE root-teardown decision both cleanup
+// switches read (opencharly/plugin-check#129). An in-place venue — kind:local (host-rooted)
+// or external-in-place — has no container lifecycle, so it must be torn down with the
+// deploy-tree verb `charly deploy del` and must NEVER reach the container verb
+// `charly remove`. A kubevirt root is also `deploy del` (its plugin owns the CR teardown);
+// only a container pod keeps `remove --purge` (the sole path to purgeDeployArtifacts).
+//
+// The mutant this kills: restoring the pre-fix predicate (an arm that tests IsExternal
+// only) makes the local row produce `["remove", …]` and fails the test.
+func TestBedRootTeardownArgsForEveryVenue(t *testing.T) {
+	const name = "check-k3s-shared-containerd"
+	cases := []struct {
+		name     string
+		d        spec.CheckBedReply
+		kubevirt bool
+		want     []string
+	}{
+		{"kind:local host venue → deploy del", spec.CheckBedReply{IsLocal: true}, false, []string{"deploy", "del", name}},
+		{"external in-place → deploy del", spec.CheckBedReply{IsExternal: true}, false, []string{"deploy", "del", name}},
+		{"kubevirt root → deploy del", spec.CheckBedReply{}, true, []string{"deploy", "del", name}},
+		{"container pod → remove --purge", spec.CheckBedReply{}, false, []string{"remove", name, "--purge"}},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := bedRootTeardownArgs(tc.d, name, tc.kubevirt)
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Fatalf("bedRootTeardownArgs(%+v, kubevirt=%v) = %v, want %v", tc.d, tc.kubevirt, got, tc.want)
+			}
+			// The defect's user-visible symptom: a local bed invoking the container verb.
+			if (tc.d.IsLocal || tc.d.IsExternal) && got[0] == "remove" {
+				t.Errorf("in-place venue %+v invoked the container verb %q; it has no container to remove", tc.d, got[0])
+			}
+		})
+	}
+}
+
+// TestBedInPlaceCoversBothInPlaceVenues pins the shared predicate: only a local or an
+// external-in-place venue is in place. A pod must not be, or its `charly config`/`start`
+// bring-up would be skipped as well.
+func TestBedInPlaceCoversBothInPlaceVenues(t *testing.T) {
+	cases := []struct {
+		name string
+		d    spec.CheckBedReply
+		want bool
+	}{
+		{"kind:local is in place", spec.CheckBedReply{IsLocal: true}, true},
+		{"external-in-place is in place", spec.CheckBedReply{IsExternal: true}, true},
+		{"local external is in place", spec.CheckBedReply{IsLocal: true, IsExternal: true}, true},
+		{"container pod is NOT in place", spec.CheckBedReply{}, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := bedInPlace(tc.d); got != tc.want {
+				t.Fatalf("bedInPlace(%+v) = %v, want %v", tc.d, got, tc.want)
+			}
+		})
+	}
+}

candy/plugin-check/bed_run.go

@@ -312,6 +312,33 @@ func vmBedDestroyArgs(d spec.CheckBedReply, opts bedRunOpts, snap *spec.VmSnapsh
 	return argv
 }
 
+// bedInPlace reports whether the bed's venue is applied IN PLACE: a kind:local
+// (host-rooted) deploy or an external-in-place substrate (spec/deploy's HostRooted and
+// ExternalInPlaceVenue). Neither has a container/VM lifecycle — their candies are applied
+// during `charly deploy add` — so their teardown is the deploy-tree verb
+// `charly deploy del`, never the container verb `charly remove`. This is the ONE
+// predicate every call site reads (R2): the isInPlace gate and the root-teardown helper
+// below, so a local bed cannot fall through into a pod arm again
+// (opencharly/plugin-check#129).
+func bedInPlace(d spec.CheckBedReply) bool {
+	return d.IsLocal || d.IsExternal
+}
+
+// bedRootTeardownArgs is the ONE root-venue teardown command for a NON-VM bed; the VM's
+// own `vm destroy` arm (which carries the domain/disk flags) stays separate. A kubevirt
+// root (its plugin owns the CR teardown) and both in-place venues are torn down with the
+// deploy verb `charly deploy del`; a container pod keeps `remove --purge`, which is the
+// ONLY path that reaches purgeDeployArtifacts (named volumes, gocryptfs volumes and the
+// synthesized <name>-overlay images). Both cleanup switches call this helper, so the two
+// halves of a bed run cannot drift — and a local bed cannot reach the container verb
+// (opencharly/plugin-check#129).
+func bedRootTeardownArgs(d spec.CheckBedReply, name string, isKubeVirt bool) []string {
+	if isKubeVirt || bedInPlace(d) {
+		return []string{"deploy", "del", name}
+	}
+	return []string{"remove", name, "--purge"}
+}
+
 // keepVenueOnSuccess is the SUCCESS tail's retention decision, and it is deliberately the
 // PRE-CHANGE gate exactly: `--keep` / `--keep-venue` / a bed's `keep_venue:` policy (all of which
 // force opts.Keep). It carries no other arm, so a passing run tears its venue down exactly as it
@@ -726,17 +753,11 @@ func runCheckBed(ctx context.Context, ex *sdk.Executor, name string, opts bedRun
 		if opts.Anchor == "" {
 			bestEffort("vm", "destroy", d.VMTemplate, "--domain", d.BedDomain, "--if-exists")
 		}
-	case isKubeVirt:
-		// A kubevirt bed's teardown is `charly deploy del` — candy/plugin-kubevirt's
-		// OpPostTeardown removes the VirtualMachine CR and stops the managed port-forward
-		// (the libvirt-domain teardown does not apply). Same in-place external shape.
-		bestEffort("deploy", "del", name)
 	default:
-		if d.IsExternal {
-			bestEffort("deploy", "del", name)
-		} else {
-			bestEffort("remove", name, "--purge")
-		}
+		// Non-VM roots: kubevirt, in-place local/external and pod all take the ONE
+		// teardown decision (bedRootTeardownArgs) — a kubevirt CR and an in-place venue
+		// are `deploy del`, a container pod is `remove --purge`, never the reverse.
+		bestEffort(bedRootTeardownArgs(d, name, isKubeVirt)...)
 		_ = deploykit.TearDownMembers(ctx, &bedNode)
 	}
 
@@ -960,12 +981,11 @@ func runCheckBed(ctx context.Context, ex *sdk.Executor, name string, opts bedRun
 		switch {
 		case d.IsVM:
 			targetErr = step("cleanup", vmBedDestroyArgs(d, opts, bedNode.Snapshot)...)
-		case isKubeVirt:
-			targetErr = step("cleanup", "deploy", "del", name)
-		case d.IsExternal:
-			targetErr = step("cleanup", "deploy", "del", name)
 		default:
-			targetErr = step("cleanup", "remove", name, "--purge")
+			// Every non-VM root takes the ONE teardown decision — kubevirt and both
+			// in-place venues via `deploy del`, a container pod via `remove --purge`.
+			// A local bed has no container engine to remove from (#129).
+			targetErr = step("cleanup", bedRootTeardownArgs(d, name, isKubeVirt)...)
 		}
 		membersErr := phase("cleanup-members", func() error {
 			return deploykit.TearDownMembers(ctx, &bedNode)
@@ -1050,8 +1070,9 @@ func runCheckBed(ctx context.Context, ex *sdk.Executor, name string, opts bedRun
 
 	// isInPlace unifies local + in-place-external: they apply candies in place during
 	// `charly deploy add` (no container/VM lifecycle — no `charly config`/`charly
-	// start`, teardown via `charly deploy del`).
-	isInPlace := d.IsLocal || d.IsExternal
+	// start`, teardown via `charly deploy del`). ONE predicate (bedInPlace) with the
+	// cleanup switches above, so the bring-up and teardown halves cannot drift.
+	isInPlace := bedInPlace(d)
 
 	// Steps 1+2: image build + check box. A VM/kubevirt substrate is an image-backed
 	// CR (no box build); a kind:local deploy carries no image. An EXTERNAL-IN-PLACE
@@ -1550,8 +1571,10 @@ func printDebugRetentionNotice(w io.Writer, name string, d spec.CheckBedReply, r
 			"  destroy: charly vm destroy %s --domain %s\n",
 			name, "charly-"+d.BedDomain, live, d.BedDomain, d.VMTemplate, d.BedDomain)
 	case d.IsLocal:
+		// A local apply has no container to remove: its teardown is the deploy-tree verb,
+		// the same one the cleanup path issues (bedRootTeardownArgs).
 		fmt.Fprintf(w, "\n[charly check run] bed %q FAILED — local apply left in place for debugging.\n"+
-			"  destroy: charly remove %s\n", name, name)
+			"  destroy: charly deploy del %s\n", name, name)
 	case d.IsExternal:
 		fmt.Fprintf(w, "\n[charly check run] bed %q FAILED — external deploy apply left in place for debugging.\n"+
 			"  destroy: charly deploy del %s\n", name, name)