diff --git a/build/resolver/driver_test.go b/build/resolver/driver_test.go index 58e8653b4cbc..09cdae5f6d65 100644 --- a/build/resolver/driver_test.go +++ b/build/resolver/driver_test.go @@ -7,6 +7,7 @@ import ( "github.com/containerd/platforms" "github.com/docker/buildx/builder" + "github.com/docker/buildx/util/platformutil" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/require" ) @@ -300,6 +301,62 @@ func TestSplitNodeMultiPlatformNoUnify(t *testing.T) { require.Equal(t, "builder-amd64-riscv64", res[1].Node().Builder) } +func TestSelectNodeWindowsOSVersion(t *testing.T) { + r := makeTestResolver(map[string][]ocispecs.Platform{ + "builder-ltsc2019": {platforms.MustParse("windows(10.0.17763)/amd64")}, + "builder-ltsc2022": {platforms.MustParse("windows(10.0.20348)/amd64")}, + }) + + res, perfect, err := r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse("windows(10.0.17763)/amd64")}, nil, platforms.Only, nil) + require.NoError(t, err) + require.True(t, perfect) + require.Len(t, res, 1) + require.Equal(t, "builder-ltsc2019", res[0].Node().Builder) + + res, perfect, err = r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse("windows(10.0.20348)/amd64")}, nil, platforms.Only, nil) + require.NoError(t, err) + require.True(t, perfect) + require.Len(t, res, 1) + require.Equal(t, "builder-ltsc2022", res[0].Node().Builder) + + // no node runs a release new enough for this image + _, perfect, err = r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse("windows(10.0.26100)/amd64")}, nil, platforms.Only, nil) + require.NoError(t, err) + require.False(t, perfect) +} + +func TestSelectNodeAdditionalPlatformsOSVersion(t *testing.T) { + // mirrors what Resolve does with the platforms reported by the workers of a + // single node: entries that only differ by OS version have to survive the + // dedupe, otherwise only one windows release can ever be matched + additional := func(int, builder.Node) []ocispecs.Platform { + return platformutil.Dedupe([]ocispecs.Platform{ + platforms.MustParse("windows(10.0.17763)/amd64"), + platforms.MustParse("windows(10.0.20348)/amd64"), + }) + } + + for _, tc := range []struct { + name string + platform string + }{ + {"ltsc2019", "windows(10.0.17763)/amd64"}, + {"ltsc2022", "windows(10.0.20348)/amd64"}, + } { + t.Run(tc.name, func(t *testing.T) { + r := makeTestResolver(map[string][]ocispecs.Platform{ + "builder-win": {platforms.MustParse("linux/amd64")}, + }) + + res, perfect, err := r.resolve(context.TODO(), []ocispecs.Platform{platforms.MustParse(tc.platform)}, nil, platforms.Only, additional) + require.NoError(t, err) + require.True(t, perfect) + require.Len(t, res, 1) + require.Equal(t, "builder-win", res[0].Node().Builder) + }) + } +} + func makeTestResolver(nodes map[string][]ocispecs.Platform) *nodeResolver { var ns []builder.Node for name, platforms := range nodes { diff --git a/builder/node.go b/builder/node.go index 9825903d6287..d5cfc346eb44 100644 --- a/builder/node.go +++ b/builder/node.go @@ -6,7 +6,6 @@ import ( "sort" "strings" - "github.com/containerd/platforms" "github.com/docker/buildx/driver" "github.com/docker/buildx/store" "github.com/docker/buildx/store/storeutil" @@ -218,10 +217,7 @@ func (n *Node) MarshalJSON() ([]byte, error) { status = "error" nerr = strings.TrimSpace(n.Err.Error()) } - var pp []string - for _, p := range n.Platforms { - pp = append(pp, platforms.Format(p)) - } + pp := platformutil.Format(n.Platforms) return json.Marshal(struct { Name string Endpoint string diff --git a/builder/node_test.go b/builder/node_test.go new file mode 100644 index 000000000000..41af82e17573 --- /dev/null +++ b/builder/node_test.go @@ -0,0 +1,47 @@ +package builder + +import ( + "encoding/json" + "testing" + + "github.com/containerd/platforms" + "github.com/docker/buildx/store" + ocispecs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/require" +) + +func TestNodeMarshalJSONPlatforms(t *testing.T) { + t.Parallel() + + n := Node{ + Node: store.Node{Name: "n0", Endpoint: "docker-container://buildx_buildkit_n0"}, + Platforms: []ocispecs.Platform{ + platforms.MustParse("windows(10.0.17763)/amd64"), + platforms.MustParse("windows(10.0.20348)/amd64"), + platforms.MustParse("windows(10.0.20348+win32k)/amd64"), + platforms.MustParse("linux/x86_64"), + }, + } + + dt, err := json.Marshal(&n) + require.NoError(t, err) + + var out struct { + Platforms []string + } + require.NoError(t, json.Unmarshal(dt, &out)) + require.Equal(t, []string{ + "windows(10.0.17763)/amd64", + "windows(10.0.20348)/amd64", + "windows(10.0.20348+win32k)/amd64", + "linux/amd64", + }, out.Platforms) +} + +func TestNodeMarshalJSONNoPlatforms(t *testing.T) { + t.Parallel() + + dt, err := json.Marshal(&Node{Node: store.Node{Name: "n0"}}) + require.NoError(t, err) + require.NotContains(t, string(dt), "Platforms") +} diff --git a/store/nodegroup.go b/store/nodegroup.go index d19a30161234..01081e36e741 100644 --- a/store/nodegroup.go +++ b/store/nodegroup.go @@ -6,7 +6,6 @@ import ( "slices" "time" - "github.com/containerd/platforms" "github.com/docker/buildx/util/confutil" "github.com/docker/buildx/util/platformutil" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" @@ -176,7 +175,7 @@ func (ng *NodeGroup) validateDuplicates(ep string, idx int) error { m := map[string]struct{}{} for _, p := range ng.Nodes[idx].Platforms { - m[platforms.FormatAll(platforms.Normalize(p))] = struct{}{} + m[platformutil.Key(p)] = struct{}{} } for i := range ng.Nodes { @@ -213,7 +212,7 @@ func (ng *NodeGroup) nextNodeName() string { func filterPlatforms(in []ocispecs.Platform, m map[string]struct{}) []ocispecs.Platform { out := make([]ocispecs.Platform, 0, len(in)) for _, p := range in { - if _, ok := m[platforms.FormatAll(platforms.Normalize(p))]; !ok { + if _, ok := m[platformutil.Key(p)]; !ok { out = append(out, p) } } diff --git a/store/nodegroup_test.go b/store/nodegroup_test.go index ece584c21e92..60000675f765 100644 --- a/store/nodegroup_test.go +++ b/store/nodegroup_test.go @@ -3,9 +3,7 @@ package store import ( "testing" - "github.com/containerd/platforms" "github.com/docker/buildx/util/platformutil" - ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/require" ) @@ -55,14 +53,6 @@ func TestNodeGroupUpdateFiltersOnlyMatchingPlatforms(t *testing.T) { err = ng.Update("n2", "ctx-b", []string{"windows(10.0.20348)/amd64", "linux/x86_64"}, true, true, nil, "", nil) require.NoError(t, err) - require.Equal(t, []string{"windows(10.0.17763)/amd64"}, formatAll(ng.Nodes[0].Platforms)) - require.Equal(t, []string{"windows(10.0.20348)/amd64", "linux/amd64"}, formatAll(ng.Nodes[1].Platforms)) -} - -func formatAll(pp []ocispecs.Platform) []string { - out := make([]string, 0, len(pp)) - for _, p := range pp { - out = append(out, platforms.FormatAll(p)) - } - return out + require.Equal(t, []string{"windows(10.0.17763)/amd64"}, platformutil.Format(ng.Nodes[0].Platforms)) + require.Equal(t, []string{"windows(10.0.20348)/amd64", "linux/amd64"}, platformutil.Format(ng.Nodes[1].Platforms)) } diff --git a/util/platformutil/parse.go b/util/platformutil/parse.go index e8dfeee8cfbd..cfe4ea028399 100644 --- a/util/platformutil/parse.go +++ b/util/platformutil/parse.go @@ -38,12 +38,19 @@ func parse(in string) (ocispecs.Platform, error) { return platforms.Parse(in) } +// Key returns the canonical string identifying a platform. Unlike +// [platforms.Format] it keeps the OS version and OS features so that Windows +// platforms from different releases do not collapse into each other. +func Key(p ocispecs.Platform) string { + return platforms.FormatAll(platforms.Normalize(p)) +} + func Dedupe(in []ocispecs.Platform) []ocispecs.Platform { m := map[string]struct{}{} out := make([]ocispecs.Platform, 0, len(in)) for _, p := range in { p := platforms.Normalize(p) - key := platforms.FormatAll(p) + key := Key(p) if _, ok := m[key]; ok { continue } @@ -58,7 +65,7 @@ func FormatInGroups(gg ...[]ocispecs.Platform) []string { out := make([]string, 0, len(gg)) for i, g := range gg { for _, p := range g { - v := platforms.FormatAll(platforms.Normalize(p)) + v := Key(p) if _, ok := m[v]; ok { continue } @@ -78,7 +85,7 @@ func Format(in []ocispecs.Platform) []string { } out := make([]string, 0, len(in)) for _, p := range in { - out = append(out, platforms.Format(p)) + out = append(out, Key(p)) } return out } diff --git a/util/platformutil/parse_test.go b/util/platformutil/parse_test.go index 1be4b6787c10..6c814b4f0094 100644 --- a/util/platformutil/parse_test.go +++ b/util/platformutil/parse_test.go @@ -25,7 +25,7 @@ func TestDedupePreservesOSVersionAndFeatures(t *testing.T) { "windows(10.0.20348)/amd64", "windows(10.0.20348+win32k)/amd64", "linux/amd64", - }, formatAll(got)) + }, Format(got)) } func TestFormatInGroupsPreservesOSVersionAndFeatures(t *testing.T) { @@ -52,10 +52,20 @@ func TestFormatInGroupsPreservesOSVersionAndFeatures(t *testing.T) { }, got) } -func formatAll(pp []ocispecs.Platform) []string { - out := make([]string, 0, len(pp)) - for _, p := range pp { - out = append(out, platforms.FormatAll(p)) - } - return out +func TestFormatPreservesOSVersionAndFeatures(t *testing.T) { + t.Parallel() + + require.Equal(t, []string{ + "windows(10.0.17763)/amd64", + "windows(10.0.20348+win32k)/amd64", + "linux/amd64", + "linux/arm/v7", + }, Format([]ocispecs.Platform{ + platforms.MustParse("windows(10.0.17763)/amd64"), + platforms.MustParse("windows(10.0.20348+win32k)/amd64"), + platforms.MustParse("linux/x86_64"), + platforms.MustParse("linux/arm/v7"), + })) + + require.Nil(t, Format(nil)) }