diff --git a/candy/plugin-deploy-pod/config_setup.go b/candy/plugin-deploy-pod/config_setup.go index 0abe9fe..eacbb62 100644 --- a/candy/plugin-deploy-pod/config_setup.go +++ b/candy/plugin-deploy-pod/config_setup.go @@ -120,6 +120,12 @@ func runPodConfigSetup(ctx context.Context, ex *sdk.Executor, c *spec.PodConfigS if rootfulUnsupported(sel.Target, rt.RunMode) { return rootfulRefusal(sel.Engine, rt.RunMode) } + // The launch argv this config step is about to emit names a runtime handler. Refuse HERE, + // before any file is written or any image work runs, when the venue cannot launch it: the + // emitted unit would restart-loop and hold the container name (plugin-containerd#16). + if err := ensureVenueRuntimeHandler(sel.Engine); err != nil { + return err + } return runConfig(ctx, ex, rt, sel, c) } diff --git a/candy/plugin-deploy-pod/config_setup_helpers.go b/candy/plugin-deploy-pod/config_setup_helpers.go index b4ea46e..967f3da 100644 --- a/candy/plugin-deploy-pod/config_setup_helpers.go +++ b/candy/plugin-deploy-pod/config_setup_helpers.go @@ -523,6 +523,11 @@ func runConfigDirect(rt *kit.ResolvedRuntime, sel podSelection, qcfg deploykit.Q if rootfulUnsupported(sel.Target, rt.RunMode) { return rootfulRefusal(sel.Engine, rt.RunMode) } + // The direct launch argv names a runtime handler; refuse one this venue cannot launch + // before anything is stopped or removed (plugin-containerd#16). + if err := ensureVenueRuntimeHandler(sel.Engine); err != nil { + return err + } engine := string(sel.Engine) name := kit.ContainerNameInstance(qcfg.BoxName, qcfg.Instance) stop := engineStopArgv(sel.Target, name) @@ -536,6 +541,10 @@ func runConfigDirect(rt *kit.ResolvedRuntime, sel podSelection, qcfg deploykit.Q cmd := exec.Command(args[0], args[1:]...) out, err := cmd.CombinedOutput() if err != nil { + // The engine creates the container before it starts the task, so a failed launch may + // leave it CREATED with its name held. Remove it with the SAME argv used above, so a + // retry is not blocked by `name ... already used` (plugin-containerd#16). + _ = exec.Command(rm[0], rm[1:]...).Run() return fmt.Errorf("%s run %s failed: %w\n%s", engine, name, err, strings.TrimSpace(string(out))) } cid := strings.TrimSpace(string(out)) diff --git a/candy/plugin-deploy-pod/engine_ops.go b/candy/plugin-deploy-pod/engine_ops.go index 40a5001..7cd0834 100644 --- a/candy/plugin-deploy-pod/engine_ops.go +++ b/candy/plugin-deploy-pod/engine_ops.go @@ -3,6 +3,8 @@ package deploypod import ( "encoding/json" "fmt" + "os/exec" + "sort" "strings" "github.com/opencharly/sdk/deploykit" @@ -136,6 +138,84 @@ func engineDefaultRuntime(c spec.EngineCapability) string { return c.RuntimeHandlers[0] } +// runtimeHandlerBinary maps a runtime-handler word to the executable the launch resolves it +// through. It mirrors the two arms of nerdctl's `generateRuntimeCOpts` (pkg/cmd/container/ +// run_runtime.go) and containerd's shim naming (pkg/shim.BinaryName), because those are the +// programs that decide whether a `--runtime ` can start: +// +// - an ABSOLUTE PATH is the executable itself; +// - a runtime TYPE (`io.containerd.*`) resolves to `containerd-shim--`; +// - every other word is a BARE OCI-runtime binary name, which nerdctl resolves with +// exec.LookPath and launches through the `io.containerd.runc.v2` shim. +// +// An unrepresentable word returns "", which the probe treats as "not installed". +func runtimeHandlerBinary(handler string) string { + if strings.Contains(handler, "/") { + return handler + } + if strings.HasPrefix(handler, "io.containerd.") || handler == "wtf.sbk.runj.v1" { + parts := strings.Split(handler, ".") + if len(parts) < 2 { + return "" + } + return "containerd-shim-" + parts[len(parts)-2] + "-" + parts[len(parts)-1] + } + return handler +} + +// venueRuntimeHandlers returns the SORTED subset of the engine's declared runtime handlers +// that this venue can actually launch: for each candidate it asks the SAME resolution the +// launch will (exec.LookPath over the runtime executable, see runtimeHandlerBinary). +// +// MEASURED on this host's rootless containerd 2.3.5 (nerdctl 2.4.1): `/usr/bin/crun` is +// absent, `nerdctl run --runtime crun` fails `failed to resolve runtime path: invalid +// runtime name crun`, and `--runtime runc` succeeds because `/usr/bin/runc` is installed. +// The venue, not the capability row, is what makes a handler runnable. +func venueRuntimeHandlers(c spec.EngineCapability) []string { + installed := make([]string, 0, len(c.RuntimeHandlers)) + for _, handler := range c.RuntimeHandlers { + binary := runtimeHandlerBinary(handler) + if binary == "" { + continue + } + if _, err := exec.LookPath(binary); err == nil { + installed = append(installed, handler) + } + } + sort.Strings(installed) + return installed +} + +// ensureVenueRuntimeHandler refuses a config/start step whose launch argv would name a +// runtime handler THIS VENUE cannot launch. Without it the capability row alone names the +// handler (`engineDefaultRuntime`), so a venue without that handler emits a unit whose +// ExecStart can never start: systemd restart-loops and the failed attempt leaves the +// container CREATED with its name held, so every retry dies `name ... already used` +// (opencharly/plugin-containerd#16). +// +// It refuses instead of falling back to another handler: a silent switch to a different +// OCI runtime is a different substrate (R3). An engine that declares NO runtime handlers +// (podman, docker) takes exactly the path it took before — no probe, no `--runtime`, no new +// failure mode. +func ensureVenueRuntimeHandler(engine spec.EngineName) error { + c, ok := container.EngineCapabilityFor(string(engine)) + if !ok || len(c.RuntimeHandlers) == 0 { + return nil + } + selected := engineDefaultRuntime(c) + installed := venueRuntimeHandlers(c) + for _, handler := range installed { + if handler == selected { + return nil + } + } + reported := strings.Join(installed, ", ") + if reported == "" { + reported = "none of " + strings.Join(c.RuntimeHandlers, ", ") + } + return fmt.Errorf("engine %q: the selected runtime handler %q is not installed on this venue (the venue reports: %s); remedy: install a %q runtime handler on the venue (the layer-containerd candy installs crun and krun), or point the deploy at a venue that has it", engine, selected, reported, selected) +} + // engineGlobalArgs returns the engine CLI's connection flags from the RESOLVED target. Only // an engine with a separate daemon endpoint declares an address (containerd), so podman and // docker are unchanged; the namespace is the resolved one (deploy > settings > capability diff --git a/candy/plugin-deploy-pod/engine_ops_test.go b/candy/plugin-deploy-pod/engine_ops_test.go index b7e3e4d..ff93d33 100644 --- a/candy/plugin-deploy-pod/engine_ops_test.go +++ b/candy/plugin-deploy-pod/engine_ops_test.go @@ -283,10 +283,13 @@ func TestResolvePodStartSystemdUnit_StartsViaSystemctl(t *testing.T) { } rt := kitResolvedRuntimeForTest("containerd", "systemd-unit", "auto") - plan, err := resolvePodStartSystemdUnit(context.Background(), nil, "demo", "", rt) + plan, sel, err := resolvePodStartSystemdUnit(context.Background(), nil, "demo", "", rt) if err != nil { t.Fatalf("resolvePodStartSystemdUnit: %v", err) } + if sel.Engine != "containerd" { + t.Fatalf("resolved selection: got engine %q, want containerd", sel.Engine) + } if plan.Mode != "systemd-unit" || plan.DirectDeploy { t.Fatalf("plan mode: got %q directDeploy=%v, want systemd-unit/false", plan.Mode, plan.DirectDeploy) } diff --git a/candy/plugin-deploy-pod/lifecycle.go b/candy/plugin-deploy-pod/lifecycle.go index 01dc00e..0da2b42 100644 --- a/candy/plugin-deploy-pod/lifecycle.go +++ b/candy/plugin-deploy-pod/lifecycle.go @@ -127,7 +127,7 @@ func podStart(ctx context.Context, exec *sdk.Executor, p lifecycleParams) (*pb.I return nil, fmt.Errorf("plugin-deploy-pod start: decode opts: %w", err) } box, instance := spec.ParseDeployKey(p.Name) - planPtr, err := resolvePodStartPlan(ctx, exec, box, instance, opts) + planPtr, sel, err := resolvePodStartPlan(ctx, exec, box, instance, opts) if err != nil { return nil, fmt.Errorf("plugin-deploy-pod start: resolve plan: %w", err) } @@ -137,7 +137,7 @@ func podStart(ctx context.Context, exec *sdk.Executor, p lifecycleParams) (*pb.I return nil, fmt.Errorf("plugin-deploy-pod start: mount encrypted volumes: %w", err) } } - if err := podContainerStart(ctx, exec, plan); err != nil { + if err := podContainerStart(ctx, exec, plan, sel); err != nil { return nil, err } if plan.Tunnel != nil { @@ -221,22 +221,51 @@ func podStartCommand(plan spec.PodLifecyclePlan) (cmd, label, target string, err // podContainerStart runs the resolved container start over the served host executor. Mirrors // StartCmd.runQuadlet / runDirect's exec. -func podContainerStart(ctx context.Context, exec *sdk.Executor, plan spec.PodLifecyclePlan) error { +// +// A FAILED start cleans up after itself. The engine creates the container BEFORE it starts +// the task, so a start that dies (a venue whose containerd cannot resolve the runtime +// handler, a missing image at task-create, …) leaves the container CREATED with its name +// held; every retry then fails `name ... already used`, and a unit-mode failure is worse +// because systemd's restart policy keeps re-creating the container. Stop the unit (no +// restart) and remove the partial container through the SAME teardown argv `charly remove` +// uses (engineRemoveArgv), so the next attempt starts from a free name +// (opencharly/plugin-containerd#16). The START error is what the operator sees; a cleanup +// failure is reported beside it, never in place of it. +func podContainerStart(ctx context.Context, ex venueExecutor, plan spec.PodLifecyclePlan, sel podSelection) error { cmd, label, target, err := podStartCommand(plan) if err != nil { return err } - return venueRun(ctx, exec, cmd, label, target) + if err := venueRun(ctx, ex, cmd, label, target); err != nil { + cleanupFailedPodStart(ctx, ex, plan, sel) + return err + } + return nil +} + +// cleanupFailedPodStart releases everything a failed start may have left behind: the unit +// (so Restart=always cannot re-create the container) and the container name itself. Both +// are best-effort and each failure is printed with what it means for the next attempt. +func cleanupFailedPodStart(ctx context.Context, ex venueExecutor, plan spec.PodLifecyclePlan, sel podSelection) { + if kit.IsUnitRunMode(plan.Mode) && !plan.DirectDeploy { + if err := podContainerStop(ctx, ex, plan); err != nil { + fmt.Fprintf(os.Stderr, "Warning: after a failed start, could not stop %s (systemd may restart it): %v\n", plan.SvcName, err) + } + } + rm := engineRemoveArgv(sel.Target, plan.ContainerName) + if err := venueRun(ctx, ex, shellJoin(rm), "remove partial container", plan.ContainerName); err != nil { + fmt.Fprintf(os.Stderr, "Warning: after a failed start, could not remove the partial container %s (a retry may fail with a name-store error): %v\n", plan.ContainerName, err) + } } // podContainerStop runs the resolved container stop: a unit run mode (quadlet or // systemd-unit) → `systemctl --user stop ` (always via systemctl so podman-stop + // Restart=always cannot restart-loop); direct → ` stop `. -func podContainerStop(ctx context.Context, exec *sdk.Executor, plan spec.PodLifecyclePlan) error { +func podContainerStop(ctx context.Context, ex venueExecutor, plan spec.PodLifecyclePlan) error { if kit.IsUnitRunMode(plan.Mode) { - return venueRun(ctx, exec, "systemctl --user stop "+shellquote.ShellQuote(plan.SvcName), "stop", plan.SvcName) + return venueRun(ctx, ex, "systemctl --user stop "+shellquote.ShellQuote(plan.SvcName), "stop", plan.SvcName) } - return venueRun(ctx, exec, plan.EngineBin+" stop "+shellquote.ShellQuote(plan.ContainerName), "stop", plan.ContainerName) + return venueRun(ctx, ex, plan.EngineBin+" stop "+shellquote.ShellQuote(plan.ContainerName), "stop", plan.ContainerName) } // podTunnelOp composes verb:tunnel over InvokeProvider with the {plugin_input:{method,config}} diff --git a/candy/plugin-deploy-pod/resolve.go b/candy/plugin-deploy-pod/resolve.go index 64fc1e4..15508f9 100644 --- a/candy/plugin-deploy-pod/resolve.go +++ b/candy/plugin-deploy-pod/resolve.go @@ -24,10 +24,14 @@ import ( // resolvePodStartPlan builds the pod START plan. The run mode is the runtime's resolved one: // quadlet threads the systemd unit name, systemd-unit threads the engine-emitted service, and // direct threads the engine `start_plan` argv. -func resolvePodStartPlan(ctx context.Context, ex *sdk.Executor, box, instance string, opts spec.PodStartOpts) (*spec.PodLifecyclePlan, error) { +// +// It also returns the SELECTION the plan was built from, so a caller that has to clean up a +// failed start can address the SAME engine target (address/namespace) the launch used, without +// re-deriving it. +func resolvePodStartPlan(ctx context.Context, ex *sdk.Executor, box, instance string, opts spec.PodStartOpts) (*spec.PodLifecyclePlan, podSelection, error) { rt, err := kit.ResolveRuntime() if err != nil { - return nil, err + return nil, podSelection{}, err } // The mode this deploy starts under is the SELECTED engine's, not the host-wide // engine.run's. Resolve the selection BEFORE the mode switch; otherwise a @@ -36,11 +40,11 @@ func resolvePodStartPlan(ctx context.Context, ex *sdk.Executor, box, instance st // engine). The branch bodies resolve the selection again against the effective rt. dc, err := loadDeploy(ctx, ex, "charly start") if err != nil { - return nil, err + return nil, podSelection{}, err } sel, err := resolvePodSelectionFromConfig(dc, box, instance, rt) if err != nil { - return nil, err + return nil, podSelection{}, err } rt = runtimeForSelection(rt, sel) switch rt.RunMode { @@ -56,30 +60,30 @@ func resolvePodStartPlan(ctx context.Context, ex *sdk.Executor, box, instance st // resolvePodStartSystemdUnit resolves the systemd-unit-mode start plan: the engine's emitted // .service (unit_emit) is started with `systemctl --user start `, exactly like the quadlet // path starts podman's generated unit. The unit file's existence is the "configured" signal. -func resolvePodStartSystemdUnit(ctx context.Context, ex *sdk.Executor, box, instance string, rt *kit.ResolvedRuntime) (*spec.PodLifecyclePlan, error) { +func resolvePodStartSystemdUnit(ctx context.Context, ex *sdk.Executor, box, instance string, rt *kit.ResolvedRuntime) (*spec.PodLifecyclePlan, podSelection, error) { dc, err := loadDeploy(ctx, ex, "charly start") if err != nil { - return nil, err + return nil, podSelection{}, err } sel, err := resolvePodSelectionFromConfig(dc, box, instance, rt) if err != nil { - return nil, err + return nil, podSelection{}, err } // The rootful refusal stays FIRST (as it was): the operator gets the reason they cannot // run this deployment, not a secondary "not configured". if rootfulUnsupported(sel.Target, rt.RunMode) { - return nil, rootfulRefusal(sel.Engine, rt.RunMode) + return nil, podSelection{}, rootfulRefusal(sel.Engine, rt.RunMode) } svc := kit.ServiceNameInstance(box, instance) svcDir, err := kit.SystemdUserDir() if err != nil { - return nil, err + return nil, podSelection{}, err } if _, err := os.Stat(filepath.Join(svcDir, svc)); err != nil { if os.IsNotExist(err) { - return nil, fmt.Errorf("not configured; run 'charly config %s' first", box) + return nil, podSelection{}, fmt.Errorf("not configured; run 'charly config %s' first", box) } - return nil, err + return nil, podSelection{}, err } plan := &spec.PodLifecyclePlan{ Mode: "systemd-unit", @@ -89,32 +93,32 @@ func resolvePodStartSystemdUnit(ctx context.Context, ex *sdk.Executor, box, inst } encJSON, err := resolvePodEncEnsurePlan(ctx, ex, dc, box, instance, false) if err != nil { - return nil, err + return nil, podSelection{}, err } plan.Enc = encJSON plan.Tunnel = resolvePodTunnelPlan(dc, box, instance) - return plan, nil + return plan, sel, nil } // resolvePodStartQuadlet resolves the quadlet-mode start plan (the deployed/bed path): the plugin // runs `systemctl --user start ` (or ` start ` through the plan's resolved // engine binary for a direct-deploy marker) + mounts encrypted volumes. -func resolvePodStartQuadlet(ctx context.Context, ex *sdk.Executor, box, instance string, rt *kit.ResolvedRuntime) (*spec.PodLifecyclePlan, error) { +func resolvePodStartQuadlet(ctx context.Context, ex *sdk.Executor, box, instance string, rt *kit.ResolvedRuntime) (*spec.PodLifecyclePlan, podSelection, error) { exists, err := kit.QuadletExistsInstance(box, instance) if err != nil { - return nil, err + return nil, podSelection{}, err } directDeploy := !exists && IsDirectDeploy(box, instance) if !exists && !directDeploy { - return nil, fmt.Errorf("not configured; run 'charly config %s' first", box) + return nil, podSelection{}, fmt.Errorf("not configured; run 'charly config %s' first", box) } dc, err := loadDeploy(ctx, ex, "charly start") if err != nil { - return nil, err + return nil, podSelection{}, err } sel, err := resolvePodSelectionFromConfig(dc, box, instance, rt) if err != nil { - return nil, err + return nil, podSelection{}, err } plan := &spec.PodLifecyclePlan{ Mode: "quadlet", @@ -127,12 +131,12 @@ func resolvePodStartQuadlet(ctx context.Context, ex *sdk.Executor, box, instance if !directDeploy { encJSON, err := resolvePodEncEnsurePlan(ctx, ex, dc, box, instance, false) if err != nil { - return nil, err + return nil, podSelection{}, err } plan.Enc = encJSON } plan.Tunnel = resolvePodTunnelPlan(dc, box, instance) - return plan, nil + return plan, sel, nil } // podRuntimeImage is the resolved pod image context shared by the start-plan resolver and the F12 @@ -212,28 +216,33 @@ func resolvePodRuntimeImageWith(ctx context.Context, ex *sdk.Executor, box, inst // plugin execs, built by the engine `start_plan` body. It returns RunArgv = buildStartArgs(…) // instead of running it, and threads the enc-ensure + tunnel legs. The sidecar-in-direct-mode // rejection is preserved (sidecars require a unit run mode). -func resolvePodStartDirect(ctx context.Context, ex *sdk.Executor, box, instance string, rt *kit.ResolvedRuntime, opts spec.PodStartOpts) (*spec.PodLifecyclePlan, error) { +func resolvePodStartDirect(ctx context.Context, ex *sdk.Executor, box, instance string, rt *kit.ResolvedRuntime, opts spec.PodStartOpts) (*spec.PodLifecyclePlan, podSelection, error) { dc, err := loadDeploy(ctx, ex, "charly start") if err != nil { - return nil, err + return nil, podSelection{}, err } sel, err := resolvePodSelectionFromConfig(dc, box, instance, rt) if err != nil { - return nil, err + return nil, podSelection{}, err } if rootfulUnsupported(sel.Target, rt.RunMode) { - return nil, rootfulRefusal(sel.Engine, rt.RunMode) + return nil, podSelection{}, rootfulRefusal(sel.Engine, rt.RunMode) + } + // The direct launch argv names a runtime handler too; refuse a handler this venue cannot + // launch BEFORE any image/volume work, exactly as the config path does. + if err := ensureVenueRuntimeHandler(sel.Engine); err != nil { + return nil, podSelection{}, err } img, err := resolvePodRuntimeImageWith(ctx, ex, box, instance, "", rt, dc, sel, opts.NoAutoDetect, opts.VolumeFlag, opts.Bind) if err != nil { - return nil, err + return nil, podSelection{}, err } detected, imageRef, meta := img.detected, img.imageRef, img.meta volumes, bindMounts := img.volumes, img.bindMounts if dc != nil { if overlay, ok := dc.Lookup(box, instance); ok && len(overlay.Sidecar) > 0 { - return nil, fmt.Errorf("image %s has sidecars configured in charly.yml; use 'charly config %s && charly start %s' (sidecars require quadlet mode)", box, box, box) + return nil, podSelection{}, fmt.Errorf("image %s has sidecars configured in charly.yml; use 'charly config %s && charly start %s' (sidecars require quadlet mode)", box, box, box) } } @@ -245,10 +254,10 @@ func resolvePodStartDirect(ctx context.Context, ex *sdk.Executor, box, instance encJSON, err := resolvePodEncEnsurePlan(ctx, ex, dc, box, instance, false) if err != nil { - return nil, err + return nil, podSelection{}, err } if err := deploykit.VerifyBindMounts(bindMounts, box); err != nil { - return nil, err + return nil, podSelection{}, err } deployEnv := meta.Env @@ -260,7 +269,7 @@ func resolvePodStartDirect(ctx context.Context, ex *sdk.Executor, box, instance } envVars, err := kit.ResolveEnvVars(startGlobalEnv, deployEnv, "", workspaceBindHost(bindMounts), opts.EnvFile, opts.Env) if err != nil { - return nil, err + return nil, podSelection{}, err } if !security.Privileged { @@ -273,20 +282,20 @@ func resolvePodStartDirect(ctx context.Context, ex *sdk.Executor, box, instance resolvedNetwork, netErr := kit.ResolveNetwork(network, string(sel.Engine)) if netErr != nil { - return nil, netErr + return nil, podSelection{}, netErr } if len(opts.Port) > 0 { ports, err = kit.ApplyPortOverrides(ports, opts.Port) if err != nil { - return nil, err + return nil, podSelection{}, err } // #55 K4: write the recomputed ports PLUGIN-SIDE (deploykit.SaveDeployState), not over the // deleted "deploy-config-save-state" host seam. deploySaveState(ctx, ex, box, instance, spec.SaveDeployStateInput{Ports: ports, SetPorts: true}) } if conflicts := kit.CheckPortAvailability(ports, rt.BindAddress, string(sel.Engine)); len(conflicts) > 0 { - return nil, fmt.Errorf("port conflicts detected:%s", kit.FormatPortConflicts(conflicts, box)) + return nil, podSelection{}, fmt.Errorf("port conflicts detected:%s", kit.FormatPortConflicts(conflicts, box)) } var deployBox *spec.DeployNode @@ -305,7 +314,7 @@ func resolvePodStartDirect(ctx context.Context, ex *sdk.Executor, box, instance workDir := deploykit.ResolveWorkingDir(volumes, bindMounts, home, box, instance) argv, err := buildStartArgs(sel, imageRef, uid, gid, ports, name, volumes, bindMounts, detected.GPU, rt.BindAddress, envVars, security, entrypoint, workDir, resolvedNetwork) if err != nil { - return nil, err + return nil, podSelection{}, err } tunnel := resolvePodTunnelPlan(dc, box, instance) @@ -313,7 +322,7 @@ func resolvePodStartDirect(ctx context.Context, ex *sdk.Executor, box, instance return &spec.PodLifecyclePlan{ Mode: "direct", ContainerName: name, RunArgv: argv, EngineBin: sel.binary(), Enc: encJSON, Tunnel: tunnel, - }, nil + }, sel, nil } // resolvePodStopPlan builds the pod STOP plan. diff --git a/candy/plugin-deploy-pod/venue_runtime_test.go b/candy/plugin-deploy-pod/venue_runtime_test.go new file mode 100644 index 0000000..84d5ffd --- /dev/null +++ b/candy/plugin-deploy-pod/venue_runtime_test.go @@ -0,0 +1,242 @@ +package deploypod + +// venue_runtime_test.go — the venue-handler gate (opencharly/plugin-containerd#16) and the +// failed-start cleanup. +// +// The gate arms describe the VENUE with PATH, because that is the resolution nerdctl applies +// to a bare `--runtime` word (exec.LookPath) — the same resolution the emitted unit's +// ExecStart will perform. A test that inherited the machine's PATH could not state which +// venue it is describing. + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/opencharly/sdk" + "github.com/opencharly/sdk/deploykit" + "github.com/opencharly/spec/hostenv" + "github.com/opencharly/spec/spec" +) + +// setVenuePath makes PATH contain ONLY the named executables, so the test states the venue +// instead of inheriting the machine's own runtime installs. +func setVenuePath(t *testing.T, binaries ...string) { + t.Helper() + dir := t.TempDir() + for _, name := range binaries { + p := filepath.Join(dir, name) + if err := os.WriteFile(p, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatalf("write venue binary %s: %v", name, err) + } + } + t.Setenv("PATH", dir) +} + +// TestEnsureVenueRuntimeHandler_RefusesHandlerAbsentFromVenue is the decision arm: a venue that +// serves runc but NOT crun must be refused with all three facts — the handler charly selected, +// the handlers the venue reports, and the remedy. +func TestEnsureVenueRuntimeHandler_RefusesHandlerAbsentFromVenue(t *testing.T) { + setVenuePath(t, "runc", "containerd-shim-runc-v2") + + err := ensureVenueRuntimeHandler("containerd") + if err == nil { + t.Fatal("a venue without crun must be refused, not launched with an unresolvable handler") + } + msg := err.Error() + for _, want := range []string{`"crun"`, "runc", "remedy"} { + if !strings.Contains(msg, want) { + t.Errorf("refusal must name %q, got: %v", want, err) + } + } + if strings.Contains(msg, `"krun"`) || strings.Contains(msg, `"nvidia"`) { + t.Errorf("the refusal must not claim the venue reports handlers it does not have: %v", err) + } + + // The observed set is exactly the engine's candidates the venue can launch — one source of + // truth (the capability row), not a second hand-written list. + c, ok := engineCapabilityForTest("containerd") + if !ok { + t.Fatal("containerd capability row missing") + } + if got := strings.Join(venueRuntimeHandlers(c), ","); got != "runc" { + t.Errorf("venue handlers: got %q, want runc", got) + } +} + +// TestRuntimeHandlerBinary_RuntimeTypeResolvesToShim pins the runtime-TYPE arm: containerd +// resolves `io.containerd.runc.v2` through its shim binary, not through a same-named executable. +func TestRuntimeHandlerBinary_RuntimeTypeResolvesToShim(t *testing.T) { + cases := map[string]string{ + "crun": "crun", + "/usr/bin/crun": "/usr/bin/crun", + "io.containerd.runc.v2": "containerd-shim-runc-v2", + "io.containerd.crun.v1": "containerd-shim-crun-v1", + "wtf.sbk.runj.v1": "containerd-shim-runj-v1", + } + for handler, want := range cases { + if got := runtimeHandlerBinary(handler); got != want { + t.Errorf("runtimeHandlerBinary(%q) = %q, want %q", handler, got, want) + } + } +} + +// TestEnsureVenueRuntimeHandler_ProceedsWhenVenueHasSelectedHandler is the control: a venue that +// serves the selected handler proceeds with NO refusal and NO fallback — the same `crun` word. +func TestEnsureVenueRuntimeHandler_ProceedsWhenVenueHasSelectedHandler(t *testing.T) { + setVenuePath(t, "crun", "runc") + + if err := ensureVenueRuntimeHandler("containerd"); err != nil { + t.Fatalf("a venue with crun must not be refused: %v", err) + } +} + +// TestEnsureVenueRuntimeHandler_PodmanAndDockerUnchanged pins the no-new-failure-mode rule: an +// engine with an empty runtime_handlers list takes exactly the path it took before, on a venue +// that has NO container runtime binaries at all. +func TestEnsureVenueRuntimeHandler_PodmanAndDockerUnchanged(t *testing.T) { + t.Setenv("PATH", t.TempDir()) // nothing installed + + for _, engine := range []spec.EngineName{"podman", "docker"} { + if err := ensureVenueRuntimeHandler(engine); err != nil { + t.Errorf("%s has no runtime_handlers list; the gate must not fire: %v", engine, err) + } + } +} + +// TestRunPodConfigSetup_RefusesHandlerAbsentFromVenue drives the REAL config entry point, so the +// gate is proved where the defect fires: `charly config` on a containerd pod deploy whose venue +// has no crun must refuse BEFORE the config walk, so no unit is emitted and no restart loop can +// start. +// +// NEGATIVE CONTROL: on origin/main this test FAILS — the config walk proceeds (panics on the +// absent executor here) instead of refusing, and its error, if any, does not name crun. +func TestRunPodConfigSetup_RefusesHandlerAbsentFromVenue(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yml") + t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmpDir, "xdg")) + + orig := hostenv.RuntimeConfigPath + defer func() { hostenv.RuntimeConfigPath = orig }() + hostenv.RuntimeConfigPath = func() (string, error) { return configPath, nil } + + t.Setenv("CHARLY_RUN_ENGINE", "containerd") + t.Setenv("CHARLY_RUN_MODE", "systemd-unit") + t.Setenv("CHARLY_ENGINE_ROOTFUL", "auto") + setVenuePath(t, "runc") // the venue serves runc; crun is absent + + origLoad := loadDeploy + defer func() { loadDeploy = origLoad }() + loadDeploy = func(context.Context, *sdk.Executor, string) (*deploykit.DeployConfig, error) { + return nil, nil // no per-host overlay: the selection resolves from the runtime level + } + + var ( + err error + proceeded bool + ) + func() { + defer func() { + if r := recover(); r != nil { + proceeded = true // the config walk ran: nothing refused it + } + }() + err = runPodConfigSetup(context.Background(), nil, &spec.PodConfigSetupRequest{Box: "demo"}) + }() + if proceeded || err == nil { + t.Fatal("charly config proceeded into the config walk on a venue without crun; it must refuse first (plugin-containerd#16)") + } + for _, want := range []string{`"crun"`, "runc", "remedy"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("refusal must name %q, got: %v", want, err) + } + } + + // A refusal means NO unit was emitted and NO quadlet was written for the deploy. + if _, statErr := os.Stat(filepath.Join(tmpDir, "xdg", "systemd", "user", "charly-demo.service")); !os.IsNotExist(statErr) { + t.Errorf("a refused config must not emit a unit (stat err=%v)", statErr) + } + if _, statErr := os.Stat(filepath.Join(tmpDir, "xdg", "containers", "systemd", "charly-demo.container")); !os.IsNotExist(statErr) { + t.Errorf("a refused config must not write a quadlet (stat err=%v)", statErr) + } +} + +// recordingVenue is the venue seam for the cleanup arms: it records every script and fails the +// one the test names. +type recordingVenue struct { + calls []string + failOn string +} + +func (v *recordingVenue) VenueCapture(_ context.Context, script string) (string, error) { + v.calls = append(v.calls, script) + if v.failOn != "" && strings.Contains(script, v.failOn) { + return "", errors.New("exit status 1") + } + return "", nil +} + +func (v *recordingVenue) VenueRunSilent(context.Context, string) error { return nil } + +// TestPodContainerStart_CleansUpFailedUnitStart proves the second half of #16: a failed unit +// start stops the unit (so Restart=always cannot re-create the container) and removes the +// partial container through the resolved containerd target, freeing the name for a retry. +func TestPodContainerStart_CleansUpFailedUnitStart(t *testing.T) { + sel := selectionForTest(t, deploykit.Selection{}, "containerd") + plan := spec.PodLifecyclePlan{ + Mode: "systemd-unit", + SvcName: "charly-demo.service", + ContainerName: "charly-demo", + EngineBin: "nerdctl", + } + venue := &recordingVenue{failOn: "systemctl --user start"} + + if err := podContainerStart(context.Background(), venue, plan, sel); err == nil { + t.Fatal("a failing start must be reported") + } + want := []string{ + "systemctl --user start 'charly-demo.service'", + "systemctl --user stop 'charly-demo.service'", + "'nerdctl' '--address' '" + sel.Target.Address + "' '--namespace' '" + sel.Target.Namespace + "' 'rm' '-f' 'charly-demo'", + } + if len(venue.calls) != len(want) { + t.Fatalf("cleanup calls: got %v, want %v", venue.calls, want) + } + for i := range want { + if venue.calls[i] != want[i] { + t.Errorf("cleanup call %d: got %q, want %q", i, venue.calls[i], want[i]) + } + } + if sel.Target.Address == "" || sel.Target.Namespace == "" { + t.Fatalf("test target is not resolved: %+v", sel.Target) + } +} + +// TestPodContainerStart_CleansUpFailedDirectStart is the same proof for a direct launch: no unit +// to stop, so the container itself must be removed with the SAME argv shape. +func TestPodContainerStart_CleansUpFailedDirectStart(t *testing.T) { + sel := selectionForTest(t, deploykit.Selection{}, "containerd") + plan := spec.PodLifecyclePlan{ + Mode: "direct", + ContainerName: "charly-demo", + EngineBin: "nerdctl", + RunArgv: []string{"nerdctl", "run", "--name", "charly-demo", "img"}, + } + venue := &recordingVenue{failOn: "'run'"} + + if err := podContainerStart(context.Background(), venue, plan, sel); err == nil { + t.Fatal("a failing start must be reported") + } + if len(venue.calls) != 2 { + t.Fatalf("cleanup calls: got %v, want the launch then one rm", venue.calls) + } + if !strings.Contains(venue.calls[1], "'rm' '-f' 'charly-demo'") { + t.Errorf("direct cleanup must remove the partial container, got %q", venue.calls[1]) + } + if strings.Contains(venue.calls[1], "systemctl") { + t.Errorf("a direct launch has no unit to stop, got %q", venue.calls[1]) + } +}