From 00e12676b09d87801a0922f623ce657fa106e77f Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 13 May 2024 14:34:56 -0700 Subject: [PATCH 1/3] tests: Add support for buildx dial-stdio This makes it so the test suite can connect to any buildx instance using `docker buildx dial-stdio`. In this case the client connection is proxied over the stdin/stdout of the buildx command. Signed-off-by: Brian Goff --- test/testenv/buildx.go | 145 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 138 insertions(+), 7 deletions(-) diff --git a/test/testenv/buildx.go b/test/testenv/buildx.go index 63451da6..99de15eb 100644 --- a/test/testenv/buildx.go +++ b/test/testenv/buildx.go @@ -1,14 +1,19 @@ package testenv import ( + "bufio" + "bytes" "context" "encoding/json" "errors" "fmt" + "io" "net" "os" "os/exec" "path/filepath" + "strconv" + "strings" "sync" "testing" @@ -18,10 +23,9 @@ import ( "github.com/cpuguy83/go-docker/container" "github.com/cpuguy83/go-docker/transport" "github.com/moby/buildkit/client" + gwclient "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/solver/pb" pkgerrors "github.com/pkg/errors" - - gwclient "github.com/moby/buildkit/frontend/gateway/client" ) type BuildxEnv struct { @@ -57,6 +61,123 @@ func (b *BuildxEnv) Load(ctx context.Context, id string, f gwclient.BuildFunc) e return nil } +func (b *BuildxEnv) version(ctx context.Context) (string, error) { + cmd := exec.CommandContext(ctx, "docker", "buildx", "version") + out, err := cmd.CombinedOutput() + if err != nil { + return "", pkgerrors.Wrap(err, string(out)) + } + + fields := strings.Fields(string(out)) + + if len(fields) != 3 { + return "", errors.New("could not determine buildx version") + } + + ver, _, _ := strings.Cut(strings.TrimPrefix(fields[1], "v"), "-") + if strings.Count(ver, ".") < 2 { + return "", fmt.Errorf("unexpected version format: %s", ver) + } + return ver, nil +} + +func (b *BuildxEnv) supportsDialStdio(ctx context.Context) (bool, error) { + ver, err := b.version(ctx) + if err != nil { + return false, err + } + + majorStr, other, _ := strings.Cut(ver, ".") + major, err := strconv.Atoi(majorStr) + if err != nil { + return false, pkgerrors.Wrapf(err, "could not parse major version number: %s", ver) + } + if major > 0 { + return true, nil + } + + minorStr, _, _ := strings.Cut(other, ".") + minor, err := strconv.Atoi(minorStr) + if err != nil { + return false, pkgerrors.Wrapf(err, "could not parse major version number: %s", ver) + } + return minor >= 13, nil +} + +var errDialStdioNotSupportedErr = errors.New("buildx dial-stdio not supported") + +func (b *BuildxEnv) dialStdio(ctx context.Context) (bool, error) { + ok, err := b.supportsDialStdio(ctx) + if err != nil { + return false, fmt.Errorf("%w: %w", errDialStdioNotSupportedErr, err) + } + + if !ok { + return false, nil + } + + c, err := client.New(ctx, "", client.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + args := []string{"buildx", "dial-stdio", "--progress=plain"} + if b.builder != "" { + args = append(args, "--builder="+b.builder) + } + + cmd := exec.CommandContext(ctx, "docker", args...) + cmd.Env = os.Environ() + + c1, c2 := net.Pipe() + cmd.Stdin = c1 + cmd.Stdout = c1 + + // Use a pipe to check when the connection is actually complete + // Also write all of stderr to an error buffer so we can have more details + // in the error message when the command fails. + r, w := io.Pipe() + errBuf := bytes.NewBuffer(nil) + ww := io.MultiWriter(w, errBuf) + cmd.Stderr = ww + + if err := cmd.Start(); err != nil { + return nil, err + } + + go func() { + err := cmd.Wait() + c1.Close() + // pkgerrors.Wrap will return nil if err is nil, otherwise it will give + // us a wrapped error with the buffered stderr fromt he command. + w.CloseWithError(pkgerrors.Wrapf(err, "%s", errBuf)) + }() + + defer r.Close() + + scanner := bufio.NewScanner(r) + for scanner.Scan() { + txt := strings.ToLower(scanner.Text()) + + if strings.HasPrefix(txt, "#1 dialing builder") && strings.HasSuffix(txt, "done") { + go func() { + // Continue draining stderr so the process does not get blocked + _, _ = io.Copy(io.Discard, r) + }() + break + } + } + if err := scanner.Err(); err != nil { + return nil, err + } + + return c2, nil + })) + + if err != nil { + return false, err + } + + b.client = c + return true, nil +} + // bootstrap is ultimately responsible for creating a buildkit client. // It looks like the buildx config on the client (typically in $HOME/.docker/buildx) to determine how to connect to the configured buildkit. func (b *BuildxEnv) bootstrap(ctx context.Context) (retErr error) { @@ -72,7 +193,7 @@ func (b *BuildxEnv) bootstrap(ctx context.Context) (retErr error) { b.supportedOnce.Do(func() { info, err := b.client.Info(ctx) if err != nil { - b.supportedErr = err + b.supportedErr = pkgerrors.WithStack(err) return } @@ -87,9 +208,19 @@ func (b *BuildxEnv) bootstrap(ctx context.Context) (retErr error) { } }() + ok, err := b.dialStdio(ctx) + if err != nil && !errors.Is(err, errDialStdioNotSupportedErr) { + return err + } + + if ok { + return nil + } + + // Fallback for older versions of buildx p, err := dockercfg.ConfigPath() if err != nil { - return err + return pkgerrors.WithStack(err) } if out, err := exec.Command("docker", "buildx", "inspect", "--bootstrap", b.builder).CombinedOutput(); err != nil { @@ -121,12 +252,12 @@ func (b *BuildxEnv) bootstrap(ctx context.Context) (retErr error) { if r.Key != "" { tr, err = transport.FromConnectionString(r.Key) if err != nil { - return err + return pkgerrors.Wrap(err, r.Key) } } else { tr, err = transport.DefaultTransport() if err != nil { - return err + return pkgerrors.WithStack(err) } } @@ -252,7 +383,7 @@ func withResolveLocal(so *client.SolveOpt) { func (b *BuildxEnv) RunTest(ctx context.Context, t *testing.T, f gwclient.BuildFunc) { c, err := b.Buildkit(ctx) if err != nil { - t.Fatal(err) + t.Fatalf("%+v", err) } ch, done := displaySolveStatus(ctx, t) From ac1aea069fd3e9a756eefd5fc2140abc69c2c6e8 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 13 May 2024 14:40:34 -0700 Subject: [PATCH 2/3] test: Remove fallback for buildx dial-stdio Remove the code that falls back to the old behavior of trying to parse the buildx directory. This makes tests require buildx >= 0.13. Signed-off-by: Brian Goff --- go.mod | 3 - go.sum | 6 -- test/testenv/buildx.go | 163 +++-------------------------------------- 3 files changed, 9 insertions(+), 163 deletions(-) diff --git a/go.mod b/go.mod index b2fb7101..2e7cb1a8 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,6 @@ toolchain go1.21.0 require ( github.com/containerd/containerd v1.7.13 - github.com/cpuguy83/dockercfg v0.3.1 - github.com/cpuguy83/go-docker v0.3.0 - github.com/cpuguy83/go-docker/buildkitopt v0.1.2 github.com/goccy/go-yaml v1.11.3 github.com/google/go-cmp v0.6.0 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go.sum b/go.sum index 337952b3..a377d724 100644 --- a/go.sum +++ b/go.sum @@ -55,12 +55,6 @@ github.com/containerd/ttrpc v1.2.2 h1:9vqZr0pxwOF5koz6N0N3kJ0zDHokrcPxIR/ZR2YFtO github.com/containerd/ttrpc v1.2.2/go.mod h1:sIT6l32Ph/H9cvnJsfXM5drIVzTr5A2flTf1G5tYZak= github.com/containerd/typeurl/v2 v2.1.1 h1:3Q4Pt7i8nYwy2KmQWIw2+1hTvwTE/6w9FqcttATPO/4= github.com/containerd/typeurl/v2 v2.1.1/go.mod h1:IDp2JFvbwZ31H8dQbEIY7sDl2L3o3HZj1hsSQlywkQ0= -github.com/cpuguy83/dockercfg v0.3.1 h1:/FpZ+JaygUR/lZP2NlFI2DVfrOEMAIKP5wWEJdoYe9E= -github.com/cpuguy83/dockercfg v0.3.1/go.mod h1:sugsbF4//dDlL/i+S+rtpIWp+5h0BHJHfjj5/jFyUJc= -github.com/cpuguy83/go-docker v0.3.0 h1:O88rocdycYvY+pUYYp0i1rRDANXHurNir3VE0F/PH3g= -github.com/cpuguy83/go-docker v0.3.0/go.mod h1:R2HgB/m54W+2dhYc70Xm78yS6o775SfN09bGIPSfQZQ= -github.com/cpuguy83/go-docker/buildkitopt v0.1.2 h1:ikh1gGd33k+SIUYvz86cN0kq9p1KhIJox5tIXcemv74= -github.com/cpuguy83/go-docker/buildkitopt v0.1.2/go.mod h1:BpQa6UGlRuOHv/oTI83l1h+14DYWwh0eMaDaC9essbg= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/test/testenv/buildx.go b/test/testenv/buildx.go index 99de15eb..75375f32 100644 --- a/test/testenv/buildx.go +++ b/test/testenv/buildx.go @@ -4,24 +4,17 @@ import ( "bufio" "bytes" "context" - "encoding/json" "errors" "fmt" "io" "net" "os" "os/exec" - "path/filepath" "strconv" "strings" "sync" "testing" - "github.com/cpuguy83/dockercfg" - "github.com/cpuguy83/go-docker" - "github.com/cpuguy83/go-docker/buildkitopt" - "github.com/cpuguy83/go-docker/container" - "github.com/cpuguy83/go-docker/transport" "github.com/moby/buildkit/client" gwclient "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/solver/pb" @@ -34,9 +27,6 @@ type BuildxEnv struct { mu sync.Mutex client *client.Client - ctr *container.Container - docker *docker.Client - supportedOnce sync.Once supportedErr error @@ -104,18 +94,9 @@ func (b *BuildxEnv) supportsDialStdio(ctx context.Context) (bool, error) { return minor >= 13, nil } -var errDialStdioNotSupportedErr = errors.New("buildx dial-stdio not supported") - -func (b *BuildxEnv) dialStdio(ctx context.Context) (bool, error) { - ok, err := b.supportsDialStdio(ctx) - if err != nil { - return false, fmt.Errorf("%w: %w", errDialStdioNotSupportedErr, err) - } - - if !ok { - return false, nil - } +var errDialStdioNotSupported = errors.New("buildx dial-stdio not supported") +func (b *BuildxEnv) dialStdio(ctx context.Context) error { c, err := client.New(ctx, "", client.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { args := []string{"buildx", "dial-stdio", "--progress=plain"} if b.builder != "" { @@ -171,15 +152,14 @@ func (b *BuildxEnv) dialStdio(ctx context.Context) (bool, error) { })) if err != nil { - return false, err + return err } b.client = c - return true, nil + return nil } // bootstrap is ultimately responsible for creating a buildkit client. -// It looks like the buildx config on the client (typically in $HOME/.docker/buildx) to determine how to connect to the configured buildkit. func (b *BuildxEnv) bootstrap(ctx context.Context) (retErr error) { if b.client != nil { return nil @@ -208,141 +188,16 @@ func (b *BuildxEnv) bootstrap(ctx context.Context) (retErr error) { } }() - ok, err := b.dialStdio(ctx) - if err != nil && !errors.Is(err, errDialStdioNotSupportedErr) { - return err - } - - if ok { - return nil - } - - // Fallback for older versions of buildx - p, err := dockercfg.ConfigPath() - if err != nil { - return pkgerrors.WithStack(err) - } - - if out, err := exec.Command("docker", "buildx", "inspect", "--bootstrap", b.builder).CombinedOutput(); err != nil { - return pkgerrors.Wrapf(err, "failed to bootstrap builder: %s", out) - } - - configBase := filepath.Join(filepath.Dir(p), "buildx") - - // builder is empty, so we need to check what the currently configured buildx builder is. - // This is stored int he buildx config in (typically) $HOME/.docker/buildx (the `dockercfg` lib determines where this actually is). - if b.builder == "" { - dt, err := os.ReadFile(filepath.Join(configBase, "current")) - if err != nil { - return pkgerrors.Wrap(err, "failed to read current builder") - } - - type ref struct { - Name string - Key string - } - var r ref - if err := json.Unmarshal(dt, &r); err != nil { - return err - } - - if r.Name == "" { - // This is the "default" buildx instance, aka dockerd's built-in buildkit. - var tr transport.Doer - if r.Key != "" { - tr, err = transport.FromConnectionString(r.Key) - if err != nil { - return pkgerrors.Wrap(err, r.Key) - } - } else { - tr, err = transport.DefaultTransport() - if err != nil { - return pkgerrors.WithStack(err) - } - } - - b.client, err = client.New(ctx, "", buildkitopt.FromDocker(tr)...) - return err - } - - b.builder = r.Name - } - - dt, err := os.ReadFile(filepath.Join(configBase, "instances", b.builder)) + ok, err := b.supportsDialStdio(ctx) if err != nil { - return pkgerrors.Wrap(err, "failed to read buildx instance config") + return fmt.Errorf("%w: %w", errDialStdioNotSupported, err) } - var cfg buildxConfig - if err := json.Unmarshal(dt, &cfg); err != nil { - return pkgerrors.Wrap(err, "failed to unmarshal buildx config") - } - - if cfg.Driver != "docker-container" { - return pkgerrors.Errorf("unsupported buildx driver: %s", cfg.Driver) - } - - if len(cfg.Nodes) == 0 { - return pkgerrors.Errorf("no buildx nodes configured") - } - - // On a typical client this would be a single node, but there could be multiple registered with he same builder name. - // We'll just try them all until we find one that works. - var errs []error - for _, n := range cfg.Nodes { - tr, err := transport.FromConnectionString(n.Endpoint) - if err != nil { - errs = append(errs, fmt.Errorf("%s: %w", n.Endpoint, err)) - continue - } - - dc := docker.NewClient(docker.WithTransport(tr)) - ctr := dc.ContainerService().NewContainer(ctx, "buildx_buildkit_"+n.Name) - - conn1, conn2 := net.Pipe() - ep, err := ctr.Exec(ctx, container.WithExecCmd("buildctl", "dial-stdio"), func(cfg *container.ExecConfig) { - cfg.Stdin = conn1 - cfg.Stdout = conn1 - cfg.Stderr = conn1 - }) - if err != nil { - conn1.Close() - conn2.Close() - errs = append(errs, fmt.Errorf("%s: %w", n.Endpoint, err)) - continue - } - - if err := ep.Start(ctx); err != nil { - conn1.Close() - conn2.Close() - errs = append(errs, fmt.Errorf("%s: %w", n.Endpoint, err)) - continue - } - - c, err := client.New(ctx, "", client.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { - return conn2, nil - })) - if err != nil { - errs = append(errs, fmt.Errorf("%s: %w", n.Endpoint, err)) - continue - } - - b.client = c - b.ctr = ctr - b.docker = dc - return nil + if !ok { + return errDialStdioNotSupported } - // Could not create a buildkit client, return all errors. - return errors.Join(errs...) -} - -type buildxConfig struct { - Driver string - Nodes []struct { - Name string - Endpoint string - } + return b.dialStdio(ctx) } func (b *BuildxEnv) Buildkit(ctx context.Context) (*client.Client, error) { From a07790740999f5e7f519150d22b210256394d4c1 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 14 May 2024 09:04:20 -0700 Subject: [PATCH 3/3] tests: Support arm64 clients and workers 1. In the test fixtures we are generating LLB from the client. This LLB was assuming the clients platform, however this should use the worker's platform. This allows the fixtures to be built from a Mac client (e.g. Docker for Mac). 2. There is currently no Windows arm64 image as such just set the tests to use amd64 as an explicit platform. Signed-off-by: Brian Goff --- frontend/windows/handle_container.go | 6 +++++- frontend/windows/handle_zip.go | 2 +- test/fixtures/phony.go | 3 ++- test/fixtures/signer.go | 3 ++- test/windows_test.go | 22 +++++++++++++++++----- 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/frontend/windows/handle_container.go b/frontend/windows/handle_container.go index 889d3e57..9c2790a1 100644 --- a/frontend/windows/handle_container.go +++ b/frontend/windows/handle_container.go @@ -20,7 +20,11 @@ const ( var ( defaultPlatform = ocispecs.Platform{ - OS: outputKey, + OS: outputKey, + // NOTE: Windows is (currently) only supported on amd64. + // Making this use runtime.GOARCH so that builds are more explicitly and not suprising. + // If/when Windows is supported on another platform (ie arm64) this will work as expected. + // Until then, if someone really wants to build an amd64 image from arm64 they'll need to set the platform explicitly in the build request. Architecture: runtime.GOARCH, } ) diff --git a/frontend/windows/handle_zip.go b/frontend/windows/handle_zip.go index 7518be30..f575c206 100644 --- a/frontend/windows/handle_zip.go +++ b/frontend/windows/handle_zip.go @@ -148,7 +148,7 @@ func buildBinaries(spec *dalec.Spec, worker llb.State, sOpt dalec.SourceOpts, ta sources, err := specToSourcesLLB(worker, spec, sOpt) if err != nil { - return llb.Scratch(), err + return llb.Scratch(), errors.Wrap(err, "could not generate sources") } patched := dalec.PatchSources(worker, spec, sources) diff --git a/test/fixtures/phony.go b/test/fixtures/phony.go index b7dadbb2..ee60f234 100644 --- a/test/fixtures/phony.go +++ b/test/fixtures/phony.go @@ -28,7 +28,8 @@ func PhonyFrontend(ctx context.Context, gwc gwclient.Client) (*gwclient.Result, return nil, err } - st := llb.Image("golang:1.21", llb.WithMetaResolver(gwc)). + p := llb.Platform(dc.BuildPlatforms[0]) + st := llb.Image("golang:1.22", llb.WithMetaResolver(gwc), p). Run( llb.Args([]string{"go", "build", "-o=/build/out", "./test/fixtures/phony"}), llb.AddEnv("CGO_ENABLED", "0"), diff --git a/test/fixtures/signer.go b/test/fixtures/signer.go index 677440f3..bece95ce 100644 --- a/test/fixtures/signer.go +++ b/test/fixtures/signer.go @@ -23,7 +23,8 @@ func PhonySigner(ctx context.Context, gwc gwclient.Client) (*gwclient.Result, er return nil, err } - st := llb.Image("golang:1.21", llb.WithMetaResolver(gwc)). + p := llb.Platform(dc.BuildPlatforms[0]) + st := llb.Image("golang:1.21", llb.WithMetaResolver(gwc), p). Run( llb.Args([]string{"go", "build", "-o=/build/out", "./test/fixtures/signer"}), llb.AddEnv("CGO_ENABLED", "0"), diff --git a/test/windows_test.go b/test/windows_test.go index a6bd828f..f277183f 100644 --- a/test/windows_test.go +++ b/test/windows_test.go @@ -21,6 +21,18 @@ func TestWindows(t *testing.T) { } func testWindows(ctx context.Context, t *testing.T, buildTarget string) { + // Windows is only supported on amd64 (ie there is no arm64 windows image currently) + // This allows the test to run on arm64 machines. + // I looked at having a good way to skip the test on non-amd64 and it all ends up + // being a bit janky and error prone. + // I'd rather just let the test run since it will work when we set an explicit platform + withAmd64Platform := func(sr *gwclient.SolveRequest) { + if sr.FrontendOpt == nil { + sr.FrontendOpt = make(map[string]string) + } + sr.FrontendOpt["platform"] = "windows/amd64" + } + t.Run("Fail when non-zero exit code during build", func(t *testing.T) { t.Parallel() spec := dalec.Spec{ @@ -42,7 +54,7 @@ func testWindows(ctx context.Context, t *testing.T, buildTarget string) { } testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) (*gwclient.Result, error) { - sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(buildTarget)) + sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(buildTarget), withAmd64Platform) sr.Evaluate = true _, err := gwc.Solve(ctx, sr) var xErr *moby_buildkit_v1_frontend.ExitError @@ -74,7 +86,7 @@ func testWindows(ctx context.Context, t *testing.T, buildTarget string) { } testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) (*gwclient.Result, error) { - sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(buildTarget)) + sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(buildTarget), withAmd64Platform) sr.Evaluate = true _, err := gwc.Solve(ctx, sr) @@ -209,7 +221,7 @@ echo "$BAR" > bar.txt } testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) (*gwclient.Result, error) { - sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(buildTarget)) + sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(buildTarget), withAmd64Platform) sr.Evaluate = true res, err := gwc.Solve(ctx, sr) if err != nil { @@ -301,7 +313,7 @@ echo "$BAR" > bar.txt }, }) - sr := newSolveRequest(withSpec(ctx, t, zipperSpec), withBuildTarget("mariner2/container")) + sr := newSolveRequest(withSpec(ctx, t, zipperSpec), withBuildTarget("mariner2/container"), withAmd64Platform) zipper := reqToState(ctx, gwc, sr, t) sr = newSolveRequest(withSpec(ctx, t, spec), withBuildTarget("windowscross/zip")) @@ -388,7 +400,7 @@ echo "$BAR" > bar.txt } testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) (*gwclient.Result, error) { - req := newSolveRequest(withBuildTarget(buildTarget), withSpec(ctx, t, spec)) + req := newSolveRequest(withBuildTarget(buildTarget), withSpec(ctx, t, spec), withAmd64Platform) return client.Solve(ctx, req) }) })