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

Allow containerd on Windows 11 to use Windows Server 2022 images #8137

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
48 changes: 47 additions & 1 deletion platforms/defaults_windows.go
Expand Up @@ -22,6 +22,7 @@ import (
"strconv"
"strings"

"github.com/Microsoft/hcsshim/osversion"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"golang.org/x/sys/windows"
)
Expand All @@ -42,6 +43,7 @@ type windowsmatcher struct {
specs.Platform
osVersionPrefix string
defaultMatcher Matcher
isClientOS bool
}

// Match matches platform with the same windows major, minor
Expand All @@ -53,6 +55,31 @@ func (m windowsmatcher) Match(p specs.Platform) bool {
if strings.HasPrefix(p.OSVersion, m.osVersionPrefix) {
return true
}
if m.isClientOS && p.OSVersion != "" {
split := strings.Split(p.OSVersion, ".")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already indirectly import a package for version parsing (go.mod, vendor). Should we just reuse it instead of maintaining a pretty fragile logic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, what this is parsing isn't semver (it's Windows 4-part OS versions like 10.0.20348.1850), so from what I can tell, that library won't actually parse them correctly. 😅

if len(split) >= 3 {
major, err := strconv.Atoi(split[0])
if err != nil {
return false
}
minor, err := strconv.Atoi(split[1])
if err != nil {
return false
}
build, err := strconv.Atoi(split[2])
if err != nil {
return false
}

if (major == 10 && minor == 0 && build >= osversion.V21H2Server) ||
(major == 10 && minor > 0) {
// Windows 11 client machines are implicitly
// compatible with 10.0.20348 (LTSC2022), even
// when the client has a newer build version.
return true
}
}
}
return p.OSVersion == ""
}

Expand All @@ -61,16 +88,35 @@ func (m windowsmatcher) Match(p specs.Platform) bool {

// Less sorts matched platforms in front of other platforms.
// For matched platforms, it puts platforms with larger revision
// number in front.
// number in front (and for Win 11 clients, larger build numbers).
func (m windowsmatcher) Less(p1, p2 specs.Platform) bool {
m1, m2 := m.Match(p1), m.Match(p2)
if m1 && m2 {
// Build number comparison will only be used for clients
// where differing build versions can match.
b1, b2 := build(p1.OSVersion), build(p2.OSVersion)
if b1 != b2 {
return b1 > b2
}

r1, r2 := revision(p1.OSVersion), revision(p2.OSVersion)
return r1 > r2
}
return m1 && !m2
}

func build(v string) int {
parts := strings.Split(v, ".")
if len(parts) < 3 {
return 0
}
r, err := strconv.Atoi(parts[2])
if err != nil {
return 0
}
return r
}

func revision(v string) int {
parts := strings.Split(v, ".")
if len(parts) < 4 {
Expand Down
121 changes: 121 additions & 0 deletions platforms/defaults_windows_test.go
Expand Up @@ -80,6 +80,7 @@ func TestMatchComparerMatch_WCOW(t *testing.T) {
defaultMatcher: &matcher{
Platform: Normalize(DefaultSpec()),
},
isClientOS: false,
}
for _, test := range []struct {
platform imagespec.Platform
Expand Down Expand Up @@ -158,6 +159,7 @@ func TestMatchComparerMatch_LCOW(t *testing.T) {
},
),
},
isClientOS: false,
}
for _, test := range []struct {
platform imagespec.Platform
Expand Down Expand Up @@ -210,6 +212,7 @@ func TestMatchComparerLess(t *testing.T) {
defaultMatcher: &matcher{
Platform: Normalize(DefaultSpec()),
},
isClientOS: false,
}
platforms := []imagespec.Platform{
{
Expand Down Expand Up @@ -268,3 +271,121 @@ func TestMatchComparerLess(t *testing.T) {
})
assert.Equal(t, expected, platforms)
}

func TestMatchComparerClientOSMatch(t *testing.T) {
m1 := windowsmatcher{
Platform: DefaultSpec(),
osVersionPrefix: "10.0.22000",
defaultMatcher: &matcher{
Platform: Normalize(DefaultSpec()),
},
isClientOS: true,
}

m2 := windowsmatcher{
Platform: DefaultSpec(),
osVersionPrefix: "10.0.22621",
defaultMatcher: &matcher{
Platform: Normalize(DefaultSpec()),
},
isClientOS: true,
}

for _, test := range []struct {
platform imagespec.Platform
match bool
}{
{
platform: imagespec.Platform{
Architecture: "amd64",
OS: "windows",
OSVersion: "10.0.17763.2114",
},
match: false,
},
{
platform: imagespec.Platform{
Architecture: "amd64",
OS: "windows",
OSVersion: "10.0.20348.169",
},
match: true,
},
{
platform: imagespec.Platform{
Architecture: "amd64",
OS: "windows",
OSVersion: "10.0.22000",
},
match: true,
},
{
platform: imagespec.Platform{
Architecture: "amd64",
OS: "windows",
},
match: true,
},
{
platform: imagespec.Platform{
Architecture: "amd64",
OS: "linux",
},
match: false,
},
} {
assert.Equal(t, test.match, m1.Match(test.platform), "should match %b, %s to %s", test.match, m1.Platform, test.platform)
assert.Equal(t, test.match, m2.Match(test.platform), "should match %b, %s to %s", test.match, m2.Platform, test.platform)
}
}

func TestMatchComparerClientOSPriority(t *testing.T) {
m1 := windowsmatcher{
Platform: DefaultSpec(),
osVersionPrefix: "10.0.22000",
defaultMatcher: &matcher{
Platform: Normalize(DefaultSpec()),
},
isClientOS: true,
}
m2 := windowsmatcher{
Platform: DefaultSpec(),
osVersionPrefix: "10.0.22621",
defaultMatcher: &matcher{
Platform: Normalize(DefaultSpec()),
},
isClientOS: true,
}
platforms := []imagespec.Platform{
{
Architecture: "amd64",
OS: "windows",
OSVersion: "10.0.20348.169",
},
{
Architecture: "amd64",
OS: "windows",
OSVersion: "10.0.22000",
},
}
expected := []imagespec.Platform{
{
Architecture: "amd64",
OS: "windows",
OSVersion: "10.0.22000",
},
{
Architecture: "amd64",
OS: "windows",
OSVersion: "10.0.20348.169",
},
}
sort.SliceStable(platforms, func(i, j int) bool {
return m1.Less(platforms[i], platforms[j])
})
assert.Equal(t, expected, platforms)
sort.SliceStable(platforms, func(i, j int) bool {
return m2.Less(platforms[i], platforms[j])
})
assert.Equal(t, expected, platforms)
}
17 changes: 17 additions & 0 deletions platforms/platforms_windows.go
Expand Up @@ -18,8 +18,24 @@ package platforms

import (
specs "github.com/opencontainers/image-spec/specs-go/v1"
"golang.org/x/sys/windows/registry"
)

func isClientOS() bool {
k, err := registry.OpenKey(registry.LOCAL_MACHINE, `SOFTWARE\Microsoft\Windows NT\CurrentVersion`, registry.QUERY_VALUE)
if err != nil {
return false
}
defer k.Close()

installationType, _, err := k.GetStringValue("InstallationType")
if err != nil {
return false
}

return installationType == "Client"
}
Comment on lines +24 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah Thinking on your comment, this kind of feels like it could be a helper in hcsshim/osversion also..

Copy link
Member

@dcantah dcantah Feb 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevpar Curious on your thoughts. This regkey approach was recommended (6 years ago however) at least as an alternative to GetProductInfo: https://learn.microsoft.com/en-us/previous-versions/windows/desktop/legacy/ee391629(v=vs.85)


// NewMatcher returns a Windows matcher that will match on osVersionPrefix if
// the platform is Windows otherwise use the default matcher
func newDefaultMatcher(platform specs.Platform) Matcher {
Expand All @@ -30,5 +46,6 @@ func newDefaultMatcher(platform specs.Platform) Matcher {
defaultMatcher: &matcher{
Platform: Normalize(platform),
},
isClientOS: isClientOS(),
}
}