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

Docker Task Driver: advanced options and privileged mode #328

Closed
jeffbaier opened this issue Oct 23, 2015 · 8 comments
Closed

Docker Task Driver: advanced options and privileged mode #328

jeffbaier opened this issue Oct 23, 2015 · 8 comments

Comments

@jeffbaier
Copy link

From reading the documentation, it looks like there are only 3 configuration options for the docker task driver: image, command, and network_mode. Then there are other nomad options that are abstracted for all the task drivers, for example port mapping and resource limiting.

But there are a lot of other docker run options missing. How can I pass some advanced config parameters to Docker? For example a big one is --privileged. Other examples would be the -i, -t and -dns flags.

If the advanced options are not possible, what is the best work around? Should I use the Fork/Exec driver to call docker run by hand with all the desired options?

@dadgar
Copy link
Contributor

dadgar commented Oct 26, 2015

Hey Jeff,

Agreed some of the more advance flags need to be added. I would not go with the fork/exec strategy because it won't allow you to wait for the container task to finish. Docker commands are passed to the dameon so docker run would succeed instantly and then the task would believe it was done. It is better to properly route the flags to docker and that is something we plan on doing!

@Sdedelbrock
Copy link
Contributor

There seems to be a handful of requests to enhance the configuration options for drivers (specifically docker). We would like to submit a PR for the Docker driver, addressing this issue as well as others. but would like to hear your opinion of how you prefer it is implemented. It seems driver based configuration best fits within the Task.Config. Unfortunately Task.Config is a map[string]string, here are two proposals:

Task Config uses map[string]string


Keep the Task.Config as a map[string]string. This is convenient as it requires very little changes, and the initial validation is still completed through mapstructure. The downside are the config options are limited, and the drivers will be forced to mutate the string values to their expected values.

Example Task Config

...
driver = "docker"
config{
    privileged = "true"
    dns = "8.8.8.8,8.8.4.4"
    auth.username = "un"
    auth.password = "pw"
}
...

Example Driver Usage

    ...
    privileged, _ := strconv.ParseBool(task.Config["privileged"])
    dns := strings.Split(task.Config["dns"], ",")
    ...

Task Config uses map[string]interface{}


Changing Task.Config to a map[string]interface{} allows for more flexible configuration, but comes with some caveats. Each item required by the driver will require type assertion, and default values that do not match the golang type default will need to manually be written or omitted. Validation will need to be moved over to the driver. In this case I think it might be wise to add a Validate method to the driver interface.

It is also possible for each driver to have their own config struct, and use mapstructure to validate the map[string]interface{}

Either option using map[string]interface{} will require a re-write for for each driver, we are more then happy to implement it if this is a preferred choice.

...
driver = "docker"
config{
    privileged = true
    dns = ["8.8.8.8", "8.8.4.4"]
    auth{
        username = "un"
        password = "pw"
    }
}
...

Example Driver Usage w/assertion

    ...
    privileged, _ := task.Config["privileged"].(bool)
    dns, _ := task.Config["dns"].([]string)
    ...

Example Driver Usage w/map structure

    ...
    type DockerDriverConfig struct {
        privileged bool
        dns []string
        auth struct{
            un string
            pw string
        }
    }

    var config DockerDriverConfig
    err := mapstructure.Decode(task.Config, &config)
    if err != nil{
        return err
    }
    privileged := config.privileged
    dns = config.dns
    ...

@dadgar
Copy link
Contributor

dadgar commented Nov 4, 2015

@Sdedelbrock: Lets keep it as a string for now. It is the least invasive change and should be fully expressive as well. Look forward to your PR! 👍

@dadgar
Copy link
Contributor

dadgar commented Nov 11, 2015

Closing at #390 enables these.

@dadgar dadgar closed this as completed Nov 11, 2015
@jeffbaier
Copy link
Author

This is a great start! It will be very helpful to have privileged as an option. However, I don't feel like this issue should be closed. There are many more options to support. Are there plans to support all the options, or will the options be restricted?

@Sdedelbrock
Copy link
Contributor

I can't speak for the nomad team, however my personal opinion is that it would be unwise to bloat the code base with options that may not be needed. Tackling the issues one by one gives a better wholistic view to the use cases, which may lead to better implementations.

If there is a configuration option you are in need of, we would be more then happy to kick off a new issue.

@dadgar
Copy link
Contributor

dadgar commented Nov 19, 2015

@Sdedelbrock hit the nail on the head. We want to support as many useful options for our users and this is best accomplished by seeing what our users need.

benbuzbee pushed a commit to benbuzbee/nomad that referenced this issue Jul 21, 2022
* add a linter, and add a lint check to the makefile, remove any existing lint violations
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, 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 Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants