Merge pull request #388 from hqhq/hq_cgroup_cleanups

Some cgroup cleanups

libcontainer/cgroups/fs/apply_raw.go

@@ -52,10 +52,10 @@ type subsystem interface {
 	Name() string
 	// Returns the stats, as 'stats', corresponding to the cgroup under 'path'.
 	GetStats(path string, stats *cgroups.Stats) error
-	// Removes the cgroup represented by 'data'.
-	Remove(*data) error
-	// Creates and joins the cgroup represented by data.
-	Apply(*data) error
+	// Removes the cgroup represented by 'cgroupData'.
+	Remove(*cgroupData) error
+	// Creates and joins the cgroup represented by 'cgroupData'.
+	Apply(*cgroupData) error
 	// Set the cgroup represented by cgroup.
 	Set(path string, cgroup *configs.Cgroup) error
 }
@@ -92,10 +92,11 @@ func getCgroupRoot() (string, error) {
 	return cgroupRoot, nil
 }
 
-type data struct {
+type cgroupData struct {
 	root   string
-	cgroup string
-	c      *configs.Cgroup
+	parent string
+	name   string
+	config *configs.Cgroup
 	pid    int
 }
 
@@ -229,58 +230,61 @@ func (m *Manager) GetPids() ([]int, error) {
 	return cgroups.GetPids(dir)
 }
 
-func getCgroupData(c *configs.Cgroup, pid int) (*data, error) {
+func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) {
 	root, err := getCgroupRoot()
 	if err != nil {
 		return nil, err
 	}
 
-	cgroup := c.Name
-	if c.Parent != "" {
-		cgroup = filepath.Join(c.Parent, cgroup)
-	}
-
-	return &data{
+	return &cgroupData{
 		root:   root,
-		cgroup: cgroup,
-		c:      c,
+		parent: c.Parent,
+		name:   c.Name,
+		config: c,
 		pid:    pid,
 	}, nil
 }
 
-func (raw *data) parent(subsystem, mountpoint, root string) (string, error) {
+func (raw *cgroupData) parentPath(subsystem, mountpoint, root string) (string, error) {
+	// Use GetThisCgroupDir instead of GetInitCgroupDir, because the creating
+	// process could in container and shared pid namespace with host, and
+	// /proc/1/cgroup could point to whole other world of cgroups.
 	initPath, err := cgroups.GetThisCgroupDir(subsystem)
 	if err != nil {
 		return "", err
 	}
+	// This is needed for nested containers, because in /proc/self/cgroup we
+	// see pathes from host, which don't exist in container.
 	relDir, err := filepath.Rel(root, initPath)
 	if err != nil {
 		return "", err
 	}
 	return filepath.Join(mountpoint, relDir), nil
 }
 
-func (raw *data) path(subsystem string) (string, error) {
+func (raw *cgroupData) path(subsystem string) (string, error) {
 	mnt, root, err := cgroups.FindCgroupMountpointAndRoot(subsystem)
 	// If we didn't mount the subsystem, there is no point we make the path.
 	if err != nil {
 		return "", err
 	}
 
+	cgPath := filepath.Join(raw.parent, raw.name)
 	// If the cgroup name/path is absolute do not look relative to the cgroup of the init process.
-	if filepath.IsAbs(raw.cgroup) {
-		return filepath.Join(raw.root, filepath.Base(mnt), raw.cgroup), nil
+	if filepath.IsAbs(cgPath) {
+		// Sometimes subsystems can be mounted togethger as 'cpu,cpuacct'.
+		return filepath.Join(raw.root, filepath.Base(mnt), cgPath), nil
 	}
 
-	parent, err := raw.parent(subsystem, mnt, root)
+	parentPath, err := raw.parentPath(subsystem, mnt, root)
 	if err != nil {
 		return "", err
 	}
 
-	return filepath.Join(parent, raw.cgroup), nil
+	return filepath.Join(parentPath, cgPath), nil
 }
 
-func (raw *data) join(subsystem string) (string, error) {
+func (raw *cgroupData) join(subsystem string) (string, error) {
 	path, err := raw.path(subsystem)
 	if err != nil {
 		return "", err

libcontainer/cgroups/fs/blkio.go

@@ -21,13 +21,13 @@ func (s *BlkioGroup) Name() string {
 	return "blkio"
 }
 
-func (s *BlkioGroup) Apply(d *data) error {
+func (s *BlkioGroup) Apply(d *cgroupData) error {
 	dir, err := d.join("blkio")
 	if err != nil && !cgroups.IsNotFound(err) {
 		return err
 	}
 
-	if err := s.Set(dir, d.c); err != nil {
+	if err := s.Set(dir, d.config); err != nil {
 		return err
 	}
 
@@ -78,7 +78,7 @@ func (s *BlkioGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *BlkioGroup) Remove(d *data) error {
+func (s *BlkioGroup) Remove(d *cgroupData) error {
 	return removePath(d.path("blkio"))
 }
 

libcontainer/cgroups/fs/blkio_test.go

@@ -89,9 +89,9 @@ func TestBlkioSetWeight(t *testing.T) {
 		"blkio.weight": strconv.Itoa(weightBefore),
 	})
 
-	helper.CgroupData.c.BlkioWeight = weightAfter
+	helper.CgroupData.config.BlkioWeight = weightAfter
 	blkio := &BlkioGroup{}
-	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -120,9 +120,9 @@ func TestBlkioSetWeightDevice(t *testing.T) {
 		"blkio.weight_device": weightDeviceBefore,
 	})
 
-	helper.CgroupData.c.BlkioWeightDevice = []*configs.WeightDevice{wd}
+	helper.CgroupData.config.BlkioWeightDevice = []*configs.WeightDevice{wd}
 	blkio := &BlkioGroup{}
-	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -157,9 +157,9 @@ func TestBlkioSetMultipleWeightDevice(t *testing.T) {
 		"blkio.weight_device": weightDeviceBefore,
 	})
 
-	helper.CgroupData.c.BlkioWeightDevice = []*configs.WeightDevice{wd1, wd2}
+	helper.CgroupData.config.BlkioWeightDevice = []*configs.WeightDevice{wd1, wd2}
 	blkio := &BlkioGroup{}
-	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -529,9 +529,9 @@ func TestBlkioSetThrottleReadBpsDevice(t *testing.T) {
 		"blkio.throttle.read_bps_device": throttleBefore,
 	})
 
-	helper.CgroupData.c.BlkioThrottleReadBpsDevice = []*configs.ThrottleDevice{td}
+	helper.CgroupData.config.BlkioThrottleReadBpsDevice = []*configs.ThrottleDevice{td}
 	blkio := &BlkioGroup{}
-	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -559,9 +559,9 @@ func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) {
 		"blkio.throttle.write_bps_device": throttleBefore,
 	})
 
-	helper.CgroupData.c.BlkioThrottleWriteBpsDevice = []*configs.ThrottleDevice{td}
+	helper.CgroupData.config.BlkioThrottleWriteBpsDevice = []*configs.ThrottleDevice{td}
 	blkio := &BlkioGroup{}
-	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -589,9 +589,9 @@ func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) {
 		"blkio.throttle.read_iops_device": throttleBefore,
 	})
 
-	helper.CgroupData.c.BlkioThrottleReadIOPSDevice = []*configs.ThrottleDevice{td}
+	helper.CgroupData.config.BlkioThrottleReadIOPSDevice = []*configs.ThrottleDevice{td}
 	blkio := &BlkioGroup{}
-	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -619,9 +619,9 @@ func TestBlkioSetThrottleWriteIOpsDevice(t *testing.T) {
 		"blkio.throttle.write_iops_device": throttleBefore,
 	})
 
-	helper.CgroupData.c.BlkioThrottleWriteIOPSDevice = []*configs.ThrottleDevice{td}
+	helper.CgroupData.config.BlkioThrottleWriteIOPSDevice = []*configs.ThrottleDevice{td}
 	blkio := &BlkioGroup{}
-	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 

libcontainer/cgroups/fs/cpu.go

@@ -19,15 +19,15 @@ func (s *CpuGroup) Name() string {
 	return "cpu"
 }
 
-func (s *CpuGroup) Apply(d *data) error {
+func (s *CpuGroup) Apply(d *cgroupData) error {
 	// We always want to join the cpu group, to allow fair cpu scheduling
 	// on a container basis
 	dir, err := d.join("cpu")
 	if err != nil && !cgroups.IsNotFound(err) {
 		return err
 	}
 
-	if err := s.Set(dir, d.c); err != nil {
+	if err := s.Set(dir, d.config); err != nil {
 		return err
 	}
 
@@ -64,7 +64,7 @@ func (s *CpuGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *CpuGroup) Remove(d *data) error {
+func (s *CpuGroup) Remove(d *cgroupData) error {
 	return removePath(d.path("cpu"))
 }
 

libcontainer/cgroups/fs/cpu_test.go

@@ -23,9 +23,9 @@ func TestCpuSetShares(t *testing.T) {
 		"cpu.shares": strconv.Itoa(sharesBefore),
 	})
 
-	helper.CgroupData.c.CpuShares = sharesAfter
+	helper.CgroupData.config.CpuShares = sharesAfter
 	cpu := &CpuGroup{}
-	if err := cpu.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -61,12 +61,12 @@ func TestCpuSetBandWidth(t *testing.T) {
 		"cpu.rt_period_us":  strconv.Itoa(rtPeriodBefore),
 	})
 
-	helper.CgroupData.c.CpuQuota = quotaAfter
-	helper.CgroupData.c.CpuPeriod = periodAfter
-	helper.CgroupData.c.CpuRtRuntime = rtRuntimeAfter
-	helper.CgroupData.c.CpuRtPeriod = rtPeriodAfter
+	helper.CgroupData.config.CpuQuota = quotaAfter
+	helper.CgroupData.config.CpuPeriod = periodAfter
+	helper.CgroupData.config.CpuRtRuntime = rtRuntimeAfter
+	helper.CgroupData.config.CpuRtPeriod = rtPeriodAfter
 	cpu := &CpuGroup{}
-	if err := cpu.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 

libcontainer/cgroups/fs/cpuacct.go

@@ -28,7 +28,7 @@ func (s *CpuacctGroup) Name() string {
 	return "cpuacct"
 }
 
-func (s *CpuacctGroup) Apply(d *data) error {
+func (s *CpuacctGroup) Apply(d *cgroupData) error {
 	// we just want to join this group even though we don't set anything
 	if _, err := d.join("cpuacct"); err != nil && !cgroups.IsNotFound(err) {
 		return err
@@ -41,7 +41,7 @@ func (s *CpuacctGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *CpuacctGroup) Remove(d *data) error {
+func (s *CpuacctGroup) Remove(d *cgroupData) error {
 	return removePath(d.path("cpuacct"))
 }
 

libcontainer/cgroups/fs/cpuset.go

@@ -20,12 +20,12 @@ func (s *CpusetGroup) Name() string {
 	return "cpuset"
 }
 
-func (s *CpusetGroup) Apply(d *data) error {
+func (s *CpusetGroup) Apply(d *cgroupData) error {
 	dir, err := d.path("cpuset")
 	if err != nil && !cgroups.IsNotFound(err) {
 		return err
 	}
-	return s.ApplyDir(dir, d.c, d.pid)
+	return s.ApplyDir(dir, d.config, d.pid)
 }
 
 func (s *CpusetGroup) Set(path string, cgroup *configs.Cgroup) error {
@@ -42,7 +42,7 @@ func (s *CpusetGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *CpusetGroup) Remove(d *data) error {
+func (s *CpusetGroup) Remove(d *cgroupData) error {
 	return removePath(d.path("cpuset"))
 }
 

libcontainer/cgroups/fs/cpuset_test.go

@@ -19,9 +19,9 @@ func TestCpusetSetCpus(t *testing.T) {
 		"cpuset.cpus": cpusBefore,
 	})
 
-	helper.CgroupData.c.CpusetCpus = cpusAfter
+	helper.CgroupData.config.CpusetCpus = cpusAfter
 	cpuset := &CpusetGroup{}
-	if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -48,9 +48,9 @@ func TestCpusetSetMems(t *testing.T) {
 		"cpuset.mems": memsBefore,
 	})
 
-	helper.CgroupData.c.CpusetMems = memsAfter
+	helper.CgroupData.config.CpusetMems = memsAfter
 	cpuset := &CpusetGroup{}
-	if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 

libcontainer/cgroups/fs/devices.go

@@ -14,15 +14,15 @@ func (s *DevicesGroup) Name() string {
 	return "devices"
 }
 
-func (s *DevicesGroup) Apply(d *data) error {
+func (s *DevicesGroup) Apply(d *cgroupData) error {
 	dir, err := d.join("devices")
 	if err != nil {
 		// We will return error even it's `not found` error, devices
 		// cgroup is hard requirement for container's security.
 		return err
 	}
 
-	if err := s.Set(dir, d.c); err != nil {
+	if err := s.Set(dir, d.config); err != nil {
 		return err
 	}
 
@@ -56,7 +56,7 @@ func (s *DevicesGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
-func (s *DevicesGroup) Remove(d *data) error {
+func (s *DevicesGroup) Remove(d *cgroupData) error {
 	return removePath(d.path("devices"))
 }
 

libcontainer/cgroups/fs/devices_test.go

@@ -41,10 +41,10 @@ func TestDevicesSetAllow(t *testing.T) {
 		"devices.deny": "a",
 	})
 
-	helper.CgroupData.c.AllowAllDevices = false
-	helper.CgroupData.c.AllowedDevices = allowedDevices
+	helper.CgroupData.config.AllowAllDevices = false
+	helper.CgroupData.config.AllowedDevices = allowedDevices
 	devices := &DevicesGroup{}
-	if err := devices.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}
 
@@ -66,10 +66,10 @@ func TestDevicesSetDeny(t *testing.T) {
 		"devices.allow": "a",
 	})
 
-	helper.CgroupData.c.AllowAllDevices = true
-	helper.CgroupData.c.DeniedDevices = deniedDevices
+	helper.CgroupData.config.AllowAllDevices = true
+	helper.CgroupData.config.DeniedDevices = deniedDevices
 	devices := &DevicesGroup{}
-	if err := devices.Set(helper.CgroupPath, helper.CgroupData.c); err != nil {
+	if err := devices.Set(helper.CgroupPath, helper.CgroupData.config); err != nil {
 		t.Fatal(err)
 	}