From 85c5218b780f61354edb1d66c2ddf43500423bdc Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 6 Nov 2017 12:27:13 -0600 Subject: [PATCH 1/5] Add support for passing device into docker driver --- client/driver/docker.go | 40 +++++++++++++++++++++ client/driver/docker_test.go | 41 ++++++++++++++++++++++ website/source/docs/drivers/docker.html.md | 19 ++++++++++ 3 files changed, 100 insertions(+) diff --git a/client/driver/docker.go b/client/driver/docker.go index 4dc6ae08f..a2243d3dc 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -142,6 +142,12 @@ type DockerMount struct { VolumeOptions []*DockerVolumeOptions `mapstructure:"volume_options"` } +type DockerDevice struct { + HostPath string `mapstructure:"host_path"` + ContainerPath string `mapstructure:"container_path"` + CgroupPermissions string `mapstructure:"cgroup_permissions"` +} + type DockerVolumeOptions struct { NoCopy bool `mapstructure:"no_copy"` Labels []map[string]string `mapstructure:"labels"` @@ -190,6 +196,7 @@ type DockerDriverConfig struct { ForcePull bool `mapstructure:"force_pull"` // Always force pull before running image, useful if your tags are mutable MacAddress string `mapstructure:"mac_address"` // Pin mac address to container SecurityOpt []string `mapstructure:"security_opt"` // Flags to pass directly to security-opt + Devices []DockerDevice `mapstructure:"devices"` // To allow mounting USB or other serial control devices } // Validate validates a docker driver config @@ -197,6 +204,22 @@ func (c *DockerDriverConfig) Validate() error { if c.ImageName == "" { return fmt.Errorf("Docker Driver needs an image name") } + if len(c.Devices) > 0 { + for _, dev := range c.Devices { + if dev.HostPath == "" { + return fmt.Errorf("host path must be set in configuration for devices") + } + + if dev.CgroupPermissions != "" { + for _, c := range dev.CgroupPermissions { + ch := string(c) + if ch != "r" && ch != "w" && ch != "m" { + return fmt.Errorf("invalid cgroup permission string: %q", dev.CgroupPermissions) + } + } + } + } + } return nil } @@ -537,6 +560,9 @@ func (d *DockerDriver) Validate(config map[string]interface{}) error { "security_opt": { Type: fields.TypeArray, }, + "devices": { + Type: fields.TypeArray, + }, }, } @@ -1020,6 +1046,20 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas } } + if len(driverConfig.Devices) > 0 { + var devices []docker.Device + for _, device := range driverConfig.Devices { + if device.HostPath != "" { + dev := docker.Device{ + PathOnHost: device.HostPath, + PathInContainer: device.ContainerPath, + CgroupPermissions: device.CgroupPermissions} + devices = append(devices, dev) + } + } + hostConfig.Devices = devices + } + // Setup mounts for _, m := range driverConfig.Mounts { hm := docker.HostMount{ diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index b93807763..2cd26a125 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -1803,3 +1803,44 @@ func TestDockerDriver_OOMKilled(t *testing.T) { t.Fatalf("timeout") } } + +func TestDockerDriver_Devices_IsInvalidConfig(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + + brokenConfigs := []interface{}{ + map[string]interface{}{ + "host_path": "", + }, + map[string]interface{}{ + "host_path": "/dev/sda1", + "cgroup_permissions": "rxb", + }, + } + + test_cases := []struct { + deviceConfig interface{} + err error + }{ + {[]interface{}{brokenConfigs[0]}, fmt.Errorf("host path must be set in configuration for devices")}, + {[]interface{}{brokenConfigs[1]}, fmt.Errorf("invalid cgroup permission string: \"rxb\"")}, + } + + for _, tc := range test_cases { + task, _, _ := dockerTask(t) + task.Config["devices"] = tc.deviceConfig + + ctx := testDockerDriverContexts(t, task) + driver := NewDockerDriver(ctx.DriverCtx) + copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") + defer ctx.AllocDir.Destroy() + + if _, err := driver.Prestart(ctx.ExecCtx, task); err == nil || err.Error() != tc.err.Error() { + t.Fatalf("error expected in prestart, got %v, expected %v", err, tc.err) + } + } +} diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index 62a1528e5..c1f3eae57 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -280,7 +280,26 @@ The `docker` driver supports the following configuration in the job spec. Only ] } ``` +* `devices` - (Optional) A list of + [devices](https://docs.docker.com/engine/reference/commandline/run/#add-host-device-to-container-device) + to be exposed the container. `host_path` is the only required field. By default, the container will be able to + `read`, `write` and `mknod` these devices. Use the optional `cgroup_permissions` field to restrict permissions. + ```hcl + config { + devices = [ + { + host_path = "/dev/sda1" + container_path = "/dev/xvdc" + cgroup_permissions = "r" + }, + { + host_path = "/dev/sda2" + container_path = "/dev/xvdd" + } + ] + } + ``` ### Container Name Nomad creates a container after pulling an image. Containers are named From b0c03e45ff5e8cc24d1607c55f88ea1a14035daa Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Mon, 6 Nov 2017 19:42:38 -0600 Subject: [PATCH 2/5] Remove unnecessary check since validate method already checks this --- client/driver/docker.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index a2243d3dc..5f4590857 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -1049,13 +1049,11 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas if len(driverConfig.Devices) > 0 { var devices []docker.Device for _, device := range driverConfig.Devices { - if device.HostPath != "" { - dev := docker.Device{ - PathOnHost: device.HostPath, - PathInContainer: device.ContainerPath, - CgroupPermissions: device.CgroupPermissions} - devices = append(devices, dev) - } + dev := docker.Device{ + PathOnHost: device.HostPath, + PathInContainer: device.ContainerPath, + CgroupPermissions: device.CgroupPermissions} + devices = append(devices, dev) } hostConfig.Devices = devices } From 929a781ae4ca4d061671395ca682ad96eb0c538a Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 8 Nov 2017 16:50:09 -0800 Subject: [PATCH 3/5] Add default value for cgroup permissions for device if not set --- client/driver/docker.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/client/driver/docker.go b/client/driver/docker.go index 5f4590857..7539a83ad 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -346,6 +346,16 @@ func NewDockerDriverConfig(task *structs.Task, env *env.TaskEnv) (*DockerDriverC dconf.ImageName = strings.Replace(dconf.ImageName, "https://", "", 1) } + // If devices are configured set default cgroup permissions + if len(dconf.Devices) > 0 { + for i, dev := range dconf.Devices { + if dev.CgroupPermissions == "" { + dev.CgroupPermissions = "rwm" + } + dconf.Devices[i] = dev + } + } + if err := dconf.Validate(); err != nil { return nil, err } From b2eeab1b8cf102fe0fda06a7ec63bf1b5f40995d Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Wed, 8 Nov 2017 17:23:23 -0800 Subject: [PATCH 4/5] Unit test (linux only) that tests mounting a device in the docker driver --- client/driver/docker_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 2cd26a125..d53679ddc 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -1844,3 +1844,34 @@ func TestDockerDriver_Devices_IsInvalidConfig(t *testing.T) { } } } + +func TestDockerDriver_Device_Success(t *testing.T) { + if !tu.IsTravis() { + t.Parallel() + } + if !testutil.DockerIsConnected(t) { + t.Skip("Docker not connected") + } + + if runtime.GOOS != "linux" { + t.Skip("test device mounts only on linux") + } + + config := map[string]interface{}{ + "host_path": "/dev/random", + "container_path": "/dev/myrandom", + } + + task, _, _ := dockerTask(t) + task.Config["devices"] = []interface{}{config} + + ctx := testDockerDriverContexts(t, task) + driver := NewDockerDriver(ctx.DriverCtx) + copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") + defer ctx.AllocDir.Destroy() + + if _, err := driver.Prestart(ctx.ExecCtx, task); err != nil { + t.Fatalf("unexpected error:%v", err) + } + +} From 8e70fd812a173bdfd84b941fd72c8ceca00bb6a5 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Sun, 12 Nov 2017 19:47:55 -0600 Subject: [PATCH 5/5] Make device mounting unit test verify configuration via docker inspect --- client/driver/docker_test.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index d53679ddc..5fa9d166f 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -27,6 +27,7 @@ import ( "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" tu "github.com/hashicorp/nomad/testutil" + "github.com/stretchr/testify/assert" ) func dockerIsRemote(t *testing.T) bool { @@ -1857,21 +1858,30 @@ func TestDockerDriver_Device_Success(t *testing.T) { t.Skip("test device mounts only on linux") } + hostPath := "/dev/random" + containerPath := "/dev/myrandom" + perms := "rwm" + + expectedDevice := docker.Device{hostPath, containerPath, perms} config := map[string]interface{}{ - "host_path": "/dev/random", - "container_path": "/dev/myrandom", + "host_path": hostPath, + "container_path": containerPath, } task, _, _ := dockerTask(t) task.Config["devices"] = []interface{}{config} - ctx := testDockerDriverContexts(t, task) - driver := NewDockerDriver(ctx.DriverCtx) - copyImage(t, ctx.ExecCtx.TaskDir, "busybox.tar") - defer ctx.AllocDir.Destroy() + client, handle, cleanup := dockerSetup(t, task) + defer cleanup() - if _, err := driver.Prestart(ctx.ExecCtx, task); err != nil { - t.Fatalf("unexpected error:%v", err) + waitForExist(t, client, handle.(*DockerHandle)) + + container, err := client.InspectContainer(handle.(*DockerHandle).ContainerID()) + if err != nil { + t.Fatalf("err: %v", err) } + assert.NotEmpty(t, container.HostConfig.Devices, "Expected one device") + assert.Equal(t, expectedDevice, container.HostConfig.Devices[0], "Incorrect device ") + }