From 84a089433d0b00e75b97fff12ec2c20233c1f960 Mon Sep 17 00:00:00 2001 From: Samuel BERTHE Date: Wed, 29 Mar 2017 16:04:42 +0200 Subject: [PATCH 01/12] feat(docker driver): adds sysctls and ulimits configs --- client/driver/docker.go | 69 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/client/driver/docker.go b/client/driver/docker.go index 7539a83ad..7cb9fb3e6 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -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 + SysctlsRaw []map[string]string `mapstructure:"sysctls"` // + Sysctls map[string]string `mapstructure:"-"` // The sysctl custom configurations + UlimitsRaw []map[string]string `mapstructure:"ulimits"` // + Ulimits []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 @@ -199,6 +203,37 @@ 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 { + splitted := strings.SplitN(ulimitRaw, ":", 2) + + soft, err := strconv.Atoi(splitted[0]) + if err != nil { + return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw) + } + + ulimit := docker.ULimit{ + Name: name, + Soft: int64(soft), + Hard: int64(soft), // default: can be override + } + + // hard limit is optional + if len(splitted) == 2 { + if hard, err := strconv.Atoi(splitted[1]); err != nil { + return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw) + } else { + ulimit.Hard = int64(hard) + } + } + + ulimits = append(ulimits, ulimit) + } + return ulimits, nil +} + // Validate validates a docker driver config func (c *DockerDriverConfig) Validate() error { if c.ImageName == "" { @@ -220,6 +255,18 @@ func (c *DockerDriverConfig) Validate() error { } } } + c.Sysctls = mapMergeStrStr(c.SysctlsRaw...) + c.Labels = mapMergeStrStr(c.LabelsRaw...) + if len(c.Logging) > 0 { + c.Logging[0].Config = mapMergeStrStr(c.Logging[0].ConfigRaw...) + } + + mergedUlimitsRaw := mapMergeStrStr(c.UlimitsRaw...) + ulimits, err := sliceMergeUlimit(mergedUlimitsRaw) + if err != nil { + return err + } + c.Ulimits = ulimits return nil } @@ -254,6 +301,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.SysctlsRaw { + for k, v := range m { + delete(m, k) + m[env.ReplaceEnv(k)] = env.ReplaceEnv(v) + } + } + + for _, m := range dconf.UlimitsRaw { + 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) @@ -506,6 +567,12 @@ func (d *DockerDriver) Validate(config map[string]interface{}) error { "userns_mode": { Type: fields.TypeString, }, + "sysctls": &fields.FieldSchema{ + Type: fields.TypeArray, + }, + "ulimits": &fields.FieldSchema{ + Type: fields.TypeArray, + }, "port_map": { Type: fields.TypeArray, }, @@ -1108,6 +1175,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.Sysctls + hostConfig.Ulimits = driverConfig.Ulimits hostConfig.NetworkMode = driverConfig.NetworkMode if hostConfig.NetworkMode == "" { From cf586996f45672377ee6de1eba3a0ab6934f6212 Mon Sep 17 00:00:00 2001 From: Samuel BERTHE Date: Wed, 29 Mar 2017 16:55:30 +0200 Subject: [PATCH 02/12] test(docker driver): testing sysctls and ulimits --- client/driver/docker_test.go | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index b8114fbb8..1127f4709 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -858,6 +858,60 @@ func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) { } } +func TestDockerDriver_Sysctls_Ulimits(t *testing.T) { + task, _, _ := dockerTask() + expectedUlimits := map[string]string{ + "nproc": "4242", + "nofiles": "2048:4096", + } + task.Config["sysctls"] = []map[string]string{ + map[string]string{ + "net.core.somaxconn": "16384", + }, + } + task.Config["ulimits"] = []map[string]string{ + expectedUlimits, + } + + client, handle, cleanup := dockerSetup(t, task) + defer cleanup() + + waitForExist(t, client, handle.(*DockerHandle)) + + container, err := client.InspectContainer(handle.(*DockerHandle).ContainerID()) + if err != nil { + t.Fatalf("err: %v", err) + } + + 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: %s, got: %s", 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() From d15f01d7df8b5c7e7283db8aae61cc9a251532cd Mon Sep 17 00:00:00 2001 From: Samuel BERTHE Date: Wed, 29 Mar 2017 18:04:37 +0200 Subject: [PATCH 03/12] :lipstick: --- client/driver/docker.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 7cb9fb3e6..843e3d67c 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -207,28 +207,26 @@ func sliceMergeUlimit(ulimitsRaw map[string]string) ([]docker.ULimit, error) { var ulimits []docker.ULimit for name, ulimitRaw := range ulimitsRaw { - splitted := strings.SplitN(ulimitRaw, ":", 2) + // hard limit is optional + if strings.Contains(ulimitRaw, ":") == false { + ulimitRaw = ulimitRaw + ":" + ulimitRaw + } + splitted := strings.SplitN(ulimitRaw, ":", 2) soft, err := strconv.Atoi(splitted[0]) if err != nil { return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw) } + hard, err := strconv.Atoi(splitted[1]) + if err != nil { + return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw) + } ulimit := docker.ULimit{ Name: name, Soft: int64(soft), - Hard: int64(soft), // default: can be override + Hard: int64(hard), } - - // hard limit is optional - if len(splitted) == 2 { - if hard, err := strconv.Atoi(splitted[1]); err != nil { - return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw) - } else { - ulimit.Hard = int64(hard) - } - } - ulimits = append(ulimits, ulimit) } return ulimits, nil From db2b09e4a037cf5e3f9bf5602bce1166c3cbaf3c Mon Sep 17 00:00:00 2001 From: Samuel BERTHE Date: Wed, 29 Mar 2017 18:52:34 +0200 Subject: [PATCH 04/12] doc(docker driver): Adds doc for ulimit and sysctl --- website/source/docs/drivers/docker.html.md | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index 46c4df5da..266c3caae 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -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. +* `sysctls` - (Optional) A key-value map of sysctl configurations to set to the + containers on start. + + ```hcl + config { + sysctls { + net.core.somaxconn = "16384" + } + } + ``` + +* `ulimits` - (Optional) A key-value map of ulimits configurations to set to the + containers on start. + + ```hcl + config { + ulimits { + 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 From 657619c0ec51068e48a34817ddc4df267b6cce6f Mon Sep 17 00:00:00 2001 From: Samuel BERTHE Date: Thu, 30 Mar 2017 16:12:36 +0200 Subject: [PATCH 05/12] Oops --- client/driver/docker_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 1127f4709..ba214849f 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -861,8 +861,8 @@ func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) { func TestDockerDriver_Sysctls_Ulimits(t *testing.T) { task, _, _ := dockerTask() expectedUlimits := map[string]string{ - "nproc": "4242", - "nofiles": "2048:4096", + "nproc": "4242", + "nofile": "2048:4096", } task.Config["sysctls"] = []map[string]string{ map[string]string{ From e6c03723189f0058a711dfca6cb9648bd999fff5 Mon Sep 17 00:00:00 2001 From: Samuel BERTHE Date: Fri, 31 Mar 2017 10:18:26 +0200 Subject: [PATCH 06/12] review(docker driver): sysctls -> sysctl + ulimits -> ulimit --- client/driver/docker.go | 29 +++++++++++----------- client/driver/docker_test.go | 8 +++--- website/source/docs/drivers/docker.html.md | 11 ++++---- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 843e3d67c..c16688860 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -176,10 +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 - SysctlsRaw []map[string]string `mapstructure:"sysctls"` // - Sysctls map[string]string `mapstructure:"-"` // The sysctl custom configurations - UlimitsRaw []map[string]string `mapstructure:"ulimits"` // - Ulimits []docker.ULimit `mapstructure:"-"` // The ulimit custom configurations + 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 @@ -242,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) @@ -253,18 +252,18 @@ func (c *DockerDriverConfig) Validate() error { } } } - c.Sysctls = mapMergeStrStr(c.SysctlsRaw...) + 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.UlimitsRaw...) - ulimits, err := sliceMergeUlimit(mergedUlimitsRaw) + mergedUlimitsRaw := mapMergeStrStr(c.UlimitRaw...) + ulimit, err := sliceMergeUlimit(mergedUlimitsRaw) if err != nil { return err } - c.Ulimits = ulimits + c.Ulimit = ulimit return nil } @@ -299,14 +298,14 @@ 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.SysctlsRaw { + for _, m := range dconf.SysctlRaw { for k, v := range m { delete(m, k) m[env.ReplaceEnv(k)] = env.ReplaceEnv(v) } } - for _, m := range dconf.UlimitsRaw { + for _, m := range dconf.UlimitRaw { for k, v := range m { delete(m, k) m[env.ReplaceEnv(k)] = env.ReplaceEnv(v) @@ -565,10 +564,10 @@ func (d *DockerDriver) Validate(config map[string]interface{}) error { "userns_mode": { Type: fields.TypeString, }, - "sysctls": &fields.FieldSchema{ + "sysctl": &fields.FieldSchema{ Type: fields.TypeArray, }, - "ulimits": &fields.FieldSchema{ + "ulimit": &fields.FieldSchema{ Type: fields.TypeArray, }, "port_map": { @@ -1173,8 +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.Sysctls - hostConfig.Ulimits = driverConfig.Ulimits + hostConfig.Sysctls = driverConfig.Sysctl + hostConfig.Ulimits = driverConfig.Ulimit hostConfig.NetworkMode = driverConfig.NetworkMode if hostConfig.NetworkMode == "" { diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index ba214849f..81d38943d 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -858,18 +858,18 @@ func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) { } } -func TestDockerDriver_Sysctls_Ulimits(t *testing.T) { +func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { task, _, _ := dockerTask() expectedUlimits := map[string]string{ "nproc": "4242", "nofile": "2048:4096", } - task.Config["sysctls"] = []map[string]string{ + task.Config["sysctl"] = []map[string]string{ map[string]string{ "net.core.somaxconn": "16384", }, } - task.Config["ulimits"] = []map[string]string{ + task.Config["ulimit"] = []map[string]string{ expectedUlimits, } @@ -888,7 +888,7 @@ func TestDockerDriver_Sysctls_Ulimits(t *testing.T) { } if want, got := 2, len(container.HostConfig.Ulimits); want != got { - t.Errorf("Wrong number of ulimit configs for docker job. Expect: %s, got: %s", 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 { diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index 266c3caae..0799a8726 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -75,6 +75,7 @@ The `docker` driver supports the following configuration in the job spec. Only } ``` +<<<<<<< 657619c0ec51068e48a34817ddc4df267b6cce6f * `dns_search_domains` - (Optional) A list of DNS search domains for the container to use. @@ -97,23 +98,23 @@ The `docker` driver supports the following configuration in the job spec. Only * `interactive` - (Optional) `true` or `false` (default). Keep STDIN open on the container. -* `sysctls` - (Optional) A key-value map of sysctl configurations to set to the - containers on start. +* `sysctl` - (Optional) A key-value map of sysctl configurations to set to the + ```hcl config { - sysctls { + sysctl { net.core.somaxconn = "16384" } } ``` -* `ulimits` - (Optional) A key-value map of ulimits configurations to set to the +* `ulimit` - (Optional) A key-value map of ulimit configurations to set to the containers on start. ```hcl config { - ulimits { + ulimit { nproc = "4242" nofile = "2048:4096" } From 48829e88d0d649c597ac97b7ca981c3f255445e7 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 17 Nov 2017 17:46:04 -0600 Subject: [PATCH 07/12] Fix test compilation after rebase --- client/driver/docker_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 81d38943d..bbbddd76d 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -859,7 +859,7 @@ func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) { } func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { - task, _, _ := dockerTask() + task, _, _ := dockerTask(t) expectedUlimits := map[string]string{ "nproc": "4242", "nofile": "2048:4096", @@ -876,9 +876,9 @@ func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { client, handle, cleanup := dockerSetup(t, task) defer cleanup() - waitForExist(t, client, handle.(*DockerHandle)) + waitForExist(t, client, handle) - container, err := client.InspectContainer(handle.(*DockerHandle).ContainerID()) + container, err := client.InspectContainer(handle.ContainerID()) if err != nil { t.Fatalf("err: %v", err) } From 9e07471e483d0a6ff8e4a1d7b97a60644b840d51 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Sat, 18 Nov 2017 09:23:09 -0600 Subject: [PATCH 08/12] Fix gofmt warnings --- client/driver/docker.go | 4 ++-- client/driver/docker_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index c16688860..20e67673c 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -564,10 +564,10 @@ func (d *DockerDriver) Validate(config map[string]interface{}) error { "userns_mode": { Type: fields.TypeString, }, - "sysctl": &fields.FieldSchema{ + "sysctl": { Type: fields.TypeArray, }, - "ulimit": &fields.FieldSchema{ + "ulimit": { Type: fields.TypeArray, }, "port_map": { diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index bbbddd76d..bc161352d 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -865,7 +865,7 @@ func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { "nofile": "2048:4096", } task.Config["sysctl"] = []map[string]string{ - map[string]string{ + { "net.core.somaxconn": "16384", }, } From bc08d31849682398df5860349e72567e372ae4d1 Mon Sep 17 00:00:00 2001 From: Preetha Date: Sun, 19 Nov 2017 16:10:56 -0600 Subject: [PATCH 09/12] Remove merge conflict marker from documentation --- website/source/docs/drivers/docker.html.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index 0799a8726..e152c554e 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -74,8 +74,7 @@ The `docker` driver supports the following configuration in the job spec. Only command = "my-command" } ``` - -<<<<<<< 657619c0ec51068e48a34817ddc4df267b6cce6f + * `dns_search_domains` - (Optional) A list of DNS search domains for the container to use. From 0d4797711b4229110371f680010008f1f3bfb8cb Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 20 Nov 2017 11:15:09 -0600 Subject: [PATCH 10/12] Address some review comments --- client/driver/docker.go | 7 +++++-- client/driver/docker_test.go | 21 ++++++++++----------- website/source/docs/drivers/docker.html.md | 2 +- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 20e67673c..57850d6d8 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -213,13 +213,16 @@ func sliceMergeUlimit(ulimitsRaw map[string]string) ([]docker.ULimit, error) { } splitted := strings.SplitN(ulimitRaw, ":", 2) + if len(splitted) < 2 { + return []docker.ULimit{}, fmt.Errorf("Malformed ulimit specification %v: %v", name, ulimitRaw) + } soft, err := strconv.Atoi(splitted[0]) if err != nil { - return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw) + return []docker.ULimit{}, fmt.Errorf("Malformed soft ulimit %v: %v", name, ulimitRaw) } hard, err := strconv.Atoi(splitted[1]) if err != nil { - return []docker.ULimit{}, fmt.Errorf("Malformed ulimit %v: %v", name, ulimitRaw) + return []docker.ULimit{}, fmt.Errorf("Malformed hard ulimit %v: %v", name, ulimitRaw) } ulimit := docker.ULimit{ diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index bc161352d..e7353da3f 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -879,22 +879,21 @@ func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { waitForExist(t, client, handle) container, err := client.InspectContainer(handle.ContainerID()) - if err != nil { - t.Fatalf("err: %v", err) - } + assert.Nil(t, err, "unexpected error: %v", err) - 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) - } + want := "16384" + got := container.HostConfig.Sysctls["net.core.somaxconn"] + assert.Equal(t, want, got, "Wrong net.core.somaxconn config for docker job. Expect: %s, got: %s", want, got) + + expectedUlimitLen := 2 + actualUlimitLen := len(container.HostConfig.Ulimits) + assert.Equal(t, want, got, "Wrong number of ulimit configs for docker job. Expect: %d, got: %d", expectedUlimitLen, actualUlimitLen) - 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 { + if expectedStr, ok := expectedUlimits[got.Name]; !ok { t.Errorf("%s config unexpected for docker job.", got.Name) } else { - if strings.Contains(expectedStr, ":") == false { + if !strings.Contains(expectedStr, ":") { expectedStr = expectedStr + ":" + expectedStr } diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index e152c554e..584e68a5b 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -98,7 +98,7 @@ The `docker` driver supports the following configuration in the job spec. Only the container. * `sysctl` - (Optional) A key-value map of sysctl configurations to set to the - + containers on start. ```hcl config { From ee4b4d859d1a7c13d406ccafbd1f0ddc4c93751b Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 20 Nov 2017 12:07:18 -0600 Subject: [PATCH 11/12] Better error validation, and added test case for invalid sysctl inputs --- client/driver/docker.go | 3 +++ client/driver/docker_test.go | 45 +++++++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 57850d6d8..05653f538 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -207,6 +207,9 @@ func sliceMergeUlimit(ulimitsRaw map[string]string) ([]docker.ULimit, error) { var ulimits []docker.ULimit for name, ulimitRaw := range ulimitsRaw { + if len(ulimitRaw) == 0 { + return []docker.ULimit{}, fmt.Errorf("Malformed ulimit specification %v: %q, cannot be empty", name, ulimitRaw) + } // hard limit is optional if strings.Contains(ulimitRaw, ":") == false { ulimitRaw = ulimitRaw + ":" + ulimitRaw diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index e7353da3f..483053fb2 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -900,13 +900,46 @@ func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { splitted := strings.SplitN(expectedStr, ":", 2) soft, _ := strconv.Atoi(splitted[0]) hard, _ := strconv.Atoi(splitted[1]) + assert.Equal(t, int64(soft), got.Soft, "Wrong soft %s ulimit for docker job. Expect: %d, got: %d", got.Name, soft, got.Soft) + assert.Equal(t, int64(hard), got.Hard, "Wrong hard %s ulimit for docker job. Expect: %d, got: %d", got.Name, hard, got.Hard) - 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_Sysctl_Ulimit_Errors(t *testing.T) { + brokenConfigs := []interface{}{ + map[string]interface{}{ + "nofile": "", + }, + map[string]interface{}{ + "nofile": "abc:1234", + }, + map[string]interface{}{ + "nofile": "1234:abc", + }, + } + + test_cases := []struct { + ulimitConfig interface{} + err error + }{ + {[]interface{}{brokenConfigs[0]}, fmt.Errorf("Malformed ulimit specification nofile: \"\", cannot be empty")}, + {[]interface{}{brokenConfigs[1]}, fmt.Errorf("Malformed soft ulimit nofile: abc:1234")}, + {[]interface{}{brokenConfigs[2]}, fmt.Errorf("Malformed hard ulimit nofile: 1234:abc")}, + } + + for _, tc := range test_cases { + task, _, _ := dockerTask(t) + task.Config["ulimit"] = tc.ulimitConfig + + ctx := testDockerDriverContexts(t, task) + driver := NewDockerDriver(ctx.DriverCtx) + copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") + defer ctx.AllocDir.Destroy() + + if _, err := driver.Prestart(ctx.ExecCtx, task); err == nil || err.Error() != tc.err.Error() { + t.Fatalf("error expected in prestart, got %v, expected %v", err, tc.err) } } } From d522149f6bc4f6d1784b7dcf12449d4580e0098b Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 20 Nov 2017 13:04:38 -0600 Subject: [PATCH 12/12] Missed assert in one place --- client/driver/docker_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 483053fb2..058361ebb 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -938,9 +938,9 @@ func TestDockerDriver_Sysctl_Ulimit_Errors(t *testing.T) { copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") defer ctx.AllocDir.Destroy() - if _, err := driver.Prestart(ctx.ExecCtx, task); err == nil || err.Error() != tc.err.Error() { - t.Fatalf("error expected in prestart, got %v, expected %v", err, tc.err) - } + _, err := driver.Prestart(ctx.ExecCtx, task) + assert.NotNil(t, err, "Expected non nil error") + assert.Equal(t, err.Error(), tc.err.Error(), "unexpected error in prestart, got %v, expected %v", err, tc.err) } }