daemon/setMounts(): do not make /dev/shm ro#36526
Conversation
|
LGTM |
|
Discussing in the maintainers meeting; looks ok to make this rw - this should probably be added to the test that was added in #28823 |
|
Need to add a test case similar to #28823 but for |
The test case checks that in case of IpcMode: private and ReadonlyRootfs: true (as in "docker run --ipc private --read-only") the resulting /dev/shm mount is NOT made read-only. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It has been pointed out that if --read-only flag is given, /dev/shm also becomes read-only in case of --ipc private. This happens because in this case the mount comes from OCI spec (since commit 7120976), and is a regression caused by that commit. The meaning of --read-only flag is to only have a "main" container filesystem read-only, not the auxiliary stuff (that includes /dev/shm, other mounts and volumes, --tmpfs, /proc, /dev and so on). So, let's make sure /dev/shm that comes from OCI spec is not made read-only. Fixes: 7120976 ("Implement none, private, and shareable ipc modes") Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #36526 +/- ##
=========================================
Coverage ? 34.66%
=========================================
Files ? 613
Lines ? 45404
Branches ? 0
=========================================
Hits ? 15739
Misses ? 27605
Partials ? 2060 |
I am a bit concerned about this being a unit test, as I had to mimic the code flow of some actual code instead of calling it; but last time I tried adding an integration test it was strongly suggested that a unit test should be done instead, so I am following the suggestion here. |
|
z failure (unrelated): 14:21:28 === RUN TestServiceWithPredefinedNetwork |
|
Opened #36547 for the flaky |
It has been pointed out that if --read-only flag is given, /dev/shm
also becomes read-only, and it does not make sense, especially for
the case of --ipc private (added in #34087).
It is still debatable whether it makes sense to have it read-only
for --ipc shareable|host|container:NNN, but let's follow the --tmpfs
logic here and do NOT make /dev/shm read-only for all the cases.
Fixes #36503