Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose docker mount options #3021

Merged
merged 3 commits into from
Aug 17, 2017
Merged

Conversation

clinta
Copy link
Contributor

@clinta clinta commented Aug 14, 2017

This PR adds support for configuring docker mount options that are available in docker API >= 1.25. These differ from the existing volume and volume_driver options in that they allow specifying a different driver per mount and allow specifying driver specific options for the volumes created.

https://docs.docker.com/engine/api/v1.25/#operation/ContainerCreate

@schmichael
Copy link
Member

Hi @clinta!

Thanks for the PR; looks great. Being able to support binds and volume_drivers in the same task would be great. However, could you limit this PR to only add support for Volumes?

Binds are already supported through the volumes driver parameter and take into account relative vs absolute paths -- as well as a config flag to allow disabling them entirely to harden clients: https://www.nomadproject.io/docs/drivers/docker.html#volumes We'd either need to (1) support such logic in both places, (2) migrate the old parameter+behavior to the new API, or (3) just remove the bind options from this new code. (3) is the easiest, and we'll probably do (2) at some point down the road to drop support for the older less featureful API.

Generic tmpfs mount support is planned, so we'd rather not have it implemented multiple places/ways. This also allows properly accounting for the tmpfs's memory usage in scheduling decisions. In the mean time the secrets/ directory is a default tmpfs tasks can use to store small amounts of data like credentials.

Let me know if you have any questions or concerns!

@clinta
Copy link
Contributor Author

clinta commented Aug 17, 2017

How does this look? I've kept the type option in the structs, so that if you wish to make use of them in the future it's easy. But it now will default to volume and throw an error on any other type.

@schmichael
Copy link
Member

Looks good! Thanks @clinta!

@schmichael schmichael merged commit ad94097 into hashicorp:master Aug 17, 2017
@schmichael
Copy link
Member

@clinta Could you provide an example job file for documentation? I'll add it or feel free to open up a PR against our docs: https://github.com/hashicorp/nomad/blob/master/website/source/docs/drivers/docker.html.md

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants