Merge pull request #47337 from vvoland/cache-fix-older-windows-25

[25.0 backport] image/cache: Ignore Build and Revision on Windows
moby · Feb 6, 2024 · aff7177 · aff7177
2 parents df96d8d + 0227d95
commit aff7177
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 2 deletions.
diff --git a/image/cache/cache.go b/image/cache/cache.go
@@ -7,7 +7,6 @@ import (
 	"reflect"
 	"strings"
 
-	"github.com/containerd/containerd/platforms"
 	"github.com/containerd/log"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/dockerversion"
@@ -250,11 +249,12 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
 			}
 
 			imgPlatform := img.Platform()
+
 			// Discard old linux/amd64 images with empty platform.
 			if imgPlatform.OS == "" && imgPlatform.Architecture == "" {
 				continue
 			}
-			if !platforms.OnlyStrict(platform).Match(imgPlatform) {
+			if !comparePlatform(platform, imgPlatform) {
 				continue
 			}
 

diff --git a/image/cache/compare.go b/image/cache/compare.go
@@ -1,7 +1,11 @@
 package cache // import "github.com/docker/docker/image/cache"
 
 import (
+	"strings"
+
+	"github.com/containerd/containerd/platforms"
 	"github.com/docker/docker/api/types/container"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // TODO: Remove once containerd image service directly uses the ImageCache and
@@ -10,6 +14,29 @@ func CompareConfig(a, b *container.Config) bool {
 	return compare(a, b)
 }
 
+func comparePlatform(builderPlatform, imagePlatform ocispec.Platform) bool {
+	// On Windows, only check the Major and Minor versions.
+	// The Build and Revision compatibility depends on whether `process` or
+	// `hyperv` isolation used.
+	//
+	// Fixes https://github.com/moby/moby/issues/47307
+	if builderPlatform.OS == "windows" && imagePlatform.OS == builderPlatform.OS {
+		// OSVersion format is:
+		// Major.Minor.Build.Revision
+		builderParts := strings.Split(builderPlatform.OSVersion, ".")
+		imageParts := strings.Split(imagePlatform.OSVersion, ".")
+
+		if len(builderParts) >= 3 && len(imageParts) >= 3 {
+			// Keep only Major & Minor.
+			builderParts[0] = imageParts[0]
+			builderParts[1] = imageParts[1]
+			imagePlatform.OSVersion = strings.Join(builderParts, ".")
+		}
+	}
+
+	return platforms.Only(builderPlatform).Match(imagePlatform)
+}
+
 // compare two Config struct. Do not container-specific fields:
 // - Image
 // - Hostname

diff --git a/image/cache/compare_test.go b/image/cache/compare_test.go
@@ -1,11 +1,15 @@
 package cache // import "github.com/docker/docker/image/cache"
 
 import (
+	"runtime"
 	"testing"
 
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/go-connections/nat"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 // Just to make life easier
@@ -124,3 +128,79 @@ func TestCompare(t *testing.T) {
 		}
 	}
 }
+
+func TestPlatformCompare(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		builder  ocispec.Platform
+		image    ocispec.Platform
+		expected bool
+	}{
+		{
+			name:     "same os and arch",
+			builder:  ocispec.Platform{Architecture: "amd64", OS: runtime.GOOS},
+			image:    ocispec.Platform{Architecture: "amd64", OS: runtime.GOOS},
+			expected: true,
+		},
+		{
+			name:     "same os different arch",
+			builder:  ocispec.Platform{Architecture: "amd64", OS: runtime.GOOS},
+			image:    ocispec.Platform{Architecture: "arm64", OS: runtime.GOOS},
+			expected: false,
+		},
+		{
+			name:     "same os smaller host variant",
+			builder:  ocispec.Platform{Variant: "v7", Architecture: "arm", OS: runtime.GOOS},
+			image:    ocispec.Platform{Variant: "v8", Architecture: "arm", OS: runtime.GOOS},
+			expected: false,
+		},
+		{
+			name:     "same os higher host variant",
+			builder:  ocispec.Platform{Variant: "v8", Architecture: "arm", OS: runtime.GOOS},
+			image:    ocispec.Platform{Variant: "v7", Architecture: "arm", OS: runtime.GOOS},
+			expected: true,
+		},
+		{
+			// Test for https://github.com/moby/moby/issues/47307
+			name:     "different build and revision",
+			builder:  ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.22621"},
+			image:    ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"},
+			expected: true,
+		},
+		{
+			name:     "different revision",
+			builder:  ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.1234"},
+			image:    ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"},
+			expected: true,
+		},
+		{
+			name:     "different major",
+			builder:  ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "11.0.17763.5329"},
+			image:    ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"},
+			expected: false,
+		},
+		{
+			name:     "different minor same osver",
+			builder:  ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"},
+			image:    ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.1.17763.5329"},
+			expected: false,
+		},
+		{
+			name:     "different arch same osver",
+			builder:  ocispec.Platform{Architecture: "arm64", OS: "windows", OSVersion: "10.0.17763.5329"},
+			image:    ocispec.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"},
+			expected: false,
+		},
+	} {
+		tc := tc
+		// OSVersion comparison is only performed by containerd platform
+		// matcher if built on Windows.
+		if (tc.image.OSVersion != "" || tc.builder.OSVersion != "") && runtime.GOOS != "windows" {
+			continue
+		}
+
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Check(t, is.Equal(comparePlatform(tc.builder, tc.image), tc.expected))
+		})
+	}
+}