You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Apologies for the rather random ticket, but I thought I'd open one for discussion.
With the containerd/platforms module now providing more options for Windows containers, we're considering using that package in a wider scope in our (Moby, BuildKit) code-base, and while looking at some changes / features, some questions popped-up. I'm opening this ticket to discuss these (looking for input / suggestions from the Microsoft and containerd maintainers).
While that utility is fairly trivial (it's mostly a strings.Split), I know there's been (sometimes very subtle) details over the Years in handling Windows versions, and it might be nice to provide a similar utility in hcsshim to provide a canonical location for parsing. Looking at containerd's function, it currently ignores any error (which may not be desirable), so when implementing, I think it'd be good to change the signature to include an error-return (which consumers can still decide to ignore depending on their use).
Provide (partial) cross-platform functionality
This is a bit of a stretch, but currently the osversion package is Windows-only. In most cases this makes sense, as obtaining the version from the system (osversion.Get()) requires it to run on a Windows host. The osversion.OSVersion struct and CheckHostAndContainerCompat utilities should be portable though, and the same could apply to the string parsing function suggested above.
With the containerd/platforms module potentially being used in cross-platform situations (client on Linux, runtime on Windows), I wonder if it would be useful to provide partial cross-platform functionality in the osversion package. For transparency; currently there's no use outside of Windows, but a discussion related to this code led me to looking; moby/moby#47330 (comment)
// Note we parse kernel.GetKernelVersion rather than system.GetOSVersion// as test binaries aren't manifested, so would otherwise report the wrong// build number.ifruntime.GOOS=="windows" {
v, err:=kernel.GetKernelVersion()
// Get gets the operating system version on Windows.
// The calling application must be manifested to get the correct version information.
funcGet() OSVersion {
once.Do(func() {
v:=*windows.RtlGetVersion()
The osversion.Get function explicitly calls out that the binary must be manifested. I'm curious if the same applies to windows.RtlGetNtVersionNumbers(), and/or if there's a discrepancy there that could potentialy be causing issues. From the Go documentation for RtlGetNtVersionNumbers;
// RtlGetNtVersionNumbers returns the version of the underlying operating system,// ignoring manifest semantics and the application compatibility layer.funcRtlGetNtVersionNumbers() (majorVersion, minorVersion, buildNumberuint32) {
@thaJeztah@vvoland I am not sure we want to have partial cross-compat functions in hccshim. Not all OS-es need to parse OSVersion in the same way and I would incline to having windows only functionality there. Alternatively, I think containerd/platform repo should have a parser for each OS osversion_windows.go , osversion_linux.go etc .
hcsshim and containerd references for the same can be switched to use containerd/platform as well.
Apologies for the rather random ticket, but I thought I'd open one for discussion.
With the containerd/platforms module now providing more options for Windows containers, we're considering using that package in a wider scope in our (Moby, BuildKit) code-base, and while looking at some changes / features, some questions popped-up. I'm opening this ticket to discuss these (looking for input / suggestions from the Microsoft and containerd maintainers).
Parsing os-version from string-format
containerd implemented a minimal
GetOsVersion
utility to parse a os-version into aOSVersion
struct https://github.com/containerd/containerd/blob/88fd474e4c0a3dd3865dfcbf0ca99fff12f5c7b7/platforms/defaults_windows.go#L67-L82While that utility is fairly trivial (it's mostly a
strings.Split
), I know there's been (sometimes very subtle) details over the Years in handling Windows versions, and it might be nice to provide a similar utility in hcsshim to provide a canonical location for parsing. Looking at containerd's function, it currently ignores any error (which may not be desirable), so when implementing, I think it'd be good to change the signature to include an error-return (which consumers can still decide to ignore depending on their use).Provide (partial) cross-platform functionality
This is a bit of a stretch, but currently the
osversion
package is Windows-only. In most cases this makes sense, as obtaining the version from the system (osversion.Get()
) requires it to run on a Windows host. Theosversion.OSVersion
struct andCheckHostAndContainerCompat
utilities should be portable though, and the same could apply to the string parsing function suggested above.With the
containerd/platforms
module potentially being used in cross-platform situations (client on Linux, runtime on Windows), I wonder if it would be useful to provide partial cross-platform functionality in the osversion package. For transparency; currently there's no use outside of Windows, but a discussion related to this code led me to looking; moby/moby#47330 (comment)Question about obtaining the host's version
This is a bit orthogonal, but I noticed that a mix of
windows.RtlGetNtVersionNumbers()
andwindows.RtlGetVersion()
is used to obtain the host's version. I recall various cases in the past where the version obtained from the system required it to be manifested, which caused some "fun" things in our CI where test-binaries are not manifested; https://github.com/moby/moby/blob/cae5d323e1f8a2b44c436f7a87bec6639841d8ec/pkg/archive/changes_test.go#L253-L257containerd's
DefaultSpec()
useswindows.RtlGetNtVersionNumbers()
https://github.com/containerd/containerd/blob/88fd474e4c0a3dd3865dfcbf0ca99fff12f5c7b7/platforms/defaults_windows.go#L30-L32
whereas
osversion.Get
useswindows.RtlGetVersion()
hcsshim/osversion/osversion_windows.go
Lines 25 to 29 in b0d91fb
The
osversion.Get
function explicitly calls out that the binary must be manifested. I'm curious if the same applies towindows.RtlGetNtVersionNumbers()
, and/or if there's a discrepancy there that could potentialy be causing issues. From the Go documentation forRtlGetNtVersionNumbers
;To add to the fun, I see in moby/moby we use
windows.GetVersion(
in some code, which looks to be "yet another" way to get the version https://github.com/moby/moby/blob/cae5d323e1f8a2b44c436f7a87bec6639841d8ec/pkg/parsers/kernel/kernel_windows.go#L38-L47The text was updated successfully, but these errors were encountered: