Fix linux mount calls not applying propagation type changes#30416
Fix linux mount calls not applying propagation type changes#30416LK4D4 merged 1 commit intomoby:masterfrom
Conversation
|
ping @cpuguy83 @justincormack PTAL |
|
Not sure the jenkins errors are related to this PR? |
|
Can we add some tests for these, as clearly there were not sufficient ones before or it would have been noticed... |
stevenh
force-pushed
the
linux-mount-fixes
branch
from
dcde4d5 to
d87a157
Compare
|
@justincormack tests added |
|
@stevenh the tests seem to be failing... |
stevenh
force-pushed
the
linux-mount-fixes
branch
2 times, most recently
from
d5c2092 to
73819b4
Compare
|
I think there may be something kernel / OS specific about slave mount reporting as this latest version works fine here on ubuntu 16 LTS here, need to investigate more. |
stevenh
force-pushed
the
linux-mount-fixes
branch
from
73819b4 to
a07a209
Compare
|
The underlying issue was false assumptions about the parent mounts propagation type in the test, locally its was shared on jenkins they must be private, so the resulting Optionals where unstable. I've restructured the test to start with a source that has known flags, so hopefully it should be good now. |
|
@justincormack this good for you now? |
|
ping @justincormack |
|
It looked good, will review again just to check.
On 1 Feb 2017 9:18 p.m., "Alexander Morozov" <notifications@github.com> wrote:
ping @justincormack <https://github.com/justincormack>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#30416 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAdcPOMTZyWIvQQZIRm2eJE2DCVtJMtxks5rYPaDgaJpZM4Lsl-9>
.
|
|
ping @justincormack |
pkg/mount/mounter_linux.go
Outdated
stevenh
force-pushed
the
linux-mount-fixes
branch
from
a07a209 to
3194a14
Compare
pkg/mount/mounter_linux.go
Outdated
There was a problem hiding this comment.
ok, unsure here. Why is device=="" or device="none" a remount? I can mount a tmpfs with these options. Why is presense of MS_REMOUNT not sufficient?
There was a problem hiding this comment.
The problem with only looking at MS_REMOUNT is that typical use cases don't do this.
An example of this can be seen in sharedsubtree_linux:
Mount(mountPoint, mountPoint, "none", "bind,rw")
These are covered in the test cases so removing the "" and "none" case you'll see the following failures:
--- FAIL: TestMount (0.22s)
--- FAIL: TestMount/none-bind,slave (0.00s)
mounter_linux_test.go:82: no such device
--- FAIL: TestMount/none-bind,rw,slave (0.00s)
mounter_linux_test.go:82: no such device
--- FAIL: TestMount/none-bind,ro,slave (0.00s)
mounter_linux_test.go:82: no such device
--- FAIL: TestSubtreePrivate (0.02s)
sharedsubtree_linux_test.go:66: no such device
--- FAIL: TestSubtreeShared (0.00s)
sharedsubtree_linux_test.go:143: no such device
--- FAIL: TestSubtreeSharedSlave (0.00s)
sharedsubtree_linux_test.go:222: no such device
--- FAIL: TestSubtreeUnbindable (0.00s)
sharedsubtree_linux_test.go:303: no such device
There was a problem hiding this comment.
Ah yes. Could we add a note in the code explaining that as it is non obvious, especially as the pkg/ code is intended for use outside Docker and external users might be confused...
Propagation type changes must be done as a separate call, in the same way as read only bind mounts. To fix this: 1. Ensure propagation type change flags aren't included in other calls. 2. Apply propagation type change in a separate call. Also: * Make it clear which parameters are ignored by passing them as empty. * Add tests to ensure Mount options are applied correctly. Fixes moby#30415 Signed-off-by: Steven Hartland <steven.hartland@multiplay.co.uk>
stevenh
force-pushed
the
linux-mount-fixes
branch
from
3194a14 to
b3b14b9
Compare
|
Windows failure unrelated and passed before comments changed. |
7 participants
Propagation type changes must be done as a separate call, in the same way as read only bind mounts.
To fix this:
Fixes #30415
Signed-off-by: Steven Hartland steven.hartland@multiplay.co.uk