Store cgroup manager on a per-container basis

When we create a container, we assign a cgroup parent based on
the current cgroup manager in use. This parent is only usable
with the cgroup manager the container is created with, so if the
default cgroup manager is later changed or overridden, the
container will not be able to start.

To solve this, store the cgroup manager that created the
container in container configuration, so we can guarantee a
container with a systemd cgroup parent will always be started
with systemd cgroups.

Unfortunately, this is very difficult to test in CI, due to the
fact that we hard-code cgroup manager on all invocations of
Podman in CI.

Fixes #7830

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This commit is contained in:
Matthew Heon 2020-10-08 15:25:06 -04:00
parent 59b5f0ac32
commit 4d800a5f45
7 changed files with 34 additions and 16 deletions

View File

@ -888,9 +888,22 @@ func (c *Container) NamespacePath(linuxNS LinuxNS) (string, error) { //nolint:in
return fmt.Sprintf("/proc/%d/ns/%s", c.state.PID, linuxNS.String()), nil return fmt.Sprintf("/proc/%d/ns/%s", c.state.PID, linuxNS.String()), nil
} }
// CgroupManager returns the cgroup manager used by the given container.
func (c *Container) CgroupManager() string {
cgroupManager := c.config.CgroupManager
if cgroupManager == "" {
cgroupManager = c.runtime.config.Engine.CgroupManager
}
return cgroupManager
}
// CGroupPath returns a cgroups "path" for a given container. // CGroupPath returns a cgroups "path" for a given container.
func (c *Container) CGroupPath() (string, error) { func (c *Container) CGroupPath() (string, error) {
cgroupManager := c.CgroupManager()
switch { switch {
case c.config.NoCgroups || c.config.CgroupsMode == "disabled":
return "", errors.Wrapf(define.ErrNoCgroups, "this container is not creating cgroups")
case c.config.CgroupsMode == cgroupSplit: case c.config.CgroupsMode == cgroupSplit:
if c.config.CgroupParent != "" { if c.config.CgroupParent != "" {
return "", errors.Errorf("cannot specify cgroup-parent with cgroup-mode %q", cgroupSplit) return "", errors.Errorf("cannot specify cgroup-parent with cgroup-mode %q", cgroupSplit)
@ -906,9 +919,9 @@ func (c *Container) CGroupPath() (string, error) {
return "", errors.Errorf("invalid cgroup for conmon %q", cg) return "", errors.Errorf("invalid cgroup for conmon %q", cg)
} }
return strings.TrimSuffix(cg, "/supervisor") + "/container", nil return strings.TrimSuffix(cg, "/supervisor") + "/container", nil
case c.runtime.config.Engine.CgroupManager == config.CgroupfsCgroupsManager: case cgroupManager == config.CgroupfsCgroupsManager:
return filepath.Join(c.config.CgroupParent, fmt.Sprintf("libpod-%s", c.ID())), nil return filepath.Join(c.config.CgroupParent, fmt.Sprintf("libpod-%s", c.ID())), nil
case c.runtime.config.Engine.CgroupManager == config.SystemdCgroupsManager: case cgroupManager == config.SystemdCgroupsManager:
if rootless.IsRootless() { if rootless.IsRootless() {
uid := rootless.GetRootlessUID() uid := rootless.GetRootlessUID()
parts := strings.SplitN(c.config.CgroupParent, "/", 2) parts := strings.SplitN(c.config.CgroupParent, "/", 2)
@ -922,7 +935,7 @@ func (c *Container) CGroupPath() (string, error) {
} }
return filepath.Join(c.config.CgroupParent, createUnitName("libpod", c.ID())), nil return filepath.Join(c.config.CgroupParent, createUnitName("libpod", c.ID())), nil
default: default:
return "", errors.Wrapf(define.ErrInvalidArg, "unsupported CGroup manager %s in use", c.runtime.config.Engine.CgroupManager) return "", errors.Wrapf(define.ErrInvalidArg, "unsupported CGroup manager %s in use", cgroupManager)
} }
} }

View File

@ -275,13 +275,16 @@ type ContainerMiscConfig struct {
StopTimeout uint `json:"stopTimeout,omitempty"` StopTimeout uint `json:"stopTimeout,omitempty"`
// Time container was created // Time container was created
CreatedTime time.Time `json:"createdTime"` CreatedTime time.Time `json:"createdTime"`
// CgroupManager is the cgroup manager used to create this container.
// If empty, the runtime default will be used.
CgroupManager string `json:"cgroupManager,omitempty"`
// NoCgroups indicates that the container will not create CGroups. It is // NoCgroups indicates that the container will not create CGroups. It is
// incompatible with CgroupParent. Deprecated in favor of CgroupsMode. // incompatible with CgroupParent. Deprecated in favor of CgroupsMode.
NoCgroups bool `json:"noCgroups,omitempty"` NoCgroups bool `json:"noCgroups,omitempty"`
// CgroupsMode indicates how the container will create cgroups // CgroupsMode indicates how the container will create cgroups
// (disabled, no-conmon, enabled). It supersedes NoCgroups. // (disabled, no-conmon, enabled). It supersedes NoCgroups.
CgroupsMode string `json:"cgroupsMode,omitempty"` CgroupsMode string `json:"cgroupsMode,omitempty"`
// Cgroup parent of the container // Cgroup parent of the container.
CgroupParent string `json:"cgroupParent"` CgroupParent string `json:"cgroupParent"`
// LogPath log location // LogPath log location
LogPath string `json:"logPath"` LogPath string `json:"logPath"`

View File

@ -729,7 +729,7 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
// CGroup parent // CGroup parent
// Need to check if it's the default, and not print if so. // Need to check if it's the default, and not print if so.
defaultCgroupParent := "" defaultCgroupParent := ""
switch c.runtime.config.Engine.CgroupManager { switch c.CgroupManager() {
case config.CgroupfsCgroupsManager: case config.CgroupfsCgroupsManager:
defaultCgroupParent = CgroupfsDefaultCgroupParent defaultCgroupParent = CgroupfsDefaultCgroupParent
case config.SystemdCgroupsManager: case config.SystemdCgroupsManager:
@ -738,6 +738,7 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
if c.config.CgroupParent != defaultCgroupParent { if c.config.CgroupParent != defaultCgroupParent {
hostConfig.CgroupParent = c.config.CgroupParent hostConfig.CgroupParent = c.config.CgroupParent
} }
hostConfig.CgroupManager = c.CgroupManager()
// PID namespace mode // PID namespace mode
pidMode := "" pidMode := ""

View File

@ -1965,6 +1965,7 @@ func (c *Container) getOCICgroupPath() (string, error) {
if err != nil { if err != nil {
return "", err return "", err
} }
cgroupManager := c.CgroupManager()
switch { switch {
case (rootless.IsRootless() && !unified) || c.config.NoCgroups: case (rootless.IsRootless() && !unified) || c.config.NoCgroups:
return "", nil return "", nil
@ -1977,14 +1978,14 @@ func (c *Container) getOCICgroupPath() (string, error) {
return "", err return "", err
} }
return filepath.Join(selfCgroup, "container"), nil return filepath.Join(selfCgroup, "container"), nil
case c.runtime.config.Engine.CgroupManager == config.SystemdCgroupsManager: case cgroupManager == config.SystemdCgroupsManager:
// When the OCI runtime is set to use Systemd as a cgroup manager, it // When the OCI runtime is set to use Systemd as a cgroup manager, it
// expects cgroups to be passed as follows: // expects cgroups to be passed as follows:
// slice:prefix:name // slice:prefix:name
systemdCgroups := fmt.Sprintf("%s:libpod:%s", path.Base(c.config.CgroupParent), c.ID()) systemdCgroups := fmt.Sprintf("%s:libpod:%s", path.Base(c.config.CgroupParent), c.ID())
logrus.Debugf("Setting CGroups for container %s to %s", c.ID(), systemdCgroups) logrus.Debugf("Setting CGroups for container %s to %s", c.ID(), systemdCgroups)
return systemdCgroups, nil return systemdCgroups, nil
case c.runtime.config.Engine.CgroupManager == config.CgroupfsCgroupsManager: case cgroupManager == config.CgroupfsCgroupsManager:
cgroupPath, err := c.CGroupPath() cgroupPath, err := c.CGroupPath()
if err != nil { if err != nil {
return "", err return "", err
@ -1992,7 +1993,7 @@ func (c *Container) getOCICgroupPath() (string, error) {
logrus.Debugf("Setting CGroup path for container %s to %s", c.ID(), cgroupPath) logrus.Debugf("Setting CGroup path for container %s to %s", c.ID(), cgroupPath)
return cgroupPath, nil return cgroupPath, nil
default: default:
return "", errors.Wrapf(define.ErrInvalidArg, "invalid cgroup manager %s requested", c.runtime.config.Engine.CgroupManager) return "", errors.Wrapf(define.ErrInvalidArg, "invalid cgroup manager %s requested", cgroupManager)
} }
} }

View File

@ -236,6 +236,9 @@ type InspectContainerHostConfig struct {
// include a Mounts field in inspect. // include a Mounts field in inspect.
// Format: <src>:<destination>[:<comma-separated options>] // Format: <src>:<destination>[:<comma-separated options>]
Binds []string `json:"Binds"` Binds []string `json:"Binds"`
// CgroupManager is the cgroup manager used by the container.
// At present, allowed values are either "cgroupfs" or "systemd".
CgroupManager string `json:"CgroupManager,omitempty"`
// CgroupMode is the configuration of the container's cgroup namespace. // CgroupMode is the configuration of the container's cgroup namespace.
// Populated as follows: // Populated as follows:
// private - a cgroup namespace has been created // private - a cgroup namespace has been created

View File

@ -57,7 +57,6 @@ type ConmonOCIRuntime struct {
path string path string
conmonPath string conmonPath string
conmonEnv []string conmonEnv []string
cgroupManager string
tmpDir string tmpDir string
exitsDir string exitsDir string
socketsDir string socketsDir string
@ -102,7 +101,6 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
runtime.runtimeFlags = runtimeFlags runtime.runtimeFlags = runtimeFlags
runtime.conmonEnv = runtimeCfg.Engine.ConmonEnvVars runtime.conmonEnv = runtimeCfg.Engine.ConmonEnvVars
runtime.cgroupManager = runtimeCfg.Engine.CgroupManager
runtime.tmpDir = runtimeCfg.Engine.TmpDir runtime.tmpDir = runtimeCfg.Engine.TmpDir
runtime.logSizeMax = runtimeCfg.Containers.LogSizeMax runtime.logSizeMax = runtimeCfg.Containers.LogSizeMax
runtime.noPivot = runtimeCfg.Engine.NoPivotRoot runtime.noPivot = runtimeCfg.Engine.NoPivotRoot
@ -149,10 +147,6 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
runtime.exitsDir = filepath.Join(runtime.tmpDir, "exits") runtime.exitsDir = filepath.Join(runtime.tmpDir, "exits")
runtime.socketsDir = filepath.Join(runtime.tmpDir, "socket") runtime.socketsDir = filepath.Join(runtime.tmpDir, "socket")
if runtime.cgroupManager != config.CgroupfsCgroupsManager && runtime.cgroupManager != config.SystemdCgroupsManager {
return nil, errors.Wrapf(define.ErrInvalidArg, "invalid cgroup manager specified: %s", runtime.cgroupManager)
}
// Create the exit files and attach sockets directories // Create the exit files and attach sockets directories
if err := os.MkdirAll(runtime.exitsDir, 0750); err != nil { if err := os.MkdirAll(runtime.exitsDir, 0750); err != nil {
// The directory is allowed to exist // The directory is allowed to exist
@ -1325,7 +1319,7 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
args = append(args, rFlags...) args = append(args, rFlags...)
} }
if r.cgroupManager == config.SystemdCgroupsManager && !ctr.config.NoCgroups && ctr.config.CgroupsMode != cgroupSplit { if ctr.CgroupManager() == config.SystemdCgroupsManager && !ctr.config.NoCgroups && ctr.config.CgroupsMode != cgroupSplit {
args = append(args, "-s") args = append(args, "-s")
} }
@ -1442,8 +1436,10 @@ func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec
} }
if mustCreateCgroup { if mustCreateCgroup {
// TODO: This should be a switch - we are not guaranteed that
// there are only 2 valid cgroup managers
cgroupParent := ctr.CgroupParent() cgroupParent := ctr.CgroupParent()
if r.cgroupManager == config.SystemdCgroupsManager { if ctr.CgroupManager() == config.SystemdCgroupsManager {
unitName := createUnitName("libpod-conmon", ctr.ID()) unitName := createUnitName("libpod-conmon", ctr.ID())
realCgroupParent := cgroupParent realCgroupParent := cgroupParent

View File

@ -208,6 +208,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
// Check CGroup parent sanity, and set it if it was not set. // Check CGroup parent sanity, and set it if it was not set.
// Only if we're actually configuring CGroups. // Only if we're actually configuring CGroups.
if !ctr.config.NoCgroups { if !ctr.config.NoCgroups {
ctr.config.CgroupManager = r.config.Engine.CgroupManager
switch r.config.Engine.CgroupManager { switch r.config.Engine.CgroupManager {
case config.CgroupfsCgroupsManager: case config.CgroupfsCgroupsManager:
if ctr.config.CgroupParent == "" { if ctr.config.CgroupParent == "" {