Remove per-container CGroup parents

Originally, it seemed like a good idea to place Conmon and the
container it managed under a shared CGroup, so we could manage
the two together. It's become increasingly clear that this is a
potential performance sore point, gains us little practical
benefit in managing Conmon, and adds extra steps to container
cleanup that interfere with Conmon postrun hooks.

Revert back to a shared CGroup for conmon processes under the
CGroup parent. This will retain per-pod conmon CGroups as well if
the pod is set to create a CGroup and act as CGroup parent for
its containers.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #1051
Approved by: umohnani8

4 files changed, 2 insertions, 64 deletions

diff --git a/libpod/container.go b/libpod/container.go
index 3708ee287..24a5f93af 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -162,9 +162,6 @@ type containerState struct {
 	// the path of the file on disk outside the container
 	BindMounts map[string]string `json:"bindMounts,omitempty"`
 
-	// CgroupCreated indicates that the container has created a cgroup
-	CgroupCreated bool `json:"cgroupCreated,omitempty"`
-
 	// UserNSRoot is the directory used as root for the container when using
 	// user namespaces.
 	UserNSRoot string `json:"userNSRoot,omitempty"`
@@ -871,7 +868,7 @@ func (c *Container) NamespacePath(ns LinuxNS) (string, error) {
 func (c *Container) CGroupPath() (string, error) {
 	switch c.runtime.config.CgroupManager {
 	case CgroupfsCgroupsManager:
-		return filepath.Join(c.config.CgroupParent, fmt.Sprintf("libpod-%s", c.ID()), "ctr"), nil
+		return filepath.Join(c.config.CgroupParent, fmt.Sprintf("libpod-%s", c.ID())), nil
 	case SystemdCgroupsManager:
 		return filepath.Join(c.config.CgroupParent, createUnitName("libpod", c.ID())), nil
 	default:
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 452be176c..94cd74c09 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -325,7 +325,6 @@ func resetState(state *containerState) error {
 	state.Interfaces = nil
 	state.Routes = nil
 	state.BindMounts = make(map[string]string)
-	state.CgroupCreated = false
 
 	return nil
 }
@@ -561,7 +560,6 @@ func (c *Container) init(ctx context.Context) error {
 	logrus.Debugf("Created container %s in OCI runtime", c.ID())
 
 	c.state.State = ContainerStateCreated
-	c.state.CgroupCreated = true
 
 	if err := c.save(); err != nil {
 		return err
@@ -832,20 +830,6 @@ func (c *Container) cleanup() error {
 		lastError = err
 	}
 
-	if err := c.cleanupCgroups(); err != nil {
-		/*
-			if lastError != nil {
-				logrus.Errorf("Error cleaning up container %s CGroups: %v", c.ID(), err)
-			} else {
-				lastError = err
-			}
-		*/
-		// For now we are going to only warn on failures to clean up cgroups
-		// We have a conflict with running podman containers cleanup in same cgroup as container
-		logrus.Warnf("Ignoring Error cleaning up container %s CGroups: %v", c.ID(), err)
-
-	}
-
 	// Unmount storage
 	if err := c.cleanupStorage(); err != nil {
 		if lastError != nil {
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 327c78045..0dc02d117 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -6,12 +6,10 @@ import (
 	"context"
 	"fmt"
 	"path"
-	"path/filepath"
 	"strings"
 	"syscall"
 	"time"
 
-	"github.com/containerd/cgroups"
 	"github.com/containers/storage/pkg/idtools"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
@@ -24,47 +22,6 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-// cleanupCgroup cleans up residual CGroups after container execution
-// This is a no-op for the systemd cgroup driver
-func (c *Container) cleanupCgroups() error {
-	if !c.state.CgroupCreated {
-		logrus.Debugf("Cgroups are not present, ignoring...")
-		return nil
-	}
-
-	if c.runtime.config.CgroupManager == SystemdCgroupsManager {
-		return nil
-	}
-
-	// Remove the base path of the container's cgroups
-	path := filepath.Join(c.config.CgroupParent, fmt.Sprintf("libpod-%s", c.ID()))
-
-	logrus.Debugf("Removing CGroup %s", path)
-
-	cgroup, err := cgroups.Load(cgroups.V1, cgroups.StaticPath(path))
-	if err != nil {
-		// It's fine for the cgroup to not exist
-		// We want it gone, it's gone
-		if err == cgroups.ErrCgroupDeleted {
-			return nil
-		}
-
-		return err
-	}
-
-	if err := cgroup.Delete(); err != nil {
-		return err
-	}
-
-	c.state.CgroupCreated = false
-
-	if c.valid {
-		return c.save()
-	}
-
-	return nil
-}
-
 func (c *Container) mountSHM(shmOptions string) error {
 	if err := unix.Mount("shm", c.config.ShmDir, "tmpfs", unix.MS_NOEXEC|unix.MS_NOSUID|unix.MS_NODEV,
 		label.FormatMountLabel(shmOptions, c.config.MountLabel)); err != nil {
diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go
index 2128b7481..2be7ee07c 100644
--- a/libpod/oci_linux.go
+++ b/libpod/oci_linux.go
@@ -28,7 +28,7 @@ func (r *OCIRuntime) moveConmonToCgroup(ctr *Container, cgroupParent string, cmd
 				logrus.Warnf("Failed to add conmon to systemd sandbox cgroup: %v", err)
 			}
 		} else {
-			cgroupPath := filepath.Join(ctr.config.CgroupParent, fmt.Sprintf("libpod-%s", ctr.ID()), "conmon")
+			cgroupPath := filepath.Join(ctr.config.CgroupParent, "conmon")
 			control, err := cgroups.New(cgroups.V1, cgroups.StaticPath(cgroupPath), &spec.LinuxResources{})
 			if err != nil {
 				logrus.Warnf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err)