Skip to content
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

Add buildx option for daemon-less BuildKit support #8172

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions pkg/skaffold/build/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact,
// ignore useCLI boolean if buildkit is enabled since buildkit is only implemented for docker CLI at the moment in skaffold.
// we might consider a different approach in the future.
// use CLI for cross-platform builds
if b.useCLI || (b.useBuildKit != nil && *b.useBuildKit) || len(a.DockerArtifact.CliFlags) > 0 || matcher.IsNotEmpty() {
if b.useCLI || b.buildx || (b.useBuildKit != nil && *b.useBuildKit) || len(a.DockerArtifact.CliFlags) > 0 || matcher.IsNotEmpty() {
imageID, err = b.dockerCLIBuild(ctx, output.GetUnderlyingWriter(out), a.ImageName, a.Workspace, dockerfile, a.ArtifactType.DockerArtifact, opts, pl)
} else {
imageID, err = b.localDocker.Build(ctx, out, a.Workspace, a.ImageName, a.ArtifactType.DockerArtifact, opts)
Expand All @@ -93,6 +93,9 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact,

func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string, workspace string, dockerfilePath string, a *latest.DockerArtifact, opts docker.BuildOptions, pl v1.Platform) (string, error) {
args := []string{"build", workspace, "--file", dockerfilePath, "-t", opts.Tag}
if b.buildx {
args = append([]string{"buildx"}, args...)
}
imgRef, err := docker.ParseReference(opts.Tag)
if err != nil {
return "", fmt.Errorf("couldn't parse image tag: %w", err)
Expand All @@ -110,6 +113,9 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string
if err != nil {
return "", fmt.Errorf("getting docker build args: %w", err)
}
if b.buildx && b.pushImages {
cliArgs = append(cliArgs, "--push")
}
args = append(args, cliArgs...)

if b.cfg.Prune() {
Expand Down Expand Up @@ -142,7 +148,11 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string
return "", tryExecFormatErr(fmt.Errorf("running build: %w", err), errBuffer)
}

return b.localDocker.ImageID(ctx, opts.Tag)
if b.buildx {
return "", nil // TODO: return id from CLI
} else {
return b.localDocker.ImageID(ctx, opts.Tag)
}
}

func (b *Builder) pullCacheFromImages(ctx context.Context, out io.Writer, a *latest.DockerArtifact, pl v1.Platform) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/build/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func TestDockerCLIBuild(t *testing.T) {
}
t.Override(&util.OSEnviron, func() []string { return []string{"KEY=VALUE"} })

builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv(test.extraEnv), test.cfg, test.localBuild.UseDockerCLI, test.localBuild.UseBuildkit, false, mockArtifactResolver{make(map[string]string)}, nil)
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv(test.extraEnv), test.cfg, test.localBuild.UseDockerCLI, false, test.localBuild.UseBuildkit, false, mockArtifactResolver{make(map[string]string)}, nil)

artifact := &latest.Artifact{
Workspace: ".",
Expand Down Expand Up @@ -242,7 +242,7 @@ func TestDockerCLICheckCacheFromArgs(t *testing.T) {
)
t.Override(&util.DefaultExecCommand, mockCmd)

builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, util.Ptr(false), false, mockArtifactResolver{make(map[string]string)}, nil)
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, false, util.Ptr(false), false, mockArtifactResolver{make(map[string]string)}, nil)
_, err := builder.Build(context.Background(), io.Discard, &a, test.tag, platform.Matcher{})
t.CheckNoError(err)
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/docker/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ Refer https://skaffold.dev/docs/references/yaml/#build-artifacts-docker for deta
"docker build . --file "+dockerfilePath+" -t tag",
))
t.Override(&docker.DefaultAuthHelper, stubAuth{})
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, nil, false, mockArtifactResolver{make(map[string]string)}, nil)
builder := NewArtifactBuilder(fakeLocalDaemonWithExtraEnv([]string{}), mockConfig{}, true, false, nil, false, mockArtifactResolver{make(map[string]string)}, nil)

artifact := &latest.Artifact{
ImageName: "test-image",
Expand Down
4 changes: 3 additions & 1 deletion pkg/skaffold/build/docker/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type Builder struct {
cfg docker.Config
pushImages bool
useCLI bool
buildx bool
useBuildKit *bool
artifacts ArtifactResolver
sourceDependencies TransitiveSourceDependenciesResolver
Expand All @@ -45,12 +46,13 @@ type TransitiveSourceDependenciesResolver interface {
}

// NewBuilder returns an new instance of a docker builder
func NewArtifactBuilder(localDocker docker.LocalDaemon, cfg docker.Config, useCLI bool, useBuildKit *bool, pushImages bool, ar ArtifactResolver, dr TransitiveSourceDependenciesResolver) *Builder {
func NewArtifactBuilder(localDocker docker.LocalDaemon, cfg docker.Config, useCLI bool, buildx bool, useBuildKit *bool, pushImages bool, ar ArtifactResolver, dr TransitiveSourceDependenciesResolver) *Builder {
return &Builder{
localDocker: localDocker,
pushImages: pushImages,
cfg: cfg,
useCLI: useCLI,
buildx: buildx,
useBuildKit: useBuildKit,
artifacts: ar,
sourceDependencies: dr,
Expand Down
9 changes: 7 additions & 2 deletions pkg/skaffold/build/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,16 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, a *latest.Ar

imageID := digestOrImageID
b.builtImages = append(b.builtImages, imageID)
return build.TagWithImageID(ctx, tag, imageID, b.localDocker)

if b.local.Buildx {
return "", nil
} else {
return build.TagWithImageID(ctx, tag, imageID, b.localDocker)
}
}

func (b *Builder) runBuildForArtifact(ctx context.Context, out io.Writer, a *latest.Artifact, tag string, platforms platform.Matcher) (string, error) {
if !b.pushImages {
if !b.local.Buildx && !b.pushImages {
// All of the builders will rely on a local Docker:
// + Either to build the image,
// + Or to docker load it.
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ type artifactBuilder interface {
func newPerArtifactBuilder(b *Builder, a *latest.Artifact) (artifactBuilder, error) {
switch {
case a.DockerArtifact != nil:
return dockerbuilder.NewArtifactBuilder(b.localDocker, b.cfg, b.local.UseDockerCLI, b.local.UseBuildkit, b.pushImages, b.artifactStore, b.sourceDependencies), nil
return dockerbuilder.NewArtifactBuilder(b.localDocker, b.cfg, b.local.UseDockerCLI, b.local.Buildx, b.local.UseBuildkit, b.pushImages, b.artifactStore, b.sourceDependencies), nil

case a.BazelArtifact != nil:
return bazel.NewArtifactBuilder(b.localDocker, b.cfg, b.pushImages), nil
Expand Down
2 changes: 2 additions & 0 deletions pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ type LocalBuild struct {
// UseDockerCLI use `docker` command-line interface instead of Docker Engine APIs.
UseDockerCLI bool `yaml:"useDockerCLI,omitempty"`

Buildx bool `yaml:"buildx,omitempty"`

// UseBuildkit use BuildKit to build Docker images. If unspecified, uses the Docker default.
UseBuildkit *bool `yaml:"useBuildkit,omitempty"`

Expand Down