From 8e9e14c684313307a6088fd6ba31f2763a8ca8ef Mon Sep 17 00:00:00 2001 From: Anuj Singh Date: Fri, 9 Jun 2023 18:12:39 -0700 Subject: [PATCH 1/2] cgroup delete: proceed to the next subsystem when a cgroup is not found 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 --- cgroup1/cgroup.go | 4 ++++ cgroup1/cgroup_test.go | 7 +++++++ cgroup1/mock_test.go | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/cgroup1/cgroup.go b/cgroup1/cgroup.go index 92b9a10b..c40b64e9 100644 --- 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 index 3c30f4e7..c855de56 100644 --- 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 index 83fca1d9..22c9bd59 100644 --- 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 +} From 280dc073edd1aa893f789f6eb784ef929ea3cc63 Mon Sep 17 00:00:00 2001 From: Anuj Singh Date: Fri, 9 Jun 2023 18:19:46 -0700 Subject: [PATCH 2/2] fix typo in cgroup1/opts/ initConfig Signed-off-by: Anuj Singh --- cgroup1/cgroup.go | 4 ++-- cgroup1/opts.go | 6 +++--- cgroup1/v1.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cgroup1/cgroup.go b/cgroup1/cgroup.go index c40b64e9..eae04f05 100644 --- a/cgroup1/cgroup.go +++ b/cgroup1/cgroup.go @@ -41,7 +41,7 @@ func New(path Path, resources *specs.LinuxResources, opts ...InitOpts) (Cgroup, return nil, err } } - subsystems, err := config.hiearchy() + subsystems, err := config.hierarchy() if err != nil { return nil, err } @@ -79,7 +79,7 @@ func Load(path Path, opts ...InitOpts) (Cgroup, error) { } } var activeSubsystems []Subsystem - subsystems, err := config.hiearchy() + subsystems, err := config.hierarchy() if err != nil { return nil, err } diff --git a/cgroup1/opts.go b/cgroup1/opts.go index 187e0f5e..3aa7f4fb 100644 --- a/cgroup1/opts.go +++ b/cgroup1/opts.go @@ -36,13 +36,13 @@ type InitOpts func(*InitConfig) error type InitConfig struct { // InitCheck can be used to check initialization errors from the subsystem InitCheck InitCheck - hiearchy Hierarchy + hierarchy Hierarchy } func newInitConfig() *InitConfig { return &InitConfig{ InitCheck: RequireDevices, - hiearchy: Default, + hierarchy: Default, } } @@ -66,7 +66,7 @@ func RequireDevices(s Subsystem, _ Path, _ error) error { // The default list is coming from /proc/self/mountinfo. func WithHiearchy(h Hierarchy) InitOpts { return func(c *InitConfig) error { - c.hiearchy = h + c.hierarchy = h return nil } } diff --git a/cgroup1/v1.go b/cgroup1/v1.go index d4c7db6f..ce025bbd 100644 --- a/cgroup1/v1.go +++ b/cgroup1/v1.go @@ -45,7 +45,7 @@ func Default() ([]Subsystem, error) { } // v1MountPoint returns the mount point where the cgroup -// mountpoints are mounted in a single hiearchy +// mountpoints are mounted in a single hierarchy func v1MountPoint() (string, error) { f, err := os.Open("/proc/self/mountinfo") if err != nil {