Skip to content

Commit

Permalink
Merge pull request #47339 from vvoland/cache-fix-older-windows-23
Browse files Browse the repository at this point in the history
[23.0 backport] image/cache: Ignore Build and Revision on Windows
  • Loading branch information
thaJeztah committed Feb 16, 2024
2 parents 5cc6748 + d10756f commit 548f37a
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 2 deletions.
4 changes: 2 additions & 2 deletions image/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"reflect"
"strings"

"github.com/containerd/containerd/platforms"
containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/dockerversion"
"github.com/docker/docker/image"
Expand Down Expand Up @@ -255,11 +254,12 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
OSFeatures: img.OSFeatures,
Variant: img.Variant,
}

// 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
34 changes: 34 additions & 0 deletions image/cache/compare.go
Original file line number Diff line number Diff line change
@@ -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,36 @@ 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, ".")

// Major and minor must match.
for i := 0; i < 2; i++ {
if len(builderParts) > i && len(imageParts) > i && builderParts[i] != imageParts[i] {
return false
}
}

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
74 changes: 74 additions & 0 deletions image/cache/compare_test.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -124,3 +128,73 @@ 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
t.Run(tc.name, func(t *testing.T) {
assert.Check(t, is.Equal(comparePlatform(tc.builder, tc.image), tc.expected))
})
}
}

0 comments on commit 548f37a

Please sign in to comment.