From d80acf805b6fa5748b627b1b510a683f14fb2526 Mon Sep 17 00:00:00 2001 From: Marcos Lilljedahl Date: Tue, 5 Apr 2022 18:17:33 -0300 Subject: [PATCH 1/3] Add experimental way to set a target platform when building Add an --experimental-platform flag to the do command to allow overriding the default auto-detected build platform until we find the time to think about the definitive multi-platform builds UX Signed-off-by: Marcos Lilljedahl --- client/client.go | 7 ++-- cmd/dagger/cmd/common/common.go | 19 +++++++++-- cmd/dagger/cmd/do.go | 1 + plancontext/platform.go | 4 --- tests/plan.bats | 18 +++++----- ...nfig_platform_failure_invalid_platform.cue | 33 ------------------- .../platform/config_platform_linux_amd64.cue | 33 ------------------- .../platform/config_platform_linux_arm64.cue | 33 ------------------- tests/plan/platform/platform.cue | 12 +++++++ 9 files changed, 44 insertions(+), 116 deletions(-) delete mode 100644 tests/plan/platform/config_platform_failure_invalid_platform.cue delete mode 100644 tests/plan/platform/config_platform_linux_amd64.cue delete mode 100644 tests/plan/platform/config_platform_linux_arm64.cue create mode 100644 tests/plan/platform/platform.cue diff --git a/client/client.go b/client/client.go index 7d109113..975a9078 100644 --- a/client/client.go +++ b/client/client.go @@ -44,6 +44,8 @@ type Config struct { CacheExports []bk.CacheOptionsEntry CacheImports []bk.CacheOptionsEntry + + TargetPlatform *specs.Platform } func New(ctx context.Context, host string, cfg Config) (*Client, error) { @@ -81,8 +83,9 @@ func (c *Client) Do(ctx context.Context, pctx *plancontext.Context, fn DoFunc) e lg := log.Ctx(ctx) eg, gctx := errgroup.WithContext(ctx) - // if platform is set through plan config, skip detection. - if !pctx.Platform.IsSet() { + if c.cfg.TargetPlatform != nil { + pctx.Platform.Set(*c.cfg.TargetPlatform) + } else { p, err := c.detectPlatform(ctx) if err != nil { return err diff --git a/cmd/dagger/cmd/common/common.go b/cmd/dagger/cmd/common/common.go index b68e16db..114f262d 100644 --- a/cmd/dagger/cmd/common/common.go +++ b/cmd/dagger/cmd/common/common.go @@ -6,7 +6,9 @@ import ( "strings" "cuelang.org/go/cue" + "github.com/containerd/containerd/platforms" "github.com/docker/buildx/util/buildflags" + specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/rs/zerolog/log" "github.com/spf13/viper" "go.dagger.io/dagger/client" @@ -95,10 +97,21 @@ func NewClient(ctx context.Context) *client.Client { lg.Fatal().Err(err).Msg("unable to parse --cache-from options") } + ep := viper.GetString("experimental-platform") + var p *specs.Platform + if len(ep) > 0 { + pp, err := platforms.Parse(ep) + if err != nil { + lg.Fatal().Err(err).Msg("invalid value for --experimental-platform") + } + p = &pp + } + cl, err := client.New(ctx, "", client.Config{ - CacheExports: cacheExports, - CacheImports: cacheImports, - NoCache: viper.GetBool("no-cache"), + CacheExports: cacheExports, + CacheImports: cacheImports, + NoCache: viper.GetBool("no-cache"), + TargetPlatform: p, }) if err != nil { lg.Fatal().Err(err).Msg("unable to create client") diff --git a/cmd/dagger/cmd/do.go b/cmd/dagger/cmd/do.go index c11c2313..bced8559 100644 --- a/cmd/dagger/cmd/do.go +++ b/cmd/dagger/cmd/do.go @@ -157,6 +157,7 @@ func init() { doCmd.Flags().StringArrayP("with", "w", []string{}, "") doCmd.Flags().StringP("plan", "p", ".", "Path to plan (defaults to current directory)") doCmd.Flags().Bool("no-cache", false, "Disable caching") + doCmd.Flags().String("experimental-platform", "", "Set target build platform (experimental)") doCmd.Flags().StringArray("cache-to", []string{}, "Cache export destinations (eg. user/app:cache, type=local,dest=path/to/dir)") doCmd.Flags().StringArray("cache-from", []string{}, diff --git a/plancontext/platform.go b/plancontext/platform.go index 4ee46b82..47815f8f 100644 --- a/plancontext/platform.go +++ b/plancontext/platform.go @@ -25,7 +25,3 @@ func (c *platformContext) SetString(platform string) error { func (c *platformContext) Set(p specs.Platform) { c.platform = &p } - -func (c *platformContext) IsSet() bool { - return c.platform != nil -} diff --git a/tests/plan.bats b/tests/plan.bats index 69692a71..97b6d894 100644 --- a/tests/plan.bats +++ b/tests/plan.bats @@ -228,15 +228,17 @@ setup() { } @test "plan/platform" { + cd "$TESTDIR" - # Run with amd64 platform - run "$DAGGER" "do" -p./plan/platform/config_platform_linux_amd64.cue verify - - # Run with arm64 platform - run "$DAGGER" "do" -p./plan/platform/config_platform_linux_arm64.cue verify - - # Run with invalid platform - run "$DAGGER" "do" -p./plan/platform/config_platform_failure_invalid_platform.cue verify + # Run with invalid platform format + run "$DAGGER" "do" --experimental-platform invalid -p./plan/platform/platform.cue test assert_failure + assert_output --partial "unknown operating system or architecture: invalid argument" + + + # Run with non-existing platform + run "$DAGGER" "do" --experimental-platform invalid/invalid -p./plan/platform/platform.cue test + assert_failure + assert_output --partial "no match for platform in manifest" } diff --git a/tests/plan/platform/config_platform_failure_invalid_platform.cue b/tests/plan/platform/config_platform_failure_invalid_platform.cue deleted file mode 100644 index 6de027d8..00000000 --- a/tests/plan/platform/config_platform_failure_invalid_platform.cue +++ /dev/null @@ -1,33 +0,0 @@ -package main - -import ( - "dagger.io/dagger" - "dagger.io/dagger/core" -) - -dagger.#Plan & { - platform: "linux/unknown" - - actions: { - image: core.#Pull & { - source: "alpine:3.15.0" - } - - writeArch: core.#Exec & { - input: image.output - always: true - args: [ - "sh", "-c", #""" - echo -n $(uname -m) >> /arch.txt - """#, - ] - } - - verify: core.#ReadFile & { - input: writeArch.output - path: "/arch.txt" - } & { - contents: "s390x" - } - } -} diff --git a/tests/plan/platform/config_platform_linux_amd64.cue b/tests/plan/platform/config_platform_linux_amd64.cue deleted file mode 100644 index 366482ec..00000000 --- a/tests/plan/platform/config_platform_linux_amd64.cue +++ /dev/null @@ -1,33 +0,0 @@ -package main - -import ( - "dagger.io/dagger" - "dagger.io/dagger/core" -) - -dagger.#Plan & { - platform: "linux/amd64" - - actions: { - image: core.#Pull & { - source: "alpine:3.15.0" - } - - writeArch: core.#Exec & { - input: image.output - always: true - args: [ - "sh", "-c", #""" - echo -n $(uname -m) >> /arch.txt - """#, - ] - } - - verify: core.#ReadFile & { - input: writeArch.output - path: "/arch.txt" - } & { - contents: "x86_64" - } - } -} diff --git a/tests/plan/platform/config_platform_linux_arm64.cue b/tests/plan/platform/config_platform_linux_arm64.cue deleted file mode 100644 index 9ffc213e..00000000 --- a/tests/plan/platform/config_platform_linux_arm64.cue +++ /dev/null @@ -1,33 +0,0 @@ -package main - -import ( - "dagger.io/dagger" - "dagger.io/dagger/core" -) - -dagger.#Plan & { - platform: "linux/arm64" - - actions: { - image: core.#Pull & { - source: "alpine:3.15.0" - } - - writeArch: core.#Exec & { - input: image.output - always: true - args: [ - "sh", "-c", #""" - echo -n $(uname -m) >> /arch.txt - """#, - ] - } - - verify: core.#ReadFile & { - input: writeArch.output - path: "/arch.txt" - } & { - contents: "aarch64" - } - } -} diff --git a/tests/plan/platform/platform.cue b/tests/plan/platform/platform.cue new file mode 100644 index 00000000..7865414b --- /dev/null +++ b/tests/plan/platform/platform.cue @@ -0,0 +1,12 @@ +package main + +import ( + "dagger.io/dagger" + "dagger.io/dagger/core" +) + +dagger.#Plan & { + actions: test: image: core.#Pull & { + source: "alpine:3.15.0" + } +} From 8969507db6a45e85cfdc3b3417c81b50f52c94da Mon Sep 17 00:00:00 2001 From: Marcos Lilljedahl Date: Wed, 6 Apr 2022 13:34:43 -0300 Subject: [PATCH 2/3] Add global --experimental flag to gatekeep some features This commit adds a global --experiemntal flag so we can start gatekeeping some features where we know beforehand that the UX will very likely change. It also refactors the current --platform flag to be avaiable under this experimental flag for the moment Signed-off-by: Marcos Lilljedahl --- ci.cue | 5 ----- cmd/dagger/cmd/common/common.go | 4 ++-- cmd/dagger/cmd/do.go | 7 ++++++- cmd/dagger/cmd/root.go | 1 + plan/plan.go | 3 ++- solver/solver.go | 1 - tests/plan.bats | 10 ++++++++-- 7 files changed, 19 insertions(+), 12 deletions(-) diff --git a/ci.cue b/ci.cue index 4474d0f5..23ff4a9c 100644 --- a/ci.cue +++ b/ci.cue @@ -14,11 +14,6 @@ import ( ) dagger.#Plan & { - // FIXME: Ideally we would want to automatically set the platform's arch identical to the host - // to avoid the performance hit caused by qemu (linter goes from <3s to >3m when arch is x86) - // Uncomment if running locally on Mac M1 to bypass qemu - // platform: "linux/aarch64" - // platform: "linux/amd64" client: filesystem: ".": read: exclude: [ "bin", diff --git a/cmd/dagger/cmd/common/common.go b/cmd/dagger/cmd/common/common.go index 114f262d..0139d286 100644 --- a/cmd/dagger/cmd/common/common.go +++ b/cmd/dagger/cmd/common/common.go @@ -97,12 +97,12 @@ func NewClient(ctx context.Context) *client.Client { lg.Fatal().Err(err).Msg("unable to parse --cache-from options") } - ep := viper.GetString("experimental-platform") + ep := viper.GetString("platform") var p *specs.Platform if len(ep) > 0 { pp, err := platforms.Parse(ep) if err != nil { - lg.Fatal().Err(err).Msg("invalid value for --experimental-platform") + lg.Fatal().Err(err).Msg("invalid value for --platform") } p = &pp } diff --git a/cmd/dagger/cmd/do.go b/cmd/dagger/cmd/do.go index bced8559..305fff51 100644 --- a/cmd/dagger/cmd/do.go +++ b/cmd/dagger/cmd/do.go @@ -42,6 +42,11 @@ var doCmd = &cobra.Command{ err error ) + switch !viper.GetBool("experimental") { + case len(viper.GetString("platform")) > 0: + lg.Fatal().Err(err).Msg("--platform requires --experimental flag") + } + if f := viper.GetString("log-format"); f == "tty" || f == "auto" && term.IsTerminal(int(os.Stdout.Fd())) { tty, err = logger.NewTTYOutput(os.Stderr) if err != nil { @@ -157,7 +162,7 @@ func init() { doCmd.Flags().StringArrayP("with", "w", []string{}, "") doCmd.Flags().StringP("plan", "p", ".", "Path to plan (defaults to current directory)") doCmd.Flags().Bool("no-cache", false, "Disable caching") - doCmd.Flags().String("experimental-platform", "", "Set target build platform (experimental)") + doCmd.Flags().String("platform", "", "Set target build platform (requires experimental)") doCmd.Flags().StringArray("cache-to", []string{}, "Cache export destinations (eg. user/app:cache, type=local,dest=path/to/dir)") doCmd.Flags().StringArray("cache-from", []string{}, diff --git a/cmd/dagger/cmd/root.go b/cmd/dagger/cmd/root.go index f49e0422..304d7cdd 100644 --- a/cmd/dagger/cmd/root.go +++ b/cmd/dagger/cmd/root.go @@ -23,6 +23,7 @@ var rootCmd = &cobra.Command{ func init() { rootCmd.PersistentFlags().String("log-format", "auto", "Log format (auto, plain, tty, json)") rootCmd.PersistentFlags().StringP("log-level", "l", "info", "Log level") + rootCmd.PersistentFlags().Bool("experimental", false, "Enable experimental features") rootCmd.PersistentPreRun = func(cmd *cobra.Command, _ []string) { go checkVersion() diff --git a/plan/plan.go b/plan/plan.go index 033b8248..6b197be4 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -109,7 +109,8 @@ func (p *Plan) Action() *Action { // configPlatform load the platform specified in the context // Buildkit will then run every operation using that platform // If platform is not define, context keep default platform -// FIXME: `platform` field temporarily disabled +// FIXME: `platform` field temporarily disabled until we decide the proper +// DX for multi-platform builds // func (p *Plan) configPlatform() error { // platformField := p.source.Lookup("platform") diff --git a/solver/solver.go b/solver/solver.go index 56ee900c..09439331 100644 --- a/solver/solver.go +++ b/solver/solver.go @@ -85,7 +85,6 @@ func (s *Solver) AddCredentials(target, username, secret string) { } func (s *Solver) Marshal(ctx context.Context, st llb.State, co ...llb.ConstraintsOpt) (*bkpb.Definition, error) { - // FIXME: do not hardcode the platform def, err := st.Marshal(ctx, co...) if err != nil { return nil, err diff --git a/tests/plan.bats b/tests/plan.bats index 97b6d894..79f9880d 100644 --- a/tests/plan.bats +++ b/tests/plan.bats @@ -232,13 +232,19 @@ setup() { cd "$TESTDIR" # Run with invalid platform format - run "$DAGGER" "do" --experimental-platform invalid -p./plan/platform/platform.cue test + run "$DAGGER" "do" --experimental --platform invalid -p./plan/platform/platform.cue test assert_failure assert_output --partial "unknown operating system or architecture: invalid argument" + # Require --experimental flag + run "$DAGGER" "do" --platform linux/arm64 -p./plan/platform/platform.cue test + assert_failure + assert_output --partial "--platform requires --experimental flag" + + # Run with non-existing platform - run "$DAGGER" "do" --experimental-platform invalid/invalid -p./plan/platform/platform.cue test + run "$DAGGER" "do" --experimental --platform invalid/invalid -p./plan/platform/platform.cue test assert_failure assert_output --partial "no match for platform in manifest" } From 2ec6a398c2a6534f075275bfc42545a53b932fdc Mon Sep 17 00:00:00 2001 From: Marcos Lilljedahl Date: Wed, 6 Apr 2022 14:29:36 -0300 Subject: [PATCH 3/3] docs: improve comment wording Signed-off-by: Marcos Lilljedahl --- client/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/client.go b/client/client.go index 975a9078..f24be978 100644 --- a/client/client.go +++ b/client/client.go @@ -110,8 +110,8 @@ func (c *Client) Do(ctx context.Context, pctx *plancontext.Context, fn DoFunc) e return eg.Wait() } -// detectPlatform will try to automatically Buildkit's target platform. -// If not possible, default platform will be used. +// detectPlatform tries using Buildkit's target platform; +// if not possible, default platform will be used. func (c *Client) detectPlatform(ctx context.Context) (*specs.Platform, error) { w, err := c.c.ListWorkers(ctx) if err != nil {