diff --git a/cmd/skaffold/app/cmd/config/set_test.go b/cmd/skaffold/app/cmd/config/set_test.go index b34845cfd59..48510a57ebb 100644 --- a/cmd/skaffold/app/cmd/config/set_test.go +++ b/cmd/skaffold/app/cmd/config/set_test.go @@ -415,7 +415,7 @@ func TestGetConfigStructWithIndex(t *testing.T) { description: "survey flag set", cfg: &config.ContextConfig{}, survey: true, - expectedIdx: []int{7}, + expectedIdx: []int{9}, }, { description: "no survey flag set", diff --git a/docs-v2/content/en/schemas/v4beta13.json b/docs-v2/content/en/schemas/v4beta13.json index 7e1cc33dbf7..4f1cf6c7d19 100755 --- a/docs-v2/content/en/schemas/v4beta13.json +++ b/docs-v2/content/en/schemas/v4beta13.json @@ -1766,6 +1766,18 @@ "[\"golang:1.10.1-alpine3.7\", \"alpine:3.7\"]" ] }, + "cacheTo": { + "items": { + "type": "string" + }, + "type": "array", + "description": "the Docker images used as cache destination. If omitted, cacheFrom is used with max mode to export all layers.", + "x-intellij-html-description": "the Docker images used as cache destination. If omitted, cacheFrom is used with max mode to export all layers.", + "default": "[]", + "examples": [ + "[\"type=registry,ref=gcr.io/k8s-skaffold/example:cache,mode=max\"]" + ] + }, "cliFlags": { "items": { "type": "string" @@ -1830,6 +1842,7 @@ "network", "addHost", "cacheFrom", + "cacheTo", "cliFlags", "pullParent", "noCache", diff --git a/docs-v2/design_proposals/buildx-transparent-support.md b/docs-v2/design_proposals/buildx-transparent-support.md new file mode 100644 index 00000000000..b1608c086ec --- /dev/null +++ b/docs-v2/design_proposals/buildx-transparent-support.md @@ -0,0 +1,284 @@ +# buildx transparent support + +* Author(s): Mariano Reingart (@reingart) +* Design Shepherd: +* Date: 2025-02-02 +* Status: Draft + +## Objectives + +Transparent local and remote container builds via buildx, a Docker CLI plugin for extended capabilities with BuildKit. + +## Background + +[buildx](https://docs.docker.com/reference/cli/docker/buildx/) is an enhanced container builder using BuildKit that can replace traditional +`docker build` command, with almost the same syntax (transparently). + +This adds several distributed features and advanced capabilities: +* local & remote build-kit builders, either running standalone, in docker containers or Kubernetes clusters +* improved cache support (registry destination, pushing full metadatada and multi-stage layers) +* improved multi-platform image building support (eg. x86_64 and arm64 combined, with emulation or cross-compilation) + +This features are very useful for corporate CI/CD, for example when using GitLab, where the use case requires: +* using ephemeral remote buildkit instances (rootless) +* using remote docker registries for cache, with multi-stage and multi-platform images support +* using a different cache destination tag for flexibility and workflows separation + +The remote BuildKit [rootless](https://github.com/moby/buildkit/blob/master/docs/rootless.md) support is useful in cases +where a privileged docker daemonis not possible or desirable due security policies (eg. untrusted images or review pipelines). +Daemon-less mode is also useful to offload container building from developers notebooks, sharing and reusing remote caches more effectively. + +The buildx command also supports exporting the build cache using `--cache-to`, useful for remote shared caches in distributed use cases. +Beside speed-ups thanks to caching improvements for metadata and multi-stage layers, this could allow different branches +to have different cache destinations (production vs development cache, with different permissions). + +Multi-platform combined image builds are directly supported by buildx, including improved caching of common layers and emulation / cross-compilation. +This could simplify pipelines and provide faster builds of complex codebases. + +References: + +* https://www.docker.com/blog/image-rebase-and-improved-remote-cache-support-in-new-buildkit/ +* https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/ + +## Proposal + +This proposal aims to improve Skaffold's build process by adding transparent support for docker buildx. +The primary goal is to enable users to leverage the advanced features of buildx, such as remote buildkit builders and improved caching, without significant changes to their existing Skaffold configurations. + +New Global Configs: + +* `buildx-builder`: Enables automatic detection of buildx and multiple builder support. +* `cache-tag`: Allows overriding the default cache tagging strategy, useful for managing caches across different branches or environments. + +Skaffold Schema changes: + +* `cacheTo` to specify custom cache destinations, adding `--cache-to` support to the Docker CLI build process when using buildx. + +Build Process Enhancements: + +* The build process now intelligently detects and utilizes buildx if available and configured. +* Reduced dependency on the local Docker daemon when using buildx, enhancing security and flexibility. + +Backward Compatibility: + +All changes are designed to be backward compatible. +If buildx is not detected or used, Skaffold will fall back to the traditional Docker builder. + +This cache behavior is intended to provide transparent user experience, with similar functionality compared to traditional local docker builds, +without additional boilerplate or different configuration / patches for a CI workflow. + +## Design approach + +`docker buildx build` can be configured to execute against a [remote buildkit instance](https://docs.docker.com/build/builders/drivers/remote/). +No docker daemon is necessary for this to work, but also that is supported by default using the [docker driver](https://docs.docker.com/build/builders/drivers/docker/). +Additionally, [docker container driver](https://docs.docker.com/build/builders/drivers/docker-container) +or [kubernetes driver](https://docs.docker.com/build/builders/drivers/kubernetes/) are available too for advanced use cases. + +Since `docker build` and `docker buildx build` essentially share the same options, no major modifications are needed to Skaffold for backward compatibility. + +This proposal implements the logic to detect if buildx is the default builder (looking for an alias in the docker config). +To [set buildx as the default builder](https://github.com/docker/buildx?tab=readme-ov-file#set-buildx-as-the-default-builder), the command `docker buildx install` should be used. + +Then, additional buildx features are availables, like multi-platform support and different cache destinations. + +Cache adjustment is extended via the the `cache-tag`, useful to point to latest or a generic cache tag, instead of the generated one for this build +(via `tagPolicy` that would be useless in most distributed cases, as it can be invalidated by minor changes, specially if using `inputDigest`). + +Multi-platform images now can be built nativelly, without multiplexing the pipeline nor additional steps to stich the different images. + +### User experience + +To use BuildKit transparently, BuildX should be installed as default builder (this creates an alias in docker config): + +``` +docker buildx install +``` + +Then Skaffold should be configured to detect buildx (default builder) and set a generic cache tag: + +``` +skaffold config set -g buildx-builder default +skaffold config set -g cache-tag cache +``` + +Example basic config, this will be sufficient for many users: + +```yaml +apiVersion: skaffold/v4beta13 +build: + artifacts: + - image: my-app-image + context: my-app-image + docker: + dockerfile: Dockerfile + cacheFrom: + - "my-app-image" + cacheTo: + - "my-app-image" + local: + useBuildkit: true + useDockerCLI: true + tryImportMissing: true +``` + +* If no tag is specified for cache, the configured `cache-tag` will be used (in this example my-app-image:cache) +* If `cacheTo` destination is not specified, the `cacheFrom` image name and tag will be adjusted, adding `type=registry,mode=max` (only if push images is enabled) + +Advanced users would prefer to create buildkit instances for multiplatform images, e.g. using the docker container driver: + +``` +docker buildx create --driver docker-container --name local +skaffold config set -g buildx-builder local +``` + +Remote builds are possible, pointing to a remote instance that could be deployed in another host or container: +``` +docker buildx create --name remote --driver remote tcp://buildkitd:2375 +skaffold config set -g buildx-builder remote +``` + +### Errors + +Example for missing builder (`ERROR: no builder "defaultx" found` if skaffold was configured incorrectly): +``` +skaffold build --default-repo localhost:5000 --platform linux/arm64,linux/amd64 --cache-artifacts=false --detect-minikube=false --push +Generating tags... + - my-app -> localhost:5000/my-app:v2.11.1-121-gc703038c9-dirty +Starting build... +Building [my-app]... +Target platforms: [linux/arm64,linux/amd64] +ERROR: no builder "defaultx" found +exit status 1. Docker build ran into internal error. Please retry. +If this keeps happening, please open an issue.. +``` + +Example of improper configuration for multi-platform emulation builds: + +``` +$ /src/out/skaffold build --default-repo localhost:5000 --platform linux/arm64,linux/amd64 --cache-artifacts=false --detect-minikube=false --push +Generating tags... + - my-app -> localhost:5000/my-app:v2.11.1-122-ga0fb3239a-dirty +Starting build... +Building [my-app]... +Target platforms: [linux/arm64,linux/amd64] +#0 building with "default" instance using docker driver + +... + +#19 [linux/arm64 builder 3/3] RUN go build -o /app main.go +#19 0.639 exec /bin/sh: exec format error +#19 ERROR: process "/bin/sh -c go build -o /app main.go" did not complete successfully: exit code: 1 +------ + > [linux/arm64 builder 3/3] RUN go build -o /app main.go: +0.639 exec /bin/sh: exec format error +------ +Dockerfile:6 +-------------------- + 4 | + 5 | COPY main.go . + 6 | >>> RUN go build -o /app main.go + 7 | + 8 | FROM alpine:3 +-------------------- +ERROR: failed to solve: process "/bin/sh -c go build -o /app main.go" did not complete successfully: exit code: 1 +running build: exit status 1. To run cross-platform builds, use a proper buildx builder. To create and select it, run: + + docker buildx create --driver docker-container --name buildkit + + skaffold config set buildx-builder buildkit + +For more details, see https://docs.docker.com/build/building/multi-platform/. + +``` + +This is a corner case, as the default docker builder should not be used for multi-platform builds. +An actionable error was returned with the step-by-step instructions. + +## Implementation plan + +For the minimal viable product (initial PR): + +* Add a new global config buildx-builder to enable buildx detection and support for different builders +* Implements logic to detect buildx is the default builder (via docker config alias) +* Remove dependency on docker daemon if using BuildX (still supported to load images if using minikube or similar) +* Add a new global config cache-tag to override default cache tagging (instead of generated tag) +* Add cacheTo to sakffold schema for the cache destination (optional, default is to use new cacheFrom + cache tag) +* Add --cache-to support in Docker CLI build if using BuildX CLI +* Add multiplatform images building support for buildkit under buildx + +Additional features, error handling and examples can be implemented in the future. + +Future work: include more BuildKit advanced features support like +[multiple build contexts](https://www.docker.com/blog/dockerfiles-now-support-multiple-build-contexts/), +for an improved mono-repo experience for large code-bases. + +Other advanced featues of buildx and buildkit includes [Attestations](https://docs.docker.com/build/metadata/attestations/). +Note: default attestation is disabled for multiplatform builds, as it creates metadata with "unknown/unknown" arch/OS, +causing issues with registry tools an libraries, see [GH discussion](https://github.com/orgs/community/discussions/45969). + +## Release plan + +The buildx support could go through the release stages Alpha -> Beta -> Stable. +This will allow time for community feedback and avoid unnecessary features or rework. + +The following features would be released at each stage: + +**Alpha** + +Implement minimal changes to support buildx (initial PR): +- buildx detection +- cache-to export +- multiplatform images + +**Beta** + +- Implement additional buildx features needed by the community, like multiple context support +- Implement additional actionable errors, if needed +- Update user-facing documentation +- Implement a new buildkit basic example using buildx + +**Stable** + +- Remove custom buildkit example using custom builder +- Implement a new buildkit advanced example using buildx and remote rootless daemon for CI + +## Automated test plan + +New test cases were implemented to cover the new functionality (added to existing test suites): + +1. Unit tests for buildx, similar to docker build. + + * `TestDockerCLIBuild`: "buildkit buildx load", "buildkit buildx push" (including both buildx detection and cache-to) + +2. Integration tests, idem: + + * `TestBuild`: "docker buildx" + * `TestBuildWithWithPlatform`: "docker buildx linux/amd64", "docker buildx linux/arm64" + * `TestBuildWithMultiPlatforms`: "build multiplatform images with buildx" + +3. Add basic and comprehensive buildx examples to the `integration/examples` + directory. + +Note that for multi-platform images and advanced examples, a registry is needed to push the images. +A docker container with a local registry was implemented in the setup of integration tests. +With this approach, all buildx tests can be run locally (not needing GCP nor any other cloud resource like a remote registry). +This avoids modifications to docker daemon config (like insecure-registry exclusions), but it needs a buildkit running within the host network, as both uses localhost. + +## Credits + +This proposal is related to [#8172](https://github.com/GoogleContainerTools/skaffold/pull/8172): "Add buildx option for daemon-less BuildKit support". +Initial code was inspired by by a prior [ebekebe fork](https://github.com/ebekebe/skaffold/commit/1c1fdeb18f4d2847e65e283fba498a14745039af). + +A more general approach was implemented in this initial PR [#9648](https://github.com/GoogleContainerTools/skaffold/pull/9648), +with configurable builders, actionable errors and multi-platform support. + +This actually fixes existing issues like: +* [#5018](https://github.com/GoogleContainerTools/skaffold/issues/5018): "Docker Buildx Integration" +* [#6732](https://github.com/GoogleContainerTools/skaffold/issues/6732): "Support building securely against remote buildkitd" +* [#9197](https://github.com/GoogleContainerTools/skaffold/issues/9197): "Support additional docker buildkit options" + +The fix proposed here uses buildx nativelly, avoiding custom build scripts, like the one in the official example: +[custom-buildx](https://github.com/GoogleContainerTools/skaffold/tree/main/examples/custom-buildx) + +Future work is related to [#2110](https://github.com/GoogleContainerTools/skaffold/issues/2110): "feature request: more control over the docker build context" + diff --git a/integration/build_test.go b/integration/build_test.go index 461454fec80..ad1ebfbb339 100644 --- a/integration/build_test.go +++ b/integration/build_test.go @@ -54,6 +54,12 @@ func TestBuild(t *testing.T) { description: "docker build", dir: "testdata/build", }, + { + setup: setupBuildX, + description: "docker buildx", + args: []string{"--config", "config", "--verbosity=trace"}, + dir: "testdata/buildx", + }, { description: "git tagger", dir: "testdata/tagPolicy", @@ -132,6 +138,7 @@ func TestBuildWithWithPlatform(t *testing.T) { dir string args []string image string + setup func(t *testing.T, workdir string) expectedPlatforms []v1.Platform }{ { @@ -146,11 +153,28 @@ func TestBuildWithWithPlatform(t *testing.T) { args: []string{"--platform", "linux/arm64"}, expectedPlatforms: []v1.Platform{{OS: "linux", Architecture: "arm64"}}, }, + { + setup: setupBuildX, + description: "docker buildx linux/amd64", + dir: "testdata/buildx", + args: []string{"--platform", "linux/amd64", "--cache-artifacts=false", "--config", "config", "--verbosity=trace"}, + expectedPlatforms: []v1.Platform{{OS: "linux", Architecture: "amd64"}}, + }, + { + setup: setupBuildX, + description: "docker buildx linux/arm64", + dir: "testdata/buildx", + args: []string{"--platform", "linux/arm64", "--cache-artifacts=false", "--config", "config", "--verbosity=trace"}, + expectedPlatforms: []v1.Platform{{OS: "linux", Architecture: "arm64"}}, + }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { MarkIntegrationTest(t, CanRunWithoutGcp) + if test.setup != nil { + test.setup(t, test.dir) + } tmpfile := testutil.TempFile(t, "", []byte{}) args := append(test.args, "--file-output", tmpfile) skaffold.Build(args...).InDir(test.dir).RunOrFail(t) @@ -169,6 +193,8 @@ func TestBuildWithMultiPlatforms(t *testing.T) { dir string args []string image string + setup func(t *testing.T, workdir string) + repo string expectedPlatforms []v1.Platform }{ { @@ -177,14 +203,27 @@ func TestBuildWithMultiPlatforms(t *testing.T) { args: []string{"--platform", "linux/arm64,linux/amd64"}, expectedPlatforms: []v1.Platform{{OS: "linux", Architecture: "arm64"}, {OS: "linux", Architecture: "amd64"}}, }, + { + setup: func(t *testing.T, dir string) { setupBuildX(t, dir); setupRegistry(t, dir) }, + repo: "localhost:5000", + description: "build multiplatform images with buildx", + dir: "testdata/buildx", + args: []string{"--platform", "linux/arm64,linux/amd64", "--cache-artifacts=false", "--config", "config", "--detect-minikube=false", "--push", "--verbosity=trace"}, + expectedPlatforms: []v1.Platform{{OS: "linux", Architecture: "arm64"}, {OS: "linux", Architecture: "amd64"}}, + }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { - MarkIntegrationTest(t, NeedsGcp) + if test.setup != nil { + MarkIntegrationTest(t, CanRunWithoutGcp) + test.setup(t, test.dir) + } else { + MarkIntegrationTest(t, NeedsGcp) + } tmpfile := testutil.TempFile(t, "", []byte{}) args := append(test.args, "--file-output", tmpfile) - skaffold.Build(args...).InDir(test.dir).RunOrFail(t) + skaffold.Build(args...).WithRepo(test.repo).InDir(test.dir).RunOrFail(t) bytes, err := os.ReadFile(tmpfile) failNowIfError(t, err) buildArtifacts, err := flags.ParseBuildOutput(bytes) @@ -315,6 +354,62 @@ func setupGitRepo(t *testing.T, dir string) { } } +// setupBuildX sets up a docker buildx builder using buildkit +func setupBuildX(t *testing.T, _ string) { + t.Cleanup(func() { + dockerArgs := [][]string{ + {"buildx", "uninstall"}, + {"buildx", "rm", "buildkit"}, + } + for _, args := range dockerArgs { + cmd := exec.Command("docker", args...) + if buf, err := util.RunCmdOut(context.Background(), cmd); err != nil { + t.Log(string(buf)) + t.Fatal(err) + } + } + }) + + dockerArgs := [][]string{ + {"buildx", "install"}, + {"buildx", "create", "--driver", "docker-container", "--name", "buildkit", "--driver-opt=network=host"}, + } + for _, args := range dockerArgs { + cmd := exec.Command("docker", args...) + if buf, err := util.RunCmdOut(context.Background(), cmd); err != nil { + t.Log(string(buf)) + t.Fatal(err) + } + } +} + +// setupRegistry deploys docker registry (in localhost, to avoid insecure-registry config) +func setupRegistry(t *testing.T, _ string) { + t.Cleanup(func() { + dockerArgs := [][]string{ + {"rm", "--force", "registry"}, + } + for _, args := range dockerArgs { + cmd := exec.Command("docker", args...) + if buf, err := util.RunCmdOut(context.Background(), cmd); err != nil { + t.Log(string(buf)) + t.Fatal(err) + } + } + }) + + dockerArgs := [][]string{ + {"run", "-d", "-p", "5000:5000", "--name", "registry", "registry"}, + } + for _, args := range dockerArgs { + cmd := exec.Command("docker", args...) + if buf, err := util.RunCmdOut(context.Background(), cmd); err != nil { + t.Log(string(buf)) + t.Fatal(err) + } + } +} + // nowInChicago returns the dateTime string as generated by the dateTime tagger func nowInChicago() string { loc, _ := tz.LoadLocation("America/Chicago") diff --git a/integration/skaffold/helper.go b/integration/skaffold/helper.go index 2ac15667c77..2243e665216 100644 --- a/integration/skaffold/helper.go +++ b/integration/skaffold/helper.go @@ -170,7 +170,9 @@ func (b *RunBuilder) WithConfig(configFile string) *RunBuilder { // WithRepo sets the default repository to be used by skaffold. func (b *RunBuilder) WithRepo(repo string) *RunBuilder { - b.repo = repo + if repo != "" { + b.repo = repo + } return b } diff --git a/integration/testdata/buildx/Dockerfile b/integration/testdata/buildx/Dockerfile new file mode 100644 index 00000000000..970447876b9 --- /dev/null +++ b/integration/testdata/buildx/Dockerfile @@ -0,0 +1,12 @@ +# syntax=docker/dockerfile:1 + +FROM golang:1.23-alpine AS builder + +COPY main.go . +RUN go build -o /app main.go + +FROM alpine:3 + +COPY --from=builder /app . + +CMD ["/app"] diff --git a/integration/testdata/buildx/config b/integration/testdata/buildx/config new file mode 100644 index 00000000000..b5e8d04f2c8 --- /dev/null +++ b/integration/testdata/buildx/config @@ -0,0 +1,11 @@ +global: + cache-tag: cache + buildx-builder: buildkit + survey: + disable-prompt: true + last-prompted: "2025-01-19T21:32:47Z" + collect-metrics: false + update-check: false + update: + last-prompted: "2025-01-19T23:10:47Z" +kubeContexts: [] diff --git a/integration/testdata/buildx/main.go b/integration/testdata/buildx/main.go new file mode 100644 index 00000000000..8c3e45267d6 --- /dev/null +++ b/integration/testdata/buildx/main.go @@ -0,0 +1,14 @@ +package main + +import ( + "fmt" + "time" +) + +func main() { + for counter := 0; ; counter++ { + fmt.Println("Hello world!", counter) + + time.Sleep(time.Second * 1) + } +} diff --git a/integration/testdata/buildx/skaffold.yaml b/integration/testdata/buildx/skaffold.yaml new file mode 100644 index 00000000000..bbd8f22a5d2 --- /dev/null +++ b/integration/testdata/buildx/skaffold.yaml @@ -0,0 +1,12 @@ +apiVersion: skaffold/v4beta12 +kind: Config +build: + artifacts: + - image: my-app + docker: + dockerfile: Dockerfile + local: + useBuildkit: true + useDockerCLI: true + tryImportMissing: true + push: false diff --git a/pkg/skaffold/build/builder_mux.go b/pkg/skaffold/build/builder_mux.go index 2985ecce973..48243c43566 100644 --- a/pkg/skaffold/build/builder_mux.go +++ b/pkg/skaffold/build/builder_mux.go @@ -38,6 +38,7 @@ type BuilderMux struct { byImageName map[string]PipelineBuilder store ArtifactStore concurrency int + buildx bool cache Cache } @@ -71,7 +72,8 @@ func NewBuilderMux(cfg Config, store ArtifactStore, cache Cache, builder func(p } } concurrency := getConcurrency(pbs, cfg.BuildConcurrency()) - return &BuilderMux{builders: pbs, byImageName: m, store: store, concurrency: concurrency, cache: cache}, nil + buildx := config.GetDetectBuildX(cfg.GlobalConfig()) + return &BuilderMux{builders: pbs, byImageName: m, store: store, concurrency: concurrency, cache: cache, buildx: buildx}, nil } // Build executes the specific image builder for each artifact in the given artifact slice. @@ -106,7 +108,8 @@ func (b *BuilderMux) Build(ctx context.Context, out io.Writer, tags tag.ImageTag } var built string - if platforms.IsMultiPlatform() && !SupportsMultiPlatformBuild(*artifact) { + // buildx creates multiplatform images via buildkit directly + if platforms.IsMultiPlatform() && !SupportsMultiPlatformBuild(*artifact) && !b.buildx { built, err = CreateMultiPlatformImage(ctx, out, artifact, tag, platforms, artifactBuilder) } else { built, err = artifactBuilder(ctx, out, artifact, tag, platforms) diff --git a/pkg/skaffold/build/cache/cache.go b/pkg/skaffold/build/cache/cache.go index ca32e87ccbc..b929f6f8b92 100644 --- a/pkg/skaffold/build/cache/cache.go +++ b/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() && docker.IsBuildXDetected() + 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 } diff --git a/pkg/skaffold/build/cache/lookup.go b/pkg/skaffold/build/cache/lookup.go index aa84c43a70d..2866a7c5a61 100644 --- a/pkg/skaffold/build/cache/lookup.go +++ b/pkg/skaffold/build/cache/lookup.go @@ -88,6 +88,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 %v", err) return failed{err} } else if isLocal { return c.lookupLocal(ctx, hash, tag, entry) @@ -123,9 +124,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 %v", err) } // Image exists remotely with a different tag @@ -153,23 +157,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: %v", 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 { diff --git a/pkg/skaffold/build/docker/docker.go b/pkg/skaffold/build/docker/docker.go index 9e9f826435d..6fa66ac7005 100644 --- a/pkg/skaffold/build/docker/docker.go +++ b/pkg/skaffold/build/docker/docker.go @@ -19,13 +19,16 @@ package docker import ( "bytes" "context" + "encoding/json" "fmt" "io" "os" "os/exec" + "strings" 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" @@ -42,11 +45,15 @@ func (b *Builder) SupportedPlatforms() platform.Matcher { } func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact, tag string, matcher platform.Matcher) (string, error) { - var pl v1.Platform - if len(matcher.Platforms) == 1 { - pl = util.ConvertToV1Platform(matcher.Platforms[0]) + var pls []v1.Platform + if len(matcher.Platforms) > 0 { + for _, plat := range matcher.Platforms { + pls = append(pls, util.ConvertToV1Platform(plat)) + } + } else { + pls = append(pls, v1.Platform{}) } - a = adjustCacheFrom(a, tag) + a = b.adjustCache(ctx, a, tag) instrumentation.AddAttributesToCurrentSpanFromContext(ctx, map[string]string{ "BuildType": "docker", "Context": instrumentation.PII(a.Workspace), @@ -62,8 +69,10 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact, return "", dockerfileNotFound(err, a.ImageName) } - if err := b.pullCacheFromImages(ctx, out, a.ArtifactType.DockerArtifact, pl); err != nil { - return "", cacheFromPullErr(err, a.ImageName) + for _, pl := range pls { + if err := b.pullCacheFromImages(ctx, out, a.ArtifactType.DockerArtifact, pl); err != nil { + return "", cacheFromPullErr(err, a.ImageName) + } } opts := docker.BuildOptions{Tag: tag, Mode: b.cfg.Mode(), ExtraBuildArgs: docker.ResolveDependencyImages(a.Dependencies, b.artifacts, true)} @@ -73,7 +82,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact, // 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.IsCrossPlatform() { - imageID, err = b.dockerCLIBuild(ctx, output.GetUnderlyingWriter(out), a.ImageName, a.Workspace, dockerfile, a.ArtifactType.DockerArtifact, opts, pl) + imageID, err = b.dockerCLIBuild(ctx, output.GetUnderlyingWriter(out), a.ImageName, a.Workspace, dockerfile, a.ArtifactType.DockerArtifact, opts, pls) } else { imageID, err = b.localDocker.Build(ctx, out, a.Workspace, a.ImageName, a.ArtifactType.DockerArtifact, opts) } @@ -82,7 +91,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 && !b.buildx { // 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) @@ -91,7 +100,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact, return imageID, nil } -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) { +func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string, workspace string, dockerfilePath string, a *latest.DockerArtifact, opts docker.BuildOptions, pls []v1.Platform) (string, error) { args := []string{"build", workspace, "--file", dockerfilePath, "-t", opts.Tag} imageInfoEnv, err := docker.EnvTags(opts.Tag) if err != nil { @@ -111,14 +120,47 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string args = append(args, "--force-rm") } - if pl.String() != "" { - args = append(args, "--platform", pl.String()) + var platforms []string + for _, pl := range pls { + if pl.String() != "" { + platforms = append(platforms, pl.String()) + } + } + if len(platforms) > 0 { + args = append(args, "--platform", strings.Join(platforms, ",")) } - 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") + } + } + + if b.buildx { + args = append(args, "--builder", config.GetBuildXBuilder(b.cfg.GlobalConfig())) } + // temporary file for buildx metadata containing the image digest: + var metadata string + if b.buildx { + metadata, err = getBuildxMetadataFile() + if err != nil { + return "", fmt.Errorf("unable to create temp file: %w", err) + } + defer os.Remove(metadata) + args = append(args, "--metadata-file", metadata) + } cmd := exec.CommandContext(ctx, "docker", args...) cmd.Env = append(util.OSEnviron(), b.localDocker.ExtraEnv()...) if b.useBuildKit != nil { @@ -127,10 +169,15 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string } else { cmd.Env = append(cmd.Env, "DOCKER_BUILDKIT=0") } - } else if pl.String() != "" { // cross-platform builds require buildkit - log.Entry(ctx).Debugf("setting DOCKER_BUILDKIT=1 for docker build for artifact %q since it targets platform %q", name, pl.String()) + } else if len(platforms) > 0 { // cross-platform builds require buildkit + log.Entry(ctx).Debugf("setting DOCKER_BUILDKIT=1 for docker build for artifact %q since it targets platform %q", name, platforms[0]) cmd.Env = append(cmd.Env, "DOCKER_BUILDKIT=1") } + if len(platforms) > 1 && b.buildx { + // avoid "unknown/unknown" architecture/OS caused by buildx default image attestation + log.Entry(ctx).Warnf("setting BUILDX_NO_DEFAULT_ATTESTATIONS=1 for docker buildx for artifact %q since it targets platform %q to avoid unknown/unknown platform issue", name, platforms[0]) + cmd.Env = append(cmd.Env, "BUILDX_NO_DEFAULT_ATTESTATIONS=1") + } cmd.Stdout = out var errBuffer bytes.Buffer @@ -138,14 +185,24 @@ func (b *Builder) dockerCLIBuild(ctx context.Context, out io.Writer, name string cmd.Stderr = stderr if err := util.RunCmd(ctx, cmd); err != nil { - return "", tryExecFormatErr(fmt.Errorf("running build: %w", err), errBuffer) + if !b.buildx { + err = tryExecFormatErr(fmt.Errorf("running build: %w", err), errBuffer) + } else { + err = tryExecFormatErrBuildX(fmt.Errorf("running build: %w", err), errBuffer) + } + return "", err } - return b.localDocker.ImageID(ctx, opts.Tag) + if !b.buildx { + return b.localDocker.ImageID(ctx, opts.Tag) + } else { + return parseBuildxMetadataFile(ctx, metadata) + } } 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 } @@ -167,26 +224,97 @@ 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: %v", err) + } else if tag != "" { + cacheTag = fmt.Sprintf("%s:%s", imgRef.BaseName, 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 © } + +// osCreateTemp allows for replacing metadata for testing purposes +var osCreateTemp = os.CreateTemp + +func getBuildxMetadataFile() (string, error) { + metadata, err := osCreateTemp("", "metadata*.json") + if err != nil { + return "", err + } + metadata.Close() + return metadata.Name(), nil +} + +func parseBuildxMetadataFile(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.config.digest"]; value != nil { + // image loaded to local docker daemon + digest = value.(string) + } else if value := metadata["containerimage.digest"]; value != nil { + // image pushed to registry + 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: %v", 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 +} diff --git a/pkg/skaffold/build/docker/docker_test.go b/pkg/skaffold/build/docker/docker_test.go index 509e3a1804f..05f3541810b 100644 --- a/pkg/skaffold/build/docker/docker_test.go +++ b/pkg/skaffold/build/docker/docker_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "os" "path/filepath" "strings" "testing" @@ -38,6 +39,13 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/testutil" ) +var metadata = `{ + "buildx.build.ref": "default/default/8tmdcf9mpy43arexbwej851qz", + "containerimage.config.digest": "sha256:0bb1817818c840e89daa8308168ac33d0d857faed1f6810638cc26169b8d3b13", + "containerimage.digest": "sha256:0bb1817818c840e77afa8308168ac33d0d23adaee1f6810638cc26169b8d3b13", + "image.name": "docker.io/library/test" +}` + func TestDockerCLIBuild(t *testing.T) { tests := []struct { description string @@ -51,6 +59,8 @@ func TestDockerCLIBuild(t *testing.T) { expectedCLIFlags []string // CLI flags expected to be autogenerated. expectedErr error wantDockerCLI bool + buildx bool // buildx detected + daemonless bool // no docker daemon expectedErrCode proto.StatusCode }{ { @@ -79,6 +89,26 @@ func TestDockerCLIBuild(t *testing.T) { expectedCLIFlags: []string{"--load"}, expectedEnv: []string{"KEY=VALUE", "DOCKER_BUILDKIT=1"}, }, + { + description: "buildkit buildx load", + localBuild: latest.LocalBuild{UseBuildkit: util.Ptr(true)}, + wantDockerCLI: true, + buildx: true, + daemonless: false, + imageName: "gcr.io/k8s-skaffold/example:tag", + expectedCLIFlags: []string{"--cache-from", "gcr.io/k8s-skaffold/example:cache", "--load", "--builder", "default", "--metadata-file", "metadata.json"}, + expectedEnv: []string{"KEY=VALUE", "DOCKER_BUILDKIT=1"}, + }, + { + description: "buildkit buildx push", + localBuild: latest.LocalBuild{UseBuildkit: util.Ptr(true), Push: util.Ptr(true)}, + wantDockerCLI: true, + buildx: true, + daemonless: true, + imageName: "gcr.io/k8s-skaffold/example:tag", + expectedCLIFlags: []string{"--cache-from", "gcr.io/k8s-skaffold/example:cache", "--cache-to", "type=registry,ref=gcr.io/k8s-skaffold/example:cache,mode=max", "--push", "--builder", "default", "--metadata-file", "metadata.json"}, + expectedEnv: []string{"KEY=VALUE", "DOCKER_BUILDKIT=1"}, + }, { description: "cliFlags", cliFlags: []string{"--platform", "linux/amd64"}, @@ -159,6 +189,14 @@ func TestDockerCLIBuild(t *testing.T) { return args, nil }) t.Override(&docker.DefaultAuthHelper, stubAuth{}) + t.Override(&osCreateTemp, func(dir, pattern string) (*os.File, error) { + tmp := t.TempFile("metadata*.json", []byte(metadata)) + os.Symlink(tmp, "metadata.json") // rename doesn't work on windows (file is still open) + return os.Open("metadata.json") + }) + t.Override(&config.GetConfigForCurrentKubectx, func(configFile string) (*config.ContextConfig, error) { + return &config.ContextConfig{CacheTag: "cache", BuildXBuilder: "default"}, nil + }) var mockCmd *testutil.FakeCmd imageName := "tag" @@ -187,7 +225,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.daemonless), test.cfg, test.localBuild.UseDockerCLI, test.localBuild.UseBuildkit, test.buildx, test.buildx && test.localBuild.Push != nil && *test.localBuild.Push, mockArtifactResolver{make(map[string]string)}, nil) artifact := &latest.Artifact{ Workspace: ".", @@ -198,9 +236,15 @@ func TestDockerCLIBuild(t *testing.T) { }, }, } + if test.buildx { + parts := strings.Split(imageName, ":") + artifact.ImageName = parts[0] + artifact.DockerArtifact.CacheFrom = []string{parts[0]} + } - _, err := builder.Build(context.Background(), io.Discard, artifact, imageName, platform.Matcher{}) + digest, err := builder.Build(context.Background(), io.Discard, artifact, imageName, platform.Matcher{}) t.CheckError(test.err != nil, err) + t.CheckTrue(digest != "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") // not empty string hash if mockCmd != nil { t.CheckDeepEqual(1, mockCmd.TimesCalled()) } @@ -268,15 +312,15 @@ 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{}, false), mockConfig{}, true, util.Ptr(false), false, false, mockArtifactResolver{make(map[string]string)}, nil) _, err := builder.Build(context.Background(), io.Discard, &a, test.tag, platform.Matcher{}) t.CheckNoError(err) }) } } -func fakeLocalDaemonWithExtraEnv(extraEnv []string) docker.LocalDaemon { - return docker.NewLocalDaemon(&testutil.FakeAPIClient{}, extraEnv, false, nil) +func fakeLocalDaemonWithExtraEnv(extraEnv []string, daemonless bool) docker.LocalDaemon { + return docker.NewLocalDaemon(&testutil.FakeAPIClient{ErrVersion: daemonless}, extraEnv, false, nil) } type mockArtifactResolver struct { diff --git a/pkg/skaffold/build/docker/errors.go b/pkg/skaffold/build/docker/errors.go index 54737df8909..74d4d525b99 100644 --- a/pkg/skaffold/build/docker/errors.go +++ b/pkg/skaffold/build/docker/errors.go @@ -170,3 +170,20 @@ func tryExecFormatErr(err error, stdErr bytes.Buffer) error { }, }) } + +func tryExecFormatErrBuildX(err error, stdErr bytes.Buffer) error { + if !execFormatErr.MatchString(stdErr.String()) { + return err + } + return sErrors.NewError(err, + &proto.ActionableErr{ + Message: err.Error(), + ErrCode: proto.StatusCode_BUILD_CROSS_PLATFORM_ERR, + Suggestions: []*proto.Suggestion{ + { + SuggestionCode: proto.SuggestionCode_BUILD_INSTALL_PLATFORM_EMULATORS, + Action: "To run cross-platform builds, use a proper buildx builder. To create and select it, run:\n\n\tdocker buildx create --driver docker-container --name buildkit\n\n\tskaffold config set buildx-builder buildkit\n\nFor more details, see https://docs.docker.com/build/building/multi-platform/", + }, + }, + }) +} diff --git a/pkg/skaffold/build/docker/errors_test.go b/pkg/skaffold/build/docker/errors_test.go index 32686d38117..c6ece2c2358 100644 --- a/pkg/skaffold/build/docker/errors_test.go +++ b/pkg/skaffold/build/docker/errors_test.go @@ -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{}, false), mockConfig{}, true, nil, false, false, mockArtifactResolver{make(map[string]string)}, nil) artifact := &latest.Artifact{ ImageName: "test-image", diff --git a/pkg/skaffold/build/docker/types.go b/pkg/skaffold/build/docker/types.go index 00ad02ff680..68ad789f6d2 100644 --- a/pkg/skaffold/build/docker/types.go +++ b/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, } diff --git a/pkg/skaffold/build/local/local.go b/pkg/skaffold/build/local/local.go index 04412a55b2d..ef6be53700f 100644 --- a/pkg/skaffold/build/local/local.go +++ b/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}) @@ -130,8 +133,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 diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go index fef419ae43a..89ddf0f8b7e 100644 --- a/pkg/skaffold/build/local/types.go +++ b/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() && docker.IsBuildXDetected() + 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 diff --git a/pkg/skaffold/config/global_config.go b/pkg/skaffold/config/global_config.go index a0dffaaae6f..872e21b222e 100644 --- a/pkg/skaffold/config/global_config.go +++ b/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"` + BuildXBuilder string `yaml:"buildx-builder,omitempty"` UpdateCheck *bool `yaml:"update-check,omitempty"` Survey *SurveyConfig `yaml:"survey,omitempty"` KindDisableLoad *bool `yaml:"kind-disable-load,omitempty"` diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index 3f4bd849c0b..3f6bdff71dd 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -217,6 +217,33 @@ func GetDebugHelpersRegistry(configFile string) (string, error) { return constants.DefaultDebugHelpersRegistry, nil } +func GetCacheTag(configFile string) (string, error) { + cfg, err := GetConfigForCurrentKubectx(configFile) + if err != nil { + log.Entry(context.TODO()).Errorf("Cannot read cache-tag from config: %v", err) + return "", err + } + if cfg.CacheTag != "" { + log.Entry(context.TODO()).Infof("Using cache-tag=%s from config", cfg.CacheTag) + } + return cfg.CacheTag, nil +} + +func GetBuildXBuilder(configFile string) string { + cfg, err := GetConfigForCurrentKubectx(configFile) + if err != nil { + log.Entry(context.TODO()).Errorf("Cannot read buildx-builder option from config: %v", err) + } else if cfg.BuildXBuilder != "" { + log.Entry(context.TODO()).Infof("Using buildx-builder=%s from config", cfg.BuildXBuilder) + return cfg.BuildXBuilder + } + return "" +} + +func GetDetectBuildX(configFile string) bool { + return GetBuildXBuilder(configFile) != "" +} + type GetClusterOpts struct { ConfigFile string DefaultRepo StringOrUndefined diff --git a/pkg/skaffold/docker/auth.go b/pkg/skaffold/docker/auth.go index f2f57745dcf..8b00e8f593a 100644 --- a/pkg/skaffold/docker/auth.go +++ b/pkg/skaffold/docker/auth.go @@ -64,14 +64,30 @@ type AuthConfigHelper interface { type credsHelper struct{} -func loadDockerConfig() (*configfile.ConfigFile, error) { +func LoadDockerConfig(access bool) (*configfile.ConfigFile, error) { cf, err := config.Load(configDir) + if err == nil && access { + // proper check of config file to detect permissions errors + if _, err = os.Stat(cf.Filename); err == nil { + var file *os.File + file, err = os.Open(cf.Filename) + if err == nil { + defer file.Close() + } + } + if err != nil { + log.Entry(context.TODO()).Warnf("cannot access docker config file %s: %v", configDir, err) + } + } + return cf, err +} + +func loadDockerConfig() (*configfile.ConfigFile, error) { + cf, err := LoadDockerConfig(false) if err != nil { return nil, fmt.Errorf("docker config: %w", err) } - gcp.AutoConfigureGCRCredentialHelper(cf) - return cf, nil } diff --git a/pkg/skaffold/docker/buildx.go b/pkg/skaffold/docker/buildx.go new file mode 100644 index 00000000000..784cc9f65ce --- /dev/null +++ b/pkg/skaffold/docker/buildx.go @@ -0,0 +1,27 @@ +/* +Copyright 2019 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package docker + +func IsBuildXDetected() bool { + // detect if buildx was installed (docker build then is an alias for docker buildx build): + // https://github.com/docker/buildx/blob/master/commands/install.go + cf, err := LoadDockerConfig(true) + if err == nil && cf.Aliases != nil { + return cf.Aliases["builder"] == "buildx" + } + return false +} diff --git a/pkg/skaffold/docker/image.go b/pkg/skaffold/docker/image.go index 1ac35b86b5d..46f7fa89c42 100644 --- a/pkg/skaffold/docker/image.go +++ b/pkg/skaffold/docker/image.go @@ -648,6 +648,10 @@ func ToCLIBuildArgs(a *latest.DockerArtifact, evaluatedArgs map[string]*string, args = append(args, "--cache-from", from) } + for _, to := range a.CacheTo { + args = append(args, "--cache-to", to) + } + for _, cliFlag := range a.CliFlags { cliFlag, err := util.ExpandEnvTemplate(cliFlag, env) if err != nil { diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index 76d2c84f35d..8baf37b1097 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -365,6 +365,16 @@ func (rc *RunContext) EnableGKEARMNodeTolerationInRenderedManifests() bool { return rc.Opts.EnableGKEARMNodeToleration } +func (rc *RunContext) DetectBuildX() bool { + if config.GetDetectBuildX(rc.GlobalConfig()) { + log.Entry(context.TODO()).Debugf("buildx detection is enabled") + return true + } else { + log.Entry(context.TODO()).Debugf("buildx detection is disabled") + return false + } +} + func (rc *RunContext) DigestSource() string { if rc.Opts.DigestSource != "" { return rc.Opts.DigestSource diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go index ec0c8b43ac5..2038ee14f03 100644 --- a/pkg/skaffold/schema/latest/config.go +++ b/pkg/skaffold/schema/latest/config.go @@ -1604,6 +1604,11 @@ type DockerArtifact struct { // For example: `["golang:1.10.1-alpine3.7", "alpine:3.7"]`. CacheFrom []string `yaml:"cacheFrom,omitempty"` + // CacheTo lists the Docker images used as cache destination. + // If omitted, cacheFrom is used with max mode to export all layers. + // For example: `["type=registry,ref=gcr.io/k8s-skaffold/example:cache,mode=max"]`. + CacheTo []string `yaml:"cacheTo,omitempty"` + // CliFlags are any additional flags to pass to the local daemon during a build. // These flags are only used during a build through the Docker CLI. CliFlags []string `yaml:"cliFlags,omitempty"`