feat: run sessions on rootless podman - #74
Conversation
The podman backend is the Docker backend driven through podman's Docker-compatible CLI. The differences are gated by docker.IsPodman(): session and auth-reconcile containers use --userns=keep-id so the host UID survives rootless podman's user namespace, builds skip --cache-from and the home-directory cache mounts (buildah commits their ancestors as root-owned), and the gateway is torn down after the session container is removed because podman refuses to remove a container whose network namespace another container still joins.
The backend now defaults to "auto", resolved once at startup before any engine call: docker when its CLI is on PATH, otherwise podman, with the podman-docker shim recognized as podman. When both are installed and a terminal is present, enclave asks once and saves the answer as "backend" in the global config.json; without a terminal it uses docker and prints a notice so scripts and --json consumers never block on a prompt.
Rootless podman copies the whole image into a keep-id mapped layer on the first run after every rebuild, silently and while blocking other podman commands, so a session looked stuck right after the gateway came up. Ctrl-C then left the gateway running with the OAuth callback port published and every later start failed the host-port check. Trigger the copy in a throwaway create before the gateway starts and announce it when it takes long; run the start under a SIGINT/SIGTERM context so an interrupted start removes the gateway and exits 130; remove a gateway whose start failed; and remove a stale gateway of the same session before the OAuth port check.
…s up termtint restored the tint on SIGINT, SIGTERM, and SIGHUP and then re-raised the signal with its default disposition. The foreground run already survives SIGINT and SIGTERM through its interrupt context so that an aborted start can remove the gateway and exit 130; with session_tint set, the re-raise killed enclave first and orphaned the gateway again, and signal.Reset also dropped the run's own handler. Begin now takes CallerHandles(signals...) and leaves those signals alone, neither restoring early nor re-raising; the run passes its interrupt set and restores through the deferred call. exec and attach keep the previous behavior, and SIGHUP still restores the tint and terminates the run.
The foreground run survives SIGINT and SIGTERM through its interrupt context so an aborted start can clean up, but once the session was attached nothing delivered a process-directed signal to the engine child: kill -TERM on the enclave process was absorbed and the session kept running, so a supervisor could not stop it. Terminal Ctrl-C is unaffected since the engine holds the TTY in raw mode and proxies it itself. The interactive run helper now relays SIGINT and SIGTERM to the child while it runs, registering before the child starts so an early signal is delivered as soon as the process exists. The engine proxies the signal into the container, the run returns with the tool's status, and the usual cleanup runs.
…vocations With both engines installed and no saved choice, resolving the "auto" backend prompted whenever stdin and stderr were terminals, so result=$(enclave tools list --json) blocked on a question and could persist a global backend choice from a command that never touches an engine. Structured output is an integration contract and must not prompt. The prompt now also requires stdout to be a terminal, is disabled for --json and --yes invocations, and the backend is not resolved at all for tools, features, config, review-target, and the extension verbs, which never use an engine and never validate the backend name.
EclipseSourceAI
left a comment
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
What this PR does
Adds a rootless podman backend by driving the existing Docker backend through podman's Docker-compatible CLI: internal/docker gets a switchable binary (SetBinary/IsPodman), and the engine differences are gated on IsPodman() (--userns=keep-id for session and auth-reconcile containers, no --cache-from, no BuildKit flags, home-directory cache mounts stripped from the rendered Dockerfile, gateway teardown after the session container). The backend default becomes auto, resolved once in app.Run from DetectCLIs(), with a one-time prompt persisted to the global config.json when both engines are installed. It also reworks interrupt handling: the foreground start runs under a SIGINT/SIGTERM context, exits 130, removes a gateway it created or one orphaned by an earlier interrupted start, and forwards signals to the engine child once attached.
Where I would focus
- Engine selection edges. Shim detection only applies to
auto; an explicit--backend dockeron apodman-dockerhost silently loseskeep-id. The engine-free action allowlist misses a few host-only verbs, which can pop the engine question where nothing needs an engine. - Build path adaptation.
--cache-fromand BuildKit flags are gated, but the buildx cache flags and thedocker buildxbuild preflight are not, so--buildx-cache-*and the preflight message stay Docker-shaped under podman. - Global engine state. The engine now lives in a package-level variable in
internal/docker, andBackend.Name()reads it instead of the instance, which is worth a deliberate call from a maintainer since it shapes how any future engine is added. - Interrupt/cleanup handling looked carefully done (post-run auth sync stays detached from the cancelled context, gateway removal ordering matches podman's netns rules, termtint no longer re-raises caller-handled signals). The keep-id warm-up create is a pragmatic answer to the image copy stall.
make buildand the touched package tests pass locally. The podman-specific behavior is covered only by CLI stubs, so the real-engine claims in the description (keep-id copy, netns join,podman version/infooutput shapes) rest on the author's manual testing.
A keep-id session joining the gateway's network namespace could not mount sysfs there, because the gateway lived in podman's default rootless user namespace. crun hides this by bind-mounting the host /sys; runc fails the create, which podman reports as an empty conmon response. The gateway now runs as root inside a keep-id user namespace and the session joins it with --userns container:<gateway> alongside the network namespace.
Name() derived the engine from the package-level CLI binary, so a Backend had no record of which engine it drives and its identity could change under it. selectBackend now passes the engine through Options and Name() reports it. The UseCLI/DetectCLIs forwards go away; internal/app already uses the CLI wrapper directly. The wrapper's package-level binary stays: image builds and engine checks use it before any backend instance exists.
…ker shim Only detection recognized the shim. A configured or typed docker on a host whose docker is podman-docker ran the shim as Docker, without --userns keep-id, leaving the bind-mounted stores unwritable under rootless podman. Such hosts are exactly where users wrote backend: docker before podman was a backend, so the explicit name now goes through the same shim check and switches to podman with a notice instead of failing.
network print and diff only render policy, ssh-init copies files, validate-extensions reads specs, and devcontainer generate resolves the build config without an engine, yet all of them detected the backend and could ask the engine question on a dual-engine host. The predicate moves next to isRunAction, where the other action predicates live.
--buildx-cache-dir, --buildx-cache-from, and --buildx-cache-to were still forwarded under podman and routed the build through podman buildx build with type=local cache specs, which podman rejects: its --cache-from and --cache-to take remote repositories. The specs are now dropped with a warning naming the flags, matching how the inline --cache-from is already skipped; podman's local layer cache still applies.
The build preflight failed whenever `<engine> buildx version` failed and told the user to install a Docker plugin. podman builds with buildah, which handles the BuildKit RUN --mount syntax natively, so the probe is skipped there; the storage check stays, since Info already reads podman's schema.
EclipseSourceAI
left a comment
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
All six threads from my earlier review are addressed in the commits since then (682e6a7, 063659c, 2d4c9c1, f3c62f5, 3609d62, 2cb14ee, 3f93ff3), each matching the fix I asked for and backed by new unit tests. I checked each against the current code and they hold up: the shim check now applies to explicit backends too, the engine-free allowlist covers the remaining host-only verbs, buildx cache specs and the buildx preflight are gated on IsPodman(), the engine now lives on the Backend instance via Options.Engine, and the gateway container name helper is deduplicated.
One more fix landed that I hadn't flagged (15a5d03, sharing the user namespace between the podman gateway and session so the session's sysfs mount doesn't fail under runc). I reviewed it against the current code: joinGatewayNamespaces/gatewayUserNS/gatewayUser are consistent, correctly scoped to podman only, and covered by new tests in both internal/backend/docker and internal/gateway. Nothing to flag there.
go build ./... and the touched package tests pass locally. No new inline comments this round.
These previous comments can be resolved as they are now handled:
- explicit
--backend dockerbehind the podman-docker shim now gets keep-id too - remaining host-only verbs no longer trigger backend detection
- buildx cache specs are dropped with a warning under podman instead of failing the build
- build preflight no longer requires docker buildx under podman
Backend.Name()now reads the engine off the instance instead of a package global- gateway container name is now derived from one helper everywhere
- rootless.md header now states podman is supported and rootless Docker is not
I can't resolve them myself as I would need write permission on this repository.
|
(I tested with Ubuntu 26.04 and with Fedora 44) |
2 participants
What it does
Closes #68 by adding a rootless podman backend and making the engine choice automatic.
--backend podmandrives the existing Docker backend through podman's Docker-compatible CLI. Engine differences are gated ondocker.IsPodman():--userns=keep-idfor session and auth-reconcile containers so the host UID survives rootless podman's user namespace, no--cache-fromand no home-directory cache mounts on build (buildah commits their parent directories as root-owned), and gateway teardown after the session container is removed. Devcontainer mode stays Docker-only.auto, resolved once at startup: docker when its CLI is onPATH, otherwise podman, with thepodman-dockershim recognized as podman. With both installed and a terminal on all three streams, enclave asks once and saves the answer asbackendin the globalconfig.json.--json,--yes, non-terminal invocations, and commands that never touch an engine (tools,features,config,review-target) never prompt.How to test
On a host with rootless podman (Fedora is one of the easiest here, it ships the short-name aliases):
The first start after a build pauses on the keep-id image copy — it should say so rather than look stuck. Ctrl-C during that phase should exit 130 and leave no
enclave-gateway-*container behind (podman ps -a). After attaching,kill -TERM <enclave pid>from another shell should stop the session. Then check detection: with both engines installed, a bareenclaveasks once and writes"backend"to~/.config/enclave/config.json, whileenclave tools list --jsonneither asks nor writes.Follow-ups
cleanup --build-cachehas nothing to reclaim under podman.Breaking changes
Review checklist