From a8b379f96254e6ba8fa76869f7d57ee8421538fe Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Thu, 6 Apr 2023 14:44:33 -0700 Subject: [PATCH] docker: default device.container_path to host_path (#16811) * docker: default device.container_path to host_path Matches docker cli behavior. Fixes #16754 --- .changelog/16811.txt | 3 ++ drivers/docker/config.go | 5 +++ drivers/docker/config_test.go | 6 ++++ drivers/docker/driver.go | 4 ++- drivers/docker/driver_test.go | 66 +++++++++++++++++++++++------------ 5 files changed, 61 insertions(+), 23 deletions(-) create mode 100644 .changelog/16811.txt diff --git a/.changelog/16811.txt b/.changelog/16811.txt new file mode 100644 index 000000000..6d0e10d08 --- /dev/null +++ b/.changelog/16811.txt @@ -0,0 +1,3 @@ +```release-note:improvement +driver/docker: Default `devices.container_path` to `devices.host_path` like Docker's CLI +``` diff --git a/drivers/docker/config.go b/drivers/docker/config.go index 0aefc96b1..956bb196a 100644 --- a/drivers/docker/config.go +++ b/drivers/docker/config.go @@ -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" } diff --git a/drivers/docker/config_test.go b/drivers/docker/config_test.go index 254973e20..95cf29f2f 100644 --- a/drivers/docker/config_test.go +++ b/drivers/docker/config_test.go @@ -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"] @@ -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"}, diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index d20db05e1..9de2b8f27 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -1035,7 +1035,7 @@ 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 { @@ -1043,6 +1043,8 @@ func (d *Driver) createContainerConfig(task *drivers.TaskConfig, driverConfig *T } 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, diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index a0016b115..c650a35bf 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -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) {