From fe1e3c792fd6a0f94225312027de69a2dcd10b14 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 27 Sep 2016 13:13:55 -0700 Subject: [PATCH] Put docker volume support behind conf flag Also add tests and fix bug with logging driver configuration. --- client/driver/docker.go | 91 +++++++++++---------- client/driver/docker_test.go | 94 ++++++++++++++++++++++ command/agent/config.go | 3 + website/source/docs/drivers/docker.html.md | 16 +++- 4 files changed, 161 insertions(+), 43 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index ad522f94b85..f1ad0bae9cf 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -58,6 +58,10 @@ const ( // driver dockerDriverAttr = "driver.docker" + dockerSELinuxLabelConfigOption = "docker.volumes.selinuxlabel" + dockerVolumesConfigOption = "docker.volumes.enabled" + dockerPrivilegedConfigOption = "docker.privileged.enabled" + // dockerTimeout is the length of time a request can be outstanding before // it is timed out. dockerTimeout = 1 * time.Minute @@ -116,27 +120,9 @@ func (c *DockerDriverConfig) Validate() error { c.PortMap = mapMergeStrInt(c.PortMapRaw...) c.Labels = mapMergeStrStr(c.LabelsRaw...) - - //if no logging is present, set default values. Otherwise, convert the raw logging data into a logging object - if len(c.Logging) != 0 { + if len(c.Logging) > 0 { c.Logging[0].Config = mapMergeStrStr(c.Logging[0].ConfigRaw...) - } else { - c.Logging = []DockerLoggingOpts{ - { - Type: "syslog", - Config: make(map[string]string), - }, - } - } - - if c.Volumes == nil { - c.Volumes = make([]string, 0) } - - if c.VolumesFrom == nil { - c.VolumesFrom = make([]string, 0) - } - return nil } @@ -358,9 +344,9 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool return false, nil } - privileged := d.config.ReadBoolDefault("docker.privileged.enabled", false) + privileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false) if privileged { - node.Attributes["docker.privileged.enabled"] = "1" + node.Attributes[dockerPrivilegedConfigOption] = "1" } // This is the first operation taken on the client so we'll try to @@ -377,10 +363,17 @@ func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool node.Attributes[dockerDriverAttr] = "1" node.Attributes["driver.docker.version"] = env.Get("Version") + + if d.config.ReadBoolDefault(dockerVolumesConfigOption, false) { + node.Attributes["driver."+dockerVolumesConfigOption] = "1" + } + return true, nil } -func (d *DockerDriver) containerBinds(alloc *allocdir.AllocDir, task *structs.Task) ([]string, error) { +func (d *DockerDriver) containerBinds(driverConfig *DockerDriverConfig, alloc *allocdir.AllocDir, + task *structs.Task) ([]string, error) { + shared := alloc.SharedDir local, ok := alloc.TaskDirs[task.Name] if !ok { @@ -394,17 +387,34 @@ func (d *DockerDriver) containerBinds(alloc *allocdir.AllocDir, task *structs.Ta allocDirBind := fmt.Sprintf("%s:%s", shared, allocdir.SharedAllocContainerPath) taskLocalBind := fmt.Sprintf("%s:%s", local, allocdir.TaskLocalContainerPath) secretDirBind := fmt.Sprintf("%s:%s", secret, allocdir.TaskSecretsContainerPath) + binds := []string{allocDirBind, taskLocalBind, secretDirBind} - if selinuxLabel := d.config.Read("docker.volumes.selinuxlabel"); selinuxLabel != "" { - allocDirBind = fmt.Sprintf("%s:%s", allocDirBind, selinuxLabel) - taskLocalBind = fmt.Sprintf("%s:%s", taskLocalBind, selinuxLabel) - secretDirBind = fmt.Sprintf("%s:%s", secretDirBind, selinuxLabel) + var merr multierror.Error + volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, false) + if len(driverConfig.Volumes) > 0 && !volumesEnabled { + merr.Errors = append(merr.Errors, fmt.Errorf(dockerVolumesConfigOption+" is false; cannot use Docker Volumes: %+q", driverConfig.Volumes)) } - return []string{ - allocDirBind, - taskLocalBind, - secretDirBind, - }, nil + + if len(driverConfig.VolumesFrom) > 0 && !volumesEnabled { + merr.Errors = append(merr.Errors, fmt.Errorf(dockerVolumesConfigOption+" is false; cannot use Docker VolumesFrom: %+q", driverConfig.VolumesFrom)) + } + + if err := merr.ErrorOrNil(); err != nil { + return nil, err + } + + if len(driverConfig.Volumes) > 0 { + binds = append(binds, driverConfig.Volumes...) + } + + if selinuxLabel := d.config.Read(dockerSELinuxLabelConfigOption); selinuxLabel != "" { + // Apply SELinux Label to each volume + for i := range binds { + binds[i] = fmt.Sprintf("%s:%s", binds[i], selinuxLabel) + } + } + + return binds, nil } // createContainer initializes a struct needed to call docker.client.CreateContainer() @@ -418,7 +428,7 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, return c, fmt.Errorf("task.Resources is empty") } - binds, err := d.containerBinds(ctx.AllocDir, task) + binds, err := d.containerBinds(driverConfig, ctx.AllocDir, task) if err != nil { return c, err } @@ -442,20 +452,15 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, memLimit := int64(task.Resources.MemoryMB) * 1024 * 1024 - if len(driverConfig.Logging) != 0 { + if len(driverConfig.Logging) == 0 { d.logger.Printf("[DEBUG] driver.docker: Setting default logging options to syslog and %s", syslogAddr) - if driverConfig.Logging[0].Type == "" { - driverConfig.Logging[0].Type = "syslog" - driverConfig.Logging[0].Config["syslog-address"] = syslogAddr + driverConfig.Logging = []DockerLoggingOpts{ + {Type: "syslog", Config: map[string]string{"syslog-address": syslogAddr}}, } } d.logger.Printf("[DEBUG] driver.docker: Using config for logging: %+v", driverConfig.Logging[0]) - //Merge nomad container binds and user specified binds - d.logger.Printf("[DEBUG] Unmodified binds from nomad: %+v\n", binds) - binds = append(binds, driverConfig.Volumes...) - hostConfig := &docker.HostConfig{ // Convert MB to bytes. This is an absolute value. Memory: memLimit, @@ -477,10 +482,12 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, d.logger.Printf("[DEBUG] driver.docker: using %d bytes memory for %s", hostConfig.Memory, task.Name) d.logger.Printf("[DEBUG] driver.docker: using %d cpu shares for %s", hostConfig.CPUShares, task.Name) d.logger.Printf("[DEBUG] driver.docker: binding directories %#v for %s", hostConfig.Binds, task.Name) - d.logger.Printf("[DEBUG] driver.docker: binding Volumes-From: %#v for %s", hostConfig.VolumesFrom, task.Name) + if d.config.ReadBoolDefault(dockerVolumesConfigOption, false) { + d.logger.Printf("[DEBUG] driver.docker: binding Volumes-From: %#v for %s", hostConfig.VolumesFrom, task.Name) + } // set privileged mode - hostPrivileged := d.config.ReadBoolDefault("docker.privileged.enabled", false) + hostPrivileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false) if driverConfig.Privileged && !hostPrivileged { return c, fmt.Errorf(`Docker privileged mode is disabled on this Nomad agent`) } diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 5029d9e787d..390652f1dca 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -4,6 +4,8 @@ import ( "fmt" "io/ioutil" "math/rand" + "os" + "path" "path/filepath" "reflect" "runtime/debug" @@ -17,6 +19,7 @@ import ( "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/driver/env" "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" tu "github.com/hashicorp/nomad/testutil" ) @@ -861,6 +864,97 @@ func TestDockerDriver_Stats(t *testing.T) { } +func setupDockerVolumes(t *testing.T, cfg *config.Config) (*structs.Task, Driver, *ExecContext, string, func()) { + if !testutil.DockerIsConnected(t) { + t.SkipNow() + } + + tmpvol, err := ioutil.TempDir("", "nomadtest_dockerdriver_volumes") + if err != nil { + t.Fatalf("error creating temporary dir: %v", err) + } + + randfn := fmt.Sprintf("test-%d", rand.Int()) + hostpath := path.Join(tmpvol, randfn) + contpath := path.Join("/mnt/vol", randfn) + + task := &structs.Task{ + Name: "ls", + Config: map[string]interface{}{ + "image": "busybox", + "load": []string{"busybox.tar"}, + "command": "touch", + "args": []string{contpath}, + "volumes": []string{fmt.Sprintf("%s:/mnt/vol", tmpvol)}, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + } + + allocDir := allocdir.NewAllocDir(filepath.Join(cfg.AllocDir, structs.GenerateUUID()), task.Resources.DiskMB) + allocDir.Build([]*structs.Task{task}) + alloc := mock.Alloc() + execCtx := NewExecContext(allocDir, alloc.ID) + cleanup := func() { + execCtx.AllocDir.Destroy() + os.RemoveAll(tmpvol) + } + + taskEnv, err := GetTaskEnv(allocDir, cfg.Node, task, alloc, "") + if err != nil { + cleanup() + t.Fatalf("Failed to get task env: %v", err) + } + + driverCtx := NewDriverContext(task.Name, cfg, cfg.Node, testLogger(), taskEnv) + driver := NewDockerDriver(driverCtx) + copyImage(execCtx, task, "busybox.tar", t) + + return task, driver, execCtx, hostpath, cleanup +} + +func TestDockerDriver_VolumesDisabled(t *testing.T) { + cfg := testConfig() + + task, driver, execCtx, _, cleanup := setupDockerVolumes(t, cfg) + defer cleanup() + + _, err := driver.Start(execCtx, task) + if err == nil { + t.Fatalf("Started driver successfully when volumes should have been disabled.") + } +} + +func TestDockerDriver_VolumesEnabled(t *testing.T) { + cfg := testConfig() + cfg.Options = map[string]string{dockerVolumesConfigOption: "true"} + + task, driver, execCtx, hostpath, cleanup := setupDockerVolumes(t, cfg) + defer cleanup() + + handle, err := driver.Start(execCtx, task) + if err != nil { + t.Fatalf("Failed to start docker driver: %v", err) + } + defer handle.Kill() + + select { + case res := <-handle.WaitCh(): + if !res.Successful() { + t.Fatalf("unexpected err: %v", res) + } + case <-time.After(time.Duration(tu.TestMultiplier()*10) * time.Second): + t.Fatalf("timeout") + } + + if _, err := ioutil.ReadFile(hostpath); err != nil { + t.Fatalf("unexpected error reading %s: %v", hostpath, err) + } +} + func copyImage(execCtx *ExecContext, task *structs.Task, image string, t *testing.T) { taskDir, _ := execCtx.AllocDir.TaskDirs[task.Name] dst := filepath.Join(taskDir, allocdir.TaskLocal, image) diff --git a/command/agent/config.go b/command/agent/config.go index 27de7fff920..bb7692289e8 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -432,6 +432,9 @@ func DevConfig() *Config { conf.Client.Options = map[string]string{ "driver.raw_exec.enable": "true", } + conf.Client.Options = map[string]string{ + "driver.docker.volumes": "true", + } return conf } diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index 6051a5fb1a9..fef8aac9412 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -137,6 +137,14 @@ The `docker` driver supports the following configuration in the job spec: * `shm_size` - (Optional) The size (bytes) of /dev/shm for the container. +* `volumes` - (Optional) A list of `host_path:container_path` strings to bind + host paths to container paths. Can only be run on clients with the + `docker.volumes.enabled` option set to true. + +* `volumes_from` - (Optional) A list of volumes to inherit from another + container. Can only be run on clients with the `docker.volumes.enabled` + option set to true. + * `work_dir` - (Optional) The working directory inside the container. ### Container Name @@ -329,8 +337,14 @@ options](/docs/agent/config.html#options): * `docker.cleanup.image` Defaults to `true`. Changing this to `false` will prevent Nomad from removing images from stopped tasks. +* `docker.volumes.enabled`: Defaults to `false`. Allows tasks to bind host + paths (`volumes`) or other containers (`volums_from`) inside their container. + Disabled by default as it removes the isolation between containers' data. + * `docker.volumes.selinuxlabel`: Allows the operator to set a SELinux - label to the allocation and task local bind-mounts to containers. + label to the allocation and task local bind-mounts to containers. If used + with `docker.volumes.enabled` set to false, the labels will still be applied + to the standard binds in the container. * `docker.privileged.enabled` Defaults to `false`. Changing this to `true` will allow containers to use `privileged` mode, which gives the containers full