diff --git a/cmd/buildah/bud.go b/cmd/buildah/bud.go index 6a3238c9f..225b426ff 100644 --- a/cmd/buildah/bud.go +++ b/cmd/buildah/bud.go @@ -148,6 +148,14 @@ func budCmd(c *cli.Context) error { return err } + if c.IsSet("layers") && c.IsSet("no-cache") { + return errors.Errorf("can only set one of 'layers' or 'no-cache'") + } + + if (c.IsSet("rm") || c.IsSet("force-rm")) && (!c.IsSet("layers") && !c.IsSet("no-cache")) { + return errors.Errorf("'rm' and 'force-rm' can only be set with either 'layers' or 'no-cache'") + } + if c.IsSet("cache-from") { logrus.Debugf("build caching not enabled so --cache-from flag has no effect") } @@ -160,14 +168,6 @@ func budCmd(c *cli.Context) error { logrus.Debugf("--disable-content-trust option specified but is ignored") } - if c.IsSet("force-rm") { - logrus.Debugf("build caching not enabled so --force-rm flag has no effect") - } - - if c.IsSet("rm") { - logrus.Debugf("build caching not enabled so --rm flag has no effect") - } - namespaceOptions, networkPolicy, err := parse.NamespaceOptions(c) if err != nil { return errors.Wrapf(err, "error parsing namespace-related options") @@ -179,34 +179,36 @@ func budCmd(c *cli.Context) error { namespaceOptions.AddOrReplace(usernsOption...) options := imagebuildah.BuildOptions{ - ContextDirectory: contextDir, - PullPolicy: pullPolicy, - Compression: imagebuildah.Gzip, - Quiet: c.Bool("quiet"), - SignaturePolicyPath: c.String("signature-policy"), - Args: args, - Output: output, - AdditionalTags: tags, - Out: stdout, - Err: stderr, - ReportWriter: reporter, - Runtime: c.String("runtime"), - RuntimeArgs: runtimeFlags, - OutputFormat: format, - SystemContext: systemContext, - NamespaceOptions: namespaceOptions, - ConfigureNetwork: networkPolicy, - CNIPluginPath: c.String("cni-plugin-path"), - CNIConfigDir: c.String("cni-config-dir"), - IDMappingOptions: idmappingOptions, - CommonBuildOpts: commonOpts, - DefaultMountsFilePath: c.GlobalString("default-mounts-file"), - IIDFile: c.String("iidfile"), - Squash: c.Bool("squash"), - Labels: c.StringSlice("label"), - Annotations: c.StringSlice("annotation"), - Layers: layers, - NoCache: c.Bool("no-cache"), + ContextDirectory: contextDir, + PullPolicy: pullPolicy, + Compression: imagebuildah.Gzip, + Quiet: c.Bool("quiet"), + SignaturePolicyPath: c.String("signature-policy"), + Args: args, + Output: output, + AdditionalTags: tags, + Out: stdout, + Err: stderr, + ReportWriter: reporter, + Runtime: c.String("runtime"), + RuntimeArgs: runtimeFlags, + OutputFormat: format, + SystemContext: systemContext, + NamespaceOptions: namespaceOptions, + ConfigureNetwork: networkPolicy, + CNIPluginPath: c.String("cni-plugin-path"), + CNIConfigDir: c.String("cni-config-dir"), + IDMappingOptions: idmappingOptions, + CommonBuildOpts: commonOpts, + DefaultMountsFilePath: c.GlobalString("default-mounts-file"), + IIDFile: c.String("iidfile"), + Squash: c.Bool("squash"), + Labels: c.StringSlice("label"), + Annotations: c.StringSlice("annotation"), + Layers: layers, + NoCache: c.Bool("no-cache"), + RemoveIntermediateCtrs: c.BoolT("rm"), + ForceRmIntermediateCtrs: c.Bool("force-rm"), } if c.Bool("quiet") { diff --git a/docs/buildah-bud.md b/docs/buildah-bud.md index 6ecc50ee3..6a009efe1 100644 --- a/docs/buildah-bud.md +++ b/docs/buildah-bud.md @@ -161,9 +161,9 @@ If a build context is not specified, and at least one Dockerfile is a local file, the directory in which it resides will be used as the build context. -**--force-rm** +**--force-rm** *bool-value* -Always remove intermediate containers after a build. Buildah does not currently support caching so this is a NOOP. +Always remove intermediate containers after a build, even if the build is unsuccessful.. **--format** @@ -197,7 +197,7 @@ OCI Runtime, using the --runtime flag. Add an image *label* (e.g. label=*value*) to the image metadata. Can be used multiple times. -**--layers** +**--layers** *bool-value* Cache intermediate images during the build process (Default is `false`). @@ -240,7 +240,7 @@ that the network namespace in which `buildah` itself is being run should be reused, or it can be the path to a network namespace which is already in use by another process. -**--no-cache** +**--no-cache** *bool-value* Do not use existing cached images for the container build. Build from the start with a new set of cached layers. @@ -269,9 +269,9 @@ Suppress output messages which indicate which instruction is being processed, and of progress when pulling images from a registry, and when writing the output image. -**--rm** +**--rm** *bool-value* -Remove intermediate containers after a successful build. Buildah does not currently support caching so this is a NOOP. +Remove intermediate containers after a successful build (default true). **--runtime** *path* @@ -518,6 +518,10 @@ buildah bud --layers -t imageName . buildah bud --no-cache -t imageName . +buildah bud --layers --force-rm -t imageName . + +buildah bud --no-cache --rm=false -t imageName . + ### Building an image using a URL This will clone the specified GitHub repository from the URL and use it as context. The Dockerfile at the root of the repository is used as Dockerfile. This only works if the GitHub repository is a dedicated repository. diff --git a/imagebuildah/build.go b/imagebuildah/build.go index 9b9a31081..58b653dce 100644 --- a/imagebuildah/build.go +++ b/imagebuildah/build.go @@ -147,6 +147,12 @@ type BuildOptions struct { // NoCache tells the builder to build the image from scratch without checking for a cache. // It creates a new set of cached images for the build. NoCache bool + // RemoveIntermediateCtrs tells the builder whether to remove intermediate containers used + // during the build process. Default is true. + RemoveIntermediateCtrs bool + // ForceRmIntermediateCtrs tells the builder to remove all intermediate containers even if + // the build was unsuccessful. + ForceRmIntermediateCtrs bool } // Executor is a buildah-based implementation of the imagebuilder.Executor @@ -196,6 +202,8 @@ type Executor struct { layers bool topLayers []string noCache bool + removeIntermediateCtrs bool + forceRmIntermediateCtrs bool } // withName creates a new child executor that will be used whenever a COPY statement uses --from=NAME. @@ -521,35 +529,37 @@ func NewExecutor(store storage.Store, options BuildOptions) (*Executor, error) { registry: options.Registry, transport: options.Transport, ignoreUnrecognizedInstructions: options.IgnoreUnrecognizedInstructions, - quiet: options.Quiet, - runtime: options.Runtime, - runtimeArgs: options.RuntimeArgs, - transientMounts: options.TransientMounts, - compression: options.Compression, - output: options.Output, - outputFormat: options.OutputFormat, - additionalTags: options.AdditionalTags, - signaturePolicyPath: options.SignaturePolicyPath, - systemContext: options.SystemContext, - volumeCache: make(map[string]string), - volumeCacheInfo: make(map[string]os.FileInfo), - log: options.Log, - out: options.Out, - err: options.Err, - reportWriter: options.ReportWriter, - namespaceOptions: options.NamespaceOptions, - configureNetwork: options.ConfigureNetwork, - cniPluginPath: options.CNIPluginPath, - cniConfigDir: options.CNIConfigDir, - idmappingOptions: options.IDMappingOptions, - commonBuildOptions: options.CommonBuildOpts, - defaultMountsFilePath: options.DefaultMountsFilePath, - iidfile: options.IIDFile, - squash: options.Squash, - labels: append([]string{}, options.Labels...), - annotations: append([]string{}, options.Annotations...), - layers: options.Layers, - noCache: options.NoCache, + quiet: options.Quiet, + runtime: options.Runtime, + runtimeArgs: options.RuntimeArgs, + transientMounts: options.TransientMounts, + compression: options.Compression, + output: options.Output, + outputFormat: options.OutputFormat, + additionalTags: options.AdditionalTags, + signaturePolicyPath: options.SignaturePolicyPath, + systemContext: options.SystemContext, + volumeCache: make(map[string]string), + volumeCacheInfo: make(map[string]os.FileInfo), + log: options.Log, + out: options.Out, + err: options.Err, + reportWriter: options.ReportWriter, + namespaceOptions: options.NamespaceOptions, + configureNetwork: options.ConfigureNetwork, + cniPluginPath: options.CNIPluginPath, + cniConfigDir: options.CNIConfigDir, + idmappingOptions: options.IDMappingOptions, + commonBuildOptions: options.CommonBuildOpts, + defaultMountsFilePath: options.DefaultMountsFilePath, + iidfile: options.IIDFile, + squash: options.Squash, + labels: append([]string{}, options.Labels...), + annotations: append([]string{}, options.Annotations...), + layers: options.Layers, + noCache: options.NoCache, + removeIntermediateCtrs: options.RemoveIntermediateCtrs, + forceRmIntermediateCtrs: options.ForceRmIntermediateCtrs, } if exec.err == nil { exec.err = os.Stderr @@ -678,6 +688,7 @@ func (b *Executor) Delete() (err error) { func (b *Executor) Execute(ctx context.Context, ib *imagebuilder.Builder, node *parser.Node) error { checkForLayers := true children := node.Children + commitName := b.output for i, node := range node.Children { step := ib.Step() if err := step.Resolve(node); err != nil { @@ -700,6 +711,12 @@ func (b *Executor) Execute(ctx context.Context, ib *imagebuilder.Builder, node * continue } + if i < len(children)-1 { + b.output = "" + } else { + b.output = commitName + } + var ( cacheID string err error @@ -739,9 +756,11 @@ func (b *Executor) Execute(ctx context.Context, ib *imagebuilder.Builder, node * // it is used to create the container for the next step. imgID = cacheID } - // Delete the intermediate container. - if err := b.Delete(); err != nil { - return errors.Wrap(err, "error deleting intermediate container") + // Delete the intermediate container if b.removeIntermediateCtrs is true. + if b.removeIntermediateCtrs { + if err := b.Delete(); err != nil { + return errors.Wrap(err, "error deleting intermediate container") + } } // Prepare for the next step with imgID as the new base image. if i != len(children)-1 { @@ -1047,7 +1066,12 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) error if err := stageExecutor.Prepare(ctx, stage.Builder, stage.Node, ""); err != nil { return err } - defer stageExecutor.Delete() + // Always remove the intermediate/build containers, even if the build was unsuccessful. + // If building with layers, remove all intermediate/build containers if b.forceRmIntermediateCtrs + // is true. + if b.forceRmIntermediateCtrs || (!b.layers && !b.noCache) { + defer stageExecutor.Delete() + } if err := stageExecutor.Execute(ctx, stage.Builder, stage.Node); err != nil { return err } @@ -1056,7 +1080,21 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) error return nil } _, err := stageExecutor.Commit(ctx, stages[len(stages)-1].Builder, "") - return err + if err != nil { + return err + } + // If building with layers and b.removeIntermediateCtrs is true + // only remove intermediate container for each step if an error + // during the build process doesn't occur. + // If the build is unsuccessful, the container created at the step + // the failure happened will persist in the container store. + // This if condition will be false if not building with layers and + // the removal of intermediate/build containers will be handled by the + // defer statement above. + if b.removeIntermediateCtrs && (b.layers || b.noCache) { + return stageExecutor.Delete() + } + return nil } // BuildDockerfiles parses a set of one or more Dockerfiles (which may be diff --git a/pkg/cli/common.go b/pkg/cli/common.go index 615917a9b..554b30a29 100644 --- a/pkg/cli/common.go +++ b/pkg/cli/common.go @@ -109,7 +109,7 @@ var ( }, cli.BoolFlag{ Name: "force-rm", - Usage: "Always remove intermediate containers after a build. The build process does not currently support caching so this is a NOOP.", + Usage: "Always remove intermediate containers after a build, even if the build is unsuccessful.", }, cli.StringFlag{ Name: "format", @@ -147,9 +147,9 @@ var ( Name: "quiet, q", Usage: "refrain from announcing build instructions and image read/write progress", }, - cli.BoolFlag{ + cli.BoolTFlag{ Name: "rm", - Usage: "Remove intermediate containers after a successful build. The build process does not currently support caching so this is a NOOP.", + Usage: "Remove intermediate containers after a successful build (default true)", }, cli.StringFlag{ Name: "runtime", diff --git a/tests/bud.bats b/tests/bud.bats index 798e49766..8adc3326f 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -37,6 +37,40 @@ load helpers rm -rf ${TESTSDIR}/bud/use-layers/mount } +@test "bud with --rm flag" { + buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t test1 ${TESTSDIR}/bud/use-layers + run buildah --debug=false containers + [ $(wc -l <<< "$output") -eq 1 ] + [ "${status}" -eq 0 ] + + buildah bud --signature-policy ${TESTSDIR}/policy.json --rm=false --layers -t test2 ${TESTSDIR}/bud/use-layers + run buildah --debug=false containers + [ $(wc -l <<< "$output") -eq 4 ] + [ "${status}" -eq 0 ] + + buildah rm -a + buildah rmi -a +} + +@test "bud with --force-rm flag" { + run buildah bud --signature-policy ${TESTSDIR}/policy.json --force-rm --layers -t test1 -f Dockerfile.fail-case ${TESTSDIR}/bud/use-layers + echo "$output" + [ "$status" -ne 0 ] + run buildah --debug=false containers + [ $(wc -l <<< "$output") -eq 1 ] + [ "${status}" -eq 0 ] + + run buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t test2 -f Dockerfile.fail-case ${TESTSDIR}/bud/use-layers + echo "$output" + [ "$status" -ne 0 ] + run buildah --debug=false containers + [ $(wc -l <<< "$output") -eq 2 ] + [ "${status}" -eq 0 ] + + buildah rm -a + buildah rmi -a +} + @test "bud from base image should have base image ENV also" { buildah bud --signature-policy ${TESTSDIR}/policy.json -t test -f Dockerfile.check-env ${TESTSDIR}/bud/env cid=$(buildah from --signature-policy ${TESTSDIR}/policy.json test) @@ -649,26 +683,6 @@ load helpers buildah rmi ${target} } -@test "bud with --force-rm noop flag" { - target=noop-image - run buildah bud --force-rm --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/run-scenarios/Dockerfile.noop-flags ${TESTSDIR}/bud/run-scenarios - echo "$output" - [ "$status" -eq 0 ] - cid=$(buildah from ${target}) - buildah rm ${cid} - buildah rmi ${target} -} - -@test "bud with --rm noop flag" { - target=noop-image - run buildah bud --rm --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/run-scenarios/Dockerfile.noop-flags ${TESTSDIR}/bud/run-scenarios - echo "$output" - [ "$status" -eq 0 ] - cid=$(buildah from ${target}) - buildah rm ${cid} - buildah rmi ${target} -} - @test "bud with --cpu-shares flag, no argument" { target=bud-flag run buildah bud --cpu-shares --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/from-scratch/Dockerfile ${TESTSDIR}/bud/from-scratch diff --git a/tests/bud/use-layers/Dockerfile b/tests/bud/use-layers/Dockerfile index a10c9e855..f043aceb6 100644 --- a/tests/bud/use-layers/Dockerfile +++ b/tests/bud/use-layers/Dockerfile @@ -1,4 +1,4 @@ FROM alpine RUN mkdir /hello RUN touch file.txt -ENV foo=bar \ No newline at end of file +ENV foo=bar diff --git a/tests/bud/use-layers/Dockerfile.2 b/tests/bud/use-layers/Dockerfile.2 index 0c2d0eed3..5b9ab0f0a 100644 --- a/tests/bud/use-layers/Dockerfile.2 +++ b/tests/bud/use-layers/Dockerfile.2 @@ -1,4 +1,4 @@ FROM alpine RUN mkdir /hello RUN touch blah.txt -ENV foo=bar \ No newline at end of file +ENV foo=bar diff --git a/tests/bud/use-layers/Dockerfile.3 b/tests/bud/use-layers/Dockerfile.3 index 12762d971..a86ba55e1 100644 --- a/tests/bud/use-layers/Dockerfile.3 +++ b/tests/bud/use-layers/Dockerfile.3 @@ -2,4 +2,4 @@ FROM alpine RUN mkdir /hello RUN touch blah.txt COPY mount world/ -ENV foo=bar \ No newline at end of file +ENV foo=bar diff --git a/tests/bud/use-layers/Dockerfile.fail-case b/tests/bud/use-layers/Dockerfile.fail-case new file mode 100644 index 000000000..6b774c0fd --- /dev/null +++ b/tests/bud/use-layers/Dockerfile.fail-case @@ -0,0 +1,5 @@ +FROM alpine +RUN mkdir /hello +RUN touch blah.txt +COPY non-existent world/ +ENV foo=bar