-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Volume Options for Podman Compatibility #8938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
It hasn't been an issue for Docker because their defaults didn't conflict, but Podman mounts volumes with nodev (and formerly noexec) by default, which would break the build. Adding `dev` and `exec` to the volume mount options ensures the needed setting whatever the defaults.
Use $DOCKER_INFO to check for podman vs dockerd, and conditionally `exec` and `dev` volume options, which are needed on podman but not allowed on dockerd.
WalkthroughAdds runtime detection to set a global Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Script as Host script
participant DockerBin as `docker` binary
participant Runtime as Container runtime (Docker/Podman)
rect rgb(240,248,255)
Note over Script: Startup — detect docker runtime
Script->>DockerBin: run `docker --version` / inspect info
DockerBin-->>Script: version string / info
alt Podman detected
Script->>Script: set DOCKER_IS_PODMAN = true (readonly)
else Not Podman
Script->>Script: set DOCKER_IS_PODMAN = false (readonly)
end
end
rect rgb(255,250,240)
Note over Script,Runtime: Container creation — mount handling
Script->>Runtime: create container with volume mounts
alt DOCKER_IS_PODMAN == true
Script->>Runtime: append ",exec,dev" to anonymous/named mount destinations
else
Script->>Runtime: use original mount destinations
end
Runtime-->>Script: container started
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/functions/host/docker.sh (1)
540-561: Architectural note: Loop device handling is invariant to runtime type.The loop device mounts and security options (lines 540–561) do not vary based on
DOCKER_IS_PODMAN. The PR summary mentions that rootful Podman may be required for operations involving loop devices. Verify that the existing loop device configuration is sufficient for Podman environments, or whether additional logic is needed to:
- Detect rootful vs rootless Podman
- Conditionally apply tighter device access for rootful Podman
- Conditionally skip certain hacks for rootless Podman
If the current approach works as-is, no changes are needed. If Podman requires conditional logic here, it should be addressed in a follow-up.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tmshlvck
Repo: armbian/build PR: 8660
File: lib/functions/host/docker.sh:519-529
Timestamp: 2025-09-22T06:01:50.284Z
Learning: In the Armbian build framework's docker.sh, DOCKER_MOUNT_OPTS is conditionally set: empty for Docker, and ",suid,dev" for Podman to handle mount permission requirements.
Learnt from: tmshlvck
Repo: armbian/build PR: 8660
File: lib/functions/host/docker.sh:519-529
Timestamp: 2025-09-22T06:01:50.284Z
Learning: In the Armbian build framework's docker.sh, DOCKER_MOUNT_OPTS is conditionally set: empty for Docker, and ",suid,dev" for Podman to handle mount permission requirements.
📚 Learning: 2025-09-22T06:01:50.284Z
Learnt from: tmshlvck
Repo: armbian/build PR: 8660
File: lib/functions/host/docker.sh:519-529
Timestamp: 2025-09-22T06:01:50.284Z
Learning: In the Armbian build framework's docker.sh, DOCKER_MOUNT_OPTS is conditionally set: empty for Docker, and ",suid,dev" for Podman to handle mount permission requirements.
Applied to files:
lib/functions/host/docker.sh
🧬 Code graph analysis (1)
lib/functions/host/docker.sh (1)
lib/functions/cli/cli-docker.sh (1)
cli_docker_run(32-112)
🔇 Additional comments (2)
lib/functions/host/docker.sh (2)
514-514: Verify thatexec,devflags are sufficient for Podman volume compatibility.The anonymous volume mount correctly applies the conditional flags. However, the prior learning from PR #8660 mentions
,suid,devas the Podman mount options, whereas this implementation uses,exec,dev.Confirm that:
- The
execflag (enables binary execution on volume) is necessary for Armbian builds.- The
suidflag (allows setuid bit) is not required for your use cases.- These two flags alone support the loop device and device access requirements mentioned in the PR summary.
523-523: Consistent application of Podman volume flags to named volumes.The named volume mount correctly mirrors the anonymous volume logic, applying
,exec,devconditionally. The exclusion of these flags from bind mounts (line 519) is appropriate, as they apply only to Docker-managed volumes, not host paths.The changes are minimal, focused, and consistent across volume types.
|
I think that "Since podman is often installed with a |
|
So, if we were to re-word that description to "when |
Co-authored-by: tabrisnet <[email protected]>
Description
This is in a similar vein to #8660 and works towards resolving issues like #7940.
Basically, this is a tiny patch adding the minimal changes needed to run the image build using podman instead of docker. Since podman is often installed with a
podman-dockershim to emulate the docker cli, the only changes needed are to use a rootful podman connection (needed for working with loop devices) and this patch to addexecanddevflags to volume mounts (since podman mounts volumes by default withnodevand sometimesnoexec)Documentation summary for feature / change
If documentation entry is predicted, please provide key elements for further implementation into main documentation and set label to "Needs Documentation". You are welcome to open a PR to documentation or you can leave following information for technical writer:
podman-dockerinstalled, runCONTAINER_HOST=unix:///run/podman/podman.sock ./compile.shorCONTAINER_CONNECTION=root ./compile.shif the rootful connection is already configured inpodmanHow Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
dockerpodman-docker(emulating the docker cli using podman)$CONTAINER_HOSTor$CONTAINER_CONNECTIONenvironment variables depending on system setupChecklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.