RFC: Add support for Podman as container engine alternative to Docker #8660
Conversation
github-actions
bot
added
size/medium
Hey @tmshlvck! 👋Thanks for submitting your first pull request to the Armbian project — we're excited to have you contributing! 🧡 If you'd like to stay informed about project updates or collaborate more closely with the team, Also, don’t forget to ⭐ star the repo if you haven’t already — and welcome aboard! 🚀 |
github-actions
bot
added
Needs review
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds runtime detection and abstraction for the container engine in lib/functions/host/docker.sh. Introduces three globals—DOCKER_COMMAND, DOCKER_NETWORK, and DOCKER_MOUNT_OPTS—initialized during a one-time info check to select between docker and podman (via sudo). Replaces direct docker calls with the dynamic command across info, image pull/list, volume/purge, build, launch, and preparation flows. Updates readiness checks and messages to accept either engine. Mount preparation now appends engine-specific options, and host networking is applied when podman is used. New globals are exposed as readonly. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/functions/host/docker.sh (1)
63-63: Guard against sudo prompts during readiness.
If sudo requires a TTY/password,$DOCKER_COMMAND infocan hang. Thesudo -ntweak above prevents this; alternatively, detect sudo -n support up front and surface a clear error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/functions/host/docker.sh(9 hunks)
🔇 Additional comments (6)
lib/functions/host/docker.sh (6)
50-60: Confirm downstream expectations for DOCKER_IN_PATH.You set DOCKER_IN_PATH="yes" only when docker is present; for Podman it remains "no". If other modules gate behavior on DOCKER_IN_PATH, Podman runs may be misclassified. Consider either setting DOCKER_IN_PATH="yes" for Podman as well or introducing a neutral ENGINE_IN_PATH flag.
89-91: Good readiness broadening to accept either engine.
This check now correctly allows Docker or Podman in PATH. LGTM.
339-356: Build/pull paths look compatible across engines.
images -qandbuild -t -fmap well to both Docker and Podman. No issues spotted.Also applies to: 376-377
601-602: Engine-conditional host networking is wired correctly.
Using DOCKER_NETWORK only when set cleanly applies host networking for Podman without impacting Docker.Also applies to: 607-608
639-641: Volume purge switched to dynamic engine correctly.
Using$DOCKER_COMMAND volume …aligns with the abstraction.
45-60: Stop treating the runtime command as a single string; add non-interactive sudo and root detection for Podman.File: lib/functions/host/docker.sh (around lines 45-60; also apply to the other Podman handling at ~71-74)
- Reason: "sudo podman" as a single scalar breaks safe argument splitting and can hang on password prompts; prefer skipping sudo when already root and use non-interactive sudo (-n) otherwise.
- elif [[ -n "$(command -v podman)" ]]; then - DOCKER_COMMAND="sudo podman" - DOCKER_NETWORK="--network host" - DOCKER_MOUNT_OPTS=",suid,dev" # Required mount options for Podman - display_alert "Podman detected" "Using Podman with sudo (required for Armbian builds)" "info" + elif [[ -n "$(command -v podman)" ]]; then + if [[ $EUID -eq 0 ]]; then + DOCKER_COMMAND="podman" + else + DOCKER_COMMAND="sudo -n podman" # non-interactive to avoid hanging on readiness checks + fi + DOCKER_NETWORK="--network host" + # For --mount, supply a plain options list; used later as options=${DOCKER_MOUNT_OPTS} + DOCKER_MOUNT_OPTS="rw,dev,suid" + display_alert "Podman detected" "Using Podman ($([[ $EUID -eq 0 ]] && echo 'root' || echo 'sudo -n'))" "info"
- Optional: refactor DOCKER_COMMAND into an array and invoke as "${DOCKER_COMMAND[@]}" everywhere to avoid future word-splitting issues.
lib/functions/host/docker.sh
Outdated
| set -- "$DOCKER_COMMAND" "$@" | ||
| fi |
There was a problem hiding this comment.
Don’t quote
set -- "$DOCKER_COMMAND" "$@" turns "sudo podman" into one token, causing command-not-found later. Allow intended splitting or (better) switch DOCKER_COMMAND to an array.
Minimal fix:
- set -- "$DOCKER_COMMAND" "$@"
+ # shellcheck disable=SC2086 # intentional word splitting for DOCKER_COMMAND
+ set -- $DOCKER_COMMAND "$@"Apply to both occurrences in this function.
Also applies to: 125-126
🤖 Prompt for AI Agents
In lib/functions/host/docker.sh around lines 117-118 (and also update the
similar occurrence at 125-126), the code uses set -- "$DOCKER_COMMAND" "$@"
which quotes the whole DOCKER_COMMAND string into a single argv (e.g. "sudo
podman") causing later command-not-found; fix by either unquoting to allow
word-splitting if DOCKER_COMMAND is a plain string: set -- $DOCKER_COMMAND "$@"
; or, preferably, change DOCKER_COMMAND to an array and use safe array
expansion: set -- "${DOCKER_COMMAND[@]}" "$@"; apply the same change to both
occurrences.
There was a problem hiding this comment.
Yes, v2 patch will be submitted.
| DOCKER_ARGS+=("--mount" "type=volume,destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}") | ||
| ;; | ||
| bind) | ||
| display_alert "Mounting" "bind mount for '${MOUNT_DIR}'" "debug" | ||
| mkdir -p "${SRC}/${MOUNT_DIR}" | ||
| DOCKER_ARGS+=("--mount" "type=bind,source=${SRC}/${MOUNT_DIR},target=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}") | ||
| DOCKER_ARGS+=("--mount" "type=bind,source=${SRC}/${MOUNT_DIR},target=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}") | ||
| ;; | ||
| namedvolume) | ||
| display_alert "Mounting" "named volume id '${volume_id}' for '${MOUNT_DIR}'" "debug" | ||
| DOCKER_ARGS+=("--mount" "type=volume,source=armbian-${volume_id},destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}") | ||
| DOCKER_ARGS+=("--mount" "type=volume,source=armbian-${volume_id},destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}") | ||
| ;; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
❓ Verification inconclusive
Fix invalid --mount syntax; pass options via options=… instead of appending raw flags to destination.
Appending ",suid,dev" after destination=… is not a valid key=value pair for Docker/Podman --mount. Use options=… so both engines parse mount flags correctly.
Apply this diff:
- DOCKER_ARGS+=("--mount" "type=volume,destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}")
+ DOCKER_ARGS+=("--mount" "type=volume,destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS:+,options=${DOCKER_MOUNT_OPTS}}")
...
- DOCKER_ARGS+=("--mount" "type=bind,source=${SRC}/${MOUNT_DIR},target=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}")
+ DOCKER_ARGS+=("--mount" "type=bind,source=${SRC}/${MOUNT_DIR},target=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS:+,options=${DOCKER_MOUNT_OPTS}}")
...
- DOCKER_ARGS+=("--mount" "type=volume,source=armbian-${volume_id},destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}")
+ DOCKER_ARGS+=("--mount" "type=volume,source=armbian-${volume_id},destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS:+,options=${DOCKER_MOUNT_OPTS}}")Fix invalid --mount syntax; pass options via options=… instead of appending raw flags to destination.
Appending ",suid,dev" (or similar) after destination=… is not a valid key=value pair for Docker/Podman --mount. Use options=… so both engines parse mount flags correctly.
Apply this diff:
- DOCKER_ARGS+=("--mount" "type=volume,destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}")
+ DOCKER_ARGS+=("--mount" "type=volume,destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS:+,options=${DOCKER_MOUNT_OPTS}}")
...
- DOCKER_ARGS+=("--mount" "type=bind,source=${SRC}/${MOUNT_DIR},target=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}")
+ DOCKER_ARGS+=("--mount" "type=bind,source=${SRC}/${MOUNT_DIR},target=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS:+,options=${DOCKER_MOUNT_OPTS}}")
...
- DOCKER_ARGS+=("--mount" "type=volume,source=armbian-${volume_id},destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}")
+ DOCKER_ARGS+=("--mount" "type=volume,source=armbian-${volume_id},destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS:+,options=${DOCKER_MOUNT_OPTS}}")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/functions/host/docker.sh around lines 519-529, the --mount invocations
append raw mount flags to the destination (destination=...${DOCKER_MOUNT_OPTS})
which is invalid; change them to use options=... instead. For each
DOCKER_ARGS+=("--mount"
"...destination=${DOCKER_ARMBIAN_TARGET_PATH}/${MOUNT_DIR}${DOCKER_MOUNT_OPTS}")
line, split DOCKER_MOUNT_OPTS into an options clause (strip any leading comma)
and append ,options=${OPTIONS} to the --mount string (omit the options= part
entirely when DOCKER_MOUNT_OPTS is empty), e.g.
destination=.../${MOUNT_DIR}[,options=${DOCKER_MOUNT_OPTS_STRIPPED}]; apply this
for volume, bind and namedvolume cases so Docker/Podman parse mount flags
correctly.
There was a problem hiding this comment.
The DOCKER_MOUNT_OPTS is set at podman detection to the required flags or kept empty for docker. There should be no potential for mixing the flags with docker.
This RFC implements Podman support for Armbian builds to address mounting issues experienced with Podman while maintaining Docker compatibility. Key changes: - Auto-detect container engine: prefers Docker, falls back to Podman - Use sudo with Podman (runs in root mode - not more secure than Docker) - Add required mount options (suid,dev) and --network host for Podman - Update all docker commands to use dynamic DOCKER_COMMAND variable Technical notes: - Podman runs with sudo/root privileges, so security model matches Docker - Requires sudo access when using Podman (security consideration) - Solves Podman-specific mount permission and networking issues - Maintains full backward compatibility with existing Docker workflows - May need documentation updates
Description
There were multiple requests for podman support:
Since rootless podman support is probably intractable at this point I am suggesting the basic support in rootful mode.
This RFC implements Podman support for Armbian builds to address mounting issues experienced with Podman while maintaining Docker compatibility.
Key changes:
Technical notes:
Documentation summary for feature / change
I believe that if this gets merged we could mention podman support as a note to the docker requirements.
How Has This Been Tested?
Checklist: