Merge pull request #4847 from cpuguy83/devices_by_dir

Support adding devices by dir

cmd/ctr/commands/commands.go

@@ -153,7 +153,7 @@ var (
 		},
 		cli.StringSliceFlag{
 			Name:  "device",
-			Usage: "add a device to a container",
+			Usage: "file path to a device to add to the container; or a path to a directory tree of devices to add to the container",
 		},
 		cli.BoolFlag{
 			Name:  "seccomp",

cmd/ctr/commands/run/run_unix.go

@@ -264,7 +264,7 @@ func NewContainer(ctx gocontext.Context, client *containerd.Client, context *cli
 			opts = append(opts, oci.WithMemoryLimit(limit))
 		}
 		for _, dev := range context.StringSlice("device") {
-			opts = append(opts, oci.WithLinuxDevice(dev, "rwm"))
+			opts = append(opts, oci.WithDevices(dev, "", "rwm"))
 		}
 	}
 

oci/spec_opts.go

@@ -1178,7 +1178,7 @@ func WithLinuxDevice(path, permissions string) SpecOpts {
 		setLinux(s)
 		setResources(s)
 
-		dev, err := deviceFromPath(path, permissions)
+		dev, err := deviceFromPath(path)
 		if err != nil {
 			return err
 		}

oci/spec_opts_linux.go

@@ -35,7 +35,7 @@ import (
 func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
 	setLinux(s)
 
-	devs, err := getDevices("/dev")
+	devs, err := getDevices("/dev", "")
 	if err != nil {
 		return err
 	}
@@ -45,7 +45,47 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 
 var errNotADevice = errors.New("not a device node")
 
-func getDevices(path string) ([]specs.LinuxDevice, error) {
+// WithDevices recursively adds devices from the passed in path and associated cgroup rules for that device.
+// If devicePath is a dir it traverses the dir to add all devices in that dir.
+// If devicePath is not a dir, it attempts to add the single device.
+// If containerPath is not set then the device path is used for the container path.
+func WithDevices(devicePath, containerPath, permissions string) SpecOpts {
+	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
+		devs, err := getDevices(devicePath, containerPath)
+		if err != nil {
+			return err
+		}
+		for _, dev := range devs {
+			s.Linux.Devices = append(s.Linux.Devices, dev)
+			s.Linux.Resources.Devices = append(s.Linux.Resources.Devices, specs.LinuxDeviceCgroup{
+				Allow:  true,
+				Type:   dev.Type,
+				Major:  &dev.Major,
+				Minor:  &dev.Minor,
+				Access: permissions,
+			})
+		}
+		return nil
+	}
+}
+
+func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) {
+	stat, err := os.Stat(path)
+	if err != nil {
+		return nil, errors.Wrap(err, "error stating device path")
+	}
+
+	if !stat.IsDir() {
+		dev, err := deviceFromPath(path)
+		if err != nil {
+			return nil, err
+		}
+		if containerPath != "" {
+			dev.Path = containerPath
+		}
+		return []specs.LinuxDevice{*dev}, nil
+	}
+
 	files, err := ioutil.ReadDir(path)
 	if err != nil {
 		return nil, err
@@ -60,7 +100,11 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 			case "pts", "shm", "fd", "mqueue", ".lxc", ".lxd-mounts", ".udev":
 				continue
 			default:
-				sub, err := getDevices(filepath.Join(path, f.Name()))
+				var cp string
+				if containerPath != "" {
+					cp = filepath.Join(containerPath, filepath.Base(f.Name()))
+				}
+				sub, err := getDevices(filepath.Join(path, f.Name()), cp)
 				if err != nil {
 					return nil, err
 				}
@@ -71,7 +115,7 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 		case f.Name() == "console":
 			continue
 		}
-		device, err := deviceFromPath(filepath.Join(path, f.Name()), "rwm")
+		device, err := deviceFromPath(filepath.Join(path, f.Name()))
 		if err != nil {
 			if err == errNotADevice {
 				continue
@@ -81,12 +125,15 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 			}
 			return nil, err
 		}
+		if containerPath != "" {
+			device.Path = filepath.Join(containerPath, filepath.Base(f.Name()))
+		}
 		out = append(out, *device)
 	}
 	return out, nil
 }
 
-func deviceFromPath(path, permissions string) (*specs.LinuxDevice, error) {
+func deviceFromPath(path string) (*specs.LinuxDevice, error) {
 	var stat unix.Stat_t
 	if err := unix.Lstat(path, &stat); err != nil {
 		return nil, err

oci/spec_opts_linux_test.go

@@ -18,9 +18,14 @@ package oci
 
 import (
 	"context"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/containerd/containerd/pkg/testutil"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"golang.org/x/sys/unix"
 )
 
 func TestAddCaps(t *testing.T) {
@@ -106,3 +111,113 @@ func TestDropCaps(t *testing.T) {
 		}
 	}
 }
+
+func TestGetDevices(t *testing.T) {
+	testutil.RequiresRoot(t)
+
+	dir, err := ioutil.TempDir("/dev", t.Name())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	zero := filepath.Join(dir, "zero")
+	if err := ioutil.WriteFile(zero, nil, 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := unix.Mount("/dev/zero", zero, "", unix.MS_BIND, ""); err != nil {
+		t.Fatal(err)
+	}
+	defer unix.Unmount(filepath.Join(dir, "zero"), unix.MNT_DETACH)
+
+	t.Run("single device", func(t *testing.T) {
+		t.Run("no container path", func(t *testing.T) {
+			devices, err := getDevices(dir, "")
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if len(devices) != 1 {
+				t.Fatalf("expected one device %v", devices)
+			}
+			if devices[0].Path != zero {
+				t.Fatalf("got unexpected device path %s", devices[0].Path)
+			}
+		})
+		t.Run("with container path", func(t *testing.T) {
+			newPath := "/dev/testNew"
+			devices, err := getDevices(dir, newPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if len(devices) != 1 {
+				t.Fatalf("expected one device %v", devices)
+			}
+			if devices[0].Path != filepath.Join(newPath, "zero") {
+				t.Fatalf("got unexpected device path %s", devices[0].Path)
+			}
+		})
+	})
+	t.Run("With symlink in dir", func(t *testing.T) {
+		if err := os.Symlink("/dev/zero", filepath.Join(dir, "zerosym")); err != nil {
+			t.Fatal(err)
+		}
+		devices, err := getDevices(dir, "")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(devices) != 1 {
+			t.Fatalf("expected one device %v", devices)
+		}
+		if devices[0].Path != filepath.Join(dir, "zero") {
+			t.Fatalf("got unexpected device path, expected %q, got %q", filepath.Join(dir, "zero"), devices[0].Path)
+		}
+	})
+	t.Run("No devices", func(t *testing.T) {
+		dir := dir + "2"
+		if err := os.MkdirAll(dir, 0755); err != nil {
+			t.Fatal(err)
+		}
+		defer os.RemoveAll(dir)
+
+		t.Run("empty dir", func(T *testing.T) {
+			devices, err := getDevices(dir, "")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(devices) != 0 {
+				t.Fatalf("expected no devices, got %+v", devices)
+			}
+		})
+		t.Run("symlink to device in dir", func(t *testing.T) {
+			if err := os.Symlink("/dev/zero", filepath.Join(dir, "zerosym")); err != nil {
+				t.Fatal(err)
+			}
+			defer os.Remove(filepath.Join(dir, "zerosym"))
+
+			devices, err := getDevices(dir, "")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(devices) != 0 {
+				t.Fatalf("expected no devices, got %+v", devices)
+			}
+		})
+		t.Run("regular file in dir", func(t *testing.T) {
+			if err := ioutil.WriteFile(filepath.Join(dir, "somefile"), []byte("hello"), 0600); err != nil {
+				t.Fatal(err)
+			}
+			defer os.Remove(filepath.Join(dir, "somefile"))
+
+			devices, err := getDevices(dir, "")
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(devices) != 0 {
+				t.Fatalf("expected no devices, got %+v", devices)
+			}
+		})
+	})
+}

oci/spec_opts_unix.go

@@ -34,7 +34,7 @@ import (
 func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
 	setLinux(s)
 
-	devs, err := getDevices("/dev")
+	devs, err := getDevices("/dev", "")
 	if err != nil {
 		return err
 	}
@@ -44,7 +44,37 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 
 var errNotADevice = errors.New("not a device node")
 
-func getDevices(path string) ([]specs.LinuxDevice, error) {
+// WithDevices recursively adds devices from the passed in path and associated cgroup rules for that device.
+// If devicePath is a dir it traverses the dir to add all devices in that dir.
+// If devicePath is not a dir, it attempts to add the signle device.
+func WithDevices(devicePath, containerPath, permissions string) SpecOpts {
+	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
+		devs, err := getDevices(devicePath, containerPath)
+		if err != nil {
+			return err
+		}
+		s.Linux.Devices = append(s.Linux.Devices, devs...)
+		return nil
+	}
+}
+
+func getDevices(path, containerPath string) ([]specs.LinuxDevice, error) {
+	stat, err := os.Stat(path)
+	if err != nil {
+		return nil, errors.Wrap(err, "error stating device path")
+	}
+
+	if !stat.IsDir() {
+		dev, err := deviceFromPath(path)
+		if err != nil {
+			return nil, err
+		}
+		if containerPath != "" {
+			dev.Path = containerPath
+		}
+		return []specs.LinuxDevice{*dev}, nil
+	}
+
 	files, err := ioutil.ReadDir(path)
 	if err != nil {
 		return nil, err
@@ -59,7 +89,11 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 			case "pts", "shm", "fd", "mqueue", ".lxc", ".lxd-mounts", ".udev":
 				continue
 			default:
-				sub, err := getDevices(filepath.Join(path, f.Name()))
+				var cp string
+				if containerPath != "" {
+					cp = filepath.Join(containerPath, filepath.Base(f.Name()))
+				}
+				sub, err := getDevices(filepath.Join(path, f.Name()), cp)
 				if err != nil {
 					return nil, err
 				}
@@ -70,7 +104,7 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 		case f.Name() == "console":
 			continue
 		}
-		device, err := deviceFromPath(filepath.Join(path, f.Name()), "rwm")
+		device, err := deviceFromPath(filepath.Join(path, f.Name()))
 		if err != nil {
 			if err == errNotADevice {
 				continue
@@ -80,12 +114,15 @@ func getDevices(path string) ([]specs.LinuxDevice, error) {
 			}
 			return nil, err
 		}
+		if containerPath != "" {
+			device.Path = filepath.Join(containerPath, filepath.Base(f.Name()))
+		}
 		out = append(out, *device)
 	}
 	return out, nil
 }
 
-func deviceFromPath(path, permissions string) (*specs.LinuxDevice, error) {
+func deviceFromPath(path string) (*specs.LinuxDevice, error) {
 	var stat unix.Stat_t
 	if err := unix.Lstat(path, &stat); err != nil {
 		return nil, err

oci/spec_opts_windows.go

@@ -74,6 +74,6 @@ func WithHostDevices(_ context.Context, _ Client, _ *containers.Container, s *Sp
 	return nil
 }
 
-func deviceFromPath(path, permissions string) (*specs.LinuxDevice, error) {
+func deviceFromPath(path string) (*specs.LinuxDevice, error) {
 	return nil, errors.New("device from path not supported on Windows")
 }

pkg/cri/opts/spec_linux.go

@@ -32,7 +32,6 @@ import (
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/oci"
-	"github.com/opencontainers/runc/libcontainer/devices"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/pkg/errors"
@@ -302,16 +301,6 @@ func ensureSharedOrSlave(path string, lookupMount func(string) (mount.Info, erro
 	return errors.Errorf("path %q is mounted on %q but it is not a shared or slave mount", path, mountInfo.Mountpoint)
 }
 
-func addDevice(s *runtimespec.Spec, rd runtimespec.LinuxDevice) {
-	for i, dev := range s.Linux.Devices {
-		if dev.Path == rd.Path {
-			s.Linux.Devices[i] = rd
-			return
-		}
-	}
-	s.Linux.Devices = append(s.Linux.Devices, rd)
-}
-
 // WithDevices sets the provided devices onto the container spec
 func WithDevices(osi osinterface.OS, config *runtime.ContainerConfig) oci.SpecOpts {
 	return func(ctx context.Context, client oci.Client, c *containers.Container, s *runtimespec.Spec) (err error) {
@@ -321,33 +310,17 @@ func WithDevices(osi osinterface.OS, config *runtime.ContainerConfig) oci.SpecOp
 		if s.Linux.Resources == nil {
 			s.Linux.Resources = &runtimespec.LinuxResources{}
 		}
+
 		for _, device := range config.GetDevices() {
 			path, err := osi.ResolveSymbolicLink(device.HostPath)
 			if err != nil {
 				return err
 			}
-			dev, err := devices.DeviceFromPath(path, device.Permissions)
+
-			if err != nil {
+			o := oci.WithDevices(path, device.ContainerPath, device.Permissions)
+			if err := o(ctx, client, c, s); err != nil {
 				return err
 			}
-			rd := runtimespec.LinuxDevice{
-				Path:  device.ContainerPath,
-				Type:  string(dev.Type),
-				Major: dev.Major,
-				Minor: dev.Minor,
-				UID:   &dev.Uid,
-				GID:   &dev.Gid,
-			}
-
-			addDevice(s, rd)
-
-			s.Linux.Resources.Devices = append(s.Linux.Resources.Devices, runtimespec.LinuxDeviceCgroup{
-				Allow:  true,
-				Type:   string(dev.Type),
-				Major:  &dev.Major,
-				Minor:  &dev.Minor,
-				Access: string(dev.Permissions),
-			})
 		}
 		return nil
 	}