Add support for Windows version filtering on pull by dmcgowan · Pull Request #35090 · moby/moby

dmcgowan · 2017-10-04T22:58:11Z

Update logic to choose manifest from manifest list to check for os version on Windows. Separate the logic for windows and unix to keep unix logic the same.

Logic implemented:

If no manifest found with matching os.version, choose first without os.version
If manifests found with matching os.version and without, choose one os.version
If multiple manifests match os.version, choose first with os.version
Match os.version based on major, minor, and build number for windows only

ping @johnstep @friism

johnstep · 2017-10-04T23:58:17Z

distribution/pull_v2_windows.go

Assuming the variable names are correct, they should be swapped in the prefix check:

strings.HasPrefix(actual, expected+".")

johnstep

LGTM

stevvooe · 2017-10-05T18:43:32Z

distribution/pull_v2_unix.go

This TODO can be removed. Features is out. Variant is ARM only.

Update logic to choose manifest from manifest list to check for os version on Windows. Separate the logic for windows and unix to keep unix logic the same. Signed-off-by: Derek McGowan <derek@mcgstyle.net>

stevvooe · 2017-10-05T20:36:27Z

LGTM

lowenna · 2017-10-06T21:59:02Z

@dmcgowan Unfortunately, this introduces a significant regression. I'm going to back it out as part of #34642 which I've rebased on top of master. As an example, you can no longer pull winspector which is a multi-OS manifest image on an RS3 host as the Windows image in the manifest list is based on RS1, so it's skipped. The image runs just fine on RS3 as a Hyper-V container. @johnstep FYI

lowenna · 2017-10-06T21:59:25Z

PS E:\docker\build\lcow> docker pull  stefanscherer/winspector
Using default tag: latest
latest: Pulling from stefanscherer/winspector
no matching manifest for windows/amd64 in the manifest list entries
PS E:\docker\build\lcow>

johnstep · 2017-10-06T22:06:45Z

Okay, that makes sense. Rather than filtering out images, we should only be changing the order to move the first compatible image to the top of the list.

lowenna · 2017-10-06T22:25:31Z

@johnstep Summarising the offline conversation - I'll leave it in for now in 34642 if you or @dmcgowan can come up with a spot fix.

johnstep · 2017-10-07T07:34:36Z

Thanks for finding this. See #35117 for a fix.

StefanScherer · 2017-10-08T12:09:12Z

What about a manifest list that covers two Windows versions to provide Docker images for RS1 and RS3 (without the need to have Hyper-V)?

I've created an Insider version and pushed a manifest list (see StefanScherer/winspector#9 for details).

$ manifest-tool inspect stefanscherer/winspector:test2
Name:   stefanscherer/winspector:test2 (Type: application/vnd.docker.distribution.manifest.list.v2+json)
Digest: sha256:d2b08f8b2e3644e844eca82ac28690a3c68aab70d265bc71a12b69f0e963b6f3
 * Contains 3 manifest references:
1    Mfst Type: application/vnd.docker.distribution.manifest.v2+json
1       Digest: sha256:2d2f088728ab8a99930df41c7355faf7e7cd30b97d1af298f72023f089a5a9bb
1  Mfst Length: 1994
1     Platform:
1           -      OS: linux
1           - OS Vers: 
1           - OS Feat: []
1           -    Arch: amd64
1           - Variant: 
1           - Feature: 
1     # Layers: 8
         layer 1: digest = sha256:7095154754192bfc2306f3b2b841ef82771b7ad39526537234adb1e74ae81a93
         layer 2: digest = sha256:7f8ede2d2484a67ae3a88912d400c46f4f76e8f62f1003ed10c6f95893603781
         layer 3: digest = sha256:3c752c95ebfb440ae472bd7c28a85adde5844abc783e8e0c12585b3055e9e804
         layer 4: digest = sha256:39c204c948870160e7e0aeaeaf01229492437a04a18345faed5e2733b0d0a64e
         layer 5: digest = sha256:087ab4f34ba7ef95cbadbbf93cd0ff8203581075eb50a2ac04f9052b08022837
         layer 6: digest = sha256:9bec334343957dcbf01d4162396961a414031c13a8c371158ee1673c3b704e30
         layer 7: digest = sha256:bcd4778b949e06f46d46b040ff6fb662abdda3b0488a897c9df2ffffae85ad27
         layer 8: digest = sha256:0d153bd8fade274b194b5ca148db5b037976802eda679b6a1ed42e54b6c7ad80

2    Mfst Type: application/vnd.docker.distribution.manifest.v2+json
2       Digest: sha256:2fee40571fb6c1e8f9e5ae2c23c52e4f774463cc92e5fed9e38e4c7680d7ccbe
2  Mfst Length: 3655
2     Platform:
2           -      OS: windows
2           - OS Vers: 10.0.14393.1593
2           - OS Feat: []
2           -    Arch: amd64
2           - Variant: 
2           - Feature: 
2     # Layers: 15
         layer 1: digest = sha256:bce2fbc256ea437a87dadac2f69aabd25bed4f56255549090056c1131fad0277
         layer 2: digest = sha256:38cc73423ca1d089e2e2374a8baf65d25d3792b22a22263c702f22f85bea6d4c
         layer 3: digest = sha256:267f08fcc3d3caf29f6f87993875c0fbe76f24b55a457a55785a6ff49275d1c7
         layer 4: digest = sha256:005551875be90a5ba8d2dd1db41beb4449163b1cd271e8b602f4c4473501ea8a
         layer 5: digest = sha256:b08cd4ea693669f695f28c9be6bf94e50fca54db60b7002e23bc131971bc1f9c
         layer 6: digest = sha256:52a3c461341c59d0814df54db74915973d81351378a4eacdfe151657c058f56b
         layer 7: digest = sha256:9fa1e8bb2400f9772dd2be2aa72b96a6770d3ecd78f9eb79af7286c11b0a0ab5
         layer 8: digest = sha256:a1312f31c5d269feb06528efc0584350d6ff326e9e93e909a811f42fcbab3cee
         layer 9: digest = sha256:f6dc9dc5fe384538ac1297caa818d55833d106e174a638c5500a110efe83cfdc
         layer 10: digest = sha256:28a4d575e76d724f3a04a3b1a1d4ede9359a3058cbced820a255be74eb4f554f
         layer 11: digest = sha256:badc8b257f42d9157305083945b6f39cce43a24a823fea4549b561dfb093f786
         layer 12: digest = sha256:a668ea7fc09789c360b82e839be2828f5f22059576d4e259492f6934f8d46b51
         layer 13: digest = sha256:1b1d7afc0e0c8819b360a8779eccc2a246e151e1716f3a5503566336a3f23320
         layer 14: digest = sha256:aa7652eb3d23499f5029f5233844f544be372ccc8c460eb9c35124c31d34e045
         layer 15: digest = sha256:6503edba0483f1987f2c3efb813cbe1e8047c3fd8c5e7469eff67d26b79ac36c

3    Mfst Type: application/vnd.docker.distribution.manifest.v2+json
3       Digest: sha256:fe868e70618dcb705ffd9d18abad2ce7fbca03bb3e9946ba694b660605cd523a
3  Mfst Length: 2508
3     Platform:
3           -      OS: windows
3           - OS Vers: 10.0.16278.1000
3           - OS Feat: []
3           -    Arch: amd64
3           - Variant: 
3           - Feature: 
3     # Layers: 10
         layer 1: digest = sha256:3cda982da924f03f7f053877b8623b1875e156b2e4367b67c97a2d46faf1d31c
         layer 2: digest = sha256:811205f974ff0313b59ee8e8f57af8d0ad94fa0e8b3979201103700956593922
         layer 3: digest = sha256:db0267c0291af29c532d1363feb282698139845f3a329e2800bb10794f4ee94b
         layer 4: digest = sha256:39606fe45686029c52a76cbf9e067ab66cd06fa5a5133467bc7a9a182eb877a3
         layer 5: digest = sha256:0dcbf3933c064d8414da5756b845a0d43255dd6bc337e4993aefc0b55371a608
         layer 6: digest = sha256:abffaaae46c5cecaea1f4f45508ff705c8db7562ca45b06be53f45a5a376b142
         layer 7: digest = sha256:326f417b869841f873cbdf542fd2918b5ece6d1299de0ad1bcf7719694eb46ee
         layer 8: digest = sha256:93f5a844fef7e68353b7d7ffaf8e2c99634b3a7d92fecf035a0b244844350929
         layer 9: digest = sha256:28a113ff085e8c3e0b844dbce223668a811f23759b397f780d183a34c6afe400
         layer 10: digest = sha256:3ce400ac9d9d6f8d164ace5c85df454d545035c635ab84a1a30802e18f152c26

johnstep · 2017-10-08T17:18:33Z

@StefanScherer Thanks, yes, that is the scenario this change supports, by moving matching version(s) to the top of the manifest list before selecting the first image. Currently, the UBR (.1593 and .1000 above) is ignored, but will be considered in the future. I have verified this works as expected, pulling the RS1 image on RS1, and the RS3 image on RS3, based on matching major, minor, and build numbers.

StefanScherer · 2017-11-08T07:08:12Z

I tried the Win10 Insider 17025 yesterday and tried to pull Windows image with a manifest list with RS1 + RS3 image. Docker engine then pulled the much bigger RS1 image (the first in the list).

Should we enhance the selection to pick the closest os.version that is lower than the host os.version?

Or would it be a best practise to provide manifest lists with multiple Windows images in a descending order (eg. RS5, RS3, RS1)?

I used the order RS1 + RS3 as older Docker engines just pick the first Windows image from the manifest list and more people are still on RS1.

thaJeztah · 2017-11-08T10:07:55Z

@StefanScherer could you also open a separate issue / proposal for that (to prevent it from going lost)? (I think ordering came up in another discussion as well, but I'll have to search which one that was; may have been in the https://github.com/docker/distribution repository)

GordonTheTurtle added the status/0-triage label Oct 4, 2017

johnstep reviewed Oct 4, 2017

View reviewed changes

dmcgowan force-pushed the windows-support-os-version branch from 30afe1c to 88df5be Compare October 5, 2017 00:13

johnstep approved these changes Oct 5, 2017

View reviewed changes

johnstep added status/2-code-review and removed status/0-triage labels Oct 5, 2017

stevvooe reviewed Oct 5, 2017

View reviewed changes

distribution/pull_v2_unix.go Outdated

Copy link

Contributor

stevvooe Oct 5, 2017

Choose a reason for hiding this comment

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

This TODO can be removed. Features is out. Variant is ARM only.

Add support for Windows version filtering on pull

38aef56

Update logic to choose manifest from manifest list to check for os version on Windows. Separate the logic for windows and unix to keep unix logic the same. Signed-off-by: Derek McGowan <derek@mcgstyle.net>

dmcgowan force-pushed the windows-support-os-version branch from 88df5be to 38aef56 Compare October 5, 2017 20:32

yongtang added status/4-merge and removed status/2-code-review labels Oct 5, 2017

yongtang merged commit b8571fd into moby:master Oct 6, 2017

thaJeztah added area/distribution Image Distribution platform/windows labels Oct 6, 2017

lowenna mentioned this pull request Oct 6, 2017

LCOW: API: Add platform to /images/create and /build #34642

Merged

johnstep mentioned this pull request Oct 11, 2017

[17.10] Backport version fix for Windows manifest lists docker-archive/docker-ce#266

Merged

This was referenced Nov 3, 2017

microsoft/dotnet nanoserver images now support Windows Server 1709 dotnet/dotnet-docker#330

Closed

microsoft/dotnet nanoserver images now support Windows Server 1709 dotnet/announcements#37

Open

mdlinville mentioned this pull request Nov 9, 2017

Document support for Windows version filtering on pull docker/docs#5261

Closed

johnstep mentioned this pull request Nov 20, 2017

[17.09] Backport version fix for Windows manifest lists docker-archive/docker-ce#327

Merged

thaJeztah mentioned this pull request Dec 12, 2018

Adding OS version info to nodes' Info struct and to the system info's API #38349

Merged

Conversation

dmcgowan commented Oct 4, 2017

Uh oh!

johnstep Oct 4, 2017

Choose a reason for hiding this comment

Uh oh!

johnstep left a comment

Choose a reason for hiding this comment

Uh oh!

stevvooe Oct 5, 2017

Choose a reason for hiding this comment

Uh oh!

stevvooe commented Oct 5, 2017

Uh oh!

lowenna commented Oct 6, 2017

Uh oh!

lowenna commented Oct 6, 2017

Uh oh!

johnstep commented Oct 6, 2017

Uh oh!

lowenna commented Oct 6, 2017

Uh oh!

johnstep commented Oct 7, 2017

Uh oh!

StefanScherer commented Oct 8, 2017

Uh oh!

johnstep commented Oct 8, 2017

Uh oh!

StefanScherer commented Nov 8, 2017

Uh oh!

thaJeztah commented Nov 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants