Build succeeded.

• containerd-build-arm64 : FAILURE in 2m 07s (non-voting)

I've speculatively enabled the snapshot tests on Windows, in case I've fixed them as part of this. If that's not the case, I'll back that enablement out for later.

Edit: Yup, a mix of complaints from hcsshim and failures due to permissions issues, plus error "unknown" from the Basic test, means this needs more work to support the snapshot test suite. Hopefully the work is in the suite, not the code...

Build succeeded.

• containerd-build-arm64 : FAILURE in 2m 28s (non-voting)

There's a bunch of refactoring opportunities I saw from this work.

Particularly, it seems that the APIs for archive.WriteDiff and archive.Apply are somewhat nonsensical when applied to windows layers, as we use the Opts to smuggle the real parameters through them, and ignore the code in favour of complete Windows-specific implementations. I think these two methods might need more insight into the thing they're being called on, to actually do their job without so much hand-holding.

Apart from tests, WriteDiff only appears to be used from the walkingDiff, and windowsDiff implementations of Compare, and for taking task checkpoints. The latter is always diffing against "", and does not interact with Windows Container layers.

Apply is used more-widely, but again only windowsDiff knows about Windows Container layers.

I suspect it would be cleaner to not involve archive in the Windows Container layers, but to directly implement those functions in windowsDiff. Particularly if we lean more on hcsshim and go-winio for the actual implementations.

Build succeeded.

• containerd-build-arm64 : FAILURE in 2m 08s (non-voting)

Build succeeded.

• containerd-build-arm64 : FAILURE in 2m 26s (non-voting)

Build succeeded.

• containerd-build-arm64 : FAILURE in 2m 09s (non-voting)

@kevpar / @jstarks - PTAL

@ambarve

Build succeeded.

• containerd-build-arm64 : FAILURE in 2m 14s (non-voting)

@ambarve - PTAL

Build succeeded.

• containerd-build-arm64 : FAILURE in 2m 00s (non-voting)

Marked as draft pending rebase on updated #4395, which will remove the winio_tar aspect of this change.

Edit: Done

Build succeeded.

• containerd-build-arm64 : SUCCESS in 5m 52s (non-voting)

Hmm. Rebasing showed a random bunch of HTTP-related failures. The Windows Integration run showed:

2021-04-05T04:02:48.2118915Z     convert_test.go:50: failed to copy: stream error: stream ID 9; HTTP_1_1_REQUIRED
2021-04-05T04:02:48.2120056Z --- FAIL: TestConvert (5.78s)

which is clearly unrelated to my changes here. The Linux suite are all failing on CRI tests, getting '403 forbidden' from GCR for some image or images. We're seeing the same thing (on Linux) on main-branch HEAD, so definitely unrelated.

We'll see if the Windows issue goes away when master is working and this is rebased again. Edit: The PRs that depend on this one (#4415, #4419) passed Windows CI, so I am pretty confident this failure isn't to do with this PR.

@TBBle - Should we just re-run the tests are are you going ot rebase on some master change that is clean?

I can't retrigger the tests AFAIK, but if you can, that'd be fine. I had a poke around, and the failures looked more like infrastructure issues than code issues, so I am not specifically waiting on a fix to rebase on-to.

I only rebased this time because I needed to rebase #4419 to verify a change, and figured I'd do the whole stack to keep my git show-branches output rational.

Build succeeded.

• containerd-build-arm64 : SUCCESS in 5m 49s (non-voting)

Gah. The rebase to current mainline branch passed: https://github.com/containerd/containerd/actions/runs/725556015
And then a comment-only fix failed in unrelated tests on Windows: https://github.com/containerd/containerd/actions/runs/725563341

2021-04-07T10:00:33.8681308Z === CONT  TestContainerKill
2021-04-07T10:00:33.8694170Z     container_test.go:620: failed to mount container storage: hcsshim::PrepareLayer - failed failed in Win32: The system cannot find the path specified. (0x3): unknown
2021-04-07T10:00:33.9335940Z --- FAIL: TestContainerKill (2.32s)

2021-04-07T10:01:33.7263019Z === CONT  TestContainerNoBinaryExists
2021-04-07T10:01:33.7264655Z     container_test.go:759: failed to create task failed to mount container storage: hcsshim::PrepareLayer - failed failed in Win32: The device is not ready. (0x15): unknown
2021-04-07T10:01:33.7552071Z --- FAIL: TestContainerNoBinaryExists (62.14s)

It's green-enough for me, since this PR doesn't actually change any of the on-disk layer data, and these look suspiciously like the HCS-was-busy-and-reported-success-too-early issue I speculated in #4419.

Notably, the two tests this PR enables, TestExport and TestExportAndImport, had passed before this point in the run

2021-04-07T09:59:53.0784565Z --- PASS: TestExport (5.86s)
2021-04-07T10:00:30.3192956Z --- PASS: TestExportAndImport (15.25s)

@TBBle - This looks to be a longstanding bug with the containerd shim here: https://github.com/microsoft/hcsshim/blob/master/cmd/containerd-shim-runhcs-v1/delete.go#L29. The code properly handles the vm/container not found but I believe unintentionally re-uses the err defined when the span status is set here: https://github.com/microsoft/hcsshim/blob/master/cmd/containerd-shim-runhcs-v1/delete.go#L36. I believe this is truly unrelated to your change and will sign off.

Thanks!

@ambarve - Would you mind taking the bug here. The shim should not return an error on the span if that error would be handled. Easiest way would be to rename the span error used and only assign in the case we actually write an error to stderr via a defer statement. But you can come up with whatever logic you like

@fuweid - I looked at the Windows error and agree this is unrelated to this change. Are you ok taking it with the transient error while @ambarve can get a fix out?

@TBBle would you mind to rebase? Your change might be not related to the error. Please rebase and retry it again. Thanks

Build succeeded.

• containerd-build-arm64 : SUCCESS in 6m 35s (non-voting)

All green!