-
Notifications
You must be signed in to change notification settings - Fork 236
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
fix: default to legacy builder instead of BuildKit with podman #900
base: main
Are you sure you want to change the base?
fix: default to legacy builder instead of BuildKit with podman #900
Conversation
@microsoft-github-policy-service agree |
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.
Looks good to me, looping in @chrmarti for extra 👀
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.
Should this only target the compose case?
I believe this would still fail with cli/src/spec-node/singleContainer.ts Lines 190 to 211 in 9ba1fda
I guess I can add the check for podman in these lines and the compose lines instead, thought to short-circuit it from the also maybe I'm missing it but is there a way to define |
48be600
to
3a80b14
Compare
- Podman doesn't support all buildkit features. - Podman main branch is now always defaulting to "DOCKER_BUILDKIT=0", See https://github.com/containers/podman/blob/f7be7a365ad3e90db5f96f269a555f6f380f9275/cmd/podman/compose.go#L164 - fix: make cli ignore `--buildkit` parameter and default to `never` when Podman is used.
3a80b14
to
8afbc9c
Compare
With Podman 4.5.1 I see buildah 1.30 with |
@chrmarti sorry didn't get to check this again, I believe I was testing with podman 5+ and it was failing, anyway I'll try to check it again soon with podman and update this accordingly |
Description
Podman doesn't support buildkit and defaults to "DOCKER_BUILDKIT=0", See https://github.com/containers/podman/blob/f7be7a365ad3e90db5f96f269a555f6f380f9275/cmd/podman/compose.go#L157-L170
This results in Podman failing builds when
BUILDKIT_INLINE_CACHE
oradditional_contexts
is used, as for example incli/src/spec-node/dockerCompose.ts
Lines 230 to 245 in 9ba1fda
A simple fix would to make cli ignore
--buildkit
parameter, default tonever
and log a warning when Podman is used.Fixes #863
Testing
A failing example would be
Dockerfile
devcontainer.json
compose.yaml
when features is used in
devcontainer.json
this would always fail with logthe classic builder doesn't support additional contexts, set DOCKER_BUILDKIT=1 to use BuildKit
I have compiled this on my machine and used it with
--buildkit auto
and--buildkit none
and it works correctly and tests are passing normally, maybe an additional testing can be added for this.