summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFabricio Voznika <fvoznika@google.com>2021-05-13 14:41:38 -0700
committergVisor bot <gvisor-bot@google.com>2021-05-13 14:44:08 -0700
commitf3478b7516e02ab341324ae1361ea9b019ae4f4e (patch)
treea691bdc8fb1cffc17c13db3f52f72f0115427fd8
parent84f04cc858644e9748a82f33b834a84c8b0fc934 (diff)
Fix problem with grouped cgroups
cgroup controllers can be grouped together (e.g. cpu,cpuacct) and that was confusing Cgroup.Install() into thinking that a cgroup directory was created by the caller, when it had being created by another controller that is grouped together. PiperOrigin-RevId: 373661336
-rw-r--r--runsc/cgroup/cgroup.go35
-rw-r--r--runsc/cgroup/cgroup_test.go4
2 files changed, 24 insertions, 15 deletions
diff --git a/runsc/cgroup/cgroup.go b/runsc/cgroup/cgroup.go
index 335e46499..66a6a0f68 100644
--- a/runsc/cgroup/cgroup.go
+++ b/runsc/cgroup/cgroup.go
@@ -342,25 +342,31 @@ func new(pid, cgroupsPath string) (*Cgroup, error) {
// already exists, it means that the caller has already provided a
// pre-configured cgroups, and 'res' is ignored.
func (c *Cgroup) Install(res *specs.LinuxResources) error {
- log.Debugf("Creating cgroup %q", c.Name)
+ log.Debugf("Installing cgroup path %q", c.Name)
- // The Cleanup object cleans up partially created cgroups when an error occurs.
- // Errors occuring during cleanup itself are ignored.
+ // Clean up partially created cgroups on error. Errors during cleanup itself
+ // are ignored.
clean := cleanup.Make(func() { _ = c.Uninstall() })
defer clean.Clean()
- for key, ctrlr := range controllers {
+ // Controllers can be symlinks to a group of controllers (e.g. cpu,cpuacct).
+ // So first check what directories need to be created. Otherwise, when
+ // the directory for one of the controllers in a group is created, it will
+ // make it seem like the directory already existed and it's not owned by the
+ // other controllers in the group.
+ var missing []string
+ for key := range controllers {
path := c.MakePath(key)
- if _, err := os.Stat(path); err == nil {
- // If cgroup has already been created; it has been setup by caller. Don't
- // make any changes to configuration, just join when sandbox/gofer starts.
- log.Debugf("Using pre-created cgroup %q", path)
- continue
+ if _, err := os.Stat(path); err != nil {
+ missing = append(missing, key)
+ } else {
+ log.Debugf("Using pre-created cgroup %q: %q", key, path)
}
-
- // Mark that cgroup resources are owned by me.
- c.Own[key] = true
-
+ }
+ for _, key := range missing {
+ ctrlr := controllers[key]
+ path := c.MakePath(key)
+ log.Debugf("Creating cgroup %q: %q", key, path)
if err := os.MkdirAll(path, 0755); err != nil {
if ctrlr.optional() && errors.Is(err, unix.EROFS) {
if err := ctrlr.skip(res); err != nil {
@@ -371,6 +377,9 @@ func (c *Cgroup) Install(res *specs.LinuxResources) error {
}
return err
}
+
+ // Only set controllers that were created by me.
+ c.Own[key] = true
if err := ctrlr.set(res, path); err != nil {
return err
}
diff --git a/runsc/cgroup/cgroup_test.go b/runsc/cgroup/cgroup_test.go
index 02cadeef4..eba40621e 100644
--- a/runsc/cgroup/cgroup_test.go
+++ b/runsc/cgroup/cgroup_test.go
@@ -60,10 +60,10 @@ var dindMountinfo = `
func TestUninstallEnoent(t *testing.T) {
c := Cgroup{
- // set a non-existent name
+ // Use a non-existent name.
Name: "runsc-test-uninstall-656e6f656e740a",
+ Own: make(map[string]bool),
}
- c.Own = make(map[string]bool)
for key := range controllers {
c.Own[key] = true
}