Full diff;

• microsoft/hcsshim@v0.6.12...v0.6.14

  • Changes to support registry modification in containers microsoft/hcsshim#272 Changes to support registry modification in containers
  • v0.6: Limit parallel starts microsoft/hcsshim#281 v0.6: Limit parallel starts
  • Changes based on PR microsoft/hcsshim#283 Fix fileattributes in the v0.6-dev branch to match current go-winio

• microsoft/go-winio@v0.4.9...v0.4.10

  • Update FileAttributes size to work across all platforms microsoft/go-winio#88 Update FileAttributes size to work across all platforms

@jhowardmsft build is failing;

16:46:12 # github.com/docker/docker/vendor/github.com/containerd/containerd/archive
16:46:12 ..\..\vendor\github.com\containerd\containerd\archive\tar_windows.go:262:27: cannot use uintptr(attr) (type uintptr) as type uint32 in assignment
16:46:23 
16:46:23 ERROR: make.ps1 failed:
16:46:23 Failed to compile daemon

@thaJeztah Hold fire on this - there's a commit missing. Changing to WIP

@thaJeztah It's because containerd/containerd needs bumping too due to it's revendoring. Dependent on containerd/containerd#2557 and a revendor of containerd into moby/moby once that is merged.

@jhowardmsft containerd/containerd#2557 is merged now 👍

Codecov Report

❗ No coverage uploaded for pull request base (master@0d9d861). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #37674   +/-   ##
=========================================
  Coverage          ?   36.03%           
=========================================
  Files             ?      609           
  Lines             ?    45063           
  Branches          ?        0           
=========================================
  Hits              ?    16238           
  Misses            ?    26596           
  Partials          ?     2229

@thaJeztah Yup, saw. Bumped containerd, fingers crossed all should be good now.

LGTM

Janky and Experimental failing on; https://jenkins.dockerproject.org/job/Docker-PRs/50499/console

20:34:41 --- FAIL: TestAuthZPluginEnsureLoadImportWorking (1.38s)
20:34:41 	daemon.go:289: [d69ba33f8b5fc] waiting for daemon to start
20:34:41 	daemon.go:321: [d69ba33f8b5fc] daemon started
20:34:41 	authz_plugin_test.go:380: assertion failed: error is not nil: unexpected EOF
20:34:41 	daemon.go:279: [d69ba33f8b5fc] exiting daemon
20:34:41 	main_test.go:57: Error while stopping the daemon d69ba33f8b5fc : exit 

not sure I've seen that one before; I'll restart but could be legit

Moving back to code-review; seeing the same failure on Janky and Experimental again, so may be legit; possibly caused by the containerd bump

@dmcgowan ^^

It looks like this has to do with the tar changes that came in.

Your vendor has removed archive/tar code in it. Maybe it is a failed vndr run but I don't think those should be removed and that will probably resolve the issue

Yeah, I saw that, and was surprised when vndr did that. However, the vendor CI job passes and matches my vndr run. I can't explain why vndr has dropped it (clearly something previously referencing it no longer does), or why it should be causing this failure. @thaJeztah Have you any clues here? This is mind boggling.

Yeah, I saw that, and was surprised when vndr did that. However, the vendor CI job passes and matches my vndr run

Oh! I know... You must have done a full vndr, but using the current "master" version of vndr, not the version that we pinned to

The vndr/archive is a manual vendor, and the current "master" of vndr wipes the vendor directory before doing the actual vendoring. See LK4D4/vndr#72 (introduced in LK4D4/vndr#65)

To work around this, either;

• only run vndr for the dependencies that were updated (vndr github.com/Microsoft/hcsshim, vndr github.com/Microsoft/go-winio)
• use the pinned version of vndr:

  moby/hack/dockerfile/install/vndr.installer

  Line 3 in 49ad3d6

  VNDR_COMMIT=a6e196d8b4b0cbbdc29aebdb20c59ac6926bb384

@thaJeztah Thanks, that explains it. I undid that part. Hopefully Janky/Experimental pass now.

Thanks! Can you squash that commit with the one that removed it?

Thanks! Can you squash that commit with the one that removed it?

Done

@thaJeztah Good to merge now? (As I need to rebase #37712 on top of it)

all green; merging