From f4728b4cf2cddceb3a4c13578146350a5f23159e Mon Sep 17 00:00:00 2001 From: TinaMor Date: Mon, 28 Oct 2024 23:16:12 +0300 Subject: [PATCH 1/2] [Windows] nerdctl build - Replace default type=docker with type=image - Use default image name only when user has not specified one Signed-off-by: Christine Murimi --- cmd/nerdctl/builder/builder_build_test.go | 2 +- docs/command-reference.md | 5 +- pkg/cmd/builder/build.go | 97 +++++++++++++++-------- 3 files changed, 68 insertions(+), 36 deletions(-) diff --git a/cmd/nerdctl/builder/builder_build_test.go b/cmd/nerdctl/builder/builder_build_test.go index e3d736f9435..ec65e90cc4e 100644 --- a/cmd/nerdctl/builder/builder_build_test.go +++ b/cmd/nerdctl/builder/builder_build_test.go @@ -81,7 +81,7 @@ CMD ["echo", "nerdctl-build-test-string"]`, testutil.CommonImage) helpers.Ensure("build", data.Get("buildCtx"), "-t", data.Identifier(), "--output=type=docker,name="+data.Identifier("ignored")) }, Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - return helpers.Command("run", "--rm", data.Identifier()) + return helpers.Command("run", "--rm", data.Identifier("ignored")) }, Cleanup: func(data test.Data, helpers test.Helpers) { helpers.Anyhow("rmi", "-f", data.Identifier()) diff --git a/docs/command-reference.md b/docs/command-reference.md index 95dfddcfd45..bfca5f79c55 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -691,7 +691,10 @@ Flags: - :whale: `--target`: Set the target build stage to build - :whale: `--build-arg`: Set build-time variables - :whale: `--no-cache`: Do not use cache when building the image -- :whale: `--output=OUTPUT`: Output destination (format: type=local,dest=path) +- :whale: `--output=OUTPUT`: Output destination (format: type=local,dest=path). See [buildctl output](https://github.com/moby/buildkit?tab=readme-ov-file#output) option for more details. + + If this flag is not specified, it defaults to `type=image,name=docker.io/library/:latest`. + - :whale: `type=local,dest=path/to/output-dir`: Local directory - :whale: `type=oci[,dest=path/to/output.tar]`: Docker/OCI dual-format tar ball (compatible with `docker buildx build`) - :whale: `type=docker[,dest=path/to/output.tar]`: Docker format tar ball (compatible with `docker buildx build`) diff --git a/pkg/cmd/builder/build.go b/pkg/cmd/builder/build.go index 9d1f0f7b05f..72c45cf6f8d 100644 --- a/pkg/cmd/builder/build.go +++ b/pkg/cmd/builder/build.go @@ -61,18 +61,22 @@ func (p platformParser) DefaultSpec() platforms.Platform { } func Build(ctx context.Context, client *containerd.Client, options types.BuilderBuildOptions) error { - buildctlBinary, buildctlArgs, needsLoading, metaFile, tags, cleanup, err := generateBuildctlArgs(ctx, client, options) + buildCtlArgs, err := generateBuildctlArgs(ctx, client, options) if err != nil { return err } - if cleanup != nil { - defer cleanup() + if buildCtlArgs.Cleanup != nil { + defer buildCtlArgs.Cleanup() } + buildctlBinary := buildCtlArgs.BuildctlBinary + buildctlArgs := buildCtlArgs.BuildctlArgs + log.L.Debugf("running %s %v", buildctlBinary, buildctlArgs) buildctlCmd := exec.Command(buildctlBinary, buildctlArgs...) buildctlCmd.Env = os.Environ() + needsLoading := buildCtlArgs.NeedsLoading var buildctlStdout io.Reader if needsLoading { buildctlStdout, err = buildctlCmd.StdoutPipe() @@ -95,6 +99,8 @@ func Build(ctx context.Context, client *containerd.Client, options types.Builder if err != nil { return err } + + // Load the image into the containerd image store if err = loadImage(ctx, buildctlStdout, options.GOptions.Namespace, options.GOptions.Address, options.GOptions.Snapshotter, options.Stdout, platMC, options.Quiet); err != nil { return err } @@ -105,7 +111,7 @@ func Build(ctx context.Context, client *containerd.Client, options types.Builder } if options.IidFile != "" { - id, err := getDigestFromMetaFile(metaFile) + id, err := getDigestFromMetaFile(buildCtlArgs.MetaFile) if err != nil { return err } @@ -114,6 +120,7 @@ func Build(ctx context.Context, client *containerd.Client, options types.Builder } } + tags := buildCtlArgs.Tags if len(tags) > 1 { log.L.Debug("Found more than 1 tag") imageService := client.ImageService() @@ -160,11 +167,15 @@ func loadImage(ctx context.Context, in io.Reader, namespace, address, snapshotte client.Close() }() r := &readCounter{Reader: in} - imgs, err := client.Import(ctx, r, containerd.WithDigestRef(archive.DigestTranslator(snapshotter)), containerd.WithSkipDigestRef(func(name string) bool { return name != "" }), containerd.WithImportPlatform(platMC)) + imgs, err := client.Import(ctx, r, + containerd.WithDigestRef(archive.DigestTranslator(snapshotter)), + containerd.WithSkipDigestRef(func(name string) bool { return name != "" }), + containerd.WithImportPlatform(platMC), + ) if err != nil { if r.N == 0 { // Avoid confusing "unrecognized image format" - return errors.New("no image was built") + return fmt.Errorf("no image was built: %w", err) } if errors.Is(err, images.ErrEmptyWalk) { err = fmt.Errorf("%w (Hint: set `--platform=PLATFORM` or `--all-platforms`)", err) @@ -192,34 +203,44 @@ func loadImage(ctx context.Context, in io.Reader, namespace, address, snapshotte return nil } -func generateBuildctlArgs(ctx context.Context, client *containerd.Client, options types.BuilderBuildOptions) (buildCtlBinary string, - buildctlArgs []string, needsLoading bool, metaFile string, tags []string, cleanup func(), err error) { +type BuildctlArgsResult struct { + BuildctlArgs []string + BuildctlBinary string + Cleanup func() + DestFile string + MetaFile string + NeedsLoading bool // Specifies whether the image needs to be loaded into the containerd image store + Tags []string +} +func generateBuildctlArgs(ctx context.Context, client *containerd.Client, options types.BuilderBuildOptions) (result BuildctlArgsResult, err error) { buildctlBinary, err := buildkitutil.BuildctlBinary() if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } + result.BuildctlBinary = buildctlBinary output := options.Output if output == "" { info, err := client.Server(ctx) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } sharable, err := isImageSharable(options.BuildKitHost, options.GOptions.Namespace, info.UUID, options.GOptions.Snapshotter, options.Platform) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } if sharable { output = "type=image,unpack=true" // ensure the target stage is unlazied (needed for any snapshotters) } else { - output = "type=docker" + // https://github.com/moby/buildkit?tab=readme-ov-file#output + // type=image is the native type for containerd + output = "type=image" if len(options.Platform) > 1 { // For avoiding `error: failed to solve: docker exporter does not currently support exporting manifest lists` // TODO: consider using type=oci for single-options.Platform build too output = "type=oci" } - needsLoading = true } } else { if !strings.Contains(output, "type=") { @@ -227,34 +248,40 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option // type=local,dest= output = fmt.Sprintf("type=local,dest=%s", output) } - if strings.Contains(output, "type=docker") || strings.Contains(output, "type=oci") { - if !strings.Contains(output, "dest=") { - needsLoading = true - } + } + + if strings.Contains(output, "type=docker") || strings.Contains(output, "type=oci") { + if !strings.Contains(output, "dest=") { + result.NeedsLoading = true } } + + var tags []string if tags = strutil.DedupeStrSlice(options.Tag); len(tags) > 0 { ref := tags[0] parsedReference, err := referenceutil.Parse(ref) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err + } + // Update the output with the the image name if it is not already set + if !strings.Contains(output, "name=") { + output += ",name=" + parsedReference.String() } - output += ",name=" + parsedReference.String() // pick the first tag and add it to output for idx, tag := range tags { parsedReference, err = referenceutil.Parse(tag) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } tags[idx] = parsedReference.String() } } else if len(tags) == 0 { output = output + ",dangling-name-prefix=" } + result.Tags = tags - buildctlArgs = buildkitutil.BuildctlBaseArgs(options.BuildKitHost) - + buildctlArgs := buildkitutil.BuildctlBaseArgs(options.BuildKitHost) buildctlArgs = append(buildctlArgs, []string{ "build", "--progress=" + options.Progress, @@ -271,9 +298,9 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option var err error dir, err = buildkitutil.WriteTempDockerfile(options.Stdin) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } - cleanup = func() { + result.Cleanup = func() { os.RemoveAll(dir) } } else { @@ -286,12 +313,12 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option } dir, file, err = buildkitutil.BuildKitFile(dir, file) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } buildCtx, err := parseContextNames(options.ExtendedBuildContext) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } for k, v := range buildCtx { @@ -306,7 +333,7 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option if isOCILayout := strings.HasPrefix(v, "oci-layout://"); isOCILayout { args, err := parseBuildContextFromOCILayout(k, v) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } buildctlArgs = append(buildctlArgs, args...) @@ -315,7 +342,7 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option path, err := filepath.Abs(v) if err != nil { - return "", nil, false, "", nil, nil, err + return result, err } buildctlArgs = append(buildctlArgs, fmt.Sprintf("--local=%s=%s", k, path)) buildctlArgs = append(buildctlArgs, fmt.Sprintf("--opt=context:%s=local:%s", k, k)) @@ -362,7 +389,7 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option } } } else { - return "", nil, false, "", nil, nil, fmt.Errorf("invalid build arg %q", ba) + return result, fmt.Errorf("invalid build arg %q", ba) } } @@ -405,7 +432,7 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option optAttestType := strings.TrimPrefix(optAttestType, "type=") buildctlArgs = append(buildctlArgs, fmt.Sprintf("--opt=attest:%s=%s", optAttestType, optAttestAttrs)) } else { - return "", nil, false, "", nil, nil, fmt.Errorf("attestation type not specified") + return result, fmt.Errorf("attestation type not specified") } } @@ -434,11 +461,11 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option if options.IidFile != "" { file, err := os.CreateTemp("", "buildkit-meta-*") if err != nil { - return "", nil, false, "", nil, cleanup, err + return result, err } defer file.Close() - metaFile = file.Name() - buildctlArgs = append(buildctlArgs, "--metadata-file="+metaFile) + result.MetaFile = file.Name() + buildctlArgs = append(buildctlArgs, "--metadata-file="+result.MetaFile) } if options.NetworkMode != "" { @@ -453,7 +480,9 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option } } - return buildctlBinary, buildctlArgs, needsLoading, metaFile, tags, cleanup, nil + result.BuildctlArgs = buildctlArgs + + return result, nil } func getDigestFromMetaFile(path string) (string, error) { From 0a72eddf42f830dd60740573c6615f0fcf206f6d Mon Sep 17 00:00:00 2001 From: TinaMor Date: Mon, 18 Nov 2024 18:46:08 +0300 Subject: [PATCH 2/2] Fix failing test --- cmd/nerdctl/builder/builder_build_test.go | 14 +++++++------- pkg/cmd/builder/build.go | 5 +---- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/cmd/nerdctl/builder/builder_build_test.go b/cmd/nerdctl/builder/builder_build_test.go index ec65e90cc4e..e7d2571a333 100644 --- a/cmd/nerdctl/builder/builder_build_test.go +++ b/cmd/nerdctl/builder/builder_build_test.go @@ -76,12 +76,12 @@ CMD ["echo", "nerdctl-build-test-string"]`, testutil.CommonImage) Expected: test.Expects(0, nil, test.Equals("nerdctl-build-test-string\n")), }, { - Description: "Successfully build with output docker, main tag still works", + Description: "Successfully build with output image, main tag still works", Setup: func(data test.Data, helpers test.Helpers) { - helpers.Ensure("build", data.Get("buildCtx"), "-t", data.Identifier(), "--output=type=docker,name="+data.Identifier("ignored")) + helpers.Ensure("build", data.Get("buildCtx"), "-t", data.Identifier(), "--output=type=image,name="+data.Identifier("ignored")) }, Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - return helpers.Command("run", "--rm", data.Identifier("ignored")) + return helpers.Command("run", "--rm", data.Identifier()) }, Cleanup: func(data test.Data, helpers test.Helpers) { helpers.Anyhow("rmi", "-f", data.Identifier()) @@ -89,17 +89,17 @@ CMD ["echo", "nerdctl-build-test-string"]`, testutil.CommonImage) Expected: test.Expects(0, nil, test.Equals("nerdctl-build-test-string\n")), }, { - Description: "Successfully build with output docker, name cannot be used", + Description: "Successfully build with output docker, named image", Setup: func(data test.Data, helpers test.Helpers) { - helpers.Ensure("build", data.Get("buildCtx"), "-t", data.Identifier(), "--output=type=docker,name="+data.Identifier("ignored")) + helpers.Ensure("build", data.Get("buildCtx"), "--output=type=docker,name="+data.Identifier()) }, Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { - return helpers.Command("run", "--rm", data.Identifier("ignored")) + return helpers.Command("run", "--rm", data.Identifier()) }, Cleanup: func(data test.Data, helpers test.Helpers) { helpers.Anyhow("rmi", "-f", data.Identifier()) }, - Expected: test.Expects(-1, nil, nil), + Expected: test.Expects(0, nil, test.Equals("nerdctl-build-test-string\n")), }, }, } diff --git a/pkg/cmd/builder/build.go b/pkg/cmd/builder/build.go index 72c45cf6f8d..2888b642e58 100644 --- a/pkg/cmd/builder/build.go +++ b/pkg/cmd/builder/build.go @@ -263,10 +263,7 @@ func generateBuildctlArgs(ctx context.Context, client *containerd.Client, option if err != nil { return result, err } - // Update the output with the the image name if it is not already set - if !strings.Contains(output, "name=") { - output += ",name=" + parsedReference.String() - } + output += ",name=" + parsedReference.String() // pick the first tag and add it to output for idx, tag := range tags {