Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for passing device into docker driver #3512

Merged
merged 5 commits into from
Nov 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions client/driver/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -190,13 +196,30 @@ 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
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this validation logic after reading https://docs.docker.com/engine/reference/commandline/run/#add-host-device-to-container-device. Also, note this sentence at the end of that documentation: Note: --device cannot be safely used with ephemeral devices. Block devices that may be removed should not be added to untrusted containers with --device. The documentation you linked above for --device-cgroup-rule for dynamic devices, which looks like something else. I could be missing something though.

We can create another follow up ticket to add configuration for --device-cgroup-rule without needing --device if this is something people need.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks for clarifying!

}
}
}
}
}
return nil
}

Expand Down Expand Up @@ -323,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
}
Expand Down Expand Up @@ -537,6 +570,9 @@ func (d *DockerDriver) Validate(config map[string]interface{}) error {
"security_opt": {
Type: fields.TypeArray,
},
"devices": {
Type: fields.TypeArray,
},
},
}

Expand Down Expand Up @@ -1020,6 +1056,18 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas
}
}

if len(driverConfig.Devices) > 0 {
var devices []docker.Device
for _, device := range driverConfig.Devices {
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{
Expand Down
82 changes: 82 additions & 0 deletions client/driver/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1803,3 +1804,84 @@ 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)
}
}
}

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")
}

hostPath := "/dev/random"
containerPath := "/dev/myrandom"
perms := "rwm"

expectedDevice := docker.Device{hostPath, containerPath, perms}
config := map[string]interface{}{
"host_path": hostPath,
"container_path": containerPath,
}

task, _, _ := dockerTask(t)
task.Config["devices"] = []interface{}{config}

client, handle, cleanup := dockerSetup(t, task)
defer cleanup()

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 ")

}
19 changes: 19 additions & 0 deletions website/source/docs/drivers/docker.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down