From 9c89ea4e08e2fbf57beee1b894d3cee34ad3add6 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 26 Nov 2018 16:45:01 -0500 Subject: [PATCH] Support docker bind mounts --- drivers/docker/config.go | 53 ++++++++++++- drivers/docker/driver.go | 27 ++++--- drivers/docker/driver_test.go | 143 ++++++++++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 16 deletions(-) diff --git a/drivers/docker/config.go b/drivers/docker/config.go index 2372fa45cbd..1d2ae281a31 100644 --- a/drivers/docker/config.go +++ b/drivers/docker/config.go @@ -6,6 +6,7 @@ import ( "strings" "time" + docker "github.com/fsouza/go-dockerclient" hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/nomad/client/structs" "github.com/hashicorp/nomad/plugins/base" @@ -242,10 +243,17 @@ var ( })), "mac_address": hclspec.NewAttr("mac_address", "string", false), "mounts": hclspec.NewBlockSet("mounts", hclspec.NewObject(map[string]*hclspec.Spec{ + "type": hclspec.NewDefault( + hclspec.NewAttr("type", "string", false), + hclspec.NewLiteral("\"volume\""), + ), "target": hclspec.NewAttr("target", "string", false), "source": hclspec.NewAttr("source", "string", false), "readonly": hclspec.NewAttr("readonly", "bool", false), - "volume_options": hclspec.NewBlockSet("volume_options", hclspec.NewObject(map[string]*hclspec.Spec{ + "bind_options": hclspec.NewBlock("bind_options", false, hclspec.NewObject(map[string]*hclspec.Spec{ + "propagation": hclspec.NewAttr("propagation", "string", false), + })), + "volume_options": hclspec.NewBlock("volume_options", false, hclspec.NewObject(map[string]*hclspec.Spec{ "no_copy": hclspec.NewAttr("no_copy", "bool", false), "labels": hclspec.NewBlockAttrs("labels", "string", false), "driver_config": hclspec.NewBlockSet("driver_config", hclspec.NewObject(map[string]*hclspec.Spec{ @@ -350,19 +358,60 @@ type DockerLogging struct { } type DockerMount struct { + Type string `codec:"type"` Target string `codec:"target"` Source string `codec:"source"` ReadOnly bool `codec:"readonly"` + BindOptions DockerBindOptions `codec:"bind_options"` VolumeOptions DockerVolumeOptions `codec:"volume_options"` } +func (m DockerMount) toDockerHostMount() (docker.HostMount, error) { + if m.Type == "" { + // for backward compatbility, as type is optional + m.Type = "volume" + } + + hm := docker.HostMount{ + Target: m.Target, + Source: m.Source, + Type: m.Type, + ReadOnly: m.ReadOnly, + } + + switch m.Type { + case "volume": + vo := m.VolumeOptions + hm.VolumeOptions = &docker.VolumeOptions{ + NoCopy: vo.NoCopy, + Labels: vo.Labels, + DriverConfig: docker.VolumeDriverConfig{ + Name: vo.DriverConfig.Name, + Options: vo.DriverConfig.Options, + }, + } + case "bind": + hm.BindOptions = &docker.BindOptions{ + Propagation: m.BindOptions.Propagation, + } + default: + return hm, fmt.Errorf(`invalid mount type, must be "bind" or "volume": %q`, m.Type) + } + + return hm, nil +} + type DockerVolumeOptions struct { NoCopy bool `codec:"no_copy"` Labels map[string]string `codec:"labels"` DriverConfig DockerVolumeDriverConfig `codec:"driver_config"` } -// VolumeDriverConfig holds a map of volume driver specific options +type DockerBindOptions struct { + Propagation string `codec:"propagation"` +} + +// DockerVolumeDriverConfig holds a map of volume driver specific options type DockerVolumeDriverConfig struct { Name string `codec:"name"` Options map[string]string `codec:"options"` diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index 4c22a17838d..323915c0129 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -745,23 +745,22 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T // Setup mounts for _, m := range driverConfig.Mounts { - hm := docker.HostMount{ - Target: m.Target, - Source: m.Source, - Type: "volume", // Only type supported - ReadOnly: m.ReadOnly, - } - vo := m.VolumeOptions - hm.VolumeOptions = &docker.VolumeOptions{ - NoCopy: vo.NoCopy, + hm, err := m.toDockerHostMount() + if err != nil { + return c, err } - dc := vo.DriverConfig - hm.VolumeOptions.DriverConfig = docker.VolumeDriverConfig{ - Name: dc.Name, + if hm.Type == "bind" { + if filepath.IsAbs(filepath.Clean(hm.Source)) { + if !d.config.Volumes.Enabled { + return c, fmt.Errorf("volumes are not enabled; cannot mount host path: %q", hm.Source) + } + } else { + // Relative paths are always allowed as they mount within a container, and treated as relative to task dir + hm.Source = filepath.Join(task.TaskDir().Dir, hm.Source) + } } - hm.VolumeOptions.DriverConfig.Options = dc.Options - hm.VolumeOptions.Labels = vo.Labels + hostConfig.Mounts = append(hostConfig.Mounts, hm) } diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index 7babd31d5e1..f047155e126 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -1733,6 +1733,149 @@ func TestDockerDriver_Mounts(t *testing.T) { } } +func TestDockerDriver_MountsSerialization(t *testing.T) { + t.Parallel() + + allocDir := "/tmp/nomad/alloc-dir" + + cases := []struct { + name string + requiresVolumes bool + passedMounts []DockerMount + expectedMounts []docker.HostMount + }{ + { + name: "basic volume", + passedMounts: []DockerMount{ + { + Target: "/nomad", + ReadOnly: true, + Source: "test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "volume", + Target: "/nomad", + Source: "test", + ReadOnly: true, + VolumeOptions: &docker.VolumeOptions{}, + }, + }, + }, + { + name: "basic bind", + passedMounts: []DockerMount{ + { + Type: "bind", + Target: "/nomad", + Source: "test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/nomad/alloc-dir/demo/test", + BindOptions: &docker.BindOptions{}, + }, + }, + }, + { + name: "basic absolute bind", + requiresVolumes: true, + passedMounts: []DockerMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/test", + BindOptions: &docker.BindOptions{}, + }, + }, + }, + { + + // FIXME: This needs to be true but we have a bug with security implications. + // The relative paths check should restrict access to alloc-dir subtree + // documenting existing behavior in test here and need to follow up in another commit + requiresVolumes: false, + + name: "bind relative outside", + passedMounts: []DockerMount{ + { + Type: "bind", + Target: "/nomad", + Source: "../../test", + }, + }, + expectedMounts: []docker.HostMount{ + { + Type: "bind", + Target: "/nomad", + Source: "/tmp/nomad/test", + BindOptions: &docker.BindOptions{}, + }, + }, + }, + } + + t.Run("with volumes enabled", func(t *testing.T) { + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + driver.config.Volumes.Enabled = true + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + task, cfg, _ := dockerTask(t) + cfg.Mounts = c.passedMounts + + task.AllocDir = allocDir + task.Name = "demo" + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + require.NoError(t, err) + require.EqualValues(t, c.expectedMounts, cc.HostConfig.Mounts) + }) + } + }) + + t.Run("with volumes disabled", func(t *testing.T) { + dh := dockerDriverHarness(t, nil) + driver := dh.Impl().(*Driver) + driver.config.Volumes.Enabled = false + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + task, cfg, _ := dockerTask(t) + cfg.Mounts = c.passedMounts + + task.AllocDir = allocDir + task.Name = "demo" + + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1") + if c.requiresVolumes { + require.Error(t, err, "volumes are not enabled") + } else { + require.NoError(t, err) + require.EqualValues(t, c.expectedMounts, cc.HostConfig.Mounts) + } + }) + } + }) + +} + // TestDockerDriver_Cleanup ensures Cleanup removes only downloaded images. func TestDockerDriver_Cleanup(t *testing.T) { if !testutil.DockerIsConnected(t) {