Add --read-only for read only container rootfs #10093
Conversation
|
@crosbymichael Tests not compiling :) |
8cdd372 to
88e1e41
Compare
|
@LK4D4 i don't know what you are talking about.... |
|
+1 for this even without |
| **--readonly**=*true*|*false* | ||
| Mount the container's root filesystem as readonly. | ||
|
|
||
| By default a container will have it's root filesystem writable allowing processes |
|
👍 |
88e1e41 to
42cbe0e
Compare
|
cooooollll the diff is so smalll! LGTM |
| @@ -1681,6 +1683,13 @@ will automatically create this directory on the host for you. In the | |||
| example above, Docker will create the `/doesnt/exist` | |||
| folder before starting your container. | |||
|
|
|||
| $ sudo docker run -v /icanwrite --readonly busybox touch /icanwrite here | |||
There was a problem hiding this comment.
Minor nit, but docker run --readonly -v /icanwrite ... could be better, as "icanwrite readonly" sounds weird.
There was a problem hiding this comment.
i agree - putting the flag you're talking about first also gives it emphasis.
|
Should we add a small warning somewhere explaining that |
|
Any mounts are still writable, if they are mounted as rw |
|
Oh right, it's only the rootfs that's readonly, my bad. Code LGTM |
| @@ -323,6 +326,7 @@ Return low-level information on the container `id` | |||
| "NetworkMode": "bridge", | |||
| "PortBindings": {}, | |||
| "Privileged": false, | |||
| "ReadonlyRootfs": false, | |||
42cbe0e to
18b612a
Compare
|
I have no idea where, but should be in "what's new" |
| a container writes files. The `--readonly` flag mounts the container's root | ||
| filesystem as read only prohibiting writes to locations other than the | ||
| specified volumes for the container. | ||
|
|
There was a problem hiding this comment.
I'd flip the first sentence to be more like
To control where a container can write files, you can combine the--readonlyflag with volumes of volume containers
Starting a discussion about readonly with Volumes made me wonder if it was out of place.
no matter tho, easy for us to discuss post-merge.
There was a problem hiding this comment.
@fredlf ! ! ! ^^^ I'm trying to apply what you told us but @SvenDowideit is saying you are wrong or I did it wrong.
There was a problem hiding this comment.
Heh, this is one of those cases where we can see the fact that writing is not code. There's no right or wrong, here, it's a question of what we most want to emphasize. @crosbymichael 's original sentence emphasizes the idea of "control over where a container writes". @SvenDowideit 's rewrite places reader emphasis on the idea of "combining the --readonly flag with volumes." Only the writer knows what he actually wanted to emphasize. But @SvenDowideit's response as a reader (another term for RET is reader-response criticism), gives an important clue: he did not have any context for mentally processing the concept of "volumes" when it was introduced. So, let's give the reader the context they need and expect: "The --readonly flag can be used in combination with volumes to control where a container writes files."
18b612a to
202471e
Compare
|
I think readonly should probably be read-only both in option and text. It's definitely read-only when used in a sentence. |
|
Otherwise LGTM |
|
@jamtur01 you mean |
|
Yes - though I'm open to be told I'm wrong as an option - but definitely the docs should say "read-only" - readonly is wrong. |
|
@tianon what do you think? |
|
I think from |
|
@tianon thanks, consistency wins, i'll make the change in docs and code. |
202471e to
84c27e0
Compare
|
@jamtur01 made the changes and changed the flag name to be consistent with other tools. |
|
LGTM |
|
Okay, I was ignored again :( |
|
@LK4D4 I saw your comment, I just don't know what to say about adding a new field to an api object, it looks to be more about endpoints. |
|
@crosbymichael |
Add a --readonly flag to allow the container's root filesystem to be mounted as readonly. This can be used in combination with volumes to force a container's process to only write to locations that will be persisted. This is useful in many cases where the admin controls where they would like developers to write files and error on any other locations. Closes moby#7923 Closes moby#8752 Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
84c27e0 to
4094070
Compare
|
@LK4D4 added |
|
LGTM |
Add --read-only for read only container rootfs
|
👍 |
Add a
--read-onlyflag to allow the container's root filesystem to bemounted as read only. This can be used in combination with volumes to
force a container's process to only write to locations that will be
persisted. This is useful in many cases where the admin controls where
they would like developers to write files and error on any other
locations.
Closes #7923
Closes #8752
Signed-off-by: Michael Crosby crosbymichael@gmail.com