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

Rebase and merge of docker sysctl support #3568

Merged
merged 12 commits into from
Nov 20, 2017
68 changes: 67 additions & 1 deletion client/driver/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ type DockerDriverConfig struct {
PortMapRaw []map[string]string `mapstructure:"port_map"` //
PortMap map[string]int `mapstructure:"-"` // A map of host port labels and the ports exposed on the container
Privileged bool `mapstructure:"privileged"` // Flag to run the container in privileged mode
SysctlRaw []map[string]string `mapstructure:"sysctl"` //
Sysctl map[string]string `mapstructure:"-"` // The sysctl custom configurations
UlimitRaw []map[string]string `mapstructure:"ulimit"` //
Ulimit []docker.ULimit `mapstructure:"-"` // The ulimit custom configurations
DNSServers []string `mapstructure:"dns_servers"` // DNS Server for containers
DNSSearchDomains []string `mapstructure:"dns_search_domains"` // DNS Search domains for containers
DNSOptions []string `mapstructure:"dns_options"` // DNS Options
Expand All @@ -199,6 +203,35 @@ type DockerDriverConfig struct {
Devices []DockerDevice `mapstructure:"devices"` // To allow mounting USB or other serial control devices
}

func sliceMergeUlimit(ulimitsRaw map[string]string) ([]docker.ULimit, error) {
var ulimits []docker.ULimit

for name, ulimitRaw := range ulimitsRaw {
// hard limit is optional
if strings.Contains(ulimitRaw, ":") == false {
ulimitRaw = ulimitRaw + ":" + ulimitRaw
}

splitted := strings.SplitN(ulimitRaw, ":", 2)
soft, err := strconv.Atoi(splitted[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check length before indexing.

if err != nil {
return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe include that it's the soft limit that is invalid? - same below for hard limit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed with the limit specification for logging.

}
hard, err := strconv.Atoi(splitted[1])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check limit before indexing.

if err != nil {
return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw)
}

ulimit := docker.ULimit{
Name: name,
Soft: int64(soft),
Hard: int64(hard),
}
ulimits = append(ulimits, ulimit)
}
return ulimits, nil
}

// Validate validates a docker driver config
func (c *DockerDriverConfig) Validate() error {
if c.ImageName == "" {
Expand All @@ -209,7 +242,6 @@ func (c *DockerDriverConfig) Validate() error {
if dev.HostPath == "" {
return fmt.Errorf("host path must be set in configuration for devices")
}

if dev.CgroupPermissions != "" {
for _, c := range dev.CgroupPermissions {
ch := string(c)
Expand All @@ -220,6 +252,18 @@ func (c *DockerDriverConfig) Validate() error {
}
}
}
c.Sysctl = mapMergeStrStr(c.SysctlRaw...)
c.Labels = mapMergeStrStr(c.LabelsRaw...)
if len(c.Logging) > 0 {
c.Logging[0].Config = mapMergeStrStr(c.Logging[0].ConfigRaw...)
}

mergedUlimitsRaw := mapMergeStrStr(c.UlimitRaw...)
ulimit, err := sliceMergeUlimit(mergedUlimitsRaw)
if err != nil {
return err
}
c.Ulimit = ulimit
return nil
}

Expand Down Expand Up @@ -254,6 +298,20 @@ func NewDockerDriverConfig(task *structs.Task, env *env.TaskEnv) (*DockerDriverC
dconf.MacAddress = env.ReplaceEnv(dconf.MacAddress)
dconf.SecurityOpt = env.ParseAndReplace(dconf.SecurityOpt)

for _, m := range dconf.SysctlRaw {
for k, v := range m {
delete(m, k)
m[env.ReplaceEnv(k)] = env.ReplaceEnv(v)
}
}

for _, m := range dconf.UlimitRaw {
for k, v := range m {
delete(m, k)
m[env.ReplaceEnv(k)] = env.ReplaceEnv(v)
}
}

for _, m := range dconf.LabelsRaw {
for k, v := range m {
delete(m, k)
Expand Down Expand Up @@ -506,6 +564,12 @@ func (d *DockerDriver) Validate(config map[string]interface{}) error {
"userns_mode": {
Type: fields.TypeString,
},
"sysctl": {
Type: fields.TypeArray,
},
"ulimit": {
Type: fields.TypeArray,
},
"port_map": {
Type: fields.TypeArray,
},
Expand Down Expand Up @@ -1108,6 +1172,8 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas
hostConfig.UTSMode = driverConfig.UTSMode
hostConfig.UsernsMode = driverConfig.UsernsMode
hostConfig.SecurityOpt = driverConfig.SecurityOpt
hostConfig.Sysctls = driverConfig.Sysctl
hostConfig.Ulimits = driverConfig.Ulimit

hostConfig.NetworkMode = driverConfig.NetworkMode
if hostConfig.NetworkMode == "" {
Expand Down
54 changes: 54 additions & 0 deletions client/driver/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,60 @@ func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) {
}
}

func TestDockerDriver_Sysctl_Ulimit(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we test the failure conditions as well?

task, _, _ := dockerTask(t)
expectedUlimits := map[string]string{
"nproc": "4242",
"nofile": "2048:4096",
}
task.Config["sysctl"] = []map[string]string{
{
"net.core.somaxconn": "16384",
},
}
task.Config["ulimit"] = []map[string]string{
expectedUlimits,
}

client, handle, cleanup := dockerSetup(t, task)
defer cleanup()

waitForExist(t, client, handle)

container, err := client.InspectContainer(handle.ContainerID())
if err != nil {
t.Fatalf("err: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use assert library for test assertions.

}

if want, got := "16384", container.HostConfig.Sysctls["net.core.somaxconn"]; want != got {
t.Errorf("Wrong net.core.somaxconn config for docker job. Expect: %s, got: %s", want, got)
}

if want, got := 2, len(container.HostConfig.Ulimits); want != got {
t.Errorf("Wrong number of ulimit configs for docker job. Expect: %d, got: %d", want, got)
}
for _, got := range container.HostConfig.Ulimits {
if expectedStr, ok := expectedUlimits[got.Name]; ok == false {
t.Errorf("%s config unexpected for docker job.", got.Name)
} else {
if strings.Contains(expectedStr, ":") == false {
expectedStr = expectedStr + ":" + expectedStr
}

splitted := strings.SplitN(expectedStr, ":", 2)
soft, _ := strconv.Atoi(splitted[0])
hard, _ := strconv.Atoi(splitted[1])

if got.Soft != int64(soft) {
t.Errorf("Wrong soft %s ulimit for docker job. Expect: %d, got: %d", got.Name, soft, got.Soft)
}
if got.Hard != int64(hard) {
t.Errorf("Wrong hard %s ulimit for docker job. Expect: %d, got: %d", got.Name, hard, got.Hard)
}
}
}
}

func TestDockerDriver_Labels(t *testing.T) {
if !tu.IsTravis() {
t.Parallel()
Expand Down
30 changes: 29 additions & 1 deletion website/source/docs/drivers/docker.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ The `docker` driver supports the following configuration in the job spec. Only
command = "my-command"
}
```

* `dns_search_domains` - (Optional) A list of DNS search domains for the container
to use.

Expand All @@ -97,6 +97,34 @@ The `docker` driver supports the following configuration in the job spec. Only
* `interactive` - (Optional) `true` or `false` (default). Keep STDIN open on
the container.

* `sysctl` - (Optional) A key-value map of sysctl configurations to set to the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is incomplete.



```hcl
config {
sysctl {
net.core.somaxconn = "16384"
}
}
```

* `ulimit` - (Optional) A key-value map of ulimit configurations to set to the
containers on start.

```hcl
config {
ulimit {
nproc = "4242"
nofile = "2048:4096"
}
}
```

* `privileged` - (Optional) `true` or `false` (default). Privileged mode gives
the container access to devices on the host. Note that this also requires the
nomad agent and docker daemon to be configured to allow privileged
containers.

* `ipc_mode` - (Optional) The IPC mode to be used for the container. The default
is `none` for a private IPC namespace. Other values are `host` for sharing
the host IPC namespace or the name or id of an existing container. Note that
Expand Down