Skip to content

Commit

Permalink
docker: default device.container_path to host_path (#16811)
Browse files Browse the repository at this point in the history
* docker: default device.container_path to host_path

Matches docker cli behavior.

Fixes #16754
  • Loading branch information
schmichael authored Apr 6, 2023
1 parent 9ed5aa1 commit db2325b
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 23 deletions.
3 changes: 3 additions & 0 deletions .changelog/16811.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
driver/docker: Default `devices.container_path` to `devices.host_path` like Docker's CLI
```
5 changes: 5 additions & 0 deletions drivers/docker/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,11 @@ func (d DockerDevice) toDockerDevice() (docker.Device, error) {
return dd, fmt.Errorf("host path must be set in configuration for devices")
}

// Docker's CLI defaults to HostPath in this case. See #16754
if dd.PathInContainer == "" {
dd.PathInContainer = d.HostPath
}

if dd.CgroupPermissions == "" {
dd.CgroupPermissions = "rwm"
}
Expand Down
6 changes: 6 additions & 0 deletions drivers/docker/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ config {
devices = [
{"host_path"="/dev/null", "container_path"="/tmp/container-null", cgroup_permissions="rwm"},
{"host_path"="/dev/random", "container_path"="/tmp/container-random"},
{"host_path"="/dev/bus/usb"},
]
dns_search_domains = ["sub.example.com", "sub2.example.com"]
dns_options = ["debug", "attempts:10"]
Expand Down Expand Up @@ -372,6 +373,11 @@ config {
ContainerPath: "/tmp/container-random",
CgroupPermissions: "",
},
{
HostPath: "/dev/bus/usb",
ContainerPath: "",
CgroupPermissions: "",
},
},
DNSSearchDomains: []string{"sub.example.com", "sub2.example.com"},
DNSOptions: []string{"debug", "attempts:10"},
Expand Down
4 changes: 3 additions & 1 deletion drivers/docker/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1035,14 +1035,16 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T
hostConfig.ShmSize = driverConfig.ShmSize
}

// Setup devices
// Setup devices from Docker-specific config
for _, device := range driverConfig.Devices {
dd, err := device.toDockerDevice()
if err != nil {
return c, err
}
hostConfig.Devices = append(hostConfig.Devices, dd)
}

// Setup devices from Nomad device plugins
for _, device := range task.Devices {
hostConfig.Devices = append(hostConfig.Devices, docker.Device{
PathOnHost: device.HostPath,
Expand Down
66 changes: 44 additions & 22 deletions drivers/docker/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2471,34 +2471,56 @@ 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{
PathOnHost: hostPath,
PathInContainer: containerPath,
CgroupPermissions: perms,
}
config := DockerDevice{
HostPath: hostPath,
ContainerPath: containerPath,
cases := []struct {
Name string
Input DockerDevice
Expected docker.Device
}{
{
Name: "AllSet",
Input: DockerDevice{
HostPath: "/dev/random",
ContainerPath: "/dev/hostrandom",
CgroupPermissions: "rwm",
},
Expected: docker.Device{
PathOnHost: "/dev/random",
PathInContainer: "/dev/hostrandom",
CgroupPermissions: "rwm",
},
},
{
Name: "OnlyHost",
Input: DockerDevice{
HostPath: "/dev/random",
},
Expected: docker.Device{
PathOnHost: "/dev/random",
PathInContainer: "/dev/random",
CgroupPermissions: "rwm",
},
},
}

task, cfg, _ := dockerTask(t)
for i := range cases {
tc := cases[i]
t.Run(tc.Name, func(t *testing.T) {
task, cfg, _ := dockerTask(t)

cfg.Devices = []DockerDevice{config}
require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
cfg.Devices = []DockerDevice{tc.Input}
require.NoError(t, task.EncodeConcreteDriverConfig(cfg))

client, driver, handle, cleanup := dockerSetup(t, task, nil)
defer cleanup()
require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second))
client, driver, handle, cleanup := dockerSetup(t, task, nil)
defer cleanup()
require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second))

container, err := client.InspectContainer(handle.containerID)
require.NoError(t, err)
container, err := client.InspectContainer(handle.containerID)
require.NoError(t, err)

require.NotEmpty(t, container.HostConfig.Devices, "Expected one device")
require.Equal(t, expectedDevice, container.HostConfig.Devices[0], "Incorrect device ")
require.NotEmpty(t, container.HostConfig.Devices, "Expected one device")
require.Equal(t, tc.Expected, container.HostConfig.Devices[0], "Incorrect device ")
})
}
}

func TestDockerDriver_Entrypoint(t *testing.T) {
Expand Down

0 comments on commit db2325b

Please sign in to comment.