Merge pull request #31 from tonistiigi/platforms-fixes

fixes for managing platforms

bake/bake.go

@@ -8,6 +8,7 @@ import (
 	"github.com/moby/buildkit/session/auth/authprovider"
 	"github.com/pkg/errors"
 	"github.com/tonistiigi/buildx/build"
+	"github.com/tonistiigi/buildx/util/platformutil"
 )
 
 func ReadTargets(ctx context.Context, files, targets, overrides []string) (map[string]Target, error) {
@@ -254,7 +255,7 @@ func toBuildOpt(t Target) (*build.Options, error) {
 		Labels:    t.Labels,
 	}
 
-	platforms, err := build.ParsePlatformSpecs(t.Platforms)
+	platforms, err := platformutil.Parse(t.Platforms)
 	if err != nil {
 		return nil, err
 	}

build/build.go

@@ -62,7 +62,7 @@ type Inputs struct {
 type DriverInfo struct {
 	Driver   driver.Driver
 	Name     string
-	Platform []string // TODO: specs.Platform
+	Platform []specs.Platform
 	Err      error
 }
 

build/platform.go

@@ -1,32 +0,0 @@
-package build
-
-import (
-	"strings"
-
-	"github.com/containerd/containerd/platforms"
-	specs "github.com/opencontainers/image-spec/specs-go/v1"
-)
-
-func ParsePlatformSpecs(platformsStr []string) ([]specs.Platform, error) {
-	if len(platformsStr) == 0 {
-		return nil, nil
-	}
-	out := make([]specs.Platform, 0, len(platformsStr))
-	for _, s := range platformsStr {
-		parts := strings.Split(s, ",")
-		if len(parts) > 1 {
-			p, err := ParsePlatformSpecs(parts)
-			if err != nil {
-				return nil, err
-			}
-			out = append(out, p...)
-			continue
-		}
-		p, err := platforms.Parse(s)
-		if err != nil {
-			return nil, err
-		}
-		out = append(out, platforms.Normalize(p))
-	}
-	return out, nil
-}

commands/build.go

@@ -14,6 +14,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/tonistiigi/buildx/build"
+	"github.com/tonistiigi/buildx/util/platformutil"
 	"github.com/tonistiigi/buildx/util/progress"
 )
 
@@ -93,7 +94,7 @@ func runBuild(dockerCli command.Cli, in buildOptions) error {
 		NetworkMode: in.networkMode,
 	}
 
-	platforms, err := build.ParsePlatformSpecs(in.platforms)
+	platforms, err := platformutil.Parse(in.platforms)
 	if err != nil {
 		return err
 	}

commands/inspect.go

@@ -11,10 +11,12 @@ import (
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/moby/buildkit/util/appcontext"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/spf13/cobra"
 	"github.com/tonistiigi/buildx/build"
 	"github.com/tonistiigi/buildx/driver"
 	"github.com/tonistiigi/buildx/store"
+	"github.com/tonistiigi/buildx/util/platformutil"
 	"github.com/tonistiigi/buildx/util/progress"
 	"golang.org/x/sync/errgroup"
 )
@@ -26,7 +28,7 @@ type inspectOptions struct {
 type dinfo struct {
 	di        *build.DriverInfo
 	info      *driver.Info
-	platforms []string
+	platforms []specs.Platform
 	err       error
 }
 
@@ -112,7 +114,7 @@ func runInspect(dockerCli command.Cli, in inspectOptions, args []string) error {
 				fmt.Fprintf(w, "Error:\t%s\n", err.Error())
 			} else {
 				fmt.Fprintf(w, "Status:\t%s\n", ngi.drivers[i].info.Status)
-				fmt.Fprintf(w, "Platforms:\t%s\n", strings.Join(append(n.Platforms, ngi.drivers[i].platforms...), ", "))
+				fmt.Fprintf(w, "Platforms:\t%s\n", strings.Join(platformutil.Format(platformutil.Dedupe(append(n.Platforms, ngi.drivers[i].platforms...))), ", "))
 			}
 		}
 	}

commands/ls.go

@@ -14,6 +14,7 @@ import (
 	"github.com/moby/buildkit/util/appcontext"
 	"github.com/spf13/cobra"
 	"github.com/tonistiigi/buildx/store"
+	"github.com/tonistiigi/buildx/util/platformutil"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -129,7 +130,7 @@ func printngi(w io.Writer, ngi *nginfo) {
 			if err != "" {
 				fmt.Fprintf(w, "  %s\t%s\t%s\n", n.Name, n.Endpoint, err)
 			} else {
-				fmt.Fprintf(w, "  %s\t%s\t%s\t%s\n", n.Name, n.Endpoint, status, strings.Join(p, ", "))
+				fmt.Fprintf(w, "  %s\t%s\t%s\t%s\n", n.Name, n.Endpoint, status, strings.Join(platformutil.Format(p), ", "))
 			}
 		}
 	}

commands/util.go

@@ -5,7 +5,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/containerd/containerd/platforms"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/context/docker"
 	dopts "github.com/docker/cli/opts"
@@ -14,6 +13,7 @@ import (
 	"github.com/tonistiigi/buildx/build"
 	"github.com/tonistiigi/buildx/driver"
 	"github.com/tonistiigi/buildx/store"
+	"github.com/tonistiigi/buildx/util/platformutil"
 	"golang.org/x/sync/errgroup"
 )
 
@@ -269,9 +269,10 @@ func loadInfoData(ctx context.Context, d *dinfo) error {
 		}
 		for _, w := range workers {
 			for _, p := range w.Platforms {
-				d.platforms = append(d.platforms, platforms.Format(p))
+				d.platforms = append(d.platforms, p)
 			}
 		}
+		d.platforms = platformutil.Dedupe(d.platforms)
 	}
 	return nil
 }

store/nodegroup.go

@@ -3,7 +3,10 @@ package store
 import (
 	"fmt"
 
+	"github.com/containerd/containerd/platforms"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
+	"github.com/tonistiigi/buildx/util/platformutil"
 )
 
 type NodeGroup struct {
@@ -15,7 +18,7 @@ type NodeGroup struct {
 type Node struct {
 	Name      string
 	Endpoint  string
-	Platforms []string
+	Platforms []specs.Platform
 }
 
 func (ng *NodeGroup) Leave(name string) error {
@@ -38,16 +41,22 @@ func (ng *NodeGroup) Update(name, endpoint string, platforms []string, endpoints
 		}
 		ng.Nodes = nil
 	}
+
+	pp, err := platformutil.Parse(platforms)
+	if err != nil {
+		return err
+	}
+
 	if i != -1 {
 		n := ng.Nodes[i]
 		if endpointsSet {
 			n.Endpoint = endpoint
 		}
 		if len(platforms) > 0 {
-			n.Platforms = platforms
+			n.Platforms = pp
 		}
 		ng.Nodes[i] = n
-		if err := ng.validateDuplicates(endpoint); err != nil {
+		if err := ng.validateDuplicates(endpoint, i); err != nil {
 			return err
 		}
 		return nil
@@ -57,7 +66,7 @@ func (ng *NodeGroup) Update(name, endpoint string, platforms []string, endpoints
 		name = ng.nextNodeName()
 	}
 
-	name, err := ValidateName(name)
+	name, err = ValidateName(name)
 	if err != nil {
 		return err
 	}
@@ -65,18 +74,17 @@ func (ng *NodeGroup) Update(name, endpoint string, platforms []string, endpoints
 	n := Node{
 		Name:      name,
 		Endpoint:  endpoint,
-		Platforms: platforms,
+		Platforms: pp,
 	}
 	ng.Nodes = append(ng.Nodes, n)
 
-	if err := ng.validateDuplicates(endpoint); err != nil {
+	if err := ng.validateDuplicates(endpoint, len(ng.Nodes)-1); err != nil {
 		return err
 	}
 	return nil
 }
 
-func (ng *NodeGroup) validateDuplicates(ep string) error {
+func (ng *NodeGroup) validateDuplicates(ep string, idx int) error {
-	// TODO: reset platforms
 	i := 0
 	for _, n := range ng.Nodes {
 		if n.Endpoint == ep {
@@ -86,6 +94,19 @@ func (ng *NodeGroup) validateDuplicates(ep string) error {
 	if i > 1 {
 		return errors.Errorf("invalid duplicate endpoint %s", ep)
 	}
+
+	m := map[string]struct{}{}
+	for _, p := range ng.Nodes[idx].Platforms {
+		m[platforms.Format(p)] = struct{}{}
+	}
+
+	for i := range ng.Nodes {
+		if i == idx {
+			continue
+		}
+		ng.Nodes[i].Platforms = filterPlatforms(ng.Nodes[i].Platforms, m)
+	}
+
 	return nil
 }
 
@@ -109,3 +130,13 @@ func (ng *NodeGroup) nextNodeName() string {
 		return name
 	}
 }
+
+func filterPlatforms(in []specs.Platform, m map[string]struct{}) []specs.Platform {
+	out := make([]specs.Platform, 0, len(in))
+	for _, p := range in {
+		if _, ok := m[platforms.Format(p)]; !ok {
+			out = append(out, p)
+		}
+	}
+	return out
+}

store/nodegroup_test.go

@@ -0,0 +1,42 @@
+package store
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/tonistiigi/buildx/util/platformutil"
+)
+
+func TestNodeGroupUpdate(t *testing.T) {
+	t.Parallel()
+
+	ng := &NodeGroup{}
+	err := ng.Update("foo", "foo0", []string{"linux/amd64"}, true, false)
+	require.NoError(t, err)
+
+	err = ng.Update("foo1", "foo1", []string{"linux/arm64", "linux/arm/v7"}, true, true)
+	require.NoError(t, err)
+
+	require.Equal(t, len(ng.Nodes), 2)
+
+	// update
+	err = ng.Update("foo", "foo2", []string{"linux/amd64", "linux/arm"}, true, false)
+	require.NoError(t, err)
+
+	require.Equal(t, len(ng.Nodes), 2)
+	require.Equal(t, []string{"linux/amd64", "linux/arm/v7"}, platformutil.Format(ng.Nodes[0].Platforms))
+	require.Equal(t, []string{"linux/arm64"}, platformutil.Format(ng.Nodes[1].Platforms))
+
+	require.Equal(t, "foo2", ng.Nodes[0].Endpoint)
+
+	// duplicate endpoint
+	err = ng.Update("foo1", "foo2", nil, true, false)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "duplicate endpoint")
+
+	err = ng.Leave("foo")
+	require.NoError(t, err)
+
+	require.Equal(t, len(ng.Nodes), 1)
+	require.Equal(t, []string{"linux/arm64"}, platformutil.Format(ng.Nodes[0].Platforms))
+}

util/platformutil/parse.go

@@ -0,0 +1,58 @@
+package platformutil
+
+import (
+	"strings"
+
+	"github.com/containerd/containerd/platforms"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
+)
+
+func Parse(platformsStr []string) ([]specs.Platform, error) {
+	if len(platformsStr) == 0 {
+		return nil, nil
+	}
+	out := make([]specs.Platform, 0, len(platformsStr))
+	for _, s := range platformsStr {
+		parts := strings.Split(s, ",")
+		if len(parts) > 1 {
+			p, err := Parse(parts)
+			if err != nil {
+				return nil, err
+			}
+			out = append(out, p...)
+			continue
+		}
+		p, err := platforms.Parse(s)
+		if err != nil {
+			return nil, err
+		}
+		out = append(out, platforms.Normalize(p))
+	}
+	return out, nil
+}
+
+func Dedupe(in []specs.Platform) []specs.Platform {
+	m := map[string]struct{}{}
+	out := make([]specs.Platform, 0, len(in))
+	for _, p := range in {
+		p := platforms.Normalize(p)
+		key := platforms.Format(p)
+		if _, ok := m[key]; ok {
+			continue
+		}
+		m[key] = struct{}{}
+		out = append(out, p)
+	}
+	return out
+}
+
+func Format(in []specs.Platform) []string {
+	if len(in) == 0 {
+		return nil
+	}
+	out := make([]string, 0, len(in))
+	for _, p := range in {
+		out = append(out, platforms.Format(p))
+	}
+	return out
+}