Merge pull request opencontainers#2500 from kolyshkin/fs-apply

libct/cgroups/fs: rework Apply()

libcontainer/cgroups/fs/blkio.go

@@ -22,12 +22,8 @@ func (s *BlkioGroup) Name() string {
 	return "blkio"
 }
 
-func (s *BlkioGroup) Apply(d *cgroupData) error {
-	_, err := d.join("blkio")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *BlkioGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *BlkioGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/cpu.go

@@ -21,17 +21,7 @@ func (s *CpuGroup) Name() string {
 	return "cpu"
 }
 
-func (s *CpuGroup) Apply(d *cgroupData) error {
-	// We always want to join the cpu group, to allow fair cpu scheduling
-	// on a container basis
-	path, err := d.path("cpu")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return s.ApplyDir(path, d.config, d.pid)
-}
-
-func (s *CpuGroup) ApplyDir(path string, cgroup *configs.Cgroup, pid int) error {
+func (s *CpuGroup) Apply(path string, d *cgroupData) error {
 	// This might happen if we have no cpu cgroup mounted.
 	// Just do nothing and don't fail.
 	if path == "" {
@@ -43,12 +33,12 @@ func (s *CpuGroup) ApplyDir(path string, cgroup *configs.Cgroup, pid int) error
 	// We should set the real-Time group scheduling settings before moving
 	// in the process because if the process is already in SCHED_RR mode
 	// and no RT bandwidth is set, adding it will fail.
-	if err := s.SetRtSched(path, cgroup); err != nil {
+	if err := s.SetRtSched(path, d.config); err != nil {
 		return err
 	}
-	// because we are not using d.join we need to place the pid into the procs file
-	// unlike the other subsystems
-	return cgroups.WriteCgroupProc(path, pid)
+	// Since we are not using join(), we need to place the pid
+	// into the procs file unlike other subsystems.
+	return cgroups.WriteCgroupProc(path, d.pid)
 }
 
 func (s *CpuGroup) SetRtSched(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/cpu_test.go

@@ -182,7 +182,9 @@ func TestCpuSetRtSchedAtApply(t *testing.T) {
 	helper.CgroupData.config.Resources.CpuRtRuntime = rtRuntimeAfter
 	helper.CgroupData.config.Resources.CpuRtPeriod = rtPeriodAfter
 	cpu := &CpuGroup{}
-	if err := cpu.ApplyDir(helper.CgroupPath, helper.CgroupData.config, 1234); err != nil {
+
+	helper.CgroupData.pid = 1234
+	if err := cpu.Apply(helper.CgroupPath, helper.CgroupData); err != nil {
 		t.Fatal(err)
 	}
 

libcontainer/cgroups/fs/cpuacct.go

@@ -40,13 +40,8 @@ func (s *CpuacctGroup) Name() string {
 	return "cpuacct"
 }
 
-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
-	}
-
-	return nil
+func (s *CpuacctGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *CpuacctGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/cpuset.go

@@ -23,12 +23,8 @@ func (s *CpusetGroup) Name() string {
 	return "cpuset"
 }
 
-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.config, d.pid)
+func (s *CpusetGroup) Apply(path string, d *cgroupData) error {
+	return s.ApplyDir(path, d.config, d.pid)
 }
 
 func (s *CpusetGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/devices.go

@@ -22,17 +22,16 @@ func (s *DevicesGroup) Name() string {
 	return "devices"
 }
 
-func (s *DevicesGroup) Apply(d *cgroupData) error {
+func (s *DevicesGroup) Apply(path string, d *cgroupData) error {
 	if d.config.SkipDevices {
 		return nil
 	}
-	_, 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 path == "" {
+		// Return error here, since devices cgroup
+		// is a hard requirement for container's security.
+		return errSubsystemDoesNotExist
 	}
-	return nil
+	return join(path, d.pid)
 }
 
 func loadEmulator(path string) (*devices.Emulator, error) {

libcontainer/cgroups/fs/freezer.go

@@ -22,12 +22,8 @@ func (s *FreezerGroup) Name() string {
 	return "freezer"
 }
 
-func (s *FreezerGroup) Apply(d *cgroupData) error {
-	_, err := d.join("freezer")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *FreezerGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *FreezerGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/fs.go

@@ -44,7 +44,7 @@ type subsystem interface {
 	// Returns the stats, as 'stats', corresponding to the cgroup under 'path'.
 	GetStats(path string, stats *cgroups.Stats) error
 	// Creates and joins the cgroup represented by 'cgroupData'.
-	Apply(*cgroupData) error
+	Apply(path string, c *cgroupData) error
 	// Set the cgroup represented by cgroup.
 	Set(path string, cgroup *configs.Cgroup) error
 }
@@ -194,7 +194,7 @@ func (m *manager) Apply(pid int) (err error) {
 		}
 		m.paths[sys.Name()] = p
 
-		if err := sys.Apply(d); err != nil {
+		if err := sys.Apply(p, d); err != nil {
 			// In the case of rootless (including euid=0 in userns), where an
 			// explicit cgroup path hasn't been set, we don't bail on error in
 			// case of permission problems. Cases where limits have been set
@@ -357,18 +357,14 @@ func (raw *cgroupData) path(subsystem string) (string, error) {
 	return filepath.Join(parentPath, raw.innerPath), nil
 }
 
-func (raw *cgroupData) join(subsystem string) (string, error) {
-	path, err := raw.path(subsystem)
-	if err != nil {
-		return "", err
+func join(path string, pid int) error {
+	if path == "" {
+		return nil
 	}
 	if err := os.MkdirAll(path, 0755); err != nil {
-		return "", err
-	}
-	if err := cgroups.WriteCgroupProc(path, raw.pid); err != nil {
-		return "", err
+		return err
 	}
-	return path, nil
+	return cgroups.WriteCgroupProc(path, pid)
 }
 
 func removePath(p string, err error) error {

libcontainer/cgroups/fs/hugetlb.go

@@ -19,12 +19,8 @@ func (s *HugetlbGroup) Name() string {
 	return "hugetlb"
 }
 
-func (s *HugetlbGroup) Apply(d *cgroupData) error {
-	_, err := d.join("hugetlb")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *HugetlbGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *HugetlbGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/memory.go

@@ -37,11 +37,8 @@ func (s *MemoryGroup) Name() string {
 	return "memory"
 }
 
-func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
-	path, err := d.path("memory")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	} else if path == "" {
+func (s *MemoryGroup) Apply(path string, d *cgroupData) (err error) {
+	if path == "" {
 		return nil
 	}
 	if memoryAssigned(d.config) {
@@ -66,11 +63,7 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
 
 	// We need to join memory cgroup after set memory limits, because
 	// kmem.limit_in_bytes can only be set when the cgroup is empty.
-	_, err = d.join("memory")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+	return join(path, d.pid)
 }
 
 func setMemoryAndSwap(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/name.go

@@ -16,10 +16,10 @@ func (s *NameGroup) Name() string {
 	return s.GroupName
 }
 
-func (s *NameGroup) Apply(d *cgroupData) error {
+func (s *NameGroup) Apply(path string, d *cgroupData) error {
 	if s.Join {
 		// ignore errors if the named cgroup does not exist
-		d.join(s.GroupName)
+		join(path, d.pid)
 	}
 	return nil
 }

libcontainer/cgroups/fs/net_cls.go

@@ -17,12 +17,8 @@ func (s *NetClsGroup) Name() string {
 	return "net_cls"
 }
 
-func (s *NetClsGroup) Apply(d *cgroupData) error {
-	_, err := d.join("net_cls")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *NetClsGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *NetClsGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/net_prio.go

@@ -15,12 +15,8 @@ func (s *NetPrioGroup) Name() string {
 	return "net_prio"
 }
 
-func (s *NetPrioGroup) Apply(d *cgroupData) error {
-	_, err := d.join("net_prio")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *NetPrioGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *NetPrioGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/perf_event.go

@@ -14,12 +14,8 @@ func (s *PerfEventGroup) Name() string {
 	return "perf_event"
 }
 
-func (s *PerfEventGroup) Apply(d *cgroupData) error {
-	// we just want to join this group even though we don't set anything
-	if _, err := d.join("perf_event"); err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *PerfEventGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *PerfEventGroup) Set(path string, cgroup *configs.Cgroup) error {

libcontainer/cgroups/fs/pids.go

@@ -19,12 +19,8 @@ func (s *PidsGroup) Name() string {
 	return "pids"
 }
 
-func (s *PidsGroup) Apply(d *cgroupData) error {
-	_, err := d.join("pids")
-	if err != nil && !cgroups.IsNotFound(err) {
-		return err
-	}
-	return nil
+func (s *PidsGroup) Apply(path string, d *cgroupData) error {
+	return join(path, d.pid)
 }
 
 func (s *PidsGroup) Set(path string, cgroup *configs.Cgroup) error {