From fa9728c5509f1ef3bb1c80055e89b910d9740efd Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 16 Sep 2021 12:08:30 +0200 Subject: [PATCH 1/4] system: avoid reading pause pid file we already know the path to the pause PID file, no need to calculate it again. Signed-off-by: Giuseppe Scrivano --- pkg/domain/infra/abi/system.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index bc98edd062..0f7492354b 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/containers/common/pkg/config" - "github.com/containers/podman/v3/libpod" "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/cgroups" "github.com/containers/podman/v3/pkg/domain/entities" @@ -121,7 +120,7 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) became, ret, err = rootless.TryJoinFromFilePaths(pausePidPath, true, paths) - if err := movePauseProcessToScope(ic.Libpod); err != nil { + if err := movePauseProcessToScope(pausePidPath); err != nil { conf, err2 := ic.Config(context.Background()) if err2 != nil { return err @@ -142,15 +141,7 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) return nil } -func movePauseProcessToScope(r *libpod.Runtime) error { - tmpDir, err := r.TmpDir() - if err != nil { - return err - } - pausePidPath, err := util.GetRootlessPauseProcessPidPathGivenDir(tmpDir) - if err != nil { - return errors.Wrapf(err, "could not get pause process pid file path") - } +func movePauseProcessToScope(pausePidPath string) error { data, err := ioutil.ReadFile(pausePidPath) if err != nil { return errors.Wrapf(err, "cannot read pause pid file") From 9c1e27fdd536f6026efe3da4360755a3e9135ca8 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 16 Sep 2021 12:14:02 +0200 Subject: [PATCH 2/4] system: always move pause process when running on systemd when running on a systemd with systemd, always try to move the pause process to its own scope. Signed-off-by: Giuseppe Scrivano --- pkg/domain/infra/abi/system.go | 13 ++----------- utils/utils.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 0f7492354b..7140618bb1 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -9,7 +9,6 @@ import ( "os/exec" "path/filepath" "strconv" - "strings" "github.com/containers/common/pkg/config" "github.com/containers/podman/v3/libpod/define" @@ -71,11 +70,7 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) if err != nil { return err } - - initCommand, err := ioutil.ReadFile("/proc/1/comm") - // On errors, default to systemd - runsUnderSystemd := err != nil || strings.TrimRight(string(initCommand), "\n") == "systemd" - + runsUnderSystemd := utils.RunsOnSystemd() unitName := fmt.Sprintf("podman-%d.scope", os.Getpid()) if runsUnderSystemd || conf.Engine.CgroupManager == config.SystemdCgroupsManager { if err := utils.RunUnderSystemdScope(os.Getpid(), "user.slice", unitName); err != nil { @@ -121,11 +116,7 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) became, ret, err = rootless.TryJoinFromFilePaths(pausePidPath, true, paths) if err := movePauseProcessToScope(pausePidPath); err != nil { - conf, err2 := ic.Config(context.Background()) - if err2 != nil { - return err - } - if conf.Engine.CgroupManager == config.SystemdCgroupsManager { + if utils.RunsOnSystemd() { logrus.Warnf("Failed to add pause process to systemd sandbox cgroup: %v", err) } else { logrus.Debugf("Failed to add pause process to systemd sandbox cgroup: %v", err) diff --git a/utils/utils.go b/utils/utils.go index 2e415130ee..e2760d2259 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -4,10 +4,12 @@ import ( "bytes" "fmt" "io" + "io/ioutil" "os" "os/exec" "strconv" "strings" + "sync" "github.com/containers/podman/v3/libpod/define" "github.com/containers/storage/pkg/archive" @@ -155,3 +157,18 @@ func RemoveScientificNotationFromFloat(x float64) (float64, error) { } return result, nil } + +var ( + runsOnSystemdOnce sync.Once + runsOnSystemd bool +) + +// RunsOnSystemd returns whether the system is using systemd +func RunsOnSystemd() bool { + runsOnSystemdOnce.Do(func() { + initCommand, err := ioutil.ReadFile("/proc/1/comm") + // On errors, default to systemd + runsOnSystemd = err != nil || strings.TrimRight(string(initCommand), "\n") == "systemd" + }) + return runsOnSystemd +} From 72534a74b3c2ff35ae1711a890406a6bce5fa44f Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 16 Sep 2021 12:44:45 +0200 Subject: [PATCH 3/4] system: move MovePauseProcessToScope to utils Signed-off-by: Giuseppe Scrivano --- pkg/domain/infra/abi/system.go | 24 +----------------------- utils/utils.go | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 7140618bb1..e326f26a85 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -3,12 +3,10 @@ package abi import ( "context" "fmt" - "io/ioutil" "net/url" "os" "os/exec" "path/filepath" - "strconv" "github.com/containers/common/pkg/config" "github.com/containers/podman/v3/libpod/define" @@ -114,14 +112,7 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) } became, ret, err = rootless.TryJoinFromFilePaths(pausePidPath, true, paths) - - if err := movePauseProcessToScope(pausePidPath); err != nil { - if utils.RunsOnSystemd() { - logrus.Warnf("Failed to add pause process to systemd sandbox cgroup: %v", err) - } else { - logrus.Debugf("Failed to add pause process to systemd sandbox cgroup: %v", err) - } - } + utils.MovePauseProcessToScope(pausePidPath) if err != nil { logrus.Error(errors.Wrapf(err, "invalid internal status, try resetting the pause process with %q", os.Args[0]+" system migrate")) os.Exit(1) @@ -132,19 +123,6 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, noMoveProcess bool) return nil } -func movePauseProcessToScope(pausePidPath string) error { - data, err := ioutil.ReadFile(pausePidPath) - if err != nil { - return errors.Wrapf(err, "cannot read pause pid file") - } - pid, err := strconv.ParseUint(string(data), 10, 0) - if err != nil { - return errors.Wrapf(err, "cannot parse pid file %s", pausePidPath) - } - - return utils.RunUnderSystemdScope(int(pid), "user.slice", "podman-pause.scope") -} - // SystemPrune removes unused data from the system. Pruning pods, containers, volumes and images. func (ic *ContainerEngine) SystemPrune(ctx context.Context, options entities.SystemPruneOptions) (*entities.SystemPruneReport, error) { var systemPruneReport = new(entities.SystemPruneReport) diff --git a/utils/utils.go b/utils/utils.go index e2760d2259..185ac48652 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -172,3 +172,28 @@ func RunsOnSystemd() bool { }) return runsOnSystemd } + +func moveProcessToScope(pidPath, slice, scope string) error { + data, err := ioutil.ReadFile(pidPath) + if err != nil { + return errors.Wrapf(err, "cannot read pid file %s", pidPath) + } + pid, err := strconv.ParseUint(string(data), 10, 0) + if err != nil { + return errors.Wrapf(err, "cannot parse pid file %s", pidPath) + } + return RunUnderSystemdScope(int(pid), slice, scope) +} + +// MovePauseProcessToScope moves the pause process used for rootless mode to keep the namespaces alive to +// a separate scope. +func MovePauseProcessToScope(pausePidPath string) { + err := moveProcessToScope(pausePidPath, "user.slice", "podman-pause.scope") + if err != nil { + if RunsOnSystemd() { + logrus.Warnf("Failed to add pause process to systemd sandbox cgroup: %v", err) + } else { + logrus.Debugf("Failed to add pause process to systemd sandbox cgroup: %v", err) + } + } +} From a2c8b5d9d6d6e46679fe9540619d4303d4b4601d Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 16 Sep 2021 12:46:21 +0200 Subject: [PATCH 4/4] runtime: move pause process to scope make sure the pause process is moved to its own scope as well as what we do when we join an existing user+mount namespace. Closes: https://github.com/containers/podman/issues/11560 [NO TESTS NEEDED] Signed-off-by: Giuseppe Scrivano --- libpod/runtime.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libpod/runtime.go b/libpod/runtime.go index d2b3d36da9..a2279e56dc 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -35,6 +35,7 @@ import ( "github.com/containers/podman/v3/pkg/rootless" "github.com/containers/podman/v3/pkg/systemd" "github.com/containers/podman/v3/pkg/util" + "github.com/containers/podman/v3/utils" "github.com/containers/storage" "github.com/containers/storage/pkg/unshare" "github.com/docker/docker/pkg/namesgenerator" @@ -543,6 +544,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) { return err } if became { + utils.MovePauseProcessToScope(pausePid) os.Exit(ret) } }