From fa9c5c55e1af0f1aef9a073504f0deb65e45f504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Mon, 5 Feb 2024 17:38:00 +0100 Subject: [PATCH 1/2] image/cache: Ignore Build and Revision on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compatibility depends on whether `hyperv` or `process` container isolation is used. This fixes cache not being used when building images based on older Windows versions on a newer Windows host. Signed-off-by: Paweł Gronowski (cherry picked from commit 91ea04089b75f3c56e60e33e27daeb0d494d79f5) Signed-off-by: Paweł Gronowski --- image/cache/cache.go | 4 +- image/cache/compare.go | 27 +++++++++++++ image/cache/compare_test.go | 80 +++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-) diff --git a/image/cache/cache.go b/image/cache/cache.go index 8b7632f5e11a1..86c099a4d4cbe 100644 --- 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 index ec1971a207829..fda4ffceed144 100644 --- 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 index 939e99f050706..0ed163f91c806 100644 --- 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/containerd/containerd/platforms" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/docker/go-connections/nat" + "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 platforms.Platform + image platforms.Platform + expected bool + }{ + { + name: "same os and arch", + builder: platforms.Platform{Architecture: "amd64", OS: runtime.GOOS}, + image: platforms.Platform{Architecture: "amd64", OS: runtime.GOOS}, + expected: true, + }, + { + name: "same os different arch", + builder: platforms.Platform{Architecture: "amd64", OS: runtime.GOOS}, + image: platforms.Platform{Architecture: "arm64", OS: runtime.GOOS}, + expected: false, + }, + { + name: "same os smaller host variant", + builder: platforms.Platform{Variant: "v7", Architecture: "arm", OS: runtime.GOOS}, + image: platforms.Platform{Variant: "v8", Architecture: "arm", OS: runtime.GOOS}, + expected: false, + }, + { + name: "same os higher host variant", + builder: platforms.Platform{Variant: "v8", Architecture: "arm", OS: runtime.GOOS}, + image: platforms.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: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.22621"}, + image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + expected: true, + }, + { + name: "different revision", + builder: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.1234"}, + image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + expected: true, + }, + { + name: "different major", + builder: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "11.0.17763.5329"}, + image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + expected: false, + }, + { + name: "different minor same osver", + builder: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.1.17763.5329"}, + expected: false, + }, + { + name: "different arch same osver", + builder: platforms.Platform{Architecture: "arm64", OS: "windows", OSVersion: "10.0.17763.5329"}, + image: platforms.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)) + }) + } +} From 0227d95f9989ef8268f349209446ef9b1343dc3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Tue, 6 Feb 2024 14:25:47 +0100 Subject: [PATCH 2/2] image/cache: Use Platform from ocispec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Gronowski (cherry picked from commit 2c01d53d9692250a7d1b18742f1e100e43615586) Signed-off-by: Paweł Gronowski --- image/cache/compare_test.go | 42 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/image/cache/compare_test.go b/image/cache/compare_test.go index 0ed163f91c806..8d6ce735e2a6a 100644 --- a/image/cache/compare_test.go +++ b/image/cache/compare_test.go @@ -4,10 +4,10 @@ import ( "runtime" "testing" - "github.com/containerd/containerd/platforms" "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" ) @@ -132,63 +132,63 @@ func TestCompare(t *testing.T) { func TestPlatformCompare(t *testing.T) { for _, tc := range []struct { name string - builder platforms.Platform - image platforms.Platform + builder ocispec.Platform + image ocispec.Platform expected bool }{ { name: "same os and arch", - builder: platforms.Platform{Architecture: "amd64", OS: runtime.GOOS}, - image: platforms.Platform{Architecture: "amd64", OS: runtime.GOOS}, + builder: ocispec.Platform{Architecture: "amd64", OS: runtime.GOOS}, + image: ocispec.Platform{Architecture: "amd64", OS: runtime.GOOS}, expected: true, }, { name: "same os different arch", - builder: platforms.Platform{Architecture: "amd64", OS: runtime.GOOS}, - image: platforms.Platform{Architecture: "arm64", OS: runtime.GOOS}, + 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: platforms.Platform{Variant: "v7", Architecture: "arm", OS: runtime.GOOS}, - image: platforms.Platform{Variant: "v8", Architecture: "arm", OS: runtime.GOOS}, + 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: platforms.Platform{Variant: "v8", Architecture: "arm", OS: runtime.GOOS}, - image: platforms.Platform{Variant: "v7", Architecture: "arm", OS: runtime.GOOS}, + 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: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.22621"}, - image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + 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: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.1234"}, - image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + 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: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "11.0.17763.5329"}, - image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + 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: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, - image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.1.17763.5329"}, + 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: platforms.Platform{Architecture: "arm64", OS: "windows", OSVersion: "10.0.17763.5329"}, - image: platforms.Platform{Architecture: "amd64", OS: "windows", OSVersion: "10.0.17763.5329"}, + 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, }, } {