Skip to content

Commit

Permalink
Merge pull request #4924 from hashicorp/f-docker-mounts
Browse files Browse the repository at this point in the history
Support bind and tmpfs docker mounts
  • Loading branch information
notnoop committed Nov 30, 2018
2 parents bff6484 + fe238dc commit 5c354ad
Show file tree
Hide file tree
Showing 4 changed files with 271 additions and 17 deletions.
71 changes: 69 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,21 @@ 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),
})),
"tmpfs_options": hclspec.NewBlock("tmpfs_options", false, hclspec.NewObject(map[string]*hclspec.Spec{
"size": hclspec.NewAttr("size", "number", false),
"mode": hclspec.NewAttr("mode", "number", 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,10 +362,56 @@ 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"`
TmpfsOptions DockerTmpfsOptions `codec:"tmpfs_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,
}
case "tmpfs":
if m.Source != "" {
return hm, fmt.Errorf(`invalid source, must be "" for tmpfs`)
}
hm.TempfsOptions = &docker.TempfsOptions{
SizeBytes: m.TmpfsOptions.SizeBytes,
Mode: m.TmpfsOptions.Mode,
}
default:
return hm, fmt.Errorf(`invalid mount type, must be "bind", "volume", "tmpfs": %q`, m.Type)
}

return hm, nil
}

type DockerVolumeOptions struct {
Expand All @@ -362,7 +420,16 @@ type DockerVolumeOptions struct {
DriverConfig DockerVolumeDriverConfig `codec:"driver_config"`
}

// VolumeDriverConfig holds a map of volume driver specific options
type DockerBindOptions struct {
Propagation string `codec:"propagation"`
}

type DockerTmpfsOptions struct {
SizeBytes int64 `codec:"size"`
Mode int `codec:"mode"`
}

// 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
167 changes: 167 additions & 0 deletions drivers/docker/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,173 @@ 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{},
},
},
},
{
name: "basic tmpfs",
requiresVolumes: false,
passedMounts: []DockerMount{
{
Type: "tmpfs",
Target: "/nomad",
TmpfsOptions: DockerTmpfsOptions{
SizeBytes: 321,
Mode: 0666,
},
},
},
expectedMounts: []docker.HostMount{
{
Type: "tmpfs",
Target: "/nomad",
TempfsOptions: &docker.TempfsOptions{
SizeBytes: 321,
Mode: 0666,
},
},
},
},
}

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
23 changes: 22 additions & 1 deletion website/source/docs/drivers/docker.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,14 @@ The `docker` driver supports the following configuration in the job spec. Only

* `mounts` - (Optional) A list of
[mounts](https://docs.docker.com/engine/reference/commandline/service_create/#add-bind-mounts-or-volumes)
to be mounted into the container. Only volume type mounts are supported.
to be mounted into the container. Volume, bind, and tmpfs type mounts are supported.

```hcl
config {
mounts = [
# sample volume mount
{
type = "volume"
target = "/path/in/container"
source = "name-of-volume"
readonly = false
Expand All @@ -314,6 +316,25 @@ The `docker` driver supports the following configuration in the job spec. Only
}
}
}
},
# sample bind mount
{
type = "bind"
target = "/path/in/container"
source = "/path/in/host"
readonly = false
bind_options {
propagation = "rshared"
}
},
# sample tmpfs mount
{
type = "tmpfs"
target = "/path/in/container"
readonly = false
tmpfs_options {
size = 100000 # size in bytes
}
}
]
}
Expand Down

0 comments on commit 5c354ad

Please sign in to comment.