From ba0a6f34e349bfb5cf70cb2062522e6af1b9578c Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 14 Jan 2020 15:05:12 +0100 Subject: podman: add new option --cgroups=no-conmon it allows to disable cgroups creation only for the conmon process. A new cgroup is created for the container payload. Signed-off-by: Giuseppe Scrivano --- cmd/podman/common.go | 2 +- docs/source/markdown/podman-create.1.md | 3 ++- docs/source/markdown/podman-run.1.md | 3 ++- libpod/container.go | 5 ++++- libpod/oci_conmon_linux.go | 8 +++++++- libpod/options.go | 21 +++++++++++---------- pkg/spec/namespaces.go | 4 ++-- pkg/spec/spec.go | 4 ++-- 8 files changed, 31 insertions(+), 19 deletions(-) diff --git a/cmd/podman/common.go b/cmd/podman/common.go index 8690be64f..f3aff1d49 100644 --- a/cmd/podman/common.go +++ b/cmd/podman/common.go @@ -158,7 +158,7 @@ func getCreateFlags(c *cliconfig.PodmanCommand) { ) createFlags.String( "cgroups", "enabled", - "control container cgroup configuration", + `control container cgroup configuration ("enabled"|"disabled"|"no-conmon")`, ) createFlags.String( "cgroup-parent", "", diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index 0236e30ba..7f0c2260c 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -78,8 +78,9 @@ If the host uses cgroups v1, the default is set to **host**. On cgroups v2 the **--cgroups**=*mode* Determines whether the container will create CGroups. -Valid values are *enabled* and *disabled*, which the default being *enabled*. +Valid values are *enabled*, *disabled*, *no-conmon*, which the default being *enabled*. The *disabled* option will force the container to not create CGroups, and thus conflicts with CGroup options (**--cgroupns** and **--cgroup-parent**). +The *no-conmon* option disables a new CGroup only for the conmon process. **--cgroup-parent**=*path* diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index f613e6668..2b1452b53 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -92,8 +92,9 @@ If the host uses cgroups v1, the default is set to **host**. On cgroups v2 the **--cgroups**=*mode* Determines whether the container will create CGroups. -Valid values are *enabled* and *disabled*, which the default being *enabled*. +Valid values are *enabled*, *disabled*, *no-conmon*, which the default being *enabled*. The *disabled* option will force the container to not create CGroups, and thus conflicts with CGroup options (**--cgroupns** and **--cgroup-parent**). +The *no-conmon* option disables a new CGroup only for the conmon process. **--cgroup-parent**=*cgroup* diff --git a/libpod/container.go b/libpod/container.go index b3cb6334a..f29cebf20 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -373,8 +373,11 @@ type ContainerConfig struct { // Time container was created CreatedTime time.Time `json:"createdTime"` // NoCgroups indicates that the container will not create CGroups. It is - // incompatible with CgroupParent. + // incompatible with CgroupParent. Deprecated in favor of CgroupsMode. NoCgroups bool `json:"noCgroups,omitempty"` + // CgroupsMode indicates how the container will create cgroups + // (disabled, no-conmon, enabled). It supersedes NoCgroups. + CgroupsMode string `json:"cgroupsMode,omitempty"` // Cgroup parent of the container CgroupParent string `json:"cgroupParent"` // LogPath log location diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 5ab0e73c4..7c7ec8b2c 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1297,11 +1297,17 @@ func startCommandGivenSelinux(cmd *exec.Cmd) error { // it then signals for conmon to start by sending nonse data down the start fd func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec.Cmd, startFd *os.File) error { mustCreateCgroup := true - // If cgroup creation is disabled - just signal. + if ctr.config.NoCgroups { mustCreateCgroup = false } + // If cgroup creation is disabled - just signal. + switch ctr.config.CgroupsMode { + case "disabled", "no-conmon": + mustCreateCgroup = false + } + if mustCreateCgroup { cgroupParent := ctr.CgroupParent() if r.cgroupManager == define.SystemdCgroupsManager { diff --git a/libpod/options.go b/libpod/options.go index 8bc5a541d..593037382 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1078,25 +1078,26 @@ func WithLogTag(tag string) CtrCreateOption { } -// WithNoCgroups disables the creation of CGroups for the new container. -func WithNoCgroups() CtrCreateOption { +// WithCgroupsMode disables the creation of CGroups for the conmon process. +func WithCgroupsMode(mode string) CtrCreateOption { return func(ctr *Container) error { if ctr.valid { return define.ErrCtrFinalized } - if ctr.config.CgroupParent != "" { - return errors.Wrapf(define.ErrInvalidArg, "NoCgroups conflicts with CgroupParent") - } - - if ctr.config.PIDNsCtr != "" { - return errors.Wrapf(define.ErrInvalidArg, "NoCgroups requires a private PID namespace and cannot be used when PID namespace is shared with another container") + switch mode { + case "disabled": + ctr.config.NoCgroups = true + ctr.config.CgroupsMode = mode + case "enabled", "no-conmon": + ctr.config.CgroupsMode = mode + default: + return errors.Wrapf(define.ErrInvalidArg, "Invalid cgroup mode %q", mode) } - ctr.config.NoCgroups = true - return nil } + } // WithCgroupParent sets the Cgroup Parent of the new container. diff --git a/pkg/spec/namespaces.go b/pkg/spec/namespaces.go index e62d4ed0a..1f98e6e25 100644 --- a/pkg/spec/namespaces.go +++ b/pkg/spec/namespaces.go @@ -213,8 +213,8 @@ func (c *CgroupConfig) ToCreateOptions(runtime *libpod.Runtime) ([]libpod.CtrCre options = append(options, libpod.WithCgroupParent(c.CgroupParent)) } - if c.Cgroups == "disabled" { - options = append(options, libpod.WithNoCgroups()) + if c.Cgroups != "" { + options = append(options, libpod.WithCgroupsMode(c.Cgroups)) } return options, nil diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 7a220012f..cae055bb0 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -358,10 +358,10 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM return nil, errors.New("cannot specify resource limits when cgroups are disabled is specified") } configSpec.Linux.Resources = &spec.LinuxResources{} - case "enabled", "": + case "enabled", "no-conmon", "": // Do nothing default: - return nil, errors.New("unrecognized option for cgroups; supported are 'default' and 'disabled'") + return nil, errors.New("unrecognized option for cgroups; supported are 'default', 'disabled', 'no-conmon'") } // Add annotations -- cgit v1.2.3-54-g00ecf From 70236f56f9e8f2044e94800de7be3925f52d6c83 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 14 Jan 2020 15:17:16 +0100 Subject: systemdgen: specify --cgroups=disabled-conmon for --new Signed-off-by: Giuseppe Scrivano --- pkg/systemdgen/systemdgen.go | 5 +++-- pkg/systemdgen/systemdgen_test.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/systemdgen/systemdgen.go b/pkg/systemdgen/systemdgen.go index 745b708e4..26b3b3756 100644 --- a/pkg/systemdgen/systemdgen.go +++ b/pkg/systemdgen/systemdgen.go @@ -153,13 +153,14 @@ func CreateContainerSystemdUnit(info *ContainerInfo, opts Options) (string, erro if len(info.CreateCommand) < index+1 { return "", errors.Errorf("container's create command is too short or invalid: %v", info.CreateCommand) } - // We're hard-coding the first four arguments and append the - // CreatCommand with a stripped command and subcomand. + // We're hard-coding the first five arguments and append the + // CreateCommand with a stripped command and subcomand. command := []string{ info.Executable, "run", "--conmon-pidfile", "/%t/%n-pid", "--cidfile", "/%t/%n-cid", + "--cgroups=no-conmon", } command = append(command, info.CreateCommand[index:]...) info.RunCommand = strings.Join(command, " ") diff --git a/pkg/systemdgen/systemdgen_test.go b/pkg/systemdgen/systemdgen_test.go index 9c6933d17..ee2429407 100644 --- a/pkg/systemdgen/systemdgen_test.go +++ b/pkg/systemdgen/systemdgen_test.go @@ -122,7 +122,7 @@ Documentation=man:podman-generate-systemd(1) [Service] Restart=always ExecStartPre=/usr/bin/rm -f /%t/%n-pid /%t/%n-cid -ExecStart=/usr/bin/podman run --conmon-pidfile /%t/%n-pid --cidfile /%t/%n-cid --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStart=/usr/bin/podman run --conmon-pidfile /%t/%n-pid --cidfile /%t/%n-cid --cgroups=no-conmon --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN ExecStop=/usr/bin/podman stop --ignore --cidfile /%t/%n-cid -t 42 ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile /%t/%n-cid PIDFile=/%t/%n-pid -- cgit v1.2.3-54-g00ecf From 1951ff168a63157fa2f4711fde283edfc4981ed3 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Tue, 14 Jan 2020 15:34:02 +0100 Subject: oci_conmon: do not create a cgroup under systemd Detect whether we are running under systemd (if the INVOCATION_ID is set). If Podman is running under a systemd service, we do not need to create a cgroup for conmon. Signed-off-by: Giuseppe Scrivano --- libpod/oci_conmon_linux.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 7c7ec8b2c..7cd42f9ab 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1308,6 +1308,11 @@ func (r *ConmonOCIRuntime) moveConmonToCgroupAndSignal(ctr *Container, cmd *exec mustCreateCgroup = false } + // $INVOCATION_ID is set by systemd when running as a service. + if os.Getenv("INVOCATION_ID") != "" { + mustCreateCgroup = false + } + if mustCreateCgroup { cgroupParent := ctr.CgroupParent() if r.cgroupManager == define.SystemdCgroupsManager { -- cgit v1.2.3-54-g00ecf