cgroup delete: proceed to the next subsystem when a cgroup is not found

singholt · singholt · commit 8e9e14c68431 · 2023-06-15T10:33:15.000-07:00
The Delete method checks for running processes within each subsystem, before deleting the cgroup from it. On certain systems, there are cases when removing a cgroup from one subsystem, also removes it from others.
For example, removing a cgroup from net_cls subsystem also removes the cgroup from net_prio. This is because both net_cls and net_prio have a symlink to 'net_cls,net_prio'. This is also true for cpu and cpuacct.

This change handles the above case, when a query to get cgroup processes within a subsystem returns a fs.ErrNotExist. In such a case, we move to the next subsystem, instead of erroring out of Delete() immediately.
This ensures that the cgroup is deleted from all subsystems that have it.

Signed-off-by: Anuj Singh &lt;singholt@amazon.com&gt;
diff --git a/cgroup1/cgroup.go b/cgroup1/cgroup.go
@@ -259,6 +259,10 @@ func (c *cgroup) Delete() error {
 		// kernel prevents cgroups with running process from being removed, check the tree is empty
 		procs, err := c.processes(s.Name(), true, cgroupProcs)
 		if err != nil {
+			// if the control group does not exist within a subsystem, then proceed to the next subsystem
+			if errors.Is(err, os.ErrNotExist) {
+				continue
+			}
 			return err
 		}
 		if len(procs) > 0 {
diff --git a/cgroup1/cgroup_test.go b/cgroup1/cgroup_test.go
@@ -459,6 +459,13 @@ func TestDelete(t *testing.T) {
 		t.Error(err)
 		return
 	}
+
+	err = mock.symLink()
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
 	if err := control.Delete(); err != nil {
 		t.Error(err)
 	}
diff --git a/cgroup1/mock_test.go b/cgroup1/mock_test.go
@@ -73,3 +73,21 @@ func (m *mockCgroup) delete() error {
 func (m *mockCgroup) hierarchy() ([]Subsystem, error) {
 	return m.subsystems, nil
 }
+
+// symLink() creates a symlink between net_cls and net_prio for testing
+// On certain Linux systems, there's a symlink from both net_cls and net_prio to "net_cls,net_prio"
+// Since we don't have a subsystem defined for "net_cls,net_prio",
+// we mock this behavior by creating a symlink directly between net_cls and net_prio
+func (m *mockCgroup) symLink() error {
+	netCLS := filepath.Join(m.root, string(NetCLS))
+	netPrio := filepath.Join(m.root, string(NetPrio))
+	// remove netCLS before creating a symlink
+	if err := os.RemoveAll(netCLS); err != nil {
+		return err
+	}
+	// create symlink between net_cls and net_prio
+	if err := os.Symlink(netPrio, netCLS); err != nil {
+		return err
+	}
+	return nil
+}

Original file line number	Diff line number	Diff line change
`@@ -459,6 +459,13 @@ func TestDelete(t *testing.T) {`
`459`	`459`	`t.Error(err)`
`460`	`460`	`return`
`461`	`461`	`}`
	`462`	`+`
	`463`	`+ err = mock.symLink()`
	`464`	`+ if err != nil {`
	`465`	`+ t.Error(err)`
	`466`	`+ return`
	`467`	`+ }`
	`468`	`+`
`462`	`469`	`if err := control.Delete(); err != nil {`
`463`	`470`	`t.Error(err)`
`464`	`471`	`}`