Skip to content

Commit

Permalink
Support docker bind mounts
Browse files Browse the repository at this point in the history
  • Loading branch information
Mahmood Ali committed Nov 27, 2018
1 parent 0af9e85 commit 9c89ea4
Show file tree
Hide file tree
Showing 3 changed files with 207 additions and 16 deletions.
53 changes: 51 additions & 2 deletions drivers/docker/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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"`
Expand Down
27 changes: 13 additions & 14 deletions drivers/docker/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
143 changes: 143 additions & 0 deletions drivers/docker/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 9c89ea4

Please sign in to comment.