feat: transparent buildx support for remote buildkit (#6732)

* detect-buildx global config option for backward compatibility
* cache-tag global config option to customize cache destination
* new CacheTo in DockerArtifact in configuration yaml (for docker build --cache-to)

* export LoadDockerConfig to read ~/.docker/config.json for buildx detection
* fix avoid loading image via buildx if no docker daemon is accessible
* fix remote lookup / import missing in buildx workaround
* fix image import if no docker daemon is available (under buildx)
* adjust cache reference preserving tag and default cacheTo if not given
* parse buildx metadata to extract ImageID digest

Initially based on ebekebe's #8172 patch
ebekebe@1c1fdeb

Signed-off-by: [email protected]

Author: reingart

pkg/skaffold/build/cache/cache.go

@@ -58,6 +58,7 @@ type cache struct {
 	isLocalImage       func(imageName string) (bool, error)
 	importMissingImage func(imageName string) (bool, error)
 	lister             DependencyLister
+	buildx             bool
 }
 
 // DependencyLister fetches a list of dependencies for an artifact
@@ -69,6 +70,7 @@ type Config interface {
 	GetPipelines() []latest.Pipeline
 	DefaultPipeline() latest.Pipeline
 	GetCluster() config.Cluster
+	DetectBuildX() bool
 	CacheArtifacts() bool
 	CacheFile() string
 	Mode() config.RunMode
@@ -118,6 +120,9 @@ func NewCache(ctx context.Context, cfg Config, isLocalImage func(imageName strin
 		return pipeline.Build.LocalBuild.TryImportMissing, nil
 	}
 
+	// for backward compatibility, extended build capabilities with BuildKit are disabled by default
+	buildx := cfg.DetectBuildX()
+
 	return &cache{
 		artifactCache:      artifactCache,
 		hashByName:         hashByName,
@@ -129,6 +134,7 @@ func NewCache(ctx context.Context, cfg Config, isLocalImage func(imageName strin
 		isLocalImage:       isLocalImage,
 		importMissingImage: importMissingImage,
 		lister:             dependencies,
+		buildx:             buildx,
 	}, nil
 }
 

pkg/skaffold/build/cache/lookup.go

@@ -87,6 +87,7 @@ func (c *cache) lookup(ctx context.Context, out io.Writer, a *latest.Artifact, t
 	}
 
 	if isLocal, err := c.isLocalImage(a.ImageName); err != nil {
+		log.Entry(ctx).Debugf("isLocalImage failed %w", err)
 		return failed{err}
 	} else if isLocal {
 		return c.lookupLocal(ctx, hash, tag, entry)
@@ -122,9 +123,12 @@ func (c *cache) lookupLocal(ctx context.Context, hash, tag string, entry ImageDe
 func (c *cache) lookupRemote(ctx context.Context, hash, tag string, platforms []specs.Platform, entry ImageDetails) cacheDetails {
 	if remoteDigest, err := docker.RemoteDigest(tag, c.cfg, nil); err == nil {
 		// Image exists remotely with the same tag and digest
+		log.Entry(ctx).Debugf("RemoteDigest: %s entry.Digest %s", remoteDigest, entry.Digest)
 		if remoteDigest == entry.Digest {
 			return found{hash: hash}
 		}
+	} else {
+		log.Entry(ctx).Debugf("RemoteDigest error %w", err)
 	}
 
 	// Image exists remotely with a different tag
@@ -152,23 +156,32 @@ func (c *cache) tryImport(ctx context.Context, a *latest.Artifact, tag string, h
 		return ImageDetails{}, fmt.Errorf("import of missing images disabled")
 	}
 
-	if !c.client.ImageExists(ctx, tag) {
-		log.Entry(ctx).Debugf("Importing artifact %s from docker registry", tag)
-		err := c.client.Pull(ctx, io.Discard, tag, pl)
+	// under buildx, docker daemon is not really needed and could be disabled
+	load := true
+	if c.buildx {
+		_, err := c.client.ServerVersion(ctx)
+		load = err == nil
+		if !load {
+			log.Entry(ctx).Debugf("Docker client error, disabling image load as using buildx: %w", err)
+		}
+	}
+	if load {
+		if !c.client.ImageExists(ctx, tag) {
+			log.Entry(ctx).Debugf("Importing artifact %s from docker registry", tag)
+			err := c.client.Pull(ctx, io.Discard, tag, pl)
+			if err != nil {
+				return entry, err
+			}
+		} else {
+			log.Entry(ctx).Debugf("Importing artifact %s from local docker", tag)
+		}
+		imageID, err := c.client.ImageID(ctx, tag)
 		if err != nil {
 			return entry, err
 		}
-	} else {
-		log.Entry(ctx).Debugf("Importing artifact %s from local docker", tag)
-	}
-
-	imageID, err := c.client.ImageID(ctx, tag)
-	if err != nil {
-		return entry, err
-	}
-
-	if imageID != "" {
-		entry.ID = imageID
+		if imageID != "" {
+			entry.ID = imageID
+		}
 	}
 
 	if digest, err := docker.RemoteDigest(tag, c.cfg, nil); err == nil {

pkg/skaffold/build/docker/docker.go

@@ -19,13 +19,15 @@ package docker
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"os"
 	"os/exec"
 
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 
+	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/instrumentation"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output"
@@ -46,7 +48,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact,
 	if len(matcher.Platforms) == 1 {
 		pl = util.ConvertToV1Platform(matcher.Platforms[0])
 	}
-	a = adjustCacheFrom(a, tag)
+	a = b.adjustCache(ctx, a, tag)
 	instrumentation.AddAttributesToCurrentSpanFromContext(ctx, map[string]string{
 		"BuildType":   "docker",
 		"Context":     instrumentation.PII(a.Workspace),
@@ -82,7 +84,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact,
 		return "", newBuildError(err, b.cfg)
 	}
 
-	if b.pushImages {
+	if !b.useCLI && b.pushImages {
 		// TODO (tejaldesai) Remove https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/errors/err_map.go#L56
 		// and instead define a pushErr() method here.
 		return b.localDocker.Push(ctx, out, tag)
@@ -120,10 +122,31 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string
 		args = append(args, "--platform", pl.String())
 	}
 
-	if b.useBuildKit != nil && *b.useBuildKit && !b.pushImages {
-		args = append(args, "--load")
+	if b.useBuildKit != nil && *b.useBuildKit {
+		if !b.pushImages {
+			load := true
+			if b.buildx {
+				// if docker daemon is not used, do not try to load the image (a buildx warning will be logged)
+				_, err := b.localDocker.ServerVersion(ctx)
+				load = err == nil
+			}
+			if load {
+				args = append(args, "--load")
+			}
+		} else if b.buildx {
+			// with buildx, push the image directly to the registry (not using the docker daemon)
+			args = append(args, "--push")
+		}
 	}
 
+	// temporary file for buildx metadata containing the image digest:
+	var metadata *os.File
+	if b.buildx {
+		metadata, err = os.CreateTemp("", "metadata.json")
+		metadata.Close()
+		defer os.Remove(metadata.Name())
+		args = append(args, "--metadata-file", metadata.Name())
+	}
 	cmd := exec.CommandContext(ctx, "docker", args...)
 	cmd.Env = append(util.OSEnviron(), b.localDocker.ExtraEnv()...)
 	if b.useBuildKit != nil {
@@ -146,11 +169,16 @@ 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 b.localDocker.ImageID(ctx, opts.Tag)
+	} else {
+		return getBuildxDigest(ctx, metadata.Name())
+	}
 }
 
 func (b *Builder) pullCacheFromImages(ctx context.Context, out io.Writer, a *latest.DockerArtifact, pl v1.Platform) error {
-	if len(a.CacheFrom) == 0 {
+	// when using buildx, avoid pulling as the builder not necessarily uses the local docker daemon
+	if len(a.CacheFrom) == 0 || b.buildx {
 		return nil
 	}
 
@@ -172,26 +200,81 @@ func (b *Builder) pullCacheFromImages(ctx context.Context, out io.Writer, a *lat
 	return nil
 }
 
-// adjustCacheFrom returns an artifact where any cache references from the artifactImage is changed to the tagged built image name instead.
-func adjustCacheFrom(a *latest.Artifact, artifactTag string) *latest.Artifact {
+// adjustCache returns an artifact where any cache references from the artifactImage is changed to the tagged built image name instead.
+// Under buildx, if cache-tag is configured, it will be used instead of the generated artifact tag (registry preserved)
+// if no cacheTo was specified in the skaffold yaml, it will add a tagged destination using the same cache source reference
+func (b *Builder) adjustCache(ctx context.Context, a *latest.Artifact, artifactTag string) *latest.Artifact {
+	cacheRef := a.ImageName // lookup value to be replaced
+	cacheTag := artifactTag // full reference to be used
 	if os.Getenv("SKAFFOLD_DISABLE_DOCKER_CACHE_ADJUSTMENT") != "" {
 		// allow this behaviour to be disabled
 		return a
 	}
-
-	if !stringslice.Contains(a.DockerArtifact.CacheFrom, a.ImageName) {
+	if b.buildx {
+		// compute the full cache reference (including registry, preserving tag)
+		tag, _ := config.GetCacheTag(b.cfg.GlobalConfig())
+		imgRef, err := docker.ParseReference(artifactTag)
+		if err != nil {
+			log.Entry(ctx).Errorf("couldn't parse image tag: %w", err)
+		} else if tag != "" {
+			cacheTag = fmt.Sprintf("%s/%s:%s", imgRef.Repo, cacheRef, tag)
+		}
+	}
+	if !stringslice.Contains(a.DockerArtifact.CacheFrom, cacheRef) {
 		return a
 	}
 
 	cf := make([]string, 0, len(a.DockerArtifact.CacheFrom))
+	ct := make([]string, 0, len(a.DockerArtifact.CacheTo))
 	for _, image := range a.DockerArtifact.CacheFrom {
-		if image == a.ImageName {
-			cf = append(cf, artifactTag)
+		if image == cacheRef {
+			// change cache reference to to the tagged image name (built or given, including registry)
+			log.Entry(ctx).Debugf("Adjusting cache source image ref: %s", cacheTag)
+			cf = append(cf, cacheTag)
+			if b.buildx {
+				// add cache destination reference, only if we're pushing to a registry and not given in config
+				if len(a.DockerArtifact.CacheTo) == 0 && b.pushImages {
+					log.Entry(ctx).Debugf("Adjusting cache destination image ref: %s", cacheTag)
+					ct = append(ct, fmt.Sprintf("type=registry,ref=%s,mode=max", cacheTag))
+				}
+			}
 		} else {
 			cf = append(cf, image)
 		}
 	}
+	// just copy any other cache destination given in the config file:
+	for _, image := range a.DockerArtifact.CacheTo {
+		ct = append(cf, image)
+	}
 	copy := *a
 	copy.DockerArtifact.CacheFrom = cf
+	copy.DockerArtifact.CacheTo = ct
 	return &copy
 }
+
+func getBuildxDigest(ctx context.Context, filename string) (string, error) {
+	var metadata map[string]interface{}
+	data, err := os.ReadFile(filename)
+	if err == nil {
+		err = json.Unmarshal(data, &metadata)
+	}
+	if err == nil {
+		// avoid panic: interface conversion: interface {} is nil, not string (if keys don't exists)
+		var digest string
+		if value := metadata["containerimage.digest"]; value != nil {
+			digest = value.(string)
+		}
+		var name string
+		if value := metadata["image.name"]; value != nil {
+			name = value.(string)
+		}
+		if digest != "" {
+			log.Entry(ctx).Debugf("Image digest found in buildx metadata: %s for %s", digest, name)
+			return digest, nil
+		}
+	}
+	log.Entry(ctx).Warnf("No digest found in buildx metadata: %w", err)
+	// if image is not pushed, it could not contain the digest log for debugging:
+	log.Entry(ctx).Debugf("Full buildx metadata: %s", data)
+	return "", err
+}

pkg/skaffold/build/docker/types.go

@@ -30,6 +30,7 @@ type Builder struct {
 	pushImages         bool
 	useCLI             bool
 	useBuildKit        *bool
+	buildx             bool
 	artifacts          ArtifactResolver
 	sourceDependencies TransitiveSourceDependenciesResolver
 }
@@ -45,13 +46,14 @@ 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, useBuildKit *bool, buildx bool, pushImages bool, ar ArtifactResolver, dr TransitiveSourceDependenciesResolver) *Builder {
 	return &Builder{
 		localDocker:        localDocker,
 		pushImages:         pushImages,
 		cfg:                cfg,
 		useCLI:             useCLI,
 		useBuildKit:        useBuildKit,
+		buildx:             buildx,
 		artifacts:          ar,
 		sourceDependencies: dr,
 	}

pkg/skaffold/build/local/local.go

@@ -89,9 +89,10 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, a *latest.Ar
 	if b.pushImages {
 		// only track images for pruning when building with docker
 		// if we're pushing a bazel image, it was built directly to the registry
+		// buildx also has its own build cache, and image load will not be attempted if no docker daemon is accessible
 		if a.DockerArtifact != nil {
 			imageID, err := b.getImageIDForTag(ctx, tag)
-			if err != nil {
+			if err != nil && !b.buildx {
 				log.Entry(ctx).Warn("unable to inspect image: built images may not be cleaned up correctly by skaffold")
 			}
 			if imageID != "" {
@@ -103,6 +104,8 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, a *latest.Ar
 		return build.TagWithDigest(tag, digest), nil
 	}
 
+	// TODO: prune buildx cache using digest?
+
 	imageID := digestOrImageID
 	if b.mode == config.RunModes.Dev {
 		artifacts, err := b.artifactStore.GetArtifacts([]*latest.Artifact{a})
@@ -123,8 +126,8 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, a *latest.Ar
 }
 
 func (b *Builder) runBuildForArtifact(ctx context.Context, out io.Writer, a *latest.Artifact, tag string, platforms platform.Matcher) (string, error) {
-	if !b.pushImages {
-		// All of the builders will rely on a local Docker:
+	if !b.buildx && !b.pushImages {
+		// Most builders will rely on a local Docker (except when using a remote buildkit via buildx):
 		// + Either to build the image,
 		// + Or to docker load it.
 		// Let's fail fast if Docker is not available

pkg/skaffold/build/local/types.go

@@ -52,6 +52,7 @@ type Builder struct {
 	skipTests          bool
 	mode               config.RunMode
 	kubeContext        string
+	buildx             bool
 	builtImages        []string
 	insecureRegistries map[string]bool
 	muted              build.Muted
@@ -66,6 +67,7 @@ type Config interface {
 	GlobalConfig() string
 	GetKubeContext() string
 	GetCluster() config.Cluster
+	DetectBuildX() bool
 	SkipTests() bool
 	Mode() config.RunMode
 	NoPruneChildren() bool
@@ -103,6 +105,9 @@ func NewBuilder(ctx context.Context, bCtx BuilderContext, buildCfg *latest.Local
 
 	tryImportMissing := buildCfg.TryImportMissing
 
+	// for backward compatibility, extended build capabilities with BuildKit are disabled by default
+	buildx := bCtx.DetectBuildX()
+
 	return &Builder{
 		local:              *buildCfg,
 		cfg:                bCtx,
@@ -111,6 +116,7 @@ func NewBuilder(ctx context.Context, bCtx BuilderContext, buildCfg *latest.Local
 		localCluster:       cluster.Local,
 		pushImages:         pushImages,
 		tryImportMissing:   tryImportMissing,
+		buildx:             buildx,
 		skipTests:          bCtx.SkipTests(),
 		mode:               bCtx.Mode(),
 		prune:              bCtx.Prune(),
@@ -133,7 +139,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.UseBuildkit, b.buildx, b.pushImages, b.artifactStore, b.sourceDependencies), nil
 
 	case a.BazelArtifact != nil:
 		return bazel.NewArtifactBuilder(b.localDocker, b.cfg, b.pushImages), nil

pkg/skaffold/config/global_config.go

@@ -33,6 +33,8 @@ type ContextConfig struct {
 	InsecureRegistries []string `yaml:"insecure-registries,omitempty"`
 	// DebugHelpersRegistry is the registry from which the debug helper images are used.
 	DebugHelpersRegistry string        `yaml:"debug-helpers-registry,omitempty"`
+	CacheTag             string        `yaml:"cache-tag,omitempty"`
+	DetectBuildX         *bool         `yaml:"detect-buildx,omitempty"`
 	UpdateCheck          *bool         `yaml:"update-check,omitempty"`
 	Survey               *SurveyConfig `yaml:"survey,omitempty"`
 	KindDisableLoad      *bool         `yaml:"kind-disable-load,omitempty"`