WIP: Mount Windows layers as volumes on the host #2287

darstahl · 2018-04-12T01:14:31Z

This is a work in progress implementation of mounting Windows volumes on the host. It currently is failing the Basic snapshotter test due to a few assumptions made by the tests. The first issue is that the root directory of the layer is a link, which is tripping up the filesystem differ. The second is mounting a parent and child layer at the same time is currently failing.

This PR is to share the current progress.

/cc @johnstep @stevvooe @dmcgowan

dmcgowan · 2018-04-16T18:55:08Z

mount/mount_windows.go

This is stripping the target down to just the directory with no path separator?

Yep. It is required for the alternate data stream (ADS). Looking back at this, I think the better way to do this is to use filepath.Split and add the ADS tag to the filename and Join again.

dmcgowan · 2018-04-16T18:55:45Z

mount/mount_windows.go

Can you explain this one? Is there any documentation for this which could be referred to in a comment?

This is an alternate data stream on the mounted layer folder. Short version is that this is used to store extra data on the same file without affecting the actual file.

Docs on alternate data streams that I can add in a comment https://blogs.technet.microsoft.com/askcore/2013/03/24/alternate-data-streams-in-ntfs/

dmcgowan · 2018-04-16T18:56:11Z

mount/mount_windows.go

Same comment here for documentation, curious what this means

I will look back at this to see if there is a better way to do it, but this is checking if the sandbox was mounted as a volume or if the function returned the path to the expanded layer on disk. \\?\ is the prefix of a volume on Windows, the volume path is of the form \\?\Volume{bc6dcf0c-7fde-4d12-9853-0d1dafad0b5b}\. If the expanded layer is returned instead of the volume GUID path, it needs to be mounted via symlink. Will add a comment to explain if I can't find a way that's obvious what it is doing.

dmcgowan · 2018-04-16T19:03:21Z

It is nice to see the tests running though. The base related changes and changes to tests look good so far.

What is the impact of this target + ":layerid" file? They will just need to be created anywhere that is mounted or is created as metadata on the original target?

darstahl · 2018-04-16T19:33:42Z

target + ":layerid" is metadata on the mounted folder to correlate back to which HCS layer is being used to mount so that it can be unmounted. See above about Alternate Data Streams for more details on what is being used to store the metadata.

darstahl · 2018-04-18T19:03:01Z

Pushed more commits that get the following tests passing on Windows:

t.Run("Basic", makeTest(name, snapshotterFn, checkSnapshotterBasic))
t.Run("StatActive", makeTest(name, snapshotterFn, checkSnapshotterStatActive))
t.Run("StatComitted", makeTest(name, snapshotterFn, checkSnapshotterStatCommitted))
t.Run("TransitivityTest", makeTest(name, snapshotterFn, checkSnapshotterTransitivity))
t.Run("PreareViewFailingtest", makeTest(name, snapshotterFn, checkSnapshotterPrepareView))
t.Run("Update", makeTest(name, snapshotterFn, checkUpdate))

dmcgowan · 2018-05-02T22:51:50Z

Can you rebase, also there is a vet complaint

gometalinter --config .gometalinter.json ./...
snapshots\testsuite\testsuite.go:51::error: unreachable code (vet)
Makefile:109: recipe for target 'check' failed
mingw32-make: *** [check] Error 1

darstahl · 2018-05-02T22:54:01Z

Vet failure is due to enabling certain tests while in development. I'm adding proper test skipping for tests that will not work on Windows in a change soon.

Will rebase.

Signed-off-by: Darren Stahl <[email protected]>

darstahl · 2018-05-05T00:51:17Z

Locally tests are passing. Debugging why they're failing in CI.

Edit: I also need to get the Continuity changes into there to vendor into containerd.

Signed-off-by: Darren Stahl <[email protected]>

darstahl · 2018-05-09T01:35:21Z

This PR is ready for review pending containerd/continuity#113

Signed-off-by: Darren Stahl <[email protected]>

codecov-io · 2018-05-11T00:20:05Z

Codecov Report

Merging #2287 into master will decrease coverage by 3.96%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master    #2287      +/-   ##
==========================================
- Coverage   49.28%   45.31%   -3.97%     
==========================================
  Files          84       93       +9     
  Lines        7424     9572    +2148     
==========================================
+ Hits         3659     4338     +679     
- Misses       3089     4532    +1443     
- Partials      676      702      +26

Flag	Coverage Δ
#linux	`49.28% <ø> (ø)`	⬆️
#windows	`41.75% <75%> (?)`

Impacted Files	Coverage Δ
snapshots/windows/windows.go	`58.62% <75%> (ø)`
oci/spec.go	`40% <0%> (-10%)`	⬇️
snapshots/native/native.go	`43.89% <0%> (-10%)`	⬇️
metadata/snapshot.go	`45.8% <0%> (-8.96%)`	⬇️
archive/compression/compression.go	`43.93% <0%> (-8.9%)`	⬇️
remotes/docker/fetcher.go	`41.42% <0%> (-7.6%)`	⬇️
content/local/writer.go	`52.63% <0%> (-7.37%)`	⬇️
archive/tar.go	`43.05% <0%> (-6.95%)`	⬇️
metadata/containers.go	`47.97% <0%> (-6.62%)`	⬇️
metadata/buckets.go	`54.66% <0%> (-5.04%)`	⬇️
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6eee2a0...a12b082. Read the comment docs.

crosbymichael · 2018-07-25T15:17:48Z

Going to close this one, we have a carry PR open and windows work is still ongoing

darstahl force-pushed the MountVol branch from b168512 to 6be0fdd Compare April 12, 2018 01:35

dmcgowan reviewed Apr 16, 2018

View reviewed changes

dmcgowan added this to the 1.2 milestone Apr 16, 2018

darstahl force-pushed the MountVol branch from 3e3fe79 to 9f17336 Compare April 19, 2018 01:10

darstahl added 10 commits May 4, 2018 17:12

Update /x/sys/windows vendor to include volume mounting

e10a628

Signed-off-by: Darren Stahl <[email protected]>

WIP: mounting Windows volumes using SetVolumeMountPath

2126d64

Signed-off-by: Darren Stahl <[email protected]>

Fork filepath.Walk to follow symlinks on the root

bcb73e8

Signed-off-by: Darren Stahl <[email protected]>

Ignore Windows layer metadata in tests

f216d8a

Signed-off-by: Darren Stahl <[email protected]>

Use filepath.Clean to remove trailing slashes

edec71c

Signed-off-by: Darren Stahl <[email protected]>

WIP: fixing view layers to create a new sandbox

699f2d2

Signed-off-by: Darren Stahl <[email protected]>

WIP: Enable more snapshotter tests on Windows

c264ea2

Signed-off-by: Darren Stahl <[email protected]>

Add TODOs

249f2d5

Signed-off-by: Darren Stahl <[email protected]>

WIP: Forked ReadLink and EvalSymlinks

0327913

Signed-off-by: Darren Stahl <[email protected]>

Removed unused forked code

22c981d

Signed-off-by: Darren Stahl <[email protected]>

darstahl force-pushed the MountVol branch from 9f17336 to 22c981d Compare May 5, 2018 00:15

darstahl added 2 commits May 7, 2018 16:41

Add TODO for recycle bin contents

0089489

Signed-off-by: Darren Stahl <[email protected]>

Workaround for CI failure due to empty folder

36e773a

Signed-off-by: Darren Stahl <[email protected]>

darstahl force-pushed the MountVol branch from daedbd6 to 36e773a Compare May 7, 2018 23:45

darstahl added 4 commits May 7, 2018 17:01

Fix Linux build?

f34d796

Signed-off-by: Darren Stahl <[email protected]>

fix containers

f1ca1f9

Signed-off-by: Darren Stahl <[email protected]>

fix unmount

df0d9cf

Signed-off-by: Darren Stahl <[email protected]>

fix build constraints

6a1bf21

Signed-off-by: Darren Stahl <[email protected]>

darstahl added 5 commits May 7, 2018 18:10

test

e87ba1f

Signed-off-by: Darren Stahl <[email protected]>

revert test

c3730f3

Signed-off-by: Darren Stahl <[email protected]>

Fix building on Linux

796e60a

Signed-off-by: Darren Stahl <[email protected]>

Mark windows_test.go as Windows only

b36adf2

Signed-off-by: Darren Stahl <[email protected]>

Use final version of continuity changes

8cb63cc

Signed-off-by: Darren Stahl <[email protected]>

darstahl added 3 commits May 9, 2018 12:03

cleanup

845b2b3

Signed-off-by: Darren Stahl <[email protected]>

More cleanup

4844ea1

Signed-off-by: Darren Stahl <[email protected]>

Fix vendoring

a12b082

Signed-off-by: Darren Stahl <[email protected]>

crosbymichael mentioned this pull request May 29, 2018

[carry] Mount Windows layers as volumes on the host #2366

Closed

crosbymichael closed this Jul 25, 2018

TBBle mentioned this pull request Jan 5, 2020

Windows: Roadmap to WCOW #1920

Closed

8 tasks

This was referenced Jul 23, 2020

Mount snapshots on Windows #4419

Closed

Mount snapshots on Windows (Isolated PR) #4425

Closed

TBBle mentioned this pull request Mar 2, 2023

Mount snapshots on Windows #8043

Merged

WIP: Mount Windows layers as volumes on the host #2287

WIP: Mount Windows layers as volumes on the host #2287

Uh oh!

Conversation

darstahl commented Apr 12, 2018

Uh oh!

dmcgowan Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

darstahl Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

dmcgowan Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

darstahl Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

dmcgowan Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

darstahl Apr 16, 2018

Choose a reason for hiding this comment

Uh oh!

dmcgowan commented Apr 16, 2018

Uh oh!

darstahl commented Apr 16, 2018

Uh oh!

darstahl commented Apr 18, 2018

Uh oh!

dmcgowan commented May 2, 2018

Uh oh!

darstahl commented May 2, 2018

Uh oh!

darstahl commented May 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darstahl commented May 9, 2018

Uh oh!

codecov-io commented May 11, 2018

Codecov Report

Uh oh!

crosbymichael commented Jul 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

darstahl commented May 5, 2018 •

edited

Loading