-
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
Improve buildkit node creation #10843
Conversation
…oad them for every service build. Signed-off-by: Silvin Lubecki <[email protected]>
961d232
to
092cd69
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## v2 #10843 +/- ##
==========================================
+ Coverage 58.30% 59.94% +1.63%
==========================================
Files 119 118 -1
Lines 10331 10076 -255
==========================================
+ Hits 6024 6040 +16
+ Misses 3710 3441 -269
+ Partials 597 595 -2
☔ View full report in Codecov by Sentry. |
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
Note to Compose team: we should make sure to mention that BUILDX_BUILDER
is now supported for up --build
as well in the release notes (see also my non-blocking review comment)
builderName := options.Builder | ||
if builderName == "" { | ||
builderName = os.Getenv("BUILDX_BUILDER") | ||
} |
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.
Hmm, so we already do a look up in cmd/build.go
:
Lines 58 to 61 in 8318f66
builderName := opts.builder | |
if builderName == "" { | |
builderName = os.Getenv("BUILDX_BUILDER") | |
} |
But that won't apply for up --build
(while this will).
@glours Do you think that --builder
should become a global flag on Compose, and let the CLI/cmd part handle the env var lookup?
[I don't think we need to resolve this as part of this PR]
What I did
Move builder and nodes initialization code up, avoiding to recreate/load them for every service build.
Added support of
BUILDX_BUILDER
env var.Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did