daemon/setMounts(): don't let user mounts overshadow spec mounts#37704
daemon/setMounts(): don't let user mounts overshadow spec mounts#37704kolyshkin wants to merge 1 commit intomoby:masterfrom
Conversation
c115e26 to
c3aaa82
Compare
Codecov Report
@@ Coverage Diff @@
## master #37704 +/- ##
=========================================
Coverage ? 36.46%
=========================================
Files ? 611
Lines ? 49545
Branches ? 0
=========================================
Hits ? 18069
Misses ? 28913
Partials ? 2563 |
c3aaa82 to
ed660bf
Compare
|
rebased; should help CI |
|
power CI timed out; doesn't look like stuck, seems it's just super slow today, need to be restarted:
experimental CI: known flaky test (#33041)
z CI: another flaky test, although the issue is closed (#34988)
|
|
All the CI failures are def unrelated |
There are some scenarios in which a user-supplied mount overshadows
a one from spec. For example, in case of `docker run -v /tmp:/sys/fs`
the spec-supplied /sys/fs/cgroup mount will not be visible from
inside the container since user-supplied mount /sys/fs will be mounted
later.
The solution in this commit is to sort mounts in alphabetical order
so that the shallower ones (like /a/b) will come before deeper ones
(like /a/b/c). Note this changes the order of mounts but AFAIK it is
not important (except for the case that this commit fixes).
Since sorting is now done for all mounts, there is no sense to
do sort of user mounts before calling setMounts(), so remove it.
NOTE there are many ways to sort mounts, such as:
1. alphabetically (what this commit does);
2. by number of slashes (as in daemon/volumes.go)
3. by checking that later mounts are not prefixes for earlier ones,
and swapping those if they are
I tried a few and settled on simple alphabetical sort, as
it does the job and its code is the simplest.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
ed660bf to
1f6a255
Compare
|
janky and ppc CI failure is #33041 janky:
power:
|
|
@coolljt0725 do you think what I propose here makes sense? |
|
@kolyshkin From our experience in using containers, user may overshadow some spec mouts, if he use |
|
Yes, I'm a bit on the fence as well; shadowing these paths sounds like a user-error (if not intended to shadow), so not sure what's best here. |
|
Needs a rebase |
|
This one is probably not required either; let's close it. |
There are some scenarios in which a user-supplied mount overshadows
a one from spec. For example, in case of
docker run -v /tmp:/sys/fsthe spec-supplied /sys/fs/cgroup mount will not be visible from
inside the container since user-supplied mount /sys/fs will be mounted
later.
The solution in this commit is to sort mounts
so that the shallower ones (like /a/b) will come before deeper ones
(like /a/b/c). Note this changes the order of mounts but AFAIK it is
not important (except for the case that this commit fixes).
Since sorting is now done for all mounts, there is no sense to
do sort of user mounts before calling setMounts(), so remove it.
NOTE there are many ways to sort mounts, such as:
and swapping those if they are
I tried a few and settled on simple alphabetical sort, as
it does the job and its code is the simplest.
Fixes #37702