diff --git a/client/driver/docker.go b/client/driver/docker.go index 6c8360aec97f..068c78f26b64 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -168,9 +168,11 @@ type DockerLoggingOpts struct { } type DockerMount struct { + Type string `mapstructure:"type"` Target string `mapstructure:"target"` Source string `mapstructure:"source"` ReadOnly bool `mapstructure:"readonly"` + BindOptions []*DockerBindOptions `mapstructure:"bind_options"` VolumeOptions []*DockerVolumeOptions `mapstructure:"volume_options"` } @@ -180,6 +182,10 @@ type DockerDevice struct { CgroupPermissions string `mapstructure:"cgroup_permissions"` } +type DockerBindOptions struct { + Propagation string `mapstructure:"propagation"` +} + type DockerVolumeOptions struct { NoCopy bool `mapstructure:"no_copy"` Labels []map[string]string `mapstructure:"labels"` @@ -390,8 +396,29 @@ func NewDockerDriverConfig(task *structs.Task, env *env.TaskEnv) (*DockerDriverC dconf.Mounts[i].Target = env.ReplaceEnv(m.Target) dconf.Mounts[i].Source = env.ReplaceEnv(m.Source) + if m.Type == "" { + // default to `volume` type for backwards compatibility + m.Type = "volume" + } + + if m.Type != "bind" && m.Type != "volume" { + return nil, fmt.Errorf(`Docker mount type must be "bind" or "volume"`) + } + + if m.Type == "bind" && len(m.VolumeOptions) > 0 { + return nil, fmt.Errorf(`volume_options invalid on "bind" mount type`) + } + + if m.Type == "volume" && len(m.BindOptions) > 0 { + return nil, fmt.Errorf(`bind_options invalid on "volume" mount type`) + } + + if len(m.BindOptions) > 1 { + return nil, fmt.Errorf("only one bind_options stanza allowed") + } + if len(m.VolumeOptions) > 1 { - return nil, fmt.Errorf("Only one volume_options stanza allowed") + return nil, fmt.Errorf("only one volume_options stanza allowed") } if len(m.VolumeOptions) == 1 { @@ -1342,26 +1369,51 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas hm := docker.HostMount{ Target: m.Target, Source: m.Source, - Type: "volume", // Only type supported + Type: m.Type, ReadOnly: m.ReadOnly, } - if len(m.VolumeOptions) == 1 { - vo := m.VolumeOptions[0] - hm.VolumeOptions = &docker.VolumeOptions{ - NoCopy: vo.NoCopy, - } - if len(vo.DriverConfig) == 1 { - dc := vo.DriverConfig[0] - hm.VolumeOptions.DriverConfig = docker.VolumeDriverConfig{ - Name: dc.Name, + if hm.Type == "bind" { + volumesEnabled := d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) + hm.Source = filepath.Clean(hm.Source) + + if filepath.IsAbs(hm.Source) { + d.logger.Println("[DEBUG] driver.docker: Job contains an absolute path.") + if !volumesEnabled { + // Disallow mounting arbitrary absolute paths + return c, fmt.Errorf("%s is false; cannot bind mount host paths: %+q", dockerVolumesConfigOption, m.Source) } - if len(dc.Options) == 1 { - hm.VolumeOptions.DriverConfig.Options = dc.Options[0] + } else { + // Expand path relative to alloc dir + hm.Source = filepath.Join(ctx.TaskDir.Dir, m.Source) + d.logger.Printf("[DEBUG] driver.docker: Job contains a relative path; expanding to %v", hm.Source) + } + + if len(m.BindOptions) == 1 { + bo := m.BindOptions[0] + hm.BindOptions = &docker.BindOptions{ + Propagation: bo.Propagation, } } - if len(vo.Labels) == 1 { - hm.VolumeOptions.Labels = vo.Labels[0] + } else { + if len(m.VolumeOptions) == 1 { + vo := m.VolumeOptions[0] + hm.VolumeOptions = &docker.VolumeOptions{ + NoCopy: vo.NoCopy, + } + + if len(vo.DriverConfig) == 1 { + dc := vo.DriverConfig[0] + hm.VolumeOptions.DriverConfig = docker.VolumeDriverConfig{ + Name: dc.Name, + } + if len(dc.Options) == 1 { + hm.VolumeOptions.DriverConfig.Options = dc.Options[0] + } + } + if len(vo.Labels) == 1 { + hm.VolumeOptions.Labels = vo.Labels[0] + } } } hostConfig.Mounts = append(hostConfig.Mounts, hm) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 588191fe523c..42fa12388d44 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -634,7 +634,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { if !res.Successful() { t.Fatalf("err: %v", res) } - case <-time.After(time.Duration(tu.TestMultiplier()*5) * time.Second): + case <-time.After(time.Duration(tu.TestMultiplier()*10) * time.Second): t.Fatalf("timeout") } @@ -1890,7 +1890,7 @@ func TestDockerDriver_VolumesEnabled(t *testing.T) { } } -func TestDockerDriver_Mounts(t *testing.T) { +func TestDockerDriver_VolumeMounts(t *testing.T) { if !tu.IsTravis() { t.Parallel() } @@ -1899,6 +1899,7 @@ func TestDockerDriver_Mounts(t *testing.T) { } goodMount := map[string]interface{}{ + "type": "volume", "target": "/nomad", "volume_options": []interface{}{ map[string]interface{}{ @@ -1936,11 +1937,27 @@ func TestDockerDriver_Mounts(t *testing.T) { Error: "", Mounts: []interface{}{goodMount, goodMount, goodMount}, }, + { + Name: "bind_options in bind mount error", + Error: "bind_options invalid on \"volume\" mount type", + Mounts: []interface{}{ + map[string]interface{}{ + "type": "volume", + "target": "/nomad", + "bind_options": []interface{}{ + map[string]interface{}{ + "propagation": "shared", + }, + }, + }, + }, + }, { Name: "multiple volume options", - Error: "Only one volume_options stanza allowed", + Error: "only one volume_options stanza allowed", Mounts: []interface{}{ map[string]interface{}{ + "type": "volume", "target": "/nomad", "volume_options": []interface{}{ map[string]interface{}{ @@ -1966,6 +1983,7 @@ func TestDockerDriver_Mounts(t *testing.T) { Error: "volume driver config may only be specified once", Mounts: []interface{}{ map[string]interface{}{ + "type": "volume", "target": "/nomad", "volume_options": []interface{}{ map[string]interface{}{ @@ -1987,6 +2005,7 @@ func TestDockerDriver_Mounts(t *testing.T) { Error: "labels may only be", Mounts: []interface{}{ map[string]interface{}{ + "type": "volume", "target": "/nomad", "volume_options": []interface{}{ map[string]interface{}{ @@ -2004,6 +2023,7 @@ func TestDockerDriver_Mounts(t *testing.T) { Error: "driver options may only", Mounts: []interface{}{ map[string]interface{}{ + "type": "volume", "target": "/nomad", "volume_options": []interface{}{ map[string]interface{}{ @@ -2070,6 +2090,312 @@ func TestDockerDriver_Mounts(t *testing.T) { } } +func setupDockerBindMount(t *testing.T, cfg *config.Config, hostpath string) (*structs.Task, Driver, *ExecContext, string, func()) { + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + + randfn := fmt.Sprintf("test-%d", rand.Int()) + hostfile := filepath.Join(hostpath, randfn) + containerPath := "/mnt/vol" + containerFile := filepath.Join(containerPath, randfn) + + task := &structs.Task{ + Name: "ls", + Driver: "docker", + Config: map[string]interface{}{ + "image": "busybox", + "load": "busybox.tar", + "command": "touch", + "args": []string{containerFile}, + "mounts": []interface{}{ + map[string]interface{}{ + "readonly": "false", + "source": hostpath, + "target": containerPath, + "type": "bind", + "bind_options": []interface{}{ + map[string]interface{}{ + "propagation": "shared", + }, + }, + }, + }, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Resources: basicResources, + } + + // Build alloc and task directory structure + allocDir := allocdir.NewAllocDir(testLogger(), filepath.Join(cfg.AllocDir, uuid.Generate())) + if err := allocDir.Build(); err != nil { + t.Fatalf("failed to build alloc dir: %v", err) + } + taskDir := allocDir.NewTaskDir(task.Name) + if err := taskDir.Build(false, nil, cstructs.FSIsolationImage); err != nil { + allocDir.Destroy() + t.Fatalf("failed to build task dir: %v", err) + } + copyImage(t, taskDir, "busybox.tar") + + // Setup driver + alloc := mock.Alloc() + logger := testLogger() + emitter := func(m string, args ...interface{}) { + logger.Printf("[EVENT] "+m, args...) + } + driverCtx := NewDriverContext(alloc.Job.Name, alloc.TaskGroup, task.Name, alloc.ID, cfg, cfg.Node, testLogger(), emitter) + driver := NewDockerDriver(driverCtx) + + // Setup execCtx + envBuilder := env.NewBuilder(cfg.Node, alloc, task, cfg.Region) + SetEnvvars(envBuilder, driver.FSIsolation(), taskDir, cfg) + execCtx := NewExecContext(taskDir, envBuilder.Build()) + + // Setup cleanup function + cleanup := func() { + allocDir.Destroy() + if filepath.IsAbs(hostpath) { + os.RemoveAll(hostpath) + } + } + return task, driver, execCtx, hostfile, cleanup +} + +func TestDockerDriver_BindMount_VolumesDisabled(t *testing.T) { + if tu.IsTravis() { + t.Skip("Need to upgrade to Ubuntu 16.04+") + } else { + t.Parallel() + } + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + + cfg := testConfig(t) + cfg.Options = map[string]string{ + dockerVolumesConfigOption: "false", + "docker.cleanup.image": "false", + } + + { + tmpvol, err := ioutil.TempDir("", "nomadtest_docker_volumesdisabled") + if err != nil { + t.Fatalf("error creating temporary dir: %v", err) + } + + task, driver, execCtx, _, cleanup := setupDockerBindMount(t, cfg, tmpvol) + defer cleanup() + + _, err = driver.Prestart(execCtx, task) + if err != nil { + t.Fatalf("error in prestart: %v", err) + } + if _, err := driver.Start(execCtx, task); err == nil { + t.Fatalf("Started driver successfully when volumes should have been disabled.") + } + } + + // Relative paths should still be allowed + { + task, driver, execCtx, fn, cleanup := setupDockerBindMount(t, cfg, ".") + defer cleanup() + + _, err := driver.Prestart(execCtx, task) + if err != nil { + t.Fatalf("error in prestart: %v", err) + } + resp, err := driver.Start(execCtx, task) + if err != nil { + t.Fatalf("err: %v", err) + } + defer resp.Handle.Kill() + + select { + case res := <-resp.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(filepath.Join(execCtx.TaskDir.Dir, fn)); err != nil { + t.Fatalf("unexpected error reading %s: %v", fn, err) + } + } +} + +func TestDockerDriver_BindMount_VolumesEnabled(t *testing.T) { + if tu.IsTravis() { + t.Skip("Need to upgrade to Ubuntu 16.04+") + } else { + t.Parallel() + } + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + + cfg := testConfig(t) + + tmpvol, err := ioutil.TempDir("", "nomadtest_docker_volumesenabled") + if err != nil { + t.Fatalf("error creating temporary dir: %v", err) + } + + // Evaluate symlinks so it works on MacOS + tmpvol, err = filepath.EvalSymlinks(tmpvol) + if err != nil { + t.Fatalf("error evaluating symlinks: %v", err) + } + + task, driver, execCtx, hostpath, cleanup := setupDockerBindMount(t, cfg, tmpvol) + defer cleanup() + + _, err = driver.Prestart(execCtx, task) + if err != nil { + t.Fatalf("error in prestart: %v", err) + } + resp, err := driver.Start(execCtx, task) + if err != nil { + t.Fatalf("Failed to start docker driver: %v", err) + } + defer resp.Handle.Kill() + + select { + case res := <-resp.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 TestDockerDriver_BindMount(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + + goodMount := map[string]interface{}{ + "type": "bind", + "target": "/nomad", + "bind_options": []interface{}{ + map[string]interface{}{ + "propagation": "shared", + }, + }, + "readonly": true, + "source": "test", + } + + cases := []struct { + Name string + Mounts []interface{} + Error string + }{ + { + Name: "good-one", + Error: "", + Mounts: []interface{}{goodMount}, + }, + { + Name: "good-many", + Error: "", + Mounts: []interface{}{goodMount, goodMount, goodMount}, + }, + { + Name: "volume_options in bind mount error", + Error: "volume_options invalid on \"bind\" mount type", + Mounts: []interface{}{ + map[string]interface{}{ + "type": "bind", + "target": "/nomad", + "volume_options": []interface{}{ + map[string]interface{}{ + "driver_config": []interface{}{ + map[string]interface{}{ + "name": "local", + }, + }, + }, + }, + }, + }, + }, + { + Name: "multiple bind options", + Error: "only one bind_options stanza allowed", + Mounts: []interface{}{ + map[string]interface{}{ + "type": "bind", + "target": "/nomad", + "bind_options": []interface{}{ + map[string]interface{}{ + "propagation": "shared", + }, + map[string]interface{}{ + "propagation": "shared", + }, + }, + }, + }, + }, + } + + task := &structs.Task{ + Name: "redis-demo", + Driver: "docker", + Config: map[string]interface{}{ + "image": "busybox", + "load": "busybox.tar", + "command": "/bin/sleep", + "args": []string{"10000"}, + }, + Resources: &structs.Resources{ + MemoryMB: 256, + CPU: 512, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + } + + for _, c := range cases { + t.Run(c.Name, func(t *testing.T) { + // Build the task + task.Config["mounts"] = c.Mounts + + ctx := testDockerDriverContexts(t, task) + driver := NewDockerDriver(ctx.DriverCtx) + copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") + defer ctx.AllocDir.Destroy() + + _, err := driver.Prestart(ctx.ExecCtx, task) + if err == nil && c.Error != "" { + t.Fatalf("expected error: %v", c.Error) + } else if err != nil { + if c.Error == "" { + t.Fatalf("unexpected error in prestart: %v", err) + } else if !strings.Contains(err.Error(), c.Error) { + t.Fatalf("expected error %q; got %v", c.Error, err) + } + } + }) + } +} + // TestDockerDriver_Cleanup ensures Cleanup removes only downloaded images. func TestDockerDriver_Cleanup(t *testing.T) { if !tu.IsTravis() { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index f22aca17af85..6e67b4e384e9 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1574,13 +1574,13 @@ func (n *Node) Stub() *NodeListStub { addr, _, _ := net.SplitHostPort(n.HTTPAddr) return &NodeListStub{ - Address: addr, - ID: n.ID, - Datacenter: n.Datacenter, - Name: n.Name, - NodeClass: n.NodeClass, - Version: n.Attributes["nomad.version"], - Drain: n.Drain, + Address: addr, + ID: n.ID, + Datacenter: n.Datacenter, + Name: n.Name, + NodeClass: n.NodeClass, + Version: n.Attributes["nomad.version"], + Drain: n.Drain, SchedulingEligibility: n.SchedulingEligibility, Status: n.Status, StatusDescription: n.StatusDescription,