Merge pull request #958 from dubstack/skip-devices

Skip updates on parent Devices cgroup

Author: hqhq

libcontainer/README.md

@@ -77,7 +77,7 @@ config := &configs.Config{
 		Parent: "system",
 		Resources: &configs.Resources{
 			MemorySwappiness: nil,
-			AllowAllDevices:  false,
+			AllowAllDevices:  nil,
 			AllowedDevices:   configs.DefaultAllowedDevices,
 		},
 	},

libcontainer/cgroups/fs/devices.go

@@ -43,21 +43,23 @@ func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error {
 		}
 		return nil
 	}
-	if !cgroup.Resources.AllowAllDevices {
-		if err := writeFile(path, "devices.deny", "a"); err != nil {
-			return err
-		}
-
-		for _, dev := range cgroup.Resources.AllowedDevices {
-			if err := writeFile(path, "devices.allow", dev.CgroupString()); err != nil {
+	if cgroup.Resources.AllowAllDevices != nil {
+		if *cgroup.Resources.AllowAllDevices == false {
+			if err := writeFile(path, "devices.deny", "a"); err != nil {
 				return err
 			}
+
+			for _, dev := range cgroup.Resources.AllowedDevices {
+				if err := writeFile(path, "devices.allow", dev.CgroupString()); err != nil {
+					return err
+				}
+			}
+			return nil
 		}
-		return nil
-	}
 
-	if err := writeFile(path, "devices.allow", "a"); err != nil {
-		return err
+		if err := writeFile(path, "devices.allow", "a"); err != nil {
+			return err
+		}
 	}
 
 	for _, dev := range cgroup.Resources.DeniedDevices {

libcontainer/cgroups/fs/devices_test.go

@@ -40,8 +40,8 @@ func TestDevicesSetAllow(t *testing.T) {
 	helper.writeFileContents(map[string]string{
 		"devices.deny": "a",
 	})
-
-	helper.CgroupData.config.Resources.AllowAllDevices = false
+	allowAllDevices := false
+	helper.CgroupData.config.Resources.AllowAllDevices = &allowAllDevices
 	helper.CgroupData.config.Resources.AllowedDevices = allowedDevices
 	devices := &DevicesGroup{}
 	if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
@@ -56,6 +56,19 @@ func TestDevicesSetAllow(t *testing.T) {
 	if value != allowedList {
 		t.Fatal("Got the wrong value, set devices.allow failed.")
 	}
+
+	// When AllowAllDevices is nil, devices.allow file should not be modified.
+	helper.CgroupData.config.Resources.AllowAllDevices = nil
+	if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
+		t.Fatal(err)
+	}
+	value, err = getCgroupParamString(helper.CgroupPath, "devices.allow")
+	if err != nil {
+		t.Fatalf("Failed to parse devices.allow - %s", err)
+	}
+	if value != allowedList {
+		t.Fatal("devices policy shouldn't have changed on AllowedAllDevices=nil.")
+	}
 }
 
 func TestDevicesSetDeny(t *testing.T) {
@@ -66,7 +79,8 @@ func TestDevicesSetDeny(t *testing.T) {
 		"devices.allow": "a",
 	})
 
-	helper.CgroupData.config.Resources.AllowAllDevices = true
+	allowAllDevices := true
+	helper.CgroupData.config.Resources.AllowAllDevices = &allowAllDevices
 	helper.CgroupData.config.Resources.DeniedDevices = deniedDevices
 	devices := &DevicesGroup{}
 	if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {

libcontainer/configs/cgroup_unix.go

@@ -36,7 +36,7 @@ type Cgroup struct {
 type Resources struct {
 	// If this is true allow access to any kind of device within the container.  If false, allow access only to devices explicitly listed in the allowed_devices list.
 	// Deprecated
-	AllowAllDevices bool `json:"allow_all_devices,omitempty"`
+	AllowAllDevices *bool `json:"allow_all_devices,omitempty"`
 	// Deprecated
 	AllowedDevices []*Device `json:"allowed_devices,omitempty"`
 	// Deprecated

libcontainer/integration/template_test.go

@@ -20,6 +20,7 @@ const defaultMountFlags = syscall.MS_NOEXEC | syscall.MS_NOSUID | syscall.MS_NOD
 // it uses a network strategy of just setting a loopback interface
 // and the default setup for devices
 func newTemplateConfig(rootfs string) *configs.Config {
+	allowAllDevices := false
 	return &configs.Config{
 		Rootfs: rootfs,
 		Capabilities: []string{
@@ -49,7 +50,7 @@ func newTemplateConfig(rootfs string) *configs.Config {
 			Path: "integration/test",
 			Resources: &configs.Resources{
 				MemorySwappiness: nil,
-				AllowAllDevices:  false,
+				AllowAllDevices:  &allowAllDevices,
 				AllowedDevices:   configs.DefaultAllowedDevices,
 			},
 		},