Windows: Try to detach sandbox on cleanup to avoid permission denied by lowenna · Pull Request #37712 · moby/moby

lowenna · 2018-08-23T23:22:53Z

A few different scenarios, but it's possible that the daemon can leak sandbox/scratch layers in the Windows graphdriver, and have side effects where containers can't be removed. Here's one such case:

Before:

C:\>start dockerd -g c:\control

C:\>docker run --rm -d --name test microsoft/windowsservercore powershell sleep 30
8058fa79bb1c1b580b32bc2e4e27f22090a27fd8cef17aaa31ae02342982e7d3

C:\>taskkill /T /IM dockerd.exe
SUCCESS: Sent termination signal to process with PID 21396, child of PID 21248.
SUCCESS: Sent termination signal to process with PID 21248, child of PID 29336.

C:\>start dockerd -g c:\control

C:\>docker ps -a
CONTAINER ID        IMAGE                         COMMAND                 CREATED             STATUS                PORTS               NAMES
8058fa79bb1c        microsoft/windowsservercore   "powershell sleep 30"   16 seconds ago      Removal In Progress                       test

C:\>docker rm -f 8058
Error response from daemon: container 8058fa79bb1c1b580b32bc2e4e27f22090a27fd8cef17aaa31ae02342982e7d3: driver "windowsfilter" failed to remove root filesystem: rename C:\control\windowsfilter\8058fa79bb1c1b580b32bc2e4e27f22090a27fd8cef17aaa31ae02342982e7d3 C:\control\windowsfilter\8058fa79bb1c1b580b32bc2e4e27f22090a27fd8cef17aaa31ae02342982e7d3-removing: Access is denied.

C:\>dir control\windowsfilter
 Volume in drive C has no label.
 Volume Serial Number is 2E9A-473A

 Directory of C:\control\windowsfilter

08/23/2018  04:16 PM    <DIR>          .
08/23/2018  04:16 PM    <DIR>          ..
08/23/2018  04:16 PM    <DIR>          8058fa79bb1c1b580b32bc2e4e27f22090a27fd8cef17aaa31ae02342982e7d3
08/23/2018  09:04 AM    <DIR>          efa7700f8e4b54cdb068d0c08d600b483846091611cf913f3ba411525207bf93
               0 File(s)              0 bytes
               4 Dir(s)  117,058,809,856 bytes free

C:\>

After:

C:\>start dockerd -g c:\control

C:\>docker run --rm -d --name test microsoft/windowsservercore powershell sleep 30
01779097b29c3d0b04373c6f1b929f79ccce72a6f43689ade7e9f4cce9a977cd

C:\>taskkill /T /IM dockerd.exe
SUCCESS: Sent termination signal to process with PID 6172, child of PID 27248.
SUCCESS: Sent termination signal to process with PID 27248, child of PID 29336.

C:\>start dockerd -g c:\control

C:\>docker ps -a
CONTAINER ID        IMAGE               COMMAND             CREATED             STATUS              PORTS               NAMES

C:\>dir control\windowsfilter
 Volume in drive C has no label.
 Volume Serial Number is 2E9A-473A

 Directory of C:\control\windowsfilter

08/23/2018  04:14 PM    <DIR>          .
08/23/2018  04:14 PM    <DIR>          ..
08/23/2018  09:04 AM    <DIR>          efa7700f8e4b54cdb068d0c08d600b483846091611cf913f3ba411525207bf93
               0 File(s)              0 bytes
               3 Dir(s)  117,096,996,864 bytes free

C:\>

lowenna · 2018-08-23T23:23:12Z

Note this might not (won't) pass vendoring until #37674 is merged.

lowenna · 2018-08-23T23:25:18Z

@johnstep PTAL. @jterry75 As mentioned offline, FYI.

lowenna · 2018-08-23T23:28:18Z

@carlfischer1. Another backport candidate.

johnstep · 2018-08-24T00:01:30Z

Kind of strange to call the error exists, since the true case is nil, but okay, I guess.

Agree this is confusing to read. Does can we just call DetachVhd directly instead of checking existence first?

I'll update to address both of these.

johnstep · 2018-08-24T00:04:46Z

Is it really worth retrying if nothing changed (the sandbox file did not exist)?

codecov · 2018-08-24T01:09:19Z

Codecov Report

❗ No coverage uploaded for pull request base (master@7aa797f). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #37712   +/-   ##
=========================================
  Coverage          ?   36.02%           
=========================================
  Files             ?      610           
  Lines             ?    45071           
  Branches          ?        0           
=========================================
  Hits              ?    16238           
  Misses            ?    26603           
  Partials          ?     2230

lowenna · 2018-08-24T01:10:08Z

@cpuguy83 @johnstep Updated. Still requires a rebase after #37674 is merged before it will fully pass CI

lowenna · 2018-08-27T17:39:24Z

Rebased. Windows CI and vendor passed (the only path affected here). Rest are completing.

@cpuguy83 are you OK with this now?

@thaJeztah Could you take a look too please?

cpuguy83

Can we add some additional context to the error messages?

cpuguy83 · 2018-08-27T20:52:13Z

Nit, but a little cleaner if we return early when "!os.IsPermission(err)"

Oooh, missed that comment @cpuguy83. Yes, that's a lot cleaner. Updated.

Signed-off-by: John Howard <[email protected]>

cpuguy83 · 2018-09-06T17:07:38Z

Can you just add some extra context to the new errors, e.g. errors.Wrap(err, "failed to detach VHD")

lowenna · 2018-09-06T17:29:33Z

Can you just add some extra context to the new errors, e.g. errors.Wrap(err, "failed to detach VHD")

@cpuguy83 Done.

cpuguy83

LGTM

cpuguy83 · 2018-09-06T17:31:31Z

Doh. Fixing....

Fixed now. Good catch 👍

Signed-off-by: John Howard <[email protected]> This is a fix for a few related scenarios where it's impossible to remove layers or containers until the host is rebooted. Generally (or at least easiest to repro) through a forced daemon kill while a container is running. Possibly slightly worse than that, as following a host reboot, the scratch layer would possibly be leaked and left on disk under the dataroot\windowsfilter directory after the container is removed. One such example of a failure: 1. run a long running container with the --rm flag docker run --rm -d --name test microsoft/windowsservercore powershell sleep 30 2. Force kill the daemon not allowing it to cleanup. Simulates a crash or a host power-cycle. 3. (re-)Start daemon 4. docker ps -a PS C:\control> docker ps -a CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES 7aff773d782b malloc "powershell start-sl…" 11 seconds ago Removal In Progress malloc 5. Try to remove PS C:\control> docker rm 7aff Error response from daemon: container 7aff773d782bbf35d95095369ffcb170b7b8f0e6f8f65d5aff42abf61234855d: driver "windowsfilter" failed to remove root filesystem: rename C:\control\windowsfilter\7aff773d782bbf35d95095369ffcb170b7b8f0e6f8f65d5aff42abf61234855d C:\control\windowsfilter\7aff773d782bbf35d95095369ffcb170b7b8f0e6f8f65d5aff42abf61234855d-removing: Access is denied. PS C:\control> Step 5 fails.

lowenna · 2018-11-09T18:18:39Z

@thaJeztah Do you know if this will make it into 18.09 EE? Not sure what the cut-off was.

thaJeztah · 2018-11-09T20:31:25Z

@jhowardmsft looks like it didn't (but will double check when I'm back at my keyboard), I can see I opened a backport request for it, but it slipped of my radar.

I think a patch release is scheduled, so I'll open backports where needed, so that it will be included in that release

lowenna · 2018-11-09T21:29:48Z

@thaJeztah Thanks. Kind of an important one as I know it fixes multiple production issues I've had to debug.

drnybble · 2019-09-17T20:55:06Z

Still seeing this with 19.03.2 on a clean Windows 2016 VM created from ISO image and fully patched.

driver "windowsfilter" failed to remove root filesystem: hcsshim::GetComputeSystems: Access is denied.

thaJeztah · 2019-09-24T20:00:50Z

@drnybble that error message looks slightly different than the one mentioned here; perhaps it's a different issue; could you open a new ticket? (and include a link to this PR and the related issue #36218 to make it easier to find)?

Bernolt · 2019-11-20T14:29:23Z

We had the same issue as @drnybble. (We used Unlocker Windows to delete the files)

leiwang008 · 2020-03-24T08:44:08Z

I am still facing this problem. I use the "Docker Desktop" in Windows 10 OS, I built image with "mcr.microsoft.com/windows/servercore:1709", during the build I got the following message

**Error removing intermediate container 720292babe10: container 720292babe105f947428d0ca5fd02533277fc8788f9303e0752ee3eadf2b4439: driver "windowsfilter" failed to remove root filesystem: failed to detach VHD: The device is not ready.: rename C:\ProgramData\Docker\windowsfilter\720292babe105f947428d0ca5fd02533277fc8788f9303e0752ee3eadf2b4439 C:\ProgramData\Docker\windowsfilter\720292babe105f947428d0ca5fd02533277fc8788f9303e0752ee3eadf2b4439-removing: Access is denied.**

I could not remove the left-behind containers with command "docker container prune --force", after I restarted my computer, I could use that command to delete the left-behind containers.

Below is the information of "Docker" that I am using

Client: Docker Engine - Community
 Version:           19.03.5
 API version:       1.40
 Go version:        go1.12.12
 Git commit:        633a0ea
 Built:             Wed Nov 13 07:22:37 2019
 OS/Arch:           windows/amd64
 Experimental:      true

Server: Docker Engine - Community
 Engine:
  Version:          19.03.5
  API version:      1.40 (minimum version 1.24)
  Go version:       go1.12.12
  Git commit:       633a0ea
  Built:            Wed Nov 13 07:36:50 2019
  OS/Arch:          windows/amd64
  Experimental:     true

aries1980 · 2021-01-06T21:54:26Z

I have the same issue as #37712 (comment) .

PS C:\Windows\system32> [System.Environment]::OSVersion.Version

Major  Minor  Build  Revision
-----  -----  -----  --------
10     0      17763  0

aka /aws/service/ami-windows-latest/Windows_Server-2019-English-Core-ECS_Optimized

PS C:\Windows\system32> docker version
Client: Mirantis Container Runtime
 Version:           19.03.13
 API version:       1.40
 Go version:        go1.13.15
 Git commit:        0c38b2d
 Built:             11/12/2020 21:33:08
 OS/Arch:           windows/amd64
 Experimental:      false

Server: Mirantis Container Runtime
 Engine:
  Version:          19.03.13
  API version:      1.40 (minimum version 1.24)
  Go version:       go1.13.15
  Git commit:       5b5efb2d49
  Built:            11/12/2020 21:31:44
  OS/Arch:          windows/amd64
  Experimental:     false

running the jenkins/inbound-agent:windowsservercore-ltsc2019 container image.

lowenna requested a review from johnstep as a code owner August 23, 2018 23:22

GordonTheTurtle added the status/0-triage label Aug 23, 2018

lowenna force-pushed the jjh/detach branch from 366ede9 to 54d54d2 Compare August 23, 2018 23:24

johnstep approved these changes Aug 24, 2018

View reviewed changes

johnstep reviewed Aug 24, 2018

View reviewed changes

lowenna force-pushed the jjh/detach branch from 54d54d2 to 1c70cde Compare August 24, 2018 01:09

swernli approved these changes Aug 24, 2018

View reviewed changes

This was referenced Aug 24, 2018

Revendor Microsoft/hcsshim and go-winio, plus container/containerd #37674

Merged

Unable to Remove Containers: driver "windowsfilter" failed to remove root filesystem #36218

Closed

lowenna force-pushed the jjh/detach branch from 1c70cde to aa26ccf Compare August 27, 2018 15:51

lowenna added status/2-code-review platform/windows rebuild/powerpc and removed status/0-triage labels Aug 27, 2018

GordonTheTurtle removed the rebuild/powerpc label Aug 27, 2018

cpuguy83 reviewed Aug 27, 2018

View reviewed changes

yongtang added the rebuild/powerpc label Sep 2, 2018

GordonTheTurtle removed the rebuild/powerpc label Sep 2, 2018

lowenna force-pushed the jjh/detach branch from aa26ccf to fc01b4f Compare September 6, 2018 16:53

Vendor Microsoft/go-winio v0.4.11

6696694

Signed-off-by: John Howard <[email protected]>

lowenna force-pushed the jjh/detach branch from fc01b4f to 09f59ac Compare September 6, 2018 16:56

lowenna force-pushed the jjh/detach branch from 09f59ac to 55b4719 Compare September 6, 2018 17:29

cpuguy83 approved these changes Sep 6, 2018

View reviewed changes

cpuguy83 reviewed Sep 6, 2018

View reviewed changes

lowenna force-pushed the jjh/detach branch from 55b4719 to efdad53 Compare September 6, 2018 20:18

lowenna added rebuild/powerpc labels Sep 6, 2018

GordonTheTurtle removed rebuild/powerpc labels Sep 6, 2018

GordonTheTurtle assigned yongtang Sep 7, 2018

yongtang added the rebuild/z label Sep 9, 2018

GordonTheTurtle removed the rebuild/z label Sep 9, 2018

yongtang merged commit 703a04e into moby:master Sep 9, 2018

lowenna deleted the jjh/detach branch September 14, 2018 18:35

thaJeztah mentioned this pull request Nov 9, 2018

[18.09 backport] Windows: DetachVhd attempt in cleanup docker-archive/engine#113

Merged

olljanat mentioned this pull request May 30, 2019

Windows graphdriver - Ignore "invalid argument" error if detach fails during Remove #39290

Closed

TBBle mentioned this pull request Apr 24, 2021

Use DeactivateLayer to unlock layers that we cannot rename containerd/containerd#5422

Merged

Conversation

lowenna commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lowenna commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lowenna commented Aug 23, 2018

Uh oh!

lowenna commented Aug 23, 2018

Uh oh!

johnstep Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

lowenna Aug 24, 2018

Choose a reason for hiding this comment

Uh oh!

johnstep Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lowenna commented Aug 24, 2018

Uh oh!

lowenna commented Aug 27, 2018

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Aug 27, 2018

Choose a reason for hiding this comment

Uh oh!

lowenna Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

cpuguy83 commented Sep 6, 2018

Uh oh!

lowenna commented Sep 6, 2018

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

lowenna Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

lowenna Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

lowenna commented Nov 9, 2018

Uh oh!

thaJeztah commented Nov 9, 2018

Uh oh!

lowenna commented Nov 9, 2018

Uh oh!

drnybble commented Sep 17, 2019

Uh oh!

thaJeztah commented Sep 24, 2019

Uh oh!

Bernolt commented Nov 20, 2019

Uh oh!

leiwang008 commented Mar 24, 2020 • edited by thaJeztah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aries1980 commented Jan 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

lowenna commented Aug 23, 2018 •

edited

Loading

lowenna commented Aug 23, 2018 •

edited

Loading

johnstep Aug 24, 2018 •

edited

Loading

johnstep Aug 24, 2018 •

edited

Loading

codecov Bot commented Aug 24, 2018 •

edited

Loading

leiwang008 commented Mar 24, 2020 •

edited by thaJeztah

Loading