Fix docker --init with /dev bind mount #37665
Conversation
daemon/oci_linux.go
Outdated
| @@ -474,7 +474,7 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c | |||
| // filter out mount overridden by a user supplied mount | |||
| continue | |||
| } | |||
| if mountDev && strings.HasPrefix(m.Destination, "/dev/") { | |||
| if mountDev && strings.HasPrefix(m.Destination, "/dev/") && m.Destination != "/dev/init" { | |||
There was a problem hiding this comment.
nitty; wondering if it would be clearer to make this a separate if (also if we should put /dev/init in a constant)
|
yes, it is better to move it to e.g. |
Codecov Report
@@ Coverage Diff @@
## master #37665 +/- ##
=========================================
Coverage ? 36.04%
=========================================
Files ? 609
Lines ? 45061
Branches ? 0
=========================================
Hits ? 16240
Misses ? 26589
Partials ? 2232 |
|
I have changed the commit to use /sbin/docker-init as a bind-mount destination for in-container init. It still won't work with bind-mounted There is still one open question: what to do if a user-supplied mount overshadows a mount from spec. Opened #37702 to address it. |
|
note all the LGTMs above are now invalid as the code is different (I should have probably close the PR and create a different one). |
|
rebased; should help CI |
In case a user wants to have a child reaper inside a container (i.e. run "docker --init") AND a bind-mounted /dev, the following error occurs: > docker run -d -v /dev:/dev --init busybox top > 088c96808c683077f04c4cc2711fddefe1f5970afc085d59e0baae779745a7cf > docker: Error response from daemon: OCI runtime create failed: container_linux.go:296: starting container process caused "exec: "/dev/init": stat /dev/init: no such file or directory": unknown. This happens because if a user-suppled /dev is provided, all the built-in /dev/xxx mounts are filtered out. To solve, let's move in-container init to /sbin, as the chance that /sbin will be bind-mounted to a container is smaller than that for /dev. While at it, let's give it more unique name (docker-init). NOTE it still won't work for the case of bind-mounted /sbin. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
rebased; should help CI (hmm, I start to sound like a broken record) |
|
ppc ci failure is #33041
z ci failure is most probably just slow/overloaded hw, also see #36907
|
|
Note this commit just changes in-container init path from /dev/init to /sbin/init, which should greatly reduce the chance to break things (as not too many users, I assume, bind-mount /sbin inside container). The ultimate fix is #37704 which rearranges the mount in proper way so even if /sbin is bind-mounted from the host, /sbin/init will still be visible. I think that both fixes make sense and should be merged. |
|
I wonder which version of docker ce already used this patch? |
|
Looks like this will be in the upcoming 19.03 release; https://github.com/docker/engine/blob/19.03/daemon/oci_linux.go#L36 (It's not in the 18.09 codebase) |
In case a user wants to have a child reaper inside a container
(i.e. run "docker --init") AND a bind-mounted /dev, the following
error occurs:
This happens because if a user-suppled /dev is provided, all the
built-in /dev/xxx mounts are filtered out.
To solve, let's move in-container init to /sbin, as the chance that
/sbin will be bind-mounted to a container is smaller than that for /dev.
While at it, let's give it more unique name (docker-init).
NOTE it still won't work for the case of bind-mounted /sbin.
Fixes #37645