Skip to content

Commit

Permalink
Setting --arch should set the TARGETARCH build arg
Browse files Browse the repository at this point in the history
Also fix a long standing FIXME in the test framework.

Signed-off-by: Daniel J Walsh <[email protected]>
  • Loading branch information
rhatdan committed Apr 20, 2024
1 parent e393e57 commit 1b80d3e
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 17 deletions.
4 changes: 4 additions & 0 deletions imagebuildah/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -221,6 +222,9 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options define.B
systemContext := options.SystemContext
for _, platform := range options.Platforms {
platformContext := *systemContext
if platform.OS == "" {
platform.OS = runtime.GOOS
}
platformSpec := internalUtil.NormalizePlatform(v1.Platform{
OS: platform.OS,
Architecture: platform.Arch,
Expand Down
20 changes: 16 additions & 4 deletions pkg/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,32 +532,41 @@ func PlatformFromOptions(c *cobra.Command) (os, arch string, err error) {
// (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
var (
os = ""
arch = ""
variant string
osFlags = 0
)
if c.Flag("os").Changed {
if os, err = c.Flags().GetString("os"); err != nil {
return nil, err
}
osFlags++
}

if c.Flag("arch").Changed {
if arch, err = c.Flags().GetString("arch"); err != nil {
return nil, err
}
osFlags++
}
if c.Flag("variant").Changed {
if variant, err = c.Flags().GetString("variant"); err != nil {
return nil, err
}
osFlags++
}
platforms = []struct{ OS, Arch, Variant string }{{os, arch, variant}}
if c.Flag("platform").Changed {
if osFlags > 0 {
return nil, fmt.Errorf("invalid --platform may not be used with --os, --arch, or --variant")
}
platforms = nil
platformSpecs, err := c.Flags().GetStringSlice("platform")
if err != nil {
return nil, fmt.Errorf("unable to parse platform: %w", err)
}
if os != "" || arch != "" || variant != "" {
return nil, fmt.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, fmt.Errorf("unable to parse platform %q: %w", pf, err)
Expand All @@ -584,6 +593,9 @@ func Platform(platform string) (os, arch, variant string, err error) {
if err != nil {
return "", "", "", fmt.Errorf("invalid platform syntax for --platform=%q: %w", platform, err)
}
/* if platformSpec.OS == "" {
platformSpec.OS = runtime.GOOS
}*/
return platformSpec.OS, platformSpec.Architecture, platformSpec.Variant, nil
}

Expand Down
1 change: 1 addition & 0 deletions pkg/parse/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ func TestSystemContextFromFlagSet(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, sc, &types.SystemContext{
BigFilesTemporaryDir: GetTempDir(),
OSChoice: "linux",
DockerInsecureSkipTLSVerify: types.OptionalBoolFalse,
DockerRegistryUserAgent: fmt.Sprintf("Buildah/%s", define.Version),
})
Expand Down
47 changes: 34 additions & 13 deletions tests/bud.bats
Original file line number Diff line number Diff line change
Expand Up @@ -242,23 +242,44 @@ expect_output --substring "hello"
# With explicit and full --platform, buildah should not warn.
run_buildah build $WITH_POLICY_JSON --platform linux/amd64/v2 \
-t source -f $containerfile
assert "$output" !~ "missing .* build argument" \
"With explicit --platform, buildah should not warn"
assert "$output" =~ "image platform \(linux/amd64\) does not match the expected platform" \
"With explicit --platform, buildah should warn about pulling difference in platform"
assert "$output" =~ "TARGETOS=linux" " --platform TARGETOS set correctly"
assert "$output" =~ "TARGETARCH=amd64" " --platform TARGETARCH set correctly"
assert "$output" =~ "TARGETVARIANT=" " --platform TARGETVARIANT set correctly"
assert "$output" =~ "TARGETPLATFORM=linux/amd64/v2" " --platform TARGETPLATFORM set correctly"

# Likewise with individual args
run_buildah build $WITH_POLICY_JSON --os linux --arch amd64 --variant v2 \
-t source -f $containerfile
assert "$output" !~ "missing .* build argument" \
"With explicit --os + --arch + --variant, buildah should not warn"

# FIXME FIXME FIXME: #4319: with --os only, buildah should not warn about OS
if false; then
run_buildah build $WITH_POLICY_JSON --os linux \
-t source -f $containerfile
assert "$output" !~ "missing.*TARGETOS" \
"With explicit --os (but no arch/variant), buildah should not warn about TARGETOS"
# FIXME: add --arch test too, and maybe make this cleaner
fi
assert "$output" =~ "image platform \(linux/amd64\) does not match the expected platform" \
"With explicit --variant, buildah should warn about pulling difference in platform"
assert "$output" =~ "TARGETOS=linux" "--os --arch --variant TARGETOS set correctly"
assert "$output" =~ "TARGETARCH=amd64" "--os --arch --variant TARGETARCH set correctly"
assert "$output" =~ "TARGETVARIANT=" "--os --arch --variant TARGETVARIANT set correctly"
assert "$output" =~ "TARGETPLATFORM=linux/amd64" "--os --arch --variant TARGETPLATFORM set correctly"

run_buildah build $WITH_POLICY_JSON --os linux -t source -f $containerfile
assert "$output" !~ "WARNING" \
"With explicit --os (but no arch/variant), buildah should not warn about TARGETOS"
assert "$output" =~ "TARGETOS=linux" "--os TARGETOS set correctly"
assert "$output" =~ "TARGETARCH=amd64" "--os TARGETARCH set correctly"
assert "$output" =~ "TARGETVARIANT=" "--os TARGETVARIANT set correctly"
assert "$output" =~ "TARGETPLATFORM=linux/amd64" "--os TARGETPLATFORM set correctly"

run_buildah build $WITH_POLICY_JSON --arch amd64 -t source -f $containerfile
assert "$output" !~ "WARNING" \
"With explicit --os (but no arch/variant), buildah should not warn about TARGETOS"
assert "$output" =~ "TARGETOS=linux" "--arch TARGETOS set correctly"
assert "$output" =~ "TARGETARCH=amd64" "--arch TARGETARCH set correctly"
assert "$output" =~ "TARGETVARIANT=" "--arch TARGETVARIANT set correctly"
assert "$output" =~ "TARGETPLATFORM=linux/amd64" "--arch TARGETPLATFORM set correctly"

for option in "--arch=arm64" "--os=windows" "--variant=v2"; do
run_buildah 125 build $WITH_POLICY_JSON --platform linux/amd64 ${option} \
-t source -f $containerfile
assert "$output" =~ "Error: building system context: invalid --platform may not be used with --os, --arch, or --variant" "can't use --platform and one of --os, --arch or --variant together"
done

}

Expand Down
4 changes: 4 additions & 0 deletions tests/bud/platform-sets-args/Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ ARG TARGETOS
ARG TARGETPLATFORM
ARG TARGETVARIANT
ENV nothing="multiarch-safe statement that will result in a built image"
run echo TARGETARCH=${TARGETARCH}
run echo TARGETOS=${TARGETOS}
run echo TARGETPLATFORM=${TARGETPLATFORM}
run echo TARGETVARIANT=${TARGETVARANT}
1 change: 1 addition & 0 deletions tests/conformance/conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3566,6 +3566,7 @@ func TestCommit(t *testing.T) {
derivedBuilder, err := buildah.NewBuilder(ctx, store, buildah.BuilderOptions{
FromImage: buildahID,
})
assert.NoErrorf(t, err, "creating a new Builder")
defer func(builder *buildah.Builder) {
err := builder.Delete()
assert.NoErrorf(t, err, "removing the derived container")
Expand Down

0 comments on commit 1b80d3e

Please sign in to comment.