RFC: Runner host mounts #140
Conversation
|
Thanks for the RFC! I'll get around to reviewing it soon. |
taylorsilva
left a comment
taylorsilva
left a comment
There was a problem hiding this comment.
Thanks so much for this RFC. No major blockers here, just some nit-pick stuff. Interested in your thoughts. I'll start trying to share the RFC as well to see if other community members have thoughts about it.
Signed-off-by: Aria <me@aria.rip>
|
I'm happy to help implement this @tcmal |
taylorsilva
left a comment
taylorsilva
left a comment
There was a problem hiding this comment.
@tcmal Sorry for the long silence from my end. I want to put momentum behind this RFC again.
I've read it again and noticed nothing mentioned about read/write permissions of the mounts. Who has control over that? Do operators get to decide if the mounts are rw/ro or is that up to the task writer to decide?
My instinct tells me we should probably let the operators decide between between rw/ro permissions for each mount. Maybe we could allow task writers to downgrade to ro for their mounts even if the operator allows rw for the given mount. What are your thoughts on this?
| - `host_mounts` is a list of `host-mount-config`s, defaulting to `[]` | ||
| - A `host-mount-config` is either: | ||
| - An object, with the following keys: | ||
| - `host` (required, non-empty): The desired path to mount from the host | ||
| - `container` (optional): The mount's location in the container. Defaults to the same as `host` | ||
| - More options may be added down the line, as required. | ||
| - A string, of format: | ||
| - `host:container`, where `host` and `container` are strings not containing `:`, and have meanings as defined above. | ||
| - or, `host`, leaving `container` to default to `host` as above | ||
| - This is just a shorthand syntax, similar to the one Docker & Docker Compose uses. |
There was a problem hiding this comment.
Could we add an example of using host_mounts as well please? Similar to what you showed in this comment: concourse/concourse#4241 (comment)
| # Answered Questions | ||
|
|
There was a problem hiding this comment.
| # Answered Questions | |
| # Answered Questions | |
| * > Which container runtimes (guardian and containerd) will support this feature? Can we support it in both runtimes? | |
| Looks like we can have this feature added to both runtimes. The garden client is the interface we have between both the guardian and containerd runtimes. We'll need to do the plumbing work ourselves for the containerd runtime, but we'll get this for free with the guardian runtime. | |
| https://github.com/cloudfoundry/garden/blob/f0775181931df979cbeee43604327bb207ae0c9d/client.go#L190-L208 | |
Adding and answering my own question.
|
|
||
| # Open Questions | ||
|
|
||
| - If the worker is itself running in a docker container, the mount will need to also be specified there. This will need to be made clear to users of the feature |
There was a problem hiding this comment.
Would be the same situation for users running Concourse on k8s, which there are a lot of.
Either way, the answer here is to document it. Can move this to the Answered section.
| # Open Questions | ||
|
|
||
| - If the worker is itself running in a docker container, the mount will need to also be specified there. This will need to be made clear to users of the feature | ||
| - Mounting directories from the host opens users up to weird permissions errors, which are often counterintuitive. For instance, group membership is not inherited from the worker user, so if the mounted directory is not world-accessible and its user/group ID doesn't match with what's specified in the container, users will get permission errors. |
There was a problem hiding this comment.
Let's move this to the Answered section as well, with the answer being that we document that the onus is on operators to ensure their host mounts have the correct permissions and suggesting some useful defaults.
If thinking rw permissions, may want to reframe this RFC a bit. At first I thought of this as ro like mounting self signed certificates but to introduce write could get weird. What if instead we "extend" the get and put prototypes to make it clear we are getting files from the host and, if allowed, putting files onto the host. That opens up Concourse, for better or for worse, to use worker local storage as job gates if we use file checksums as the version. Obviously this is an entirely different direction than just mounting a folder, but just an initial thought when reading the proposal to allow rw permissions. |
5 participants
Rendered
Discussed briefly in concourse/concourse#4241