From ae08e01e49c7e2f07fb03e6caa5d3626d0b2b10d Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 22 Jun 2021 10:52:49 -0400 Subject: [PATCH] bud: teach --platform to take a list Add a pkg/parse.PlatformsFromOptions() which understands a "variant" value as an optional third value in an OS/ARCH[/VARIANT] argument value, which accepts a comma-separated list of them, and which returns a list of platforms. Teach "from" and "pull" about the --platform option and add integration tests for them, warning if --platform was given multiple values. Add a define.BuildOptions.JobSemaphore which an imagebuildah executor will use in preference to one that it might allocate for itself. In main(), allocate a JobSemaphore if the number of jobs is not 0 (which we treat as "unlimited", and continue to allow executors to do). In addManifest(), take a lock on the manifest list's image ID so that we don't overwrite changes that another thread might be making while we're attempting to make changes to it. In main(), create an empty list if the list doesn't already exist before we start down this path, so that we don't get two threads trying to create that manifest list at the same time later on. Two processes could still try to create the same list twice, but it's an incremental improvement. Finally, if we've been given multiple platforms to build for, run their builds concurrently and gather up their results. Signed-off-by: Nalin Dahyabhai --- buildah.go | 6 +- cmd/buildah/bud.go | 172 ++++++++++++++++++++----------- cmd/buildah/from.go | 7 ++ cmd/buildah/pull.go | 9 ++ cmd/buildah/unshare.go | 2 +- cmd/buildah/version.go | 4 + commit.go | 6 ++ config.go | 18 +++- contrib/completions/bash/buildah | 1 + define/build.go | 4 + docs/buildah-bud.md | 28 +++-- docs/buildah-from.md | 20 +++- docs/buildah-pull.md | 16 ++- docs/buildah-unshare.md | 2 +- imagebuildah/executor.go | 17 +-- import.go | 2 +- new.go | 2 +- pkg/cli/common.go | 4 +- pkg/cli/common_test.go | 2 +- pkg/parse/parse.go | 123 ++++++++++++++-------- tests/bud.bats | 28 +++++ tests/bud/multiarch/Dockerfile | 10 ++ tests/from.bats | 38 ++++++- tests/pull.bats | 17 +++ 24 files changed, 400 insertions(+), 138 deletions(-) create mode 100644 tests/bud/multiarch/Dockerfile diff --git a/buildah.go b/buildah.go index 10eed6c9b..ae10cdae0 100644 --- a/buildah.go +++ b/buildah.go @@ -397,7 +397,7 @@ func OpenBuilder(store storage.Store, container string) (*Builder, error) { return nil, errors.Errorf("container %q is not a %s container (is a %q container)", container, define.Package, b.Type) } b.store = store - b.fixupConfig() + b.fixupConfig(nil) b.setupLogger() return b, nil } @@ -433,7 +433,7 @@ func OpenBuilderByPath(store storage.Store, path string) (*Builder, error) { err = json.Unmarshal(buildstate, &b) if err == nil && b.Type == containerType && builderMatchesPath(b, abs) { b.store = store - b.fixupConfig() + b.fixupConfig(nil) b.setupLogger() return b, nil } @@ -471,7 +471,7 @@ func OpenAllBuilders(store storage.Store) (builders []*Builder, err error) { if err == nil && b.Type == containerType { b.store = store b.setupLogger() - b.fixupConfig() + b.fixupConfig(nil) builders = append(builders, b) continue } diff --git a/cmd/buildah/bud.go b/cmd/buildah/bud.go index e08707bde..5bbeac55d 100644 --- a/cmd/buildah/bud.go +++ b/cmd/buildah/bud.go @@ -13,10 +13,16 @@ import ( buildahcli "github.com/containers/buildah/pkg/cli" "github.com/containers/buildah/pkg/parse" "github.com/containers/buildah/util" + "github.com/containers/common/libimage" + "github.com/containers/common/libimage/manifests" "github.com/containers/common/pkg/auth" + "github.com/containers/image/v5/manifest" + "github.com/containers/storage" + "github.com/hashicorp/go-multierror" "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "golang.org/x/sync/semaphore" ) type budOptions struct { @@ -299,7 +305,7 @@ func budCmd(c *cobra.Command, inputArgs []string, iopts budOptions) error { } namespaceOptions.AddOrReplace(usernsOption...) - imageOS, arch, err := parse.PlatformFromOptions(c) + platforms, err := parse.PlatformsFromOptions(c) if err != nil { return err } @@ -309,75 +315,119 @@ func budCmd(c *cobra.Command, inputArgs []string, iopts budOptions) error { return errors.Wrapf(err, "unable to obtain decrypt config") } - options := define.BuildOptions{ - AddCapabilities: iopts.CapAdd, - AdditionalTags: tags, - Annotations: iopts.Annotation, - Architecture: arch, - Args: args, - BlobDirectory: iopts.BlobCache, - CNIConfigDir: iopts.CNIConfigDir, - CNIPluginPath: iopts.CNIPlugInPath, - CommonBuildOpts: commonOpts, - Compression: compression, - ConfigureNetwork: networkPolicy, - ContextDirectory: contextDir, - DefaultMountsFilePath: globalFlagResults.DefaultMountsFile, - Devices: iopts.Devices, - DropCapabilities: iopts.CapDrop, - Err: stderr, - ForceRmIntermediateCtrs: iopts.ForceRm, - From: iopts.From, - IDMappingOptions: idmappingOptions, - IIDFile: iopts.Iidfile, - In: stdin, - Isolation: isolation, - Labels: iopts.Label, - Layers: layers, - LogRusage: iopts.LogRusage, - Manifest: iopts.Manifest, - MaxPullPushRetries: maxPullPushRetries, - NamespaceOptions: namespaceOptions, - NoCache: iopts.NoCache, - OS: imageOS, - Out: stdout, - Output: output, - OutputFormat: format, - PullPolicy: pullPolicy, - PullPushRetryDelay: pullPushRetryDelay, - Quiet: iopts.Quiet, - RemoveIntermediateCtrs: iopts.Rm, - ReportWriter: reporter, - Runtime: iopts.Runtime, - RuntimeArgs: runtimeFlags, - RusageLogFile: iopts.RusageLogFile, - SignBy: iopts.SignBy, - SignaturePolicyPath: iopts.SignaturePolicy, - Squash: iopts.Squash, - SystemContext: systemContext, - Target: iopts.Target, - TransientMounts: iopts.Volumes, - OciDecryptConfig: decConfig, - Jobs: &iopts.Jobs, + var ( + jobSemaphore *semaphore.Weighted + builds multierror.Group + ) + if iopts.Jobs > 0 { + jobSemaphore = semaphore.NewWeighted(int64(iopts.Jobs)) + iopts.Jobs = 0 } - if iopts.IgnoreFile != "" { - excludes, err := parseDockerignore(iopts.IgnoreFile) + if iopts.Manifest != "" { + // Ensure that the list's ID is known before we spawn off any + // goroutines that'll want to modify it, so that they don't + // race and create two lists, one of which will rapidly become + // ignored. + rt, err := libimage.RuntimeFromStore(store, nil) + if err != nil { + return err + } + _, err = rt.LookupManifestList(iopts.Manifest) + if err != nil && errors.Cause(err) == storage.ErrImageUnknown { + list := manifests.Create() + _, err = list.SaveToImage(store, "", []string{iopts.Manifest}, manifest.DockerV2ListMediaType) + } if err != nil { return err } - options.Excludes = excludes } + var excludes []string + if iopts.IgnoreFile != "" { + if excludes, err = parseDockerignore(iopts.IgnoreFile); err != nil { + return err + } + } + var timestamp *time.Time if c.Flag("timestamp").Changed { - timestamp := time.Unix(iopts.Timestamp, 0).UTC() - options.Timestamp = ×tamp + t := time.Unix(iopts.Timestamp, 0).UTC() + timestamp = &t } + for _, platform := range platforms { + platform := platform + builds.Go(func() error { + platformContext := *systemContext + platformContext.OSChoice = platform.OS + platformContext.ArchitectureChoice = platform.Arch + platformContext.VariantChoice = platform.Variant + options := define.BuildOptions{ + AddCapabilities: iopts.CapAdd, + AdditionalTags: tags, + Annotations: iopts.Annotation, + Architecture: platform.Arch, + Args: args, + BlobDirectory: iopts.BlobCache, + CNIConfigDir: iopts.CNIConfigDir, + CNIPluginPath: iopts.CNIPlugInPath, + CommonBuildOpts: commonOpts, + Compression: compression, + ConfigureNetwork: networkPolicy, + ContextDirectory: contextDir, + DefaultMountsFilePath: globalFlagResults.DefaultMountsFile, + Devices: iopts.Devices, + DropCapabilities: iopts.CapDrop, + Err: stderr, + ForceRmIntermediateCtrs: iopts.ForceRm, + From: iopts.From, + IDMappingOptions: idmappingOptions, + IIDFile: iopts.Iidfile, + In: stdin, + Isolation: isolation, + Labels: iopts.Label, + Layers: layers, + LogRusage: iopts.LogRusage, + Manifest: iopts.Manifest, + MaxPullPushRetries: maxPullPushRetries, + NamespaceOptions: namespaceOptions, + NoCache: iopts.NoCache, + OS: platform.OS, + Out: stdout, + Output: output, + OutputFormat: format, + PullPolicy: pullPolicy, + PullPushRetryDelay: pullPushRetryDelay, + Quiet: iopts.Quiet, + RemoveIntermediateCtrs: iopts.Rm, + ReportWriter: reporter, + Runtime: iopts.Runtime, + RuntimeArgs: runtimeFlags, + RusageLogFile: iopts.RusageLogFile, + SignBy: iopts.SignBy, + SignaturePolicyPath: iopts.SignaturePolicy, + Squash: iopts.Squash, + SystemContext: &platformContext, + Target: iopts.Target, + TransientMounts: iopts.Volumes, + OciDecryptConfig: decConfig, + Jobs: &iopts.Jobs, + JobSemaphore: jobSemaphore, + Excludes: excludes, + Timestamp: timestamp, + } + if iopts.Quiet { + options.ReportWriter = ioutil.Discard + } - if iopts.Quiet { - options.ReportWriter = ioutil.Discard + _, _, err = imagebuildah.BuildDockerfiles(getContext(), store, options, dockerfiles...) + return err + }) } - - _, _, err = imagebuildah.BuildDockerfiles(getContext(), store, options, dockerfiles...) - return err + if merr := builds.Wait(); merr != nil { + if merr.Len() == 1 { + return merr.Errors[0] + } + return merr.ErrorOrNil() + } + return nil } // discoverContainerfile tries to find a Containerfile or a Dockerfile within the provided `path`. diff --git a/cmd/buildah/from.go b/cmd/buildah/from.go index 20edca92f..92da8cc56 100644 --- a/cmd/buildah/from.go +++ b/cmd/buildah/from.go @@ -195,6 +195,13 @@ func fromCmd(c *cobra.Command, args []string, iopts fromReply) error { if err != nil { return errors.Wrapf(err, "error building system context") } + platforms, err := parse.PlatformsFromOptions(c) + if err != nil { + return err + } + if len(platforms) > 1 { + logrus.Warnf("ignoring platforms other than %+v: %+v", platforms[0], platforms[1:]) + } pullFlagsCount := 0 if c.Flag("pull").Changed { diff --git a/cmd/buildah/pull.go b/cmd/buildah/pull.go index 16e3919f8..4788b61cf 100644 --- a/cmd/buildah/pull.go +++ b/cmd/buildah/pull.go @@ -11,6 +11,7 @@ import ( "github.com/containers/buildah/pkg/parse" "github.com/containers/common/pkg/auth" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -67,6 +68,7 @@ func init() { flags.BoolVarP(&opts.quiet, "quiet", "q", false, "don't output progress information when pulling images") flags.String("os", runtime.GOOS, "prefer `OS` instead of the running OS for choosing images") flags.String("arch", runtime.GOARCH, "prefer `ARCH` instead of the architecture of the machine for choosing images") + flags.StringSlice("platform", []string{parse.DefaultPlatform()}, "prefer OS/ARCH instead of the current operating system and architecture for choosing images") flags.String("variant", "", "override the `variant` of the specified image") flags.BoolVar(&opts.tlsVerify, "tls-verify", true, "require HTTPS and verify certificates when accessing the registry. TLS verification cannot be used when talking to an insecure registry.") if err := flags.MarkHidden("blob-cache"); err != nil { @@ -94,6 +96,13 @@ func pullCmd(c *cobra.Command, args []string, iopts pullOptions) error { if err != nil { return errors.Wrapf(err, "error building system context") } + platforms, err := parse.PlatformsFromOptions(c) + if err != nil { + return err + } + if len(platforms) > 1 { + logrus.Warnf("ignoring platforms other than %+v: %+v", platforms[0], platforms[1:]) + } store, err := getStore(c) if err != nil { diff --git a/cmd/buildah/unshare.go b/cmd/buildah/unshare.go index 9e97da0ae..6a10f7367 100644 --- a/cmd/buildah/unshare.go +++ b/cmd/buildah/unshare.go @@ -33,7 +33,7 @@ func init() { unshareCommand.SetUsageTemplate(UsageTemplate()) flags := unshareCommand.Flags() flags.SetInterspersed(false) - flags.StringSliceVar(&unshareMounts, "mount", []string{}, "mount the specified containers (default [])") + flags.StringSliceVarP(&unshareMounts, "mount", "m", []string{}, "mount the specified containers (default [])") rootCmd.AddCommand(unshareCommand) } diff --git a/cmd/buildah/version.go b/cmd/buildah/version.go index 1f7977c34..0566b718c 100644 --- a/cmd/buildah/version.go +++ b/cmd/buildah/version.go @@ -7,6 +7,7 @@ import ( "strconv" "time" + "github.com/containerd/containerd/platforms" cniversion "github.com/containernetworking/cni/pkg/version" "github.com/containers/buildah/define" iversion "github.com/containers/image/v5/version" @@ -33,6 +34,7 @@ type versionInfo struct { GitCommit string `json:"gitCommit"` Built string `json:"built"` OsArch string `json:"osArch"` + BuildPlatform string `json:"buildPlatform"` } type versionOptions struct { @@ -83,6 +85,7 @@ func versionCmd(opts versionOptions) error { GitCommit: GitCommit, Built: time.Unix(buildTime, 0).Format(time.ANSIC), OsArch: runtime.GOOS + "/" + runtime.GOARCH, + BuildPlatform: platforms.DefaultString(), } if opts.json { @@ -106,6 +109,7 @@ func versionCmd(opts versionOptions) error { //Prints out the build time in readable format fmt.Println("Built: ", version.Built) fmt.Println("OS/Arch: ", version.OsArch) + fmt.Println("BuildPlatform: ", version.BuildPlatform) return nil } diff --git a/commit.go b/commit.go index d0186f27c..c2afd84c7 100644 --- a/commit.go +++ b/commit.go @@ -183,6 +183,12 @@ func (b *Builder) addManifest(ctx context.Context, manifestName string, imageSpe create = true list = manifests.Create() } else { + locker, err := manifests.LockerForImage(b.store, manifestList.ID()) + if err != nil { + return "", err + } + locker.Lock() + defer locker.Unlock() _, list, err = manifests.LoadFromImage(b.store, manifestList.ID()) if err != nil { return "", err diff --git a/config.go b/config.go index b71f08aa3..a3da64e6b 100644 --- a/config.go +++ b/config.go @@ -51,7 +51,7 @@ func unmarshalConvertedConfig(ctx context.Context, dest interface{}, img types.I return nil } -func (b *Builder) initConfig(ctx context.Context, img types.Image) error { +func (b *Builder) initConfig(ctx context.Context, img types.Image, sys *types.SystemContext) error { if img != nil { // A pre-existing image, as opposed to a "FROM scratch" new one. rawManifest, manifestMIMEType, err := img.Manifest(ctx) if err != nil { @@ -95,11 +95,11 @@ func (b *Builder) initConfig(ctx context.Context, img types.Image) error { } b.setupLogger() - b.fixupConfig() + b.fixupConfig(sys) return nil } -func (b *Builder) fixupConfig() { +func (b *Builder) fixupConfig(sys *types.SystemContext) { if b.Docker.Config != nil { // Prefer image-level settings over those from the container it was built from. b.Docker.ContainerConfig = *b.Docker.Config @@ -114,10 +114,18 @@ func (b *Builder) fixupConfig() { b.OCIv1.Created = &now } if b.OS() == "" { - b.SetOS(runtime.GOOS) + if sys != nil && sys.OSChoice != "" { + b.SetOS(sys.OSChoice) + } else { + b.SetOS(runtime.GOOS) + } } if b.Architecture() == "" { - b.SetArchitecture(runtime.GOARCH) + if sys != nil && sys.ArchitectureChoice != "" { + b.SetArchitecture(sys.ArchitectureChoice) + } else { + b.SetArchitecture(runtime.GOARCH) + } } if b.Format == define.Dockerv2ImageManifest && b.Hostname() == "" { b.SetHostname(stringid.TruncateID(stringid.GenerateRandomID())) diff --git a/contrib/completions/bash/buildah b/contrib/completions/bash/buildah index 7e5a9d4ec..f267eaa1b 100644 --- a/contrib/completions/bash/buildah +++ b/contrib/completions/bash/buildah @@ -434,6 +434,7 @@ return 1 --os --pid --platform + --platforms --runtime --runtime-flag --security-opt diff --git a/define/build.go b/define/build.go index dc3708c20..0ea369963 100644 --- a/define/build.go +++ b/define/build.go @@ -7,6 +7,7 @@ import ( "github.com/containers/image/v5/types" encconfig "github.com/containers/ocicrypt/config" "github.com/containers/storage/pkg/archive" + "golang.org/x/sync/semaphore" ) // CommonBuildOptions are resources that can be defined by flags for both buildah from and build-using-dockerfile @@ -214,7 +215,10 @@ type BuildOptions struct { // encrypted if non-nil. If nil, it does not attempt to decrypt an image. OciDecryptConfig *encconfig.DecryptConfig // Jobs is the number of stages to run in parallel. If not specified it defaults to 1. + // Ignored if a JobSemaphore is provided. Jobs *int + // JobSemaphore, for when you want Jobs to be shared with more than just this build. + JobSemaphore *semaphore.Weighted // LogRusage logs resource usage for each step. LogRusage bool // File to which the Rusage logs will be saved to instead of stdout diff --git a/docs/buildah-bud.md b/docs/buildah-bud.md index f3ded098c..9cff06f47 100644 --- a/docs/buildah-bud.md +++ b/docs/buildah-bud.md @@ -44,7 +44,7 @@ Note: this information is not present in Docker image formats, so it is discarde **--arch**="ARCH" -Set the ARCH of the image to be pulled to the provided value instead of using the architecture of the host. (Examples: aarch64, arm, i686, ppc64le, s390x, x86_64) +Set the ARCH of the image to be pulled to the provided value instead of using the architecture of the host. (Examples: arm, arm64, 386, amd64, ppc64le, s390x) **--authfile** *path* @@ -406,13 +406,29 @@ that the PID namespace in which `buildah` itself is being run should be reused, or it can be the path to a PID namespace which is already in use by another process. -**--platform**="OS/ARCH" +**--platform**="OS/ARCH[/VARIANT]" -Set the OS/ARCH of the image to the provided value instead of using the current -operating system and architecture of the host (for example `linux/arm`). If -`--platform` is set, then the values of the `--arch` and `--os` options will be +Set the OS/ARCH of the built image (and its base image, if your build uses one) +to the provided value instead of using the current operating system and +architecture of the host (for example `linux/arm`). If `--platform` is set, +then the values of the `--arch`, `--os`, and `--variant` options will be overridden. +The `--platform` flag can be specified more than once, or given a +comma-separated list of values as its argument. When more than one platform is +specified, the `--manifest` option should be used instead of the `--tag` +option. + +OS/ARCH pairs are those used by the Go Programming Language. In several cases +the ARCH value for a platform differs from one produced by other tools such as +the `arch` command. Valid OS and architecture name combinations are listed as +values for $GOOS and $GOARCH at https://golang.org/doc/install/source#environment, +and can also be found by running `go tool dist list`. + +While `buildah bud` is happy to use base images and build images for any +platform that exists, `RUN` instructions will not be able to succeed without +the help of emulation provided by packages like `qemu-user-static`. + **--pull** When the flag is enabled, attempt to pull the latest image from the registries @@ -513,7 +529,7 @@ Use --stdin to be able to interact from the terminal during the build. Specifies the name which will be assigned to the resulting image if the build process completes successfully. -If _imageName_ does not include a registry name, the registry name *localhost* will be prepended to the image name. +If _imageName_ does not include a registry name component, the registry name *localhost* will be prepended to the image name. **--target** *stageName* diff --git a/docs/buildah-from.md b/docs/buildah-from.md index 281814c65..d3c1e6fbd 100644 --- a/docs/buildah-from.md +++ b/docs/buildah-from.md @@ -51,7 +51,7 @@ Add a line to /etc/hosts. The format is hostname:ip. The **--add-host** option c **--arch**="ARCH" -Set the ARCH of the image to be pulled to the provided value instead of using the architecture of the host. (Examples: aarch64, arm, i686, ppc64le, s390x, x86_64) +Set the ARCH of the image to be pulled to the provided value instead of using the architecture of the host. (Examples: arm, arm64, 386, amd64, ppc64le, s390x) **--authfile** *path* @@ -295,6 +295,24 @@ that the PID namespace in which `Buildah` itself is being run should be reused, or it can be the path to a PID namespace which is already in use by another process. +**--platform**="OS/ARCH[/VARIANT]" + +Set the OS/ARCH of the image to be pulled +to the provided value instead of using the current operating system and +architecture of the host (for example `linux/arm`). If `--platform` +is set, then the values of the `--arch`, `--os`, and `--variant` options will +be overridden. + +OS/ARCH pairs are those used by the Go Programming Language. In several cases +the ARCH value for a platform differs from one produced by other tools such as +the `arch` command. Valid OS and architecture name combinations are listed as +values for $GOOS and $GOARCH at https://golang.org/doc/install/source#environment, +and can also be found by running `go tool dist list`. + +While `buildah from` is happy to pull an image for any platform that exists, +`buildah run` will not be able to run binaries provided by that image without +the help of emulation provided by packages like `qemu-user-static`. + **--pull** When the flag is enabled, attempt to pull the latest image from the registries diff --git a/docs/buildah-pull.md b/docs/buildah-pull.md index 2ca3faa04..015ddae9a 100644 --- a/docs/buildah-pull.md +++ b/docs/buildah-pull.md @@ -26,7 +26,7 @@ All tagged images in the repository will be pulled. **--arch**="ARCH" -Set the ARCH of the image to be pulled to the provided value instead of using the architecture of the host. (Examples: aarch64, arm, i686, ppc64le, s390x, x86_64) +Set the ARCH of the image to be pulled to the provided value instead of using the architecture of the host. (Examples: arm, arm64, 386, amd64, ppc64le, s390x) **--authfile** *path* @@ -60,9 +60,19 @@ If an image needs to be pulled from the registry, suppress progress output. Set the OS of the image to be pulled instead of using the current operating system of the host. -**--os**="OS" +**--platform**="OS/ARCH[/VARIANT]" -Set the OS of the image to be pulled to the provided value instead of using the current operating system of the host. +Set the OS/ARCH of the image to be pulled +to the provided value instead of using the current operating system and +architecture of the host (for example `linux/arm`). If `--platform` +is set, then the values of the `--arch`, `--os`, and `--variant` options will +be overridden. + +OS/ARCH pairs are those used by the Go Programming Language. In several cases +the ARCH value for a platform differs from one produced by other tools such as +the `arch` command. Valid OS and architecture name combinations are listed as +values for $GOOS and $GOARCH at https://golang.org/doc/install/source#environment, +and can also be found by running `go tool dist list`. **--policy**=**always**|**missing**|**never** diff --git a/docs/buildah-unshare.md b/docs/buildah-unshare.md index 731c5360d..cc25bec9a 100644 --- a/docs/buildah-unshare.md +++ b/docs/buildah-unshare.md @@ -20,7 +20,7 @@ It is also useful if you want to use the `buildah mount` command. If an unprivi buildah unshare. Executing `buildah mount` fails for unprivileged users unless the user is running inside a `buildah unshare` session. ## OPTIONS -**--mount** [*VARIABLE=]containerNameOrID* +**--mount**, **-m** [*VARIABLE=]containerNameOrID* Mount the *containerNameOrID* container while running *command*, and set the environment variable *VARIABLE* to the path of the mountpoint. If *VARIABLE* diff --git a/imagebuildah/executor.go b/imagebuildah/executor.go index 606015ba7..b088a9395 100644 --- a/imagebuildah/executor.go +++ b/imagebuildah/executor.go @@ -252,6 +252,7 @@ func NewExecutor(logger *logrus.Logger, store storage.Store, options define.Buil retryPullPushDelay: options.PullPushRetryDelay, ociDecryptConfig: options.OciDecryptConfig, terminatedStage: make(map[string]struct{}), + stagesSemaphore: options.JobSemaphore, jobs: jobs, logRusage: options.LogRusage, rusageLogFile: rusageLogFile, @@ -618,14 +619,16 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image ch := make(chan Result) - jobs := int64(b.jobs) - if jobs < 0 { - return "", nil, errors.New("error building: invalid value for jobs. It must be a positive integer") - } else if jobs == 0 { - jobs = int64(len(stages)) - } + if b.stagesSemaphore == nil { + jobs := int64(b.jobs) + if jobs < 0 { + return "", nil, errors.New("error building: invalid value for jobs. It must be a positive integer") + } else if jobs == 0 { + jobs = int64(len(stages)) + } - b.stagesSemaphore = semaphore.NewWeighted(jobs) + b.stagesSemaphore = semaphore.NewWeighted(jobs) + } var wg sync.WaitGroup wg.Add(len(stages)) diff --git a/import.go b/import.go index 11494c5e3..0029a95e2 100644 --- a/import.go +++ b/import.go @@ -102,7 +102,7 @@ func importBuilderDataFromImage(ctx context.Context, store storage.Store, system }, } - if err := builder.initConfig(ctx, image); err != nil { + if err := builder.initConfig(ctx, image, systemContext); err != nil { return nil, errors.Wrapf(err, "error preparing image configuration") } diff --git a/new.go b/new.go index 9df4a0ed0..85a0f0b31 100644 --- a/new.go +++ b/new.go @@ -299,7 +299,7 @@ func newBuilder(ctx context.Context, store storage.Store, options BuilderOptions } } - if err := builder.initConfig(ctx, src); err != nil { + if err := builder.initConfig(ctx, src, systemContext); err != nil { return nil, errors.Wrapf(err, "error preparing image configuration") } err = builder.Save() diff --git a/pkg/cli/common.go b/pkg/cli/common.go index 83230716e..c645ce697 100644 --- a/pkg/cli/common.go +++ b/pkg/cli/common.go @@ -207,7 +207,6 @@ func GetBudFlags(flags *BudResults) pflag.FlagSet { fs.StringVar(&flags.Manifest, "manifest", "", "add the image to the specified manifest list. Creates manifest if it does not exist") fs.BoolVar(&flags.NoCache, "no-cache", false, "Do not use existing cached images for the container build. Build from the start with a new set of cached layers.") fs.String("os", runtime.GOOS, "set the OS to the provided value instead of the current operating system of the host") - fs.String("platform", parse.DefaultPlatform(), "set the OS/ARCH to the provided value instead of the current operating system and architecture of the host (for example `linux/arm`)") fs.BoolVar(&flags.Pull, "pull", true, "pull the image from the registry if newer or not present in store, if false, only pull the image if not present") fs.BoolVar(&flags.PullAlways, "pull-always", false, "pull the image even if the named image is present in store") fs.BoolVar(&flags.PullNever, "pull-never", false, "do not pull the image, use the image present in store if available") @@ -251,7 +250,6 @@ func GetBudFlagsCompletions() commonComp.FlagCompletions { flagCompletion["logfile"] = commonComp.AutocompleteDefault flagCompletion["manifest"] = commonComp.AutocompleteDefault flagCompletion["os"] = commonComp.AutocompleteNone - flagCompletion["platform"] = commonComp.AutocompleteNone flagCompletion["runtime-flag"] = commonComp.AutocompleteNone flagCompletion["secret"] = commonComp.AutocompleteNone flagCompletion["sign-by"] = commonComp.AutocompleteNone @@ -295,6 +293,7 @@ func GetFromAndBudFlags(flags *FromAndBudResults, usernsResults *UserNSResults, fs.StringVar(&flags.MemorySwap, "memory-swap", "", "swap limit equal to memory plus swap: '-1' to enable unlimited swap") fs.String("arch", runtime.GOARCH, "set the ARCH of the image to the provided value instead of the architecture of the host") fs.String("os", runtime.GOOS, "prefer `OS` instead of the running OS when pulling images") + fs.StringSlice("platform", []string{parse.DefaultPlatform()}, "set the OS/ARCH/VARIANT of the image to the provided value instead of the current operating system and architecture of the host (for example `linux/arm`)") fs.String("variant", "", "override the `variant` of the specified image") fs.StringArrayVar(&flags.SecurityOpt, "security-opt", []string{}, "security options (default [])") fs.StringVar(&flags.ShmSize, "shm-size", defaultContainerConfig.Containers.ShmSize, "size of '/dev/shm'. The format is ``.") @@ -333,6 +332,7 @@ func GetFromAndBudFlagsCompletions() commonComp.FlagCompletions { flagCompletion["memory"] = commonComp.AutocompleteNone flagCompletion["memory-swap"] = commonComp.AutocompleteNone flagCompletion["os"] = commonComp.AutocompleteNone + flagCompletion["platform"] = commonComp.AutocompleteNone flagCompletion["security-opt"] = commonComp.AutocompleteNone flagCompletion["shm-size"] = commonComp.AutocompleteNone flagCompletion["ulimit"] = commonComp.AutocompleteNone diff --git a/pkg/cli/common_test.go b/pkg/cli/common_test.go index 9c7e1860a..a1c29a75d 100644 --- a/pkg/cli/common_test.go +++ b/pkg/cli/common_test.go @@ -22,7 +22,7 @@ func testFlagCompletion(t *testing.T, flags pflag.FlagSet, flagCompletions compl // make sure no unnecessary flag completion functions are defined for name := range flagCompletions { if flag := flags.Lookup(name); flag == nil { - t.Errorf("Flag %q does not exists but has a shell completion function set.", name) + t.Errorf("Flag %q does not exist but has a shell completion function set.", name) } } } diff --git a/pkg/parse/parse.go b/pkg/parse/parse.go index 5ba2f51d0..8241cd675 100644 --- a/pkg/parse/parse.go +++ b/pkg/parse/parse.go @@ -556,35 +556,45 @@ func SystemContextFromOptions(c *cobra.Command) (*types.SystemContext, error) { } ctx.DockerRegistryUserAgent = fmt.Sprintf("Buildah/%s", define.Version) if c.Flag("os") != nil && c.Flag("os").Changed { - if os, err := c.Flags().GetString("os"); err == nil { - ctx.OSChoice = os + var os string + if os, err = c.Flags().GetString("os"); err != nil { + return nil, err } + ctx.OSChoice = os } if c.Flag("arch") != nil && c.Flag("arch").Changed { - if arch, err := c.Flags().GetString("arch"); err == nil { - ctx.ArchitectureChoice = arch + var arch string + if arch, err = c.Flags().GetString("arch"); err != nil { + return nil, err } + ctx.ArchitectureChoice = arch } if c.Flag("variant") != nil && c.Flag("variant").Changed { - if variant, err := c.Flags().GetString("variant"); err == nil { - ctx.VariantChoice = variant + var variant string + if variant, err = c.Flags().GetString("variant"); err != nil { + return nil, err } + ctx.VariantChoice = variant } if c.Flag("platform") != nil && c.Flag("platform").Changed { - if platform, err := c.Flags().GetString("platform"); err == nil { - os, arch, variant, err := Platform(platform) - if err != nil { - return nil, err - } - if ctx.OSChoice != "" || - ctx.ArchitectureChoice != "" || - ctx.VariantChoice != "" { - return nil, errors.Errorf("invalid --platform may not be used with --os, --arch, or --variant") - } - ctx.OSChoice = os - ctx.ArchitectureChoice = arch - ctx.VariantChoice = variant + var specs []string + if specs, err = c.Flags().GetStringSlice("platform"); err != nil { + return nil, err } + if len(specs) == 0 || specs[0] == "" { + return nil, errors.Errorf("unable to parse --platform value %v", specs) + } + platform := specs[0] + os, arch, variant, err := Platform(platform) + if err != nil { + return nil, err + } + if ctx.OSChoice != "" || ctx.ArchitectureChoice != "" || ctx.VariantChoice != "" { + return nil, errors.Errorf("invalid --platform may not be used with --os, --arch, or --variant") + } + ctx.OSChoice = os + ctx.ArchitectureChoice = arch + ctx.VariantChoice = variant } ctx.BigFilesTemporaryDir = GetTempDir() @@ -599,32 +609,57 @@ func getAuthFile(authfile string) string { } // PlatformFromOptions parses the operating system (os) and architecture (arch) -// from the provided command line options. +// from the provided command line options. Deprecated in favor of +// PlatformsFromOptions(), but kept here because it's part of our API. func PlatformFromOptions(c *cobra.Command) (os, arch string, err error) { + platforms, err := PlatformsFromOptions(c) + if err != nil { + return "", "", err + } + if len(platforms) < 1 { + return "", "", errors.Errorf("invalid platform syntax for --platform (use OS/ARCH[/VARIANT])") + } + return platforms[0].OS, platforms[0].Arch, nil +} +// PlatformsFromOptions parses the operating system (os) and architecture +// (arch) from the provided command line options. If --platform used, it +// also returns the list of platforms that were passed in as its argument. +func PlatformsFromOptions(c *cobra.Command) (platforms []struct{ OS, Arch, Variant string }, err error) { + var os, arch, variant string if c.Flag("os").Changed { - if selectedOS, err := c.Flags().GetString("os"); err == nil { - os = selectedOS + if os, err = c.Flags().GetString("os"); err != nil { + return nil, err } } if c.Flag("arch").Changed { - if selectedArch, err := c.Flags().GetString("arch"); err == nil { - arch = selectedArch + if arch, err = c.Flags().GetString("arch"); err != nil { + return nil, err } } - + if c.Flag("variant").Changed { + if variant, err = c.Flags().GetString("variant"); err != nil { + return nil, err + } + } + platforms = []struct{ OS, Arch, Variant string }{{os, arch, variant}} if c.Flag("platform").Changed { - if pf, err := c.Flags().GetString("platform"); err == nil { - selectedOS, selectedArch, _, err := Platform(pf) - if err != nil { - return "", "", errors.Wrap(err, "unable to parse platform") + platforms = nil + platformSpecs, err := c.Flags().GetStringSlice("platform") + if err != nil { + return nil, errors.Wrap(err, "unable to parse platform") + } + if os != "" || arch != "" || variant != "" { + return nil, errors.Errorf("invalid --platform may not be used with --os, --arch, or --variant") + } + for _, pf := range platformSpecs { + if os, arch, variant, err = Platform(pf); err != nil { + return nil, errors.Wrapf(err, "unable to parse platform %q", pf) } - arch = selectedArch - os = selectedOS + platforms = append(platforms, struct{ OS, Arch, Variant string }{os, arch, variant}) } } - - return os, arch, nil + return platforms, nil } const platformSep = "/" @@ -634,18 +669,24 @@ func DefaultPlatform() string { return runtime.GOOS + platformSep + runtime.GOARCH } -// Platform separates the platform string into os, arch and variant +// Platform separates the platform string into os, arch and variant, +// accepting any of $arch, $os/$arch, or $os/$arch/$variant. func Platform(platform string) (os, arch, variant string, err error) { split := strings.Split(platform, platformSep) - if len(split) < 2 { - return "", "", "", errors.Errorf("invalid platform syntax for %q (use OS/ARCH)", platform) - } - os = split[0] - arch = split[1] - if len(split) == 3 { + switch len(split) { + case 3: variant = split[2] + fallthrough + case 2: + arch = split[1] + os = split[0] + return + case 1: + if platform == "local" { + return Platform(DefaultPlatform()) + } } - return + return "", "", "", errors.Errorf("invalid platform syntax for %q (use OS/ARCH[/VARIANT][,...])", platform) } func parseCreds(creds string) (string, string) { diff --git a/tests/bud.bats b/tests/bud.bats index ceeed1f7c..ae29ef4fa 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -3300,3 +3300,31 @@ _EOF run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json -t secretreq -f ${TESTSDIR}/bud/run-mounts/Dockerfile.secret-required ${TESTSDIR}/bud/run-mounts expect_output --substring "secret required but no secret with id mysecret found" } + +@test "bud-multiple-platform-values" { + outputlist=testlist + # check if we can run a couple of 32-bit versions of an image, and if we can, + # assume that emulation for other architectures is in place. + os=`go env GOOS` + run_buildah from --signature-policy ${TESTSDIR}/policy.json --name try-386 --platform=$os/386 alpine + run buildah run try-386 true + if test $status -ne 0 ; then + skip "unable to run 386 container, assuming emulation is not available" + fi + run_buildah from --signature-policy ${TESTSDIR}/policy.json --name try-arm --platform=$os/arm alpine + run buildah run try-arm true + if test $status -ne 0 ; then + skip "unable to run arm container, assuming emulation is not available" + fi + # build for those architectures - RUN gets exercised + run_buildah bud --signature-policy ${TESTSDIR}/policy.json --jobs=0 --platform=$os/arm,$os/386 --manifest $outputlist ${TESTSDIR}/bud/multiarch + run_buildah manifest inspect $outputlist + list="$output" + run jq -r '.manifests[0].digest' <<< "$list" + d1="$output" + run jq -r '.manifests[1].digest' <<< "$list" + d2="$output" + test -n "$d1" + test -n "$d2" + test "$d1" != "$d2" +} diff --git a/tests/bud/multiarch/Dockerfile b/tests/bud/multiarch/Dockerfile new file mode 100644 index 000000000..9f0d863b9 --- /dev/null +++ b/tests/bud/multiarch/Dockerfile @@ -0,0 +1,10 @@ +FROM alpine AS base +RUN cp /etc/apk/arch /root/arch-base + +FROM alpine +# Make sure that non-default arch doesn't mess with copying from previous stages. +COPY --from=base /root/arch-base /root/ +# Make sure that COPY --from=image uses the image for the preferred architecture. +COPY --from=alpine /etc/apk/arch /root/ +RUN cmp /etc/apk/arch /root/arch +RUN cmp /etc/apk/arch /root/arch-base diff --git a/tests/from.bats b/tests/from.bats index 41e232de0..ff94d426d 100644 --- a/tests/from.bats +++ b/tests/from.bats @@ -471,15 +471,45 @@ load helpers @test "from --arch test" { skip_if_no_runtime + _prefetch alpine run_buildah from --quiet --pull --signature-policy ${TESTSDIR}/policy.json --arch=arm64 alpine + other=$output + run_buildah from --quiet --pull --signature-policy ${TESTSDIR}/policy.json --arch=$(go env GOARCH) alpine cid=$output -# run_buildah run $cid arch -# expect_output "aarch64" + run_buildah copy --from $other $cid /etc/apk/arch /root/other-arch + run_buildah run $cid cat /root/other-arch + expect_output "aarch64" run_buildah from --quiet --pull --signature-policy ${TESTSDIR}/policy.json --arch=s390x alpine + other=$output + run_buildah copy --from $other $cid /etc/apk/arch /root/other-arch + run_buildah run $cid cat /root/other-arch + expect_output "s390x" +} + +@test "from --platform test" { + skip_if_no_runtime + + run_buildah version + platform=$(grep ^BuildPlatform: <<< "$output") + echo "$platform" + platform=${platform##* } + echo "$platform" + + _prefetch alpine + run_buildah from --quiet --pull --signature-policy ${TESTSDIR}/policy.json --platform=linux/arm64 alpine + other=$output + run_buildah from --quiet --pull --signature-policy ${TESTSDIR}/policy.json --platform=${platform} alpine cid=$output -# run_buildah run $cid arch -# expect_output "s390x" + run_buildah copy --from $other $cid /etc/apk/arch /root/other-arch + run_buildah run $cid cat /root/other-arch + expect_output "aarch64" + + run_buildah from --quiet --pull --signature-policy ${TESTSDIR}/policy.json --platform=linux/s390x alpine + other=$output + run_buildah copy --from $other $cid /etc/apk/arch /root/other-arch + run_buildah run $cid cat /root/other-arch + expect_output "s390x" } @test "from --authfile test" { diff --git a/tests/pull.bats b/tests/pull.bats index d732a2578..588280e96 100644 --- a/tests/pull.bats +++ b/tests/pull.bats @@ -336,6 +336,23 @@ load helpers run_buildah rmi alpine } +@test "pull --platform" { + mkdir ${TESTDIR}/buildahtest + run_buildah 125 pull --signature-policy ${TESTSDIR}/policy.json --platform linux/bogus alpine + expect_output --substring "no image found in manifest list" + + # Make sure missing image works + run_buildah pull -q --signature-policy ${TESTSDIR}/policy.json --platform linux/arm64 alpine + + run_buildah inspect --format "{{ .Docker.Architecture }}" alpine + expect_output arm64 + + run_buildah inspect --format "{{ .OCIv1.Architecture }}" alpine + expect_output arm64 + + run_buildah rmi alpine +} + @test "pull image with TMPDIR set" { testdir=${TESTDIR}/buildah-test mkdir -p $testdir