diff --git a/candy/plugin-deploy-pod/config_remove.go b/candy/plugin-deploy-pod/config_remove.go index 53d999e..bd9af9f 100644 --- a/candy/plugin-deploy-pod/config_remove.go +++ b/candy/plugin-deploy-pod/config_remove.go @@ -47,6 +47,9 @@ func runPodConfigRemove(ctx context.Context, ex *sdk.Executor, c *spec.PodConfig if err != nil { return err } + // Disable the unit the SELECTED engine actually emitted, not the mode auto-detected for the + // host-wide engine.run (opencharly/charly#1109's config/start pair shares this resolution). + rt = runtimeForSelection(rt, sel) if rt.RunMode == "direct" || IsDirectDeploy(boxName, c.Instance) { name := kit.ContainerNameInstance(boxName, c.Instance) diff --git a/candy/plugin-deploy-pod/config_setup.go b/candy/plugin-deploy-pod/config_setup.go index 864a2cf..0abe9fe 100644 --- a/candy/plugin-deploy-pod/config_setup.go +++ b/candy/plugin-deploy-pod/config_setup.go @@ -113,6 +113,10 @@ func runPodConfigSetup(ctx context.Context, ex *sdk.Executor, c *spec.PodConfigS if err != nil { return err } + // The deploy's own engine: overrides the host-wide engine.run, so the emitted persistence + // mode must be THAT engine's (containerd → systemd-unit), never the mode auto-detected for + // a different global engine (opencharly/charly#1109). + rt = runtimeForSelection(rt, sel) if rootfulUnsupported(sel.Target, rt.RunMode) { return rootfulRefusal(sel.Engine, rt.RunMode) } @@ -202,7 +206,7 @@ func runConfig(ctx context.Context, ex *sdk.Executor, rt *kit.ResolvedRuntime, s if err != nil { return err } - if err := ensureImagePresent(ctx, ex, imageRef, sel.Target); err != nil { + if err := ensureImagePresent(ctx, ex, imageRef, sel.Target, c.Box); err != nil { return err } // ExtractMetadata PLUGIN-SIDE (#55 coneC-dsh — the pod-config-ensure-image host seam shrinks to diff --git a/candy/plugin-deploy-pod/config_setup_helpers.go b/candy/plugin-deploy-pod/config_setup_helpers.go index d893588..b4ea46e 100644 --- a/candy/plugin-deploy-pod/config_setup_helpers.go +++ b/candy/plugin-deploy-pod/config_setup_helpers.go @@ -596,7 +596,7 @@ func provisionData(ctx context.Context, ex *sdk.Executor, rt *kit.ResolvedRuntim dataRef = resolved } } - if err := ensureImagePresent(ctx, ex, dataRef, sel.Target); err != nil { + if err := ensureImagePresent(ctx, ex, dataRef, sel.Target, c.Box); err != nil { return fmt.Errorf("extracting metadata from data image %s: %w", dataRef, err) } dm, err := deploykit.ExtractMetadata(dataEngine, dataRef) @@ -707,7 +707,7 @@ func updateAllDeployedQuadlets(ctx context.Context, ex *sdk.Executor, rt *kit.Re fmt.Fprintf(os.Stderr, "Warning: could not resolve the engine for %s, skipping quadlet update: %v\n", key, selErr) continue } - if err := ensureImagePresent(ctx, ex, imageRef, updateSel.Target); err != nil { + if err := ensureImagePresent(ctx, ex, imageRef, updateSel.Target, boxName); err != nil { fmt.Fprintf(os.Stderr, "Warning: could not read metadata for %s, skipping quadlet update\n", key) continue } diff --git a/candy/plugin-deploy-pod/go.mod b/candy/plugin-deploy-pod/go.mod index 4c18f52..7951d05 100644 --- a/candy/plugin-deploy-pod/go.mod +++ b/candy/plugin-deploy-pod/go.mod @@ -2,7 +2,7 @@ module github.com/opencharly/plugin-deploy-pod/candy/plugin-deploy-pod go 1.26.8 -require github.com/opencharly/sdk v0.2026296.1207 +require github.com/opencharly/sdk v0.2026309.1926 require ( github.com/containerd/containerd/v2 v2.4.1 // indirect @@ -29,7 +29,7 @@ require ( github.com/mattn/go-isatty v0.0.17 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/oklog/run v1.1.0 // indirect - github.com/opencharly/spec v0.2026296.1036 + github.com/opencharly/spec v0.2026309.1917 github.com/pelletier/go-toml/v2 v2.4.3 // indirect github.com/protocolbuffers/txtpbfmt v0.0.0-20260217160748-a481f6a22f94 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect diff --git a/candy/plugin-deploy-pod/go.sum b/candy/plugin-deploy-pod/go.sum index f709514..5c1b82d 100644 --- a/candy/plugin-deploy-pod/go.sum +++ b/candy/plugin-deploy-pod/go.sum @@ -90,8 +90,12 @@ github.com/opencharly/plugin-init/candy/plugin-init v0.2026237.1424 h1:Bldh1DoES github.com/opencharly/plugin-init/candy/plugin-init v0.2026237.1424/go.mod h1:tzB4pMZllnxK6Jpyeq5jxg4EQe1p544rQkhNdmPn788= github.com/opencharly/sdk v0.2026296.1207 h1:FcLqgjV8q6TsFo8cQ4pcAWLYTKM37BFeTj6dpUGf9pY= github.com/opencharly/sdk v0.2026296.1207/go.mod h1:JATa64xjMSgqEdOhRRm4iiptgfaetvmHOwybt9kKZko= +github.com/opencharly/sdk v0.2026309.1926 h1:DLhLwsT5E17bEYh6iGVRs+BYz6r1bUQ04zq5UgnlExw= +github.com/opencharly/sdk v0.2026309.1926/go.mod h1:r8zMVnKjFjkOaIlRTmlRA/jCtDhRn1doLR0PedHhDgs= github.com/opencharly/spec v0.2026296.1036 h1:F3Up2zG5hLw2SFVOR772cMQ1JegU/+HjoK8dOPGR/AU= github.com/opencharly/spec v0.2026296.1036/go.mod h1:VrjzjP2glMbDwP4JNm50OV0MraHFZnFKTl+bVlouoso= +github.com/opencharly/spec v0.2026309.1917 h1:DHjf0t7TloPeG44PNSxqlZBvxWv1ssEZ+4UnQZi94E0= +github.com/opencharly/spec v0.2026309.1917/go.mod h1:VrjzjP2glMbDwP4JNm50OV0MraHFZnFKTl+bVlouoso= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040= diff --git a/candy/plugin-deploy-pod/image_ensure.go b/candy/plugin-deploy-pod/image_ensure.go index 6e6f7d1..e458e28 100644 --- a/candy/plugin-deploy-pod/image_ensure.go +++ b/candy/plugin-deploy-pod/image_ensure.go @@ -86,7 +86,7 @@ func newImageStore(ex *sdk.Executor, target deploykit.ContainerdTarget) (deployk // store itself (no host round-trip): (1) the target store already addresses it, (2) otherwise // build:ensure (W3.6) stages it — pulling from the registry or building locally — and the store // is re-probed to prove the boundary. -func ensureImagePresent(ctx context.Context, ex *sdk.Executor, imageRef string, target deploykit.ContainerdTarget) error { +func ensureImagePresent(ctx context.Context, ex *sdk.Executor, imageRef string, target deploykit.ContainerdTarget, box string) error { store, err := newImageStore(ex, target) if err != nil { return err @@ -106,8 +106,15 @@ func ensureImagePresent(ctx context.Context, ex *sdk.Executor, imageRef string, return nil } // Step 4 — build:ensure. It receives the SAME resolved engine word, so the image lands in - // the store just probed. - if err := dispatchBuildEnsurePeer(ctx, ex, imageRef, string(target.Engine)); err != nil { + // the store just probed, AND the project dir: a bed's image is built LOCALLY under the + // project's registry/name, so with no dir build:ensure cannot rebuild it into the target + // store and fails "buildkitdrive: build: no context" (measured: a disposable pod bed whose + // image is not registry-published cannot reach a containerd store without this). + dir, derr := projectDirFor(ctx, ex, box) + if derr != nil { + return fmt.Errorf("%w: %s: resolving project dir for the build fallback: %v", spec.ErrImageNotLocal, imageRef, derr) + } + if err := dispatchBuildEnsurePeer(ctx, ex, imageRef, string(target.Engine), dir); err != nil { return fmt.Errorf("%w: %s: %v", spec.ErrImageNotLocal, imageRef, err) } present, err = store.Has(ctx, imageRef) @@ -126,10 +133,11 @@ func ensureImagePresent(ctx context.Context, ex *sdk.Executor, imageRef string, // generic Executor.InvokeProvider peer-dispatch leg instead (class-agnostic — "build"/"ensure" is // treated identically to any other (class, word) pair). The request carries the SAME run engine // the pod will launch on, so the image lands in the store that runs it. -func dispatchBuildEnsurePeer(ctx context.Context, ex *sdk.Executor, imageRef, runEngine string) error { +func dispatchBuildEnsurePeer(ctx context.Context, ex *sdk.Executor, imageRef, runEngine, dir string) error { params, err := json.Marshal(spec.BuildEnsureRequest{ Image: imageRef, RunEngine: runEngine, + Dir: dir, }) if err != nil { return err diff --git a/candy/plugin-deploy-pod/image_ensure_test.go b/candy/plugin-deploy-pod/image_ensure_test.go new file mode 100644 index 0000000..d80ccfc --- /dev/null +++ b/candy/plugin-deploy-pod/image_ensure_test.go @@ -0,0 +1,47 @@ +package deploypod + +import ( + "encoding/json" + "testing" + + "github.com/opencharly/sdk" + "github.com/opencharly/spec/spec" +) + +// TestDispatchBuildEnsurePeer_CarriesProjectDir is the regression witness for the +// containerd-bed blocker measured while proving opencharly/charly#1109: a disposable pod +// bed's image is built LOCALLY under the project's registry/name, so when the run engine's +// store lacks it and the registry pull misses, build:ensure must rebuild it FROM THE +// PROJECT. Without `dir` the build fallback fails "buildkitdrive: build: no context", so +// the deploy never reaches config/start — and a pod bed can never run on containerd. +// +// Fails on the pre-fix request (the field was absent; the function had no dir parameter). +func TestDispatchBuildEnsurePeer_CarriesProjectDir(t *testing.T) { + const ( + image = "ghcr.io/opencharly/probe-engine-image:check-engine-containerd-pod-2026.309.1930" + dir = "/home/operator/project" + ) + client := &recordExecutorClient{} + if err := dispatchBuildEnsurePeer(t.Context(), sdk.NewInProcExecutor(client), image, "containerd", dir); err != nil { + t.Fatalf("dispatchBuildEnsurePeer: %v", err) + } + if len(client.invoked) != 1 || client.invoked[0] != "build:ensure" { + t.Fatalf("dispatches = %v, want exactly [build:ensure]", client.invoked) + } + if len(client.params) != 1 { + t.Fatalf("captured params = %d, want 1", len(client.params)) + } + var req spec.BuildEnsureRequest + if err := json.Unmarshal(client.params[0], &req); err != nil { + t.Fatalf("decode the build:ensure request: %v", err) + } + if req.Image != image { + t.Errorf("Image = %q, want %q", req.Image, image) + } + if req.RunEngine != "containerd" { + t.Errorf("RunEngine = %q, want containerd", req.RunEngine) + } + if req.Dir != dir { + t.Fatalf("Dir = %q, want %q — build:ensure cannot rebuild a locally-built bed image into the containerd store without the project context", req.Dir, dir) + } +} diff --git a/candy/plugin-deploy-pod/lifecycle.go b/candy/plugin-deploy-pod/lifecycle.go index 6e473ee..01dc00e 100644 --- a/candy/plugin-deploy-pod/lifecycle.go +++ b/candy/plugin-deploy-pod/lifecycle.go @@ -38,6 +38,11 @@ type lifecycleParams struct { EngineBin string `json:"engine_bin"` // host-resolved container engine binary (PostTeardown) Cmd []string `json:"cmd"` Plan json.RawMessage `json:"plan"` // host-resolved spec.PodLifecyclePlan (OpStart/OpStop) — the K4 deep-body move + // Node is the dispatch-merged DeployNode the fleet target marshals as `node` + // (plugin-fleet/deploy_target.go marshalDeployOpParams). PrepareVenue reads its authored + // `engine:` selector into the persisted state patch so config/start resolve the engine the + // deploy was ADDED with (opencharly/charly#1109); the other lifecycle ops ignore it. + Node *spec.Deploy `json:"node,omitempty"` } // isLifecycleOp reports whether op is a substrate-lifecycle Op (vs. the OpExecute deploy walk). @@ -413,15 +418,26 @@ func podPrepareVenue(ctx context.Context, exec *sdk.Executor, p lifecycleParams) "Overlay image ready: " + resolvedImage, "To start the container, run: charly start " + reply.DeployName, } - r.State = prepareVenueState(resolvedImage, reply.BaseImage) + r.State = prepareVenueState(resolvedImage, reply.BaseImage, baseName, p.Node) } return marshalReply(r) } // prepareVenueState encodes the deploy-state delta a PrepareVenue reply carries back for the host -// to persist: the concrete add_candy overlay ref, and ONLY when an overlay was actually built -// (resolvedImage differs from the base). Returns nil otherwise, so a no-overlay deploy writes -// nothing. +// to persist: +// +// - the concrete add_candy overlay ref, ONLY when an overlay was actually built +// (resolvedImage differs from the base); +// - the deploy node's authored `engine:` selector, whenever the node selected one; when it +// is present, the deploy's box/image name and the `pod` target ride along so a newly +// created entry is a complete image-backed pod entry. +// +// Returns nil when there is neither an overlay nor a selector. The engine write is deliberately +// NOT gated on the overlay: a plain `pod:` deploy with `engine: containerd` must still remember +// its engine (opencharly/charly#1109). Carrying box + target alongside it is what keeps the +// entry valid when an older sdk/plugin-fleet (which does not yet know `engine`) decodes this +// patch: the entry is still a complete pod entry, and only the new field is dropped until those +// consumers adopt it. // // It marshals the TYPED spec.SaveDeployStateInput. That is the whole point of the helper, and it // is not stylistic: the host decodes this blob into that struct, whose wire key is the @@ -432,12 +448,33 @@ func podPrepareVenue(ctx context.Context, exec *sdk.Executor, p lifecycleParams) // image ready", the write path still took its lock (leaving a charly.yml.lock next to a charly.yml // that was never created), and the next `charly config` then resolved the deploy to the BASE image // and rendered a quadlet running it — every add_candy overlay pod deploy silently ran the wrong -// image. Marshal the wire struct, never a key map. -func prepareVenueState(resolvedImage, baseImage string) json.RawMessage { - if resolvedImage == "" || resolvedImage == baseImage { +// image. Marshal the wire struct, never a key map. The `engine` field rides the same contract and +// would be dropped by the same mistake. +func prepareVenueState(resolvedImage, baseImage, box string, node *spec.Deploy) json.RawMessage { + var in spec.SaveDeployStateInput + if resolvedImage != "" && resolvedImage != baseImage { + in.ResolvedImage = resolvedImage + } + if node != nil { + in.Engine = node.Engine + } + if in.ResolvedImage == "" && in.Engine == "" { + // Preserve the pre-existing no-overlay, no-selector contract: nothing to persist. return nil } - state, err := json.Marshal(spec.SaveDeployStateInput{ResolvedImage: resolvedImage}) + if in.Engine != "" { + // An engine selector can arrive with NO overlay, so give the new entry the image that + // makes it a complete image-backed pod entry. The node-form writer derives the `pod:` + // discriminator from Target or an image/workload indicator; an engine-only patch has + // neither, so without this the writer would emit a targetless `group:` entry and the + // next load would reject `engine` there (spec/deploy_nodeform.go deployDiscForEntity). + // An older sdk/plugin-fleet drops only the `engine` key and still writes a valid entry. + // applyDeployState seeds Image/Target only when the entry has none, so an existing + // operator value is never rewritten. + in.Box = box + in.Target = "pod" + } + state, err := json.Marshal(in) if err != nil { return nil } diff --git a/candy/plugin-deploy-pod/prepare_venue_state_test.go b/candy/plugin-deploy-pod/prepare_venue_state_test.go index c083268..3c75b0e 100644 --- a/candy/plugin-deploy-pod/prepare_venue_state_test.go +++ b/candy/plugin-deploy-pod/prepare_venue_state_test.go @@ -19,7 +19,7 @@ func TestPrepareVenueState_DecodesIntoTheHostsWireType(t *testing.T) { overlay = "check-stepkind-emit-pod-overlay:0fafbf994885" base = "ghcr.io/opencharly/check-pod:2026.216.2119" ) - state := prepareVenueState(overlay, base) + state := prepareVenueState(overlay, base, "", nil) if len(state) == 0 { t.Fatal("prepareVenueState returned no state for a built overlay") } @@ -46,15 +46,72 @@ func TestPrepareVenueState_DecodesIntoTheHostsWireType(t *testing.T) { } } -// TestPrepareVenueState_NoOverlayWritesNothing: a deploy with no add_candy overlay resolves to the -// base image itself, and must persist NO resolved_image — otherwise the base ref would be pinned -// into the overlay slot and survive later rebuilds. +// TestPrepareVenueState_NoOverlayWritesNothing: a deploy with no add_candy overlay and no +// engine selector resolves to the base image itself, and must persist NOTHING — otherwise the +// base ref would be pinned into the overlay slot and survive later rebuilds. A box name alone +// must not create an entry either. func TestPrepareVenueState_NoOverlayWritesNothing(t *testing.T) { const base = "ghcr.io/opencharly/check-pod:2026.216.2119" - if got := prepareVenueState(base, base); got != nil { - t.Fatalf("prepareVenueState(base, base) = %s, want nil", got) + if got := prepareVenueState(base, base, "", nil); got != nil { + t.Fatalf("prepareVenueState(base, base, \"\", nil) = %s, want nil", got) } - if got := prepareVenueState("", base); got != nil { - t.Fatalf("prepareVenueState(\"\", base) = %s, want nil", got) + if got := prepareVenueState("", base, "", nil); got != nil { + t.Fatalf("prepareVenueState(\"\", base, \"\", nil) = %s, want nil", got) + } + // The negative control for TestPrepareVenueState_CarriesEngine: a node that selected no + // engine must keep the old "nothing to persist" outcome even when the box name is known. + if got := prepareVenueState(base, base, "probe-engine-image", &spec.Deploy{}); got != nil { + t.Fatalf("prepareVenueState(base, base, box, engine-less node) = %s, want nil", got) + } +} + +// TestPrepareVenueState_CarriesEngine is the regression witness for opencharly/charly#1109: +// a pod deploy's authored `engine:` selector must reach the per-host overlay even when no +// add_candy overlay was built. The host's config/start path resolves its run mode from that +// entry, so a selector dropped here is silently replaced by the host-wide engine.run and +// `charly config` emits a podman quadlet for an `engine: containerd` deploy. Fails on the +// pre-fix helper, which returned nil whenever resolvedImage == baseImage. +func TestPrepareVenueState_CarriesEngine(t *testing.T) { + const base = "ghcr.io/opencharly/check-pod:2026.216.2119" + state := prepareVenueState(base, base, "probe-engine-image", &spec.Deploy{Engine: spec.EngineName("containerd")}) + if len(state) == 0 { + t.Fatal("prepareVenueState dropped the authored engine for a no-overlay deploy") + } + var in spec.SaveDeployStateInput + if err := json.Unmarshal(state, &in); err != nil { + t.Fatalf("host decode of the state blob: %v", err) + } + if in.Engine != spec.EngineName("containerd") { + t.Fatalf("decoded Engine = %q, want containerd — the state blob does not carry the authored selector", in.Engine) + } + if in.ResolvedImage != "" { + t.Fatalf("decoded ResolvedImage = %q for a no-overlay deploy; the base ref must not be pinned into the overlay slot", in.ResolvedImage) + } + // The entry must be a complete image-backed pod entry: the engine-only patch names the box + // and the substrate so the node-form writer emits `pod:` (never a targetless `group:` that + // would reject `engine` on the next load). + if in.Box != "probe-engine-image" { + t.Fatalf("decoded Box = %q, want probe-engine-image", in.Box) + } + if in.Target != "pod" { + t.Fatalf("decoded Target = %q, want pod — an engine-only patch without a target marshals as a `group:` entry", in.Target) + } +} + +// TestPrepareVenueState_CarriesEngineAndOverlay proves the fields travel together when a +// real overlay was built: the engine is not traded for the resolved image, and the entry +// carries the box name. +func TestPrepareVenueState_CarriesEngineAndOverlay(t *testing.T) { + const ( + overlay = "probe-overlay:0fafbf994885" + base = "ghcr.io/opencharly/pod-crabbox:2026.309.1054" + ) + state := prepareVenueState(overlay, base, "probe-engine-image", &spec.Deploy{Engine: spec.EngineName("containerd")}) + var in spec.SaveDeployStateInput + if err := json.Unmarshal(state, &in); err != nil { + t.Fatalf("host decode of the state blob: %v", err) + } + if in.Engine != spec.EngineName("containerd") || in.ResolvedImage != overlay || in.Box != "probe-engine-image" { + t.Fatalf("state = %+v, want engine=containerd, resolved_image=%s, box=probe-engine-image", in, overlay) } } diff --git a/candy/plugin-deploy-pod/resolve.go b/candy/plugin-deploy-pod/resolve.go index cf9a401..64fc1e4 100644 --- a/candy/plugin-deploy-pod/resolve.go +++ b/candy/plugin-deploy-pod/resolve.go @@ -29,6 +29,20 @@ func resolvePodStartPlan(ctx context.Context, ex *sdk.Executor, box, instance st if err != nil { return nil, 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 + // `pod:` + `engine: containerd` deploy on a podman-default host is started through the + // quadlet branch and fails "not configured" (or, worse, starts a unit for the wrong + // 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 + } + sel, err := resolvePodSelectionFromConfig(dc, box, instance, rt) + if err != nil { + return nil, err + } + rt = runtimeForSelection(rt, sel) switch rt.RunMode { case "quadlet": return resolvePodStartQuadlet(ctx, ex, box, instance, rt) @@ -171,7 +185,7 @@ func resolvePodRuntimeImageWith(ctx context.Context, ex *sdk.Executor, box, inst if err != nil { return nil, err } - if err := ensureImagePresent(ctx, ex, imageRef, sel.Target); err != nil { + if err := ensureImagePresent(ctx, ex, imageRef, sel.Target, box); err != nil { return nil, err } metaPtr, err := deploykit.ExtractMetadata(string(sel.Engine), imageRef) diff --git a/candy/plugin-deploy-pod/resolve_ref.go b/candy/plugin-deploy-pod/resolve_ref.go index 8648135..178b98f 100644 --- a/candy/plugin-deploy-pod/resolve_ref.go +++ b/candy/plugin-deploy-pod/resolve_ref.go @@ -129,6 +129,18 @@ var ( loadUnifiedForBox = loaderkit.LoadUnifiedViaExecutor ) +// projectDirFor resolves the project dir through the SAME "deploy-plugins-connect" preamble +// resolveDeployBoxNameLocal uses. build:ensure needs it to rebuild a project/box image into +// the deploy's RUN-engine store when a registry pull misses — a bed's locally-built image is +// never registry-published, so without the dir that fallback fails "build: no context". +func projectDirFor(ctx context.Context, ex *sdk.Executor, box string) (string, error) { + var pre spec.DeployPluginsConnectReply + if err := projectConnect(ctx, ex, box, &pre); err != nil { + return "", err + } + return pre.Dir, nil +} + // deployKeyToBoxLocal mirrors the former host deploy-key→box loader-read: user (per-host) overlay // wins over the project config; each is tried DeployKey-then-bare-key. func deployKeyToBoxLocal(ctx context.Context, ex *sdk.Executor, dc *deploykit.DeployConfig, box, instance string) string { diff --git a/candy/plugin-deploy-pod/selection.go b/candy/plugin-deploy-pod/selection.go index 5e77f01..595d6f2 100644 --- a/candy/plugin-deploy-pod/selection.go +++ b/candy/plugin-deploy-pod/selection.go @@ -6,6 +6,7 @@ import ( "github.com/opencharly/sdk" "github.com/opencharly/sdk/deploykit" "github.com/opencharly/sdk/kit" + "github.com/opencharly/spec/container" "github.com/opencharly/spec/spec" ) @@ -80,3 +81,32 @@ func resolvePodSelectionFor(ctx context.Context, ex *sdk.Executor, box, instance } return resolvePodSelectionFromConfig(dc, box, instance, rt) } + +// detectRunMode is the host-availability half of runtimeForSelection. It is a package var so a +// unit test can pin the capability-table answer without depending on the runner's systemd-user +// session; production reads the ONE host probe in spec/hostenv. +var detectRunMode = kit.DetectRunMode + +// runtimeForSelection returns rt with the persistence/supervision MODE re-derived from the +// deploy's resolved engine. rt.RunMode was resolved for rt.RunEngine (the host-wide engine.run: +// env > settings > detection); a deploy-level `engine:` overrides that WORD, and the mode is a +// capability FACT of the selected word — containerd persists as a `systemd-unit`, podman as a +// quadlet, docker direct. Keeping the global mode emits a podman quadlet for a containerd deploy +// and `charly start` then runs podman (opencharly/charly#1109). +// +// `direct` is engine-agnostic (the engine CLI is executed with no unit), so an explicit or +// host-degraded direct mode is preserved. A unit mode is re-derived through the same +// capability+availability rule ResolveRuntime uses, so a host without a systemd-user session +// still degrades to direct. An unchanged engine, a nil rt, or an unresolved selection is a +// no-op. +func runtimeForSelection(rt *kit.ResolvedRuntime, sel podSelection) *kit.ResolvedRuntime { + if rt == nil || sel.Engine == "" || string(sel.Engine) == rt.RunEngine { + return rt + } + out := *rt + out.RunEngine = string(sel.Engine) + if container.IsUnitRunMode(out.RunMode) { + out.RunMode = detectRunMode(string(sel.Engine)) + } + return &out +} diff --git a/candy/plugin-deploy-pod/selection_test.go b/candy/plugin-deploy-pod/selection_test.go index 1eac2c5..f838ac3 100644 --- a/candy/plugin-deploy-pod/selection_test.go +++ b/candy/plugin-deploy-pod/selection_test.go @@ -130,6 +130,7 @@ type recordExecutorClient struct { scripts []string handle func(script string) *pb.CaptureReply invoked []string + params [][]byte } func (c *recordExecutorClient) RunCapture(_ context.Context, in *pb.RunRequest, _ ...grpc.CallOption) (*pb.CaptureReply, error) { @@ -139,6 +140,7 @@ func (c *recordExecutorClient) RunCapture(_ context.Context, in *pb.RunRequest, func (c *recordExecutorClient) InvokeProvider(_ context.Context, in *pb.InvokeProviderRequest, _ ...grpc.CallOption) (*pb.InvokeReply, error) { c.invoked = append(c.invoked, in.GetClass()+":"+in.GetReserved()) + c.params = append(c.params, in.GetParamsJson()) return &pb.InvokeReply{}, nil } @@ -153,7 +155,7 @@ func TestEnsureImagePresent_ProbesResolvedStore(t *testing.T) { client := &recordExecutorClient{handle: func(string) *pb.CaptureReply { return &pb.CaptureReply{ExitCode: 0} }} - if err := ensureImagePresent(t.Context(), sdk.NewInProcExecutor(client), "myimage:latest", sel.Target); err != nil { + if err := ensureImagePresent(t.Context(), sdk.NewInProcExecutor(client), "myimage:latest", sel.Target, "myimage"); err != nil { t.Fatalf("ensureImagePresent: %v", err) } if len(client.scripts) != 1 { @@ -166,6 +168,16 @@ func TestEnsureImagePresent_ProbesResolvedStore(t *testing.T) { }) t.Run("absent reaches build:ensure and re-probes", func(t *testing.T) { + // build:ensure now carries the project dir (a locally-built bed image cannot be + // staged into a containerd store without it). Stub the host preamble seam so this + // in-proc test does not need a live host builder. + prevProjectConnect := projectConnect + projectConnect = func(_ context.Context, _ *sdk.Executor, _ string, pre *spec.DeployPluginsConnectReply) error { + pre.Dir = "/project" + return nil + } + t.Cleanup(func() { projectConnect = prevProjectConnect }) + probes := 0 client := &recordExecutorClient{handle: func(script string) *pb.CaptureReply { if strings.Contains(script, "image inspect") { @@ -181,7 +193,7 @@ func TestEnsureImagePresent_ProbesResolvedStore(t *testing.T) { // The engine's own daemon probe (`info`) answers, so exit 1 means absent. return &pb.CaptureReply{ExitCode: 0} }} - if err := ensureImagePresent(t.Context(), sdk.NewInProcExecutor(client), "myimage:latest", sel.Target); err != nil { + if err := ensureImagePresent(t.Context(), sdk.NewInProcExecutor(client), "myimage:latest", sel.Target, "myimage"); err != nil { t.Fatalf("ensureImagePresent: %v", err) } if len(client.invoked) != 1 || client.invoked[0] != "build:ensure" { @@ -197,3 +209,81 @@ func TestEnsureImagePresent_ProbesResolvedStore(t *testing.T) { } }) } + +// TestRuntimeForSelection_DerivesModeFromDeployEngine is the regression witness for +// opencharly/charly#1109's config/start half: the persistence mode is a capability FACT of +// the SELECTED engine. The host-wide rt.RunMode is auto-detected for rt.RunEngine (podman +// → quadlet on a podman-default host), so a deploy-level `engine: containerd` kept running +// the quadlet path and emitted `ExecStart=/usr/bin/podman run`. The test stubs the host probe +// so the capability-table answer is pinned without depending on the runner's systemd session. +func TestRuntimeForSelection_DerivesModeFromDeployEngine(t *testing.T) { + prev := detectRunMode + t.Cleanup(func() { detectRunMode = prev }) + + var probed []string + detectRunMode = func(engine string) string { + probed = append(probed, engine) + if engine == "containerd" { + return "systemd-unit" + } + return "quadlet" + } + + rt := &kit.ResolvedRuntime{RunEngine: "podman", RunMode: "quadlet"} + sel, err := resolvePodSelection(deploykit.Selection{Engine: "containerd"}, rt) + if err != nil { + t.Fatalf("resolvePodSelection: %v", err) + } + if sel.Engine != "containerd" { + t.Fatalf("selection engine = %q, want containerd", sel.Engine) + } + + got := runtimeForSelection(rt, sel) + if got == rt { + t.Fatal("runtimeForSelection returned the global runtime unchanged; the containerd deploy would emit a podman quadlet") + } + if got.RunMode != "systemd-unit" { + t.Fatalf("effective RunMode = %q, want systemd-unit (containerd's capability mode)", got.RunMode) + } + if got.RunEngine != "containerd" { + t.Fatalf("effective RunEngine = %q, want containerd", got.RunEngine) + } + if len(probed) != 1 || probed[0] != "containerd" { + t.Fatalf("host run-mode probe = %v, want exactly [containerd]", probed) + } + // Negative control: the input runtime is a value copy, never mutated. + if rt.RunMode != "quadlet" || rt.RunEngine != "podman" { + t.Fatalf("runtimeForSelection mutated its input: %+v", rt) + } +} + +// TestRuntimeForSelection_PreservesDirect: direct is engine-agnostic (the engine CLI is +// executed with no unit), so an explicit or host-degraded direct mode survives a deploy-level +// engine change; only a unit mode is re-derived. +func TestRuntimeForSelection_PreservesDirect(t *testing.T) { + rt := &kit.ResolvedRuntime{RunEngine: "podman", RunMode: "direct"} + sel, err := resolvePodSelection(deploykit.Selection{Engine: "containerd"}, rt) + if err != nil { + t.Fatalf("resolvePodSelection: %v", err) + } + got := runtimeForSelection(rt, sel) + if got.RunMode != "direct" { + t.Fatalf("effective RunMode = %q, want direct preserved", got.RunMode) + } + if got.RunEngine != "containerd" { + t.Fatalf("effective RunEngine = %q, want containerd", got.RunEngine) + } +} + +// TestRuntimeForSelection_SameEngineIsNoOp: an unchanged engine returns the SAME runtime, so +// the existing explicit/host-degraded mode behaviour is untouched. +func TestRuntimeForSelection_SameEngineIsNoOp(t *testing.T) { + rt := &kit.ResolvedRuntime{RunEngine: "containerd", RunMode: "systemd-unit"} + sel, err := resolvePodSelection(deploykit.Selection{}, rt) + if err != nil { + t.Fatalf("resolvePodSelection: %v", err) + } + if got := runtimeForSelection(rt, sel); got != rt { + t.Fatalf("runtimeForSelection rebuilt the runtime for the same engine: %+v", got) + } +} diff --git a/candy/plugin-deploy-pod/systemd_unit.go b/candy/plugin-deploy-pod/systemd_unit.go index 107aafd..7601c79 100644 --- a/candy/plugin-deploy-pod/systemd_unit.go +++ b/candy/plugin-deploy-pod/systemd_unit.go @@ -75,7 +75,11 @@ func writeSystemdUnitConfig(rt *kit.ResolvedRuntime, sel podSelection, qcfg depl StartArgv: argv, StopArgv: engineStopArgv(sel.Target, name), Description: "charly deployment " + name, - WorkingDir: workingDir, + // NO systemd WorkingDirectory=: this is a HOST unit that runs the engine CLI, and the + // engine's own `run -w ` (already in StartArgv) sets the CONTAINER's + // workdir. Passing the container path here made systemd CHDIR to /home/user on the + // host and fail status=200/CHDIR on the first live containerd pod bed (measured). + WorkingDir: "", }) if err != nil { return nil, err