Volumes should have default propagation property "rprivate" #32851
Conversation
|
I think following commit introduce this change. @cpuguy83 can you please have a look. From fc7b904 Mon Sep 17 00:00:00 2001 |
volume/volume.go
Outdated
| Propagation: GetPropagation(mode), | ||
| } | ||
|
|
||
| spec.BindOptions = &mounttypes.BindOptions{ |
There was a problem hiding this comment.
We should not change the spec itself since this is what the user defined.
Instead we should update the returned mountpoint here: https://github.com/moby/moby/blob/master/volume/volume.go#L314
There was a problem hiding this comment.
And if not, we should have a unit test to make sure the default is set :)
rhvgoyal
force-pushed
the
volume-propagation
branch
from
d261976 to
d43ecde
Compare
|
Pushed new patch. let me see what test cases are broken and will fix them. |
volume/volume.go
Outdated
| } else { | ||
| // If user did not specify a propagation mode, get | ||
| // default propagation mode. | ||
| mp.Propagation = GetPropagation("") |
There was a problem hiding this comment.
I think we have a DefaultMountPropagation const that would be a little nicer here.
There was a problem hiding this comment.
ok, will use that.
Until and unless user has specified a propagation property for volume, they should default to "rprivate" and it should be passed to runc. We can't make it conditional on HasPropagation(). GetPropagation() returns default of rprivate if noting was passed in by user. If we don't pass "rprivate" to runc, then bind mount could be shared even if user did not ask for it. For example, mount two volumes in a container. One is "shared" while other's propagation is not specified by caller. If both volume has same source mount point of "shared", then second volume will also be shared inside container (instead of being private). Signed-off-by: Vivek Goyal <[email protected]>
rhvgoyal
force-pushed
the
volume-propagation
branch
from
d43ecde to
af8a143
Compare
|
@cpuguy83 PTAL. Failure of janky and experimental is not due to my PR. I have fixed unit tests and they will also ensure that default propagation mode is passed properly. |
|
Can this PR be merged now. It has passed all tests and has one LGTM. |
|
It has mine too, but I accidentally lost my ability to merge in the move... |
Until and unless user has specified a propagation property for volume, they
should default to "rprivate" and it should be passed to runc.
We can't make it conditional on HasPropagation(). GetPropagation() returns
default of rprivate if noting was passed in by user.
If we don't pass "rprivate" to runc, then bind mount could be shared even
if user did not ask for it. For example, mount two volumes in a container.
One is "shared" while other's propagation is not specified by caller. If
both volume has same source mount point of "shared", then second volume
will also be shared inside container (instead of being private).
Signed-off-by: Vivek Goyal [email protected]
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)