Skip to content

Commit

Permalink
docker: introduce a new mount syntax
Browse files Browse the repository at this point in the history 
Introduce a new more-block friendly for specifying mounts:

```hcl
config {
  image = "..."

  mount {
    type = "..."
    target = "target-path"
    volume_options { ... }
  }
}
```

We introduce this as the current `mounts` require a list of mounts, that
`hcl2` doesn't allow to have a map syntax.
  • Loading branch information
Mahmood Ali committed Dec 15, 2020
1 parent 3a7f455 commit c135f2e
Show file tree
Hide file tree
Showing 5 changed files with 213 additions and 47 deletions.
60 changes: 35 additions & 25 deletions drivers/docker/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,32 @@ var (
"disable_log_collection": hclspec.NewAttr("disable_log_collection", "bool", false),
})

// mountBodySpec is the hcl specification for the `mount` block
mountBodySpec = 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),
"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.NewAttr("labels", "list(map(string))", false),
"driver_config": hclspec.NewBlock("driver_config", false, hclspec.NewObject(map[string]*hclspec.Spec{
"name": hclspec.NewAttr("name", "string", false),
"options": hclspec.NewAttr("options", "list(map(string))", false),
})),
})),
})

// taskConfigSpec is the hcl specification for the driver config section of
// a task within a job. It is returned in the TaskConfigSchema RPC
taskConfigSpec = hclspec.NewObject(map[string]*hclspec.Spec{
Expand Down Expand Up @@ -331,30 +357,11 @@ var (
})),
"mac_address": hclspec.NewAttr("mac_address", "string", false),
"memory_hard_limit": hclspec.NewAttr("memory_hard_limit", "number", false),
"mounts": hclspec.NewBlockList("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),
"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.NewAttr("labels", "list(map(string))", false),
"driver_config": hclspec.NewBlock("driver_config", false, hclspec.NewObject(map[string]*hclspec.Spec{
"name": hclspec.NewAttr("name", "string", false),
"options": hclspec.NewAttr("options", "list(map(string))", false),
})),
})),
})),
// mount and mounts are effectively aliases, but `mounts` is meant for pre-1.0
// assignment syntax `mounts = [{type="..." ..."}]` while
// `mount` is 1.0 repeated block syntax `mount { type = "..." }`
"mount": hclspec.NewBlockList("mount", mountBodySpec),
"mounts": hclspec.NewBlockList("mounts", mountBodySpec),
"network_aliases": hclspec.NewAttr("network_aliases", "list(string)", false),
"network_mode": hclspec.NewAttr("network_mode", "string", false),
"runtime": hclspec.NewAttr("runtime", "string", false),
Expand Down Expand Up @@ -426,7 +433,7 @@ type TaskConfig struct {
Logging DockerLogging `codec:"logging"`
MacAddress string `codec:"mac_address"`
MemoryHardLimit int64 `codec:"memory_hard_limit"`
Mounts []DockerMount `codec:"mounts"`
Mounts []DockerMount `codec:"mount"`
NetworkAliases []string `codec:"network_aliases"`
NetworkMode string `codec:"network_mode"`
Runtime string `codec:"runtime"`
Expand All @@ -448,6 +455,9 @@ type TaskConfig struct {
Volumes []string `codec:"volumes"`
VolumeDriver string `codec:"volume_driver"`
WorkDir string `codec:"work_dir"`

// MountsList supports the pre-1.0 mounts array syntax
MountsList []DockerMount `codec:"mounts"`
}

type DockerAuth struct {
Expand Down
47 changes: 47 additions & 0 deletions drivers/docker/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func TestConfig_ParseHCL(t *testing.T) {
Image: "redis:3.2",
Devices: []DockerDevice{},
Mounts: []DockerMount{},
MountsList: []DockerMount{},
CPUCFSPeriod: 100000,
ImagePullTimeout: "5m",
},
Expand Down Expand Up @@ -56,6 +57,7 @@ func TestConfig_ParseJSON(t *testing.T) {
expected: TaskConfig{
Image: "bash:3",
Mounts: []DockerMount{},
MountsList: []DockerMount{},
Devices: []DockerDevice{},
CPUCFSPeriod: 100000,
ImagePullTimeout: "5m",
Expand All @@ -67,6 +69,7 @@ func TestConfig_ParseJSON(t *testing.T) {
expected: TaskConfig{
Image: "bash:3",
Mounts: []DockerMount{},
MountsList: []DockerMount{},
Devices: []DockerDevice{},
CPUCFSPeriod: 100000,
ImagePullTimeout: "5m",
Expand All @@ -78,6 +81,7 @@ func TestConfig_ParseJSON(t *testing.T) {
expected: TaskConfig{
Image: "bash:3",
Mounts: []DockerMount{},
MountsList: []DockerMount{},
Devices: []DockerDevice{},
CPUCFSPeriod: 100000,
ImagePullTimeout: "5m",
Expand All @@ -89,6 +93,7 @@ func TestConfig_ParseJSON(t *testing.T) {
expected: TaskConfig{
Image: "bash:3",
Mounts: []DockerMount{},
MountsList: []DockerMount{},
Devices: []DockerDevice{},
CPUCFSPeriod: 100000,
ImagePullTimeout: "5m",
Expand Down Expand Up @@ -229,6 +234,27 @@ config {
}
mac_address = "02:42:ac:11:00:02"
memory_hard_limit = 512
mount {
type = "bind"
target ="/mount-bind-target"
source = "/bind-source-mount"
readonly = true
bind_options {
propagation = "rshared"
}
}
mount {
type = "tmpfs"
target ="/mount-tmpfs-target"
readonly = true
tmpfs_options {
size = 30000
mode = 0777
}
}
mounts = [
{
type = "bind"
Expand Down Expand Up @@ -361,6 +387,27 @@ config {
MacAddress: "02:42:ac:11:00:02",
MemoryHardLimit: 512,
Mounts: []DockerMount{
{
Type: "bind",
Target: "/mount-bind-target",
Source: "/bind-source-mount",
ReadOnly: true,
BindOptions: DockerBindOptions{
Propagation: "rshared",
},
},
{
Type: "tmpfs",
Target: "/mount-tmpfs-target",
Source: "",
ReadOnly: true,
TmpfsOptions: DockerTmpfsOptions{
SizeBytes: 30000,
Mode: 511,
},
},
},
MountsList: []DockerMount{
{
Type: "bind",
Target: "/bind-target",
Expand Down
59 changes: 37 additions & 22 deletions drivers/docker/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -947,32 +947,18 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T

// Setup mounts
for _, m := range driverConfig.Mounts {
hm, err := m.toDockerHostMount()
hm, err := d.toDockerMount(&m, task)
if err != nil {
return c, err
}

switch hm.Type {
case "bind":
hm.Source = expandPath(task.TaskDir().Dir, hm.Source)

// paths inside alloc dir are always allowed as they mount within
// a container, and treated as relative to task dir
if !d.config.Volumes.Enabled && !isParentPath(task.AllocDir, hm.Source) {
return c, fmt.Errorf(
"volumes are not enabled; cannot mount host path: %q %q",
hm.Source, task.AllocDir)
}
case "tmpfs":
// no source, so no sandbox check required
default: // "volume", but also any new thing that comes along
if !d.config.Volumes.Enabled {
return c, fmt.Errorf(
"volumes are not enabled; cannot mount volume: %q", hm.Source)
}
hostConfig.Mounts = append(hostConfig.Mounts, *hm)
}
for _, m := range driverConfig.MountsList {
hm, err := d.toDockerMount(&m, task)
if err != nil {
return c, err
}

hostConfig.Mounts = append(hostConfig.Mounts, hm)
hostConfig.Mounts = append(hostConfig.Mounts, *hm)
}

// Setup DNS
Expand Down Expand Up @@ -1177,6 +1163,35 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T
}, nil
}

func (d *Driver) toDockerMount(m *DockerMount, task *drivers.TaskConfig) (*docker.HostMount, error) {
hm, err := m.toDockerHostMount()
if err != nil {
return nil, err
}

switch hm.Type {
case "bind":
hm.Source = expandPath(task.TaskDir().Dir, hm.Source)

// paths inside alloc dir are always allowed as they mount within
// a container, and treated as relative to task dir
if !d.config.Volumes.Enabled && !isParentPath(task.AllocDir, hm.Source) {
return nil, fmt.Errorf(
"volumes are not enabled; cannot mount host path: %q %q",
hm.Source, task.AllocDir)
}
case "tmpfs":
// no source, so no sandbox check required
default: // "volume", but also any new thing that comes along
if !d.config.Volumes.Enabled {
return nil, fmt.Errorf(
"volumes are not enabled; cannot mount volume: %q", hm.Source)
}
}

return &hm, nil
}

// detectIP of Docker container. Returns the first IP found as well as true if
// the IP should be advertised (bridge network IPs return false). Returns an
// empty string and false if no IP could be found.
Expand Down
79 changes: 79 additions & 0 deletions drivers/docker/driver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"reflect"
"runtime"
"runtime/debug"
"sort"
"strings"
"syscall"
"testing"
Expand Down Expand Up @@ -1150,6 +1151,84 @@ func TestDockerDriver_CreateContainerConfig_Logging(t *testing.T) {
}
}

func TestDockerDriver_CreateContainerConfig_Mounts(t *testing.T) {
t.Parallel()

task, cfg, ports := dockerTask(t)
defer freeport.Return(ports)

cfg.Mounts = []DockerMount{
DockerMount{
Type: "bind",
Target: "/map-bind-target",
Source: "/map-source",
},
DockerMount{
Type: "tmpfs",
Target: "/map-tmpfs-target",
},
}
cfg.MountsList = []DockerMount{
{
Type: "bind",
Target: "/list-bind-target",
Source: "/list-source",
},
{
Type: "tmpfs",
Target: "/list-tmpfs-target",
},
}

expectedSrcPrefix := "/"
if runtime.GOOS == "windows" {
expectedSrcPrefix = "redis-demo\\"
}
expected := []docker.HostMount{
// from mount map
{
Type: "bind",
Target: "/map-bind-target",
Source: expectedSrcPrefix + "map-source",
BindOptions: &docker.BindOptions{},
},
{
Type: "tmpfs",
Target: "/map-tmpfs-target",
TempfsOptions: &docker.TempfsOptions{},
},
// from mount list
{
Type: "bind",
Target: "/list-bind-target",
Source: expectedSrcPrefix + "list-source",
BindOptions: &docker.BindOptions{},
},
{
Type: "tmpfs",
Target: "/list-tmpfs-target",
TempfsOptions: &docker.TempfsOptions{},
},
}

require.NoError(t, task.EncodeConcreteDriverConfig(cfg))

dh := dockerDriverHarness(t, nil)
driver := dh.Impl().(*Driver)
driver.config.Volumes.Enabled = true

cc, err := driver.createContainerConfig(task, cfg, "org/repo:0.1")
require.NoError(t, err)

found := cc.HostConfig.Mounts
sort.Slice(found, func(i, j int) bool { return strings.Compare(found[i].Target, found[j].Target) < 0 })
sort.Slice(expected, func(i, j int) bool {
return strings.Compare(expected[i].Target, expected[j].Target) < 0
})

require.Equal(t, expected, found)
}

func TestDockerDriver_CreateContainerConfigWithRuntimes(t *testing.T) {
if !tu.IsCI() {
t.Parallel()
Expand Down
Loading

0 comments on commit c135f2e

Please sign in to comment.