Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[25.0 backport] image/cache: Ignore Build and Revision on Windows #47337

Merged
merged 2 commits into from Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions image/cache/cache.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
27 changes: 27 additions & 0 deletions 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
Expand All @@ -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
Expand Down
80 changes: 80 additions & 0 deletions 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
Expand Down Expand Up @@ -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))
})
}
}