-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Changes from 11 commits
84a0894
cf58699
d15f01d
db2b09e
657619c
e6c0372
48829e8
9e07471
bc08d31
0d47977
ee4b4d8
d522149
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -199,6 +203,41 @@ 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 { | ||
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 | ||
} | ||
|
||
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 soft ulimit %v: %v", name, ulimitRaw) | ||
} | ||
hard, err := strconv.Atoi(splitted[1]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 hard 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 == "" { | ||
|
@@ -209,7 +248,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) | ||
|
@@ -220,6 +258,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 | ||
} | ||
|
||
|
@@ -254,6 +304,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) | ||
|
@@ -506,6 +570,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, | ||
}, | ||
|
@@ -1108,6 +1178,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 == "" { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -858,6 +858,92 @@ func TestDockerDriver_NetworkAliases_Bridge(t *testing.T) { | |
} | ||
} | ||
|
||
func TestDockerDriver_Sysctl_Ulimit(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()) | ||
assert.Nil(t, err, "unexpected error: %v", err) | ||
|
||
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) | ||
|
||
for _, got := range container.HostConfig.Ulimits { | ||
if expectedStr, ok := expectedUlimits[got.Name]; !ok { | ||
t.Errorf("%s config unexpected for docker job.", got.Name) | ||
} else { | ||
if !strings.Contains(expectedStr, ":") { | ||
expectedStr = expectedStr + ":" + expectedStr | ||
} | ||
|
||
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) | ||
|
||
} | ||
} | ||
} | ||
|
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use assert here :) |
||
} | ||
} | ||
} | ||
|
||
func TestDockerDriver_Labels(t *testing.T) { | ||
if !tu.IsTravis() { | ||
t.Parallel() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sentence is incomplete. |
||
containers on start. | ||
|
||
```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 | ||
|
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.
Check length before indexing.