-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Expanded docker driver options #390
Expanded docker driver options #390
Conversation
- Added `priviliged` option to task config to allow containers to run in priviliged mode. - Added `dns-servers` option to task config to allow containers to use custom DNS servers - Added `search-domains` option to task config to allow containers to use custom dns search domains - Added authentication options (under key namespace `auth.*`) to allow authentication on a task level for docker remote. - Updated site docs to reflect changes
…cker-driver-options * 'master' of https://github.com/hashicorp/nomad: (59 commits) Move the executor and spawn package into driver Remove file watching Check if the PID is alive instead of heartbeating through modify time Update CHANGELOG.md nomad/watch: add a note about the Item struct go fmt this file Vet errors Search path Update website Make a basic executor that can be shared and fix some fingerprinting/tests Small improvements Use const value for AWS metadata URL Create Spawn pkg that handles IPC with the spawn-daemon and update exec_linux to use that Fixed the restart policy syntax Introducing vars to create default batch and service restart policies Fixed the tests Declaring Batch and Service default restart policies Fixing tests to not create a TG without restart policies This option only work -> This option only works leave -> leaving ...
@@ -166,6 +167,32 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task) (do | |||
d.logger.Printf("[DEBUG] driver.docker: using %d cpu shares for %s", hostConfig.CPUShares, task.Config["image"]) | |||
d.logger.Printf("[DEBUG] driver.docker: binding directories %#v for %s", hostConfig.Binds, task.Config["image"]) | |||
|
|||
// set privileged (fallback to false) | |||
hostConfig.Privileged, _ = strconv.ParseBool(task.Config["privileged"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you handle the error here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since privileged is a potentially dangerous flag, we want to give the Nomad operator control over whether this is allowed. So what should happen is there should be a name key/value to enable or disable the docker driver from allowing the privileged flag:
That would go here: https://github.com/hashicorp/nomad/blob/master/client/config/config.go#L55
Here is an example:
Line 76 in cb811dd
_, err = strconv.ParseBool(d.config.ReadDefault("docker.cleanup.container", "true")) |
And the default should be that privileged is not enabled. The docker driver should also set a node attribute to indicate if it allows privileged. Let me know if you need clarification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a better example: https://github.com/hashicorp/nomad/blob/master/client/driver/raw_exec.go#L50
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes perfect sense, I'll update the PR
- Added error checking on priviliged mode. - Added `docker.privileged.enabled` to client config/fingerprint
@@ -121,6 +141,11 @@ The `docker` driver has the following configuration options: | |||
* `docker.cleanup.image` Defaults to `true`. Changing this to `false` will | |||
prevent Nomad from removing images from stopped tasks. | |||
|
|||
* `docker.privileged.enabled` Defaults to `false`. Changing this to `true` will | |||
allow containers to use "privileged" mode, which gives the containers full access | |||
to the host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put a period after
There is also a some build errors but coming together nicely 👍 |
Thanks @Sdedelbrock |
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. |
This pull request adds additional task level options for the docker driver. Addresses #328
privileged
option to task config to allow containers to run inprivileged mode.
dns-servers
option to task config to allow containers to usecustom DNS servers
search-domains
option to task config to allow containers touse custom dns search domains
auth.*
) to allowauthentication on a task level for docker remote.
docker.privileged.enabled
to client configExample Task Config with new options
Example Client Config with new options