Support default platform via containers.conf and CONTAINER_DEFAULT_PLATFORM#28154
Support default platform via containers.conf and CONTAINER_DEFAULT_PLATFORM#28154TrevorBurnham wants to merge 1 commit intocontainers:mainfrom
Conversation
27ec27b to
787b31a
Compare
Luap99
left a comment
There was a problem hiding this comment.
How to properly update and test changes here so ci can run.
Overall I do not think we should use DOCKER_DEFAULT_PLATFORM at all. If we really need and env it should be a different one IMO
And now regarding the client vs server side and code deduplication I think we might be able best to handle this once on the common package in the libimage runtime in the pull function.
I guess we would still need to handle the build part though.
cmd/podman/common/build.go
Outdated
| // If no explicit --platform, --os, --arch, or --variant was given, | ||
| // fall back to DOCKER_DEFAULT_PLATFORM env var, then containers.conf platform. | ||
| if !c.Flag("platform").Changed && !c.Flag("os").Changed && !c.Flag("arch").Changed && !c.Flag("variant").Changed { | ||
| defOS, defArch, defVariant, pErr := util.DefaultPlatform(podmanConfig.ContainersConfDefaultsRO.Engine.Platform) |
There was a problem hiding this comment.
the handling of such containers.conf fields should not be on the client side generally, I know we have been very inconsistent but I will try to enforce that for all new code
In general the code like that is wrong because something like "local" would be resolved on client side while the server can have a different arch.
8fc6215 to
fcf76b4
Compare
Done, thanks!
Supporting an env var strikes me as the most straightforward way to solve the problem described on #25641, where a user wants to specify the platform but doesn't have the ability to pass a Personally I really like the idea of supporting |
3a70883 to
9f490e3
Compare
I dont see where DOCKER_HOST is used? It is being set by clients that like to talk to our socket but podman itself does not need to read it, i.e. having that set will not make podman-remote talk to that endpoint at all. WE use CONTAINER_HOST instead. DOCKER_CONFIG is used by our code but IMO there is no downside to reading that file AFAICT, DOCKER_DEFAULT_PLATFORM would actively alter the podman behavior and I Don;t like that as people can use podman and docker in parallel if they want so using one env for both does not seem right. If people want an env I am fine adding a new one but I do not want to use a DOCKER_ one as we do now own that. |
…ATFORM Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
9f490e3 to
90deceb
Compare
|
Got it. I saw references to I've rebased the PR branch and updated it to use |
Depends on: containers/container-libs#669
Closes: #25641
This PR adds support for setting a default platform for image operations (pull, build, create, run) without requiring
--platformon every command.The default platform is resolved in the following priority order:
--platform(or--arch/--os) flagCONTAINER_DEFAULT_PLATFORMenvironment variable (new)containers.conf[engine] platform setting (new)Checklist
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?
Yes: