-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
delegate build to buildx bake #12300
Conversation
pkg/compose/build.go
Outdated
return imageIDs, err | ||
} | ||
|
||
if b, ok := os.LookupEnv("COMPOSE_BAKE"); ok { |
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.
Perhaps could also be worth (if compose reads the cli config) to consider either an option in features
, or plugins
(plugins allows plugin-specific options to be set), which would allow opt-in/opt-out of this without having to use an env-var;
https://github.com/docker/cli/blob/9861ce90fd6b8ddca19db5f803dcbef9a583e9e1/cli/config/configfile/file.go#L42-L44
Plugins map[string]map[string]string `json:"plugins,omitempty"`
Aliases map[string]string `json:"aliases,omitempty"`
Features map[string]string `json:"features,omitempty"`
(in addition to an env-var probably)
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.
we never used this mechanism in the past for optional/experimental docker compose features
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.
Yeah, my thinking here was that the cli-config would more easily allow this to be set as a default, which could also allow (e.g.) it to be set through docker desktop "settings".
84fad3d
to
9be4fef
Compare
c678ddf
to
731da92
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12300 +/- ##
==========================================
- Coverage 50.14% 49.66% -0.48%
==========================================
Files 154 155 +1
Lines 15060 15310 +250
==========================================
+ Hits 7552 7604 +52
- Misses 6738 6931 +193
- Partials 770 775 +5 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
if dockerCli.ConfigFile().Plugins["compose"]["build"] == "bake" { | ||
b, ok = "true", true | ||
} |
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.
SGTM, but maybe we should error out on unexpected/invalid values?
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.
LGTM, tests ok
We should open follow up PRs to add bake flavour of existing e2e build tests
I was indeed looking for configuration on CI to run build-related tests with this new option enabled. |
Just some quick blurbs;
|
I don't think this is a good solution. 1st, this would break separation of concerns, and force the docker CLI to be aware about compose sub-commands and UX (only a subset of service might be requested for build). Also, this would interfere with other commands which involve building images but also some compose actions, typically |
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
731da92
to
b8a0254
Compare
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | minor | `v2.30.3` -> `v2.31.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>docker/compose (docker/compose)</summary> ### [`v2.31.0`](https://github.com/docker/compose/releases/tag/v2.31.0) [Compare Source](docker/compose@v2.30.3...v2.31.0) #### What's Changed ##### ✨ Improvements - Delegate build to buildx bake by [@​ndeloof](https://github.com/ndeloof) [(12300)](docker/compose#12300) - Add commit command by [@​jarqvi](https://github.com/jarqvi) [(12268)](docker/compose#12268) ##### 🐛 Fixes - Fix(config): Print service names with --no-interpolate by [@​idsulik](https://github.com/idsulik) [(12282)](docker/compose#12282) - Remove obsolete containers first on scale down by [@​ndeloof](https://github.com/ndeloof) [(12272)](docker/compose#12272) - Fix compose images that return a different image with the same ID by [@​koooge](https://github.com/koooge) [(12278)](docker/compose#12278) - Emit events for building images by [@​felixfontein](https://github.com/felixfontein) [(11498)](docker/compose#11498) - Fix support for --remove-orphans on `docker compose run` by [@​ndeloof](https://github.com/ndeloof) [(12288)](docker/compose#12288) - Push empty descriptor layer when using OCI version 1.1 for Compose artifact by [@​glours](https://github.com/glours) [(12289)](docker/compose#12289) - Detect network config changes and recreate if needed by [@​ndeloof](https://github.com/ndeloof) [(12267)](docker/compose#12267) - Update wait-timeout flag usage to include the unit by [@​terev](https://github.com/terev) [(12316)](docker/compose#12316) - Use service.stop to stop dependent containers by [@​ndeloof](https://github.com/ndeloof) [(12322)](docker/compose#12322) - Only check attached networks on running containers by [@​ndeloof](https://github.com/ndeloof) [(12327)](docker/compose#12327) - Only stop dependent containers ... if there's some by [@​ndeloof](https://github.com/ndeloof) [(12328)](docker/compose#12328) ##### 🔧 Internal - Pass stale bot inactivity limit from 6 to 3 months by [@​glours](https://github.com/glours) [(12284)](docker/compose#12284) - Ci: enable testifylint linter by [@​mmorel-35](https://github.com/mmorel-35) [(11761)](docker/compose#11761) - Remove ddev e2e tests by [@​glours](https://github.com/glours) [(12291)](docker/compose#12291) - Gha: test against docker engine v27.4.0 by [@​thaJeztah](https://github.com/thaJeztah) [(12299)](docker/compose#12299) - Run build tests against bake by [@​ndeloof](https://github.com/ndeloof) [(12325)](docker/compose#12325) ##### ⚙️ Dependencies - Build(deps): bump golang.org/x/sync from `0.8.0` to `0.9.0` by [@​dependabot](https://github.com/dependabot) [(12277)](docker/compose#12277) - Build(deps): bump golang.org/x/sys from `0.26.0` to `0.27.0` by [@​dependabot](https://github.com/dependabot) [(12276)](docker/compose#12276) - Build(deps): bump github.com/moby/buildkit `v0.17.1`, github.com/docker/buildx `v0.18.0` by [@​thaJeztah](https://github.com/thaJeztah) [(12298)](docker/compose#12298) - Build(deps): bump docker/docker `v27.4.0-rc.2`, docker/cli `v27.4.0-rc.2` by [@​thaJeztah](https://github.com/thaJeztah) [(12306)](docker/compose#12306) - Build(deps): bump github.com/stretchr/testify from `1.9.0` to `1.10.0` by [@​dependabot](https://github.com/dependabot) [(12319)](docker/compose#12319) - Build(deps): bump github.com/compose-spec/compose-go/v2 from `2.4.5-0.20241111154218-9d02caaf8465` to `2.4.5` by [@​dependabot](https://github.com/dependabot) [(12324)](docker/compose#12324) - Build(deps): bump github.com/moby/buildkit from `0.17.1` to `0.17.2` by [@​dependabot](https://github.com/dependabot) [(12320)](docker/compose#12320) - Bump google.golang.org/grpc to v1.68.0 and containerd to `v1.7.24` by [@​glours](https://github.com/glours) [(12329)](docker/compose#12329) #### New Contributors - [@​terev](https://github.com/terev) made their first contribution in docker/compose#12316 **Full Changelog**: docker/compose@v2.30.3...v2.31.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
What I did
introduce support for COMPOSE_BAKE to opt-in for build to be delegated to buildx bake. Compose generates a bake json config from application model and executes bake, which (we assume) better knows how to orchestrate build steps for best efficiency.
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did