Merge pull request #4933 from hashicorp/f-mount-device

Mount Devices in container based drivers

drivers/docker/config.go

@@ -356,6 +356,28 @@ type DockerDevice struct {
 	CgroupPermissions string `codec:"cgroup_permissions"`
 }
 
+func (d DockerDevice) toDockerDevice() (docker.Device, error) {
+	dd := docker.Device{
+		PathOnHost:        d.HostPath,
+		PathInContainer:   d.ContainerPath,
+		CgroupPermissions: d.CgroupPermissions,
+	}
+
+	if d.HostPath == "" {
+		return dd, fmt.Errorf("host path must be set in configuration for devices")
+	}
+
+	if dd.CgroupPermissions == "" {
+		dd.CgroupPermissions = "rwm"
+	}
+
+	if !validateCgroupPermission(dd.CgroupPermissions) {
+		return dd, fmt.Errorf("invalid cgroup permission string: %q", dd.CgroupPermissions)
+	}
+
+	return dd, nil
+}
+
 type DockerLogging struct {
 	Type   string            `codec:"type"`
 	Config map[string]string `codec:"config"`

drivers/docker/driver.go

@@ -718,29 +718,20 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T
 		}
 	}
 
-	if len(driverConfig.Devices) > 0 {
-		var devices []docker.Device
-		for _, device := range driverConfig.Devices {
-			if device.HostPath == "" {
-				return c, fmt.Errorf("host path must be set in configuration for devices")
-			}
-			if device.CgroupPermissions != "" {
-				for _, char := range device.CgroupPermissions {
-					ch := string(char)
-					if ch != "r" && ch != "w" && ch != "m" {
-						return c, fmt.Errorf("invalid cgroup permission string: %q", device.CgroupPermissions)
-					}
-				}
-			} else {
-				device.CgroupPermissions = "rwm"
-			}
-			dev := docker.Device{
-				PathOnHost:        device.HostPath,
-				PathInContainer:   device.ContainerPath,
-				CgroupPermissions: device.CgroupPermissions}
-			devices = append(devices, dev)
+	// Setup devices
+	for _, device := range driverConfig.Devices {
+		dd, err := device.toDockerDevice()
+		if err != nil {
+			return c, err
 		}
-		hostConfig.Devices = devices
+		hostConfig.Devices = append(hostConfig.Devices, dd)
+	}
+	for _, device := range task.Devices {
+		hostConfig.Devices = append(hostConfig.Devices, docker.Device{
+			PathOnHost:        device.HostPath,
+			PathInContainer:   device.TaskPath,
+			CgroupPermissions: device.Permissions,
+		})
 	}
 
 	// Setup mounts
@@ -763,6 +754,14 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T
 
 		hostConfig.Mounts = append(hostConfig.Mounts, hm)
 	}
+	for _, m := range task.Mounts {
+		hostConfig.Mounts = append(hostConfig.Mounts, docker.HostMount{
+			Type:     "bind",
+			Target:   m.TaskPath,
+			Source:   m.HostPath,
+			ReadOnly: m.Readonly,
+		})
+	}
 
 	// set DNS search domains and extra hosts
 	hostConfig.DNSSearch = driverConfig.DNSSearchDomains

drivers/docker/driver_test.go

@@ -11,6 +11,7 @@ import (
 	"reflect"
 	"runtime"
 	"runtime/debug"
+	"sort"
 	"strconv"
 	"strings"
 	"testing"
@@ -1908,6 +1909,95 @@ func TestDockerDriver_MountsSerialization(t *testing.T) {
 
 }
 
+// TestDockerDriver_CreateContainerConfig_MountsCombined asserts that
+// devices and mounts set by device managers/plugins are honored
+// and present in docker.CreateContainerOptions, and that it is appended
+// to any devices/mounts a user sets in the task config.
+func TestDockerDriver_CreateContainerConfig_MountsCombined(t *testing.T) {
+	t.Parallel()
+
+	task, cfg, _ := dockerTask(t)
+
+	task.Devices = []*drivers.DeviceConfig{
+		{
+			HostPath:    "/dev/fuse",
+			TaskPath:    "/container/dev/task-fuse",
+			Permissions: "rw",
+		},
+	}
+	task.Mounts = []*drivers.MountConfig{
+		{
+			HostPath: "/tmp/task-mount",
+			TaskPath: "/container/tmp/task-mount",
+			Readonly: true,
+		},
+	}
+
+	cfg.Devices = []DockerDevice{
+		{
+			HostPath:          "/dev/stdout",
+			ContainerPath:     "/container/dev/cfg-stdout",
+			CgroupPermissions: "rwm",
+		},
+	}
+	cfg.Mounts = []DockerMount{
+		{
+			Type:     "bind",
+			Source:   "/tmp/cfg-mount",
+			Target:   "/container/tmp/cfg-mount",
+			ReadOnly: false,
+		},
+	}
+
+	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
+
+	dh := dockerDriverHarness(t, nil)
+	driver := dh.Impl().(*Driver)
+
+	c, err := driver.createContainerConfig(task, cfg, "org/repo:0.1")
+	require.NoError(t, err)
+
+	expectedMounts := []docker.HostMount{
+		{
+			Type:        "bind",
+			Source:      "/tmp/cfg-mount",
+			Target:      "/container/tmp/cfg-mount",
+			ReadOnly:    false,
+			BindOptions: &docker.BindOptions{},
+		},
+		{
+			Type:     "bind",
+			Source:   "/tmp/task-mount",
+			Target:   "/container/tmp/task-mount",
+			ReadOnly: true,
+		},
+	}
+	foundMounts := c.HostConfig.Mounts
+	sort.Slice(foundMounts, func(i, j int) bool {
+		return foundMounts[i].Target < foundMounts[j].Target
+	})
+	require.EqualValues(t, expectedMounts, foundMounts)
+
+	expectedDevices := []docker.Device{
+		{
+			PathOnHost:        "/dev/stdout",
+			PathInContainer:   "/container/dev/cfg-stdout",
+			CgroupPermissions: "rwm",
+		},
+		{
+			PathOnHost:        "/dev/fuse",
+			PathInContainer:   "/container/dev/task-fuse",
+			CgroupPermissions: "rw",
+		},
+	}
+
+	foundDevices := c.HostConfig.Devices
+	sort.Slice(foundDevices, func(i, j int) bool {
+		return foundDevices[i].PathInContainer < foundDevices[j].PathInContainer
+	})
+	require.EqualValues(t, expectedDevices, foundDevices)
+}
+
 // TestDockerDriver_Cleanup ensures Cleanup removes only downloaded images.
 func TestDockerDriver_Cleanup(t *testing.T) {
 	if !testutil.DockerIsConnected(t) {

drivers/docker/utils.go

@@ -188,3 +188,15 @@ func authIsEmpty(auth *docker.AuthConfiguration) bool {
 		auth.Email == "" &&
 		auth.ServerAddress == ""
 }
+
+func validateCgroupPermission(s string) bool {
+	for _, c := range s {
+		switch c {
+		case 'r', 'w', 'm':
+		default:
+			return false
+		}
+	}
+
+	return true
+}

drivers/docker/utils_test.go

@@ -0,0 +1,37 @@
+package docker
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateCgroupPermission(t *testing.T) {
+	positiveCases := []string{
+		"r",
+		"rw",
+		"rwm",
+		"mr",
+		"mrw",
+		"",
+	}
+
+	for _, c := range positiveCases {
+		t.Run("positive case: "+c, func(t *testing.T) {
+			require.True(t, validateCgroupPermission(c))
+		})
+	}
+
+	negativeCases := []string{
+		"q",
+		"asdf",
+		"rq",
+	}
+
+	for _, c := range negativeCases {
+		t.Run("negative case: "+c, func(t *testing.T) {
+			require.False(t, validateCgroupPermission(c))
+		})
+	}
+
+}

drivers/lxc/lxc.go

@@ -12,7 +12,8 @@ import (
 	"time"
 
 	"github.com/hashicorp/nomad/plugins/drivers"
-	lxc "gopkg.in/lxc/go-lxc.v2"
+	ldevices "github.com/opencontainers/runc/libcontainer/devices"
+	"gopkg.in/lxc/go-lxc.v2"
 )
 
 var (
@@ -91,13 +92,43 @@ func networkTypeConfigKey() string {
 }
 
 func (d *Driver) mountVolumes(c *lxc.Container, cfg *drivers.TaskConfig, taskConfig TaskConfig) error {
+	mounts, err := d.mountEntries(cfg, taskConfig)
+	if err != nil {
+		return err
+	}
+
+	devCgroupAllows, err := d.devicesCgroupEntries(cfg)
+	if err != nil {
+		return err
+	}
+
+	for _, mnt := range mounts {
+		if err := c.SetConfigItem("lxc.mount.entry", mnt); err != nil {
+			return fmt.Errorf("error setting bind mount %q error: %v", mnt, err)
+		}
+	}
+
+	for _, cgroupDev := range devCgroupAllows {
+		if err := c.SetConfigItem("lxc.cgroup.devices.allow", cgroupDev); err != nil {
+			return fmt.Errorf("error setting cgroup permission %q error: %v", cgroupDev, err)
+		}
+	}
+
+	return nil
+}
+
+// mountEntries compute the mount entries to be set on the container
+func (d *Driver) mountEntries(cfg *drivers.TaskConfig, taskConfig TaskConfig) ([]string, error) {
 	// Bind mount the shared alloc dir and task local dir in the container
 	mounts := []string{
 		fmt.Sprintf("%s local none rw,bind,create=dir", cfg.TaskDir().LocalDir),
 		fmt.Sprintf("%s alloc none rw,bind,create=dir", cfg.TaskDir().SharedAllocDir),
 		fmt.Sprintf("%s secrets none rw,bind,create=dir", cfg.TaskDir().SecretsDir),
 	}
 
+	mounts = append(mounts, d.formatTaskMounts(cfg.Mounts)...)
+	mounts = append(mounts, d.formatTaskDevices(cfg.Devices)...)
+
 	volumesEnabled := d.config.AllowVolumes
 
 	for _, volDesc := range taskConfig.Volumes {
@@ -106,23 +137,75 @@ func (d *Driver) mountVolumes(c *lxc.Container, cfg *drivers.TaskConfig, taskCon
 
 		if filepath.IsAbs(paths[0]) {
 			if !volumesEnabled {
-				return fmt.Errorf("absolute bind-mount volume in config but volumes are disabled")
+				return nil, fmt.Errorf("absolute bind-mount volume in config but volumes are disabled")
 			}
 		} else {
 			// Relative source paths are treated as relative to alloc dir
 			paths[0] = filepath.Join(cfg.TaskDir().Dir, paths[0])
 		}
 
-		mounts = append(mounts, fmt.Sprintf("%s %s none rw,bind,create=dir", paths[0], paths[1]))
+		// LXC assumes paths are relative with respect to rootfs
+		target := strings.TrimLeft(paths[1], "/")
+		mounts = append(mounts, fmt.Sprintf("%s %s none rw,bind,create=dir", paths[0], target))
 	}
 
-	for _, mnt := range mounts {
-		if err := c.SetConfigItem("lxc.mount.entry", mnt); err != nil {
-			return fmt.Errorf("error setting bind mount %q error: %v", mnt, err)
+	return mounts, nil
+
+}
+
+func (d *Driver) devicesCgroupEntries(cfg *drivers.TaskConfig) ([]string, error) {
+	entries := make([]string, len(cfg.Devices))
+
+	for i, d := range cfg.Devices {
+		hd, err := ldevices.DeviceFromPath(d.HostPath, d.Permissions)
+		if err != nil {
+			return nil, err
 		}
+
+		entries[i] = hd.CgroupString()
 	}
 
-	return nil
+	return entries, nil
+}
+
+func (d *Driver) formatTaskMounts(mounts []*drivers.MountConfig) []string {
+	result := make([]string, len(mounts))
+
+	for i, m := range mounts {
+		result[i] = d.formatMount(m.HostPath, m.TaskPath, m.Readonly)
+	}
+
+	return result
+}
+
+func (d *Driver) formatTaskDevices(devices []*drivers.DeviceConfig) []string {
+	result := make([]string, len(devices))
+
+	for i, m := range devices {
+		result[i] = d.formatMount(m.HostPath, m.TaskPath,
+			!strings.Contains(m.Permissions, "w"))
+	}
+
+	return result
+}
+
+func (d *Driver) formatMount(hostPath, taskPath string, readOnly bool) string {
+	typ := "dir"
+	s, err := os.Stat(hostPath)
+	if err != nil {
+		d.logger.Warn("failed to find mount host path type, defaulting to dir type", "path", hostPath, "error", err)
+	} else if !s.IsDir() {
+		typ = "file"
+	}
+
+	perm := "rw"
+	if readOnly {
+		perm = "ro"
+	}
+
+	// LXC assumes paths are relative with respect to rootfs
+	target := strings.TrimLeft(taskPath, "/")
+	return fmt.Sprintf("%s %s none %s,bind,create=%s", hostPath, target, perm, typ)
 }
 
 func (d *Driver) setResourceLimits(c *lxc.Container, cfg *drivers.TaskConfig) error {