floki volume support #36
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rlupton20
force-pushed
the
volumes
branch
2 times, most recently
from
4fed2c3 to
7c91acd
Compare
|
@maxdymond I've been working on adding function to help with build caching (see issue #35). I'm currently pausing to figure out whether I want to automatically mount the new volumes in Would suggest stepping through the commits - I tried to write good messages to motivate each one, and on their own they are more or less comprehensible. As a sum it's a morass of refactoring and new function... |
|
I think I'm happy with this as a first chunk of work. I want to rename |
|
I also need to write the docs. I should probably automate the docs build next because it's woefully out of date, and without the automation it basically won't happen. |
maxdymond
reviewed
Oct 29, 2019
|
It should, by virtue of the way the host folders are named, have shared folder support too (the hash is used when shared is false, and not when it's true). Cool - I'll make some markups and merge this in (making a note of a few things we ought to consider later too). |
|
I also updated the code to use the configuration file to key the volume name, rather than the root directory - this avoids accidental conflicts when people use multiple configuration files in one directory! |
|
Just need to write documentation and changelog, then I'll merge. |
This helps reduce the amount of parameter passing that is needed in functions which figure out how to launch the container. The location of the configuration file, and the directory it is located in (or which needs to be mounted into the container) is part of how `floki` is run, so belongs in the `Environment` structure. This commit doesn't fundamentally change the function in any way, it just moves it. Signed-off-by: Richard Lupton <[email protected]>
This introduces configuration parsing for volumes in floki.
Currently the configuration isn't interpreted, so no volumes are actually added to the running floki container.
However this provides the intended configuration semantics.
Volumes can be configured in `floki.yaml` under the `volumes` key:
```
volumes:
cargo-registry:
shared: true
mount: /home/rust/.cargo/registry
something-else:
mount: /something
```
Intended interpretation
-----------------------
The `shared` key indicates that this volume should be shared across all containers using
the same key (with `shared` also set). Setting `shared` may be elided, in which case it
defaults to false, and the volume will only be used by containers using the same `floki.yaml`
configuration files.
Signed-off-by: Richard Lupton <[email protected]>
This commit adds function for deciding what host paths should be used to back configured volumes. No volumes are mounted, but a debug log is emitted to allow inspection of the generated mounts. Signed-off-by: Richard Lupton <[email protected]>
Now mount the volumes specified in the configuration file. This currently has the following limitations: - docker creates the host folders on demand (with root access control) - any corresponding dind containers don't yet receive the volume mounts Signed-off-by: Richard Lupton <[email protected]>
Make sure the host directories which back floki volumes are created before the container is launched. This ensures that the folders are created with the users permissions. Files in this folder that are added by the container may not be owned by the host user however. Signed-off-by: Richard Lupton <[email protected]>
Previously there was some awkward packaging of the command to run in docker, which relied on the Command object knowing about which shell to use. Now `Command` doesn't need to care about the command that will be run in the container, that is resolved in the `interpret` module instead. It can now run an arbitrary command inide the docker container. This additional flexibility is motivated by a desire to unify the `command` module with `dind`, so the same pieces of logic don't need to be replicated in slightly different ways. This will make it much easier to keep the main `floki` container and `dind` containers in sync. Signed-off-by: Richard Lupton <[email protected]>
Added further function to `DocekrCommandBuilder` so that `Dind` can be implemented in terms of `Command`. This allows the functionality of `Command` to be used to program `Dind`. Launching `dind` was also moved to the high-level interpreting functions, rather than being buried deep in one of the command functions. This makes it clearer when dind is launched. `DockerCommandBuilder`s can now be instantiated as daemons, which produce a handle to the daemon docker container. When this handle is dropped, the container is cleaned up. Signed-off-by: Richard Lupton <[email protected]>
Flatten the creation of the `Dind` and `DockerCommandBuilder` structures, and remove some unecessary `mut` declarations. Additional odd bits of cleanup. Signed-off-by: Richard Lupton <[email protected]>
Even though the volumes are intended to be used for build caches, they may be used for other purposes, so they ought to be identified as they are. Signed-off-by: Richard Lupton <[email protected]>
Mostly these markups pertain to adding some docstrings, and renaming a variable to make its use a bit clearer. Signed-off-by: Richard Lupton <[email protected]>
This means that different configuration files in the same folder will not accidently share volumes if given the same key. Some additional work was required to normalize the filepaths and ensure that they were absolute where needed. An additional test was added to cover the motivating case. Signed-off-by: Richard Lupton <[email protected]>
Signed-off-by: Richard Lupton <[email protected]>
Signed-off-by: Richard Lupton <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This starts to address #35, by adding
flokivolumes as outliined in the issue.