-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: make useBuildkit
field nullable across all config versions
#6612
Conversation
setting `useBuildkit: false` actually turns off buildkit
Codecov Report
@@ Coverage Diff @@
## main #6612 +/- ##
==========================================
- Coverage 70.48% 69.99% -0.50%
==========================================
Files 515 520 +5
Lines 23150 23603 +453
==========================================
+ Hits 16317 16520 +203
- Misses 5776 6001 +225
- Partials 1057 1082 +25
Continue to review full report at Codecov.
|
@@ -54,7 +54,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latestV1.Artifact | |||
|
|||
var imageID string | |||
|
|||
if b.useCLI || b.useBuildKit { | |||
if b.useCLI || (b.useBuildKit != nil && *b.useBuildKit) { |
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.
Worth a comment here to say that we ignore the useCLI setting if buildkit is requested as we currently use the Docker CLI to use buildkit. We could consider a different approach in the future.
@nkubala fyi: this PR changes previously-published config schemas to turn |
fix: setting
useBuildkit: false
actually turns off buildkit for docker builderRelated: #4936
Description
Currently setting
useBuildkit: false
in theskaffold.yaml
config doesn't actually disable buildkit. This PR fixes that by settingDOCKER_BUILDKIT =0
env variable whenuseBuildkit: false
is set in the config.In
pkg/skaffold/build/docker/docker.go
:This change in itself would however break backwards compatibility since for previous versions of the config even if
useBuildkit
property is not set, it'd default tofalse
and now start disabling buildkit which is not how it worked previously (so that bug essentially became a feature, that ifuseBuildkit
is unspecified it should use the Docker default).So we additionally make the config change to all config versions, so that a missing
useBuildkit
field can be identified separately from an explicitly setuseBuildkit: false
User facing changes (remove if N/A)
There are no user facing changes other than if users were setting
useBuildkit: false
in the config, skaffold will now actually disable buildkit.Follow-up Work (remove if N/A)
Fix #4936 after this is merged.