From 9087f2e827d971ad2e5b5b6a57048a3d211776ee Mon Sep 17 00:00:00 2001 From: lifubang Date: Thu, 23 Apr 2020 21:33:47 +0800 Subject: [PATCH] fix path error in systemd when stopped When we use cgroup with systemd driver, the cgroup path will be auto removed by systemd when all processes exited. So we should check cgroup path exists when we access the cgroup path, for example in `kill/ps`, or else we will got an error. Signed-off-by: lifubang --- libcontainer/cgroups/cgroups.go | 3 ++ libcontainer/cgroups/fs/apply_raw.go | 4 +++ libcontainer/cgroups/fs2/fs2.go | 4 +++ libcontainer/cgroups/systemd/unsupported.go | 4 +++ libcontainer/cgroups/systemd/v1.go | 4 +++ libcontainer/cgroups/systemd/v2.go | 4 +++ libcontainer/container_linux.go | 22 +++++++++++--- libcontainer/container_linux_test.go | 33 ++++++++++++++++++--- 8 files changed, 70 insertions(+), 8 deletions(-) diff --git a/libcontainer/cgroups/cgroups.go b/libcontainer/cgroups/cgroups.go index 2bb2d1e81..071f8f2e3 100644 --- a/libcontainer/cgroups/cgroups.go +++ b/libcontainer/cgroups/cgroups.go @@ -47,6 +47,9 @@ type Manager interface { // GetFreezerState retrieves the current FreezerState of the cgroup. GetFreezerState() (configs.FreezerState, error) + + // Whether the cgroup path exists or not + Exists() bool } type NotFoundError struct { diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index b5bace369..5ccf4000d 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -392,3 +392,7 @@ func (m *manager) GetFreezerState() (configs.FreezerState, error) { } return freezer.(*FreezerGroup).GetState(dir) } + +func (m *manager) Exists() bool { + return cgroups.PathExists(m.paths["devices"]) +} diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go index 69b9948a4..7be26211a 100644 --- a/libcontainer/cgroups/fs2/fs2.go +++ b/libcontainer/cgroups/fs2/fs2.go @@ -262,3 +262,7 @@ func (m *manager) GetCgroups() (*configs.Cgroup, error) { func (m *manager) GetFreezerState() (configs.FreezerState, error) { return getFreezer(m.dirPath) } + +func (m *manager) Exists() bool { + return cgroups.PathExists(m.dirPath) +} diff --git a/libcontainer/cgroups/systemd/unsupported.go b/libcontainer/cgroups/systemd/unsupported.go index 46d3becaa..cb4d00c88 100644 --- a/libcontainer/cgroups/systemd/unsupported.go +++ b/libcontainer/cgroups/systemd/unsupported.go @@ -65,3 +65,7 @@ func Freeze(c *configs.Cgroup, state configs.FreezerState) error { func (m *Manager) GetCgroups() (*configs.Cgroup, error) { return nil, errors.New("Systemd not supported") } + +func (m *Manager) Exists() bool { + return false +} diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index bd54342ac..c2fa57c5a 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -481,3 +481,7 @@ func (m *legacyManager) GetFreezerState() (configs.FreezerState, error) { } return freezer.(*fs.FreezerGroup).GetState(path) } + +func (m *legacyManager) Exists() bool { + return cgroups.PathExists(m.paths["devices"]) +} diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index e78de667a..cbcc15a1d 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -370,3 +370,7 @@ func (m *unifiedManager) GetFreezerState() (configs.FreezerState, error) { } return fsMgr.GetFreezerState() } + +func (m *unifiedManager) Exists() bool { + return cgroups.PathExists(m.path) +} diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 10e684b82..6f1c56aee 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -169,7 +169,17 @@ func (c *linuxContainer) OCIState() (*specs.State, error) { } func (c *linuxContainer) Processes() ([]int, error) { - pids, err := c.cgroupManager.GetAllPids() + var pids []int + status, err := c.currentStatus() + if err != nil { + return pids, err + } + // for systemd cgroup, the unit's cgroup path will be auto removed if container's all processes exited + if status == Stopped && !c.cgroupManager.Exists() { + return pids, nil + } + + pids, err = c.cgroupManager.GetAllPids() if err != nil { return nil, newSystemErrorWithCause(err, "getting all container pids from cgroups") } @@ -385,13 +395,17 @@ func (c *linuxContainer) start(process *Process) error { func (c *linuxContainer) Signal(s os.Signal, all bool) error { c.m.Lock() defer c.m.Unlock() - if all { - return signalAllProcesses(c.cgroupManager, s) - } status, err := c.currentStatus() if err != nil { return err } + if all { + // for systemd cgroup, the unit's cgroup path will be auto removed if container's all processes exited + if status == Stopped && !c.cgroupManager.Exists() { + return nil + } + return signalAllProcesses(c.cgroupManager, s) + } // to avoid a PID reuse attack if status == Running || status == Created || status == Paused { if err := c.initProcess.signal(s); err != nil { diff --git a/libcontainer/container_linux_test.go b/libcontainer/container_linux_test.go index 72f0835c0..187517330 100644 --- a/libcontainer/container_linux_test.go +++ b/libcontainer/container_linux_test.go @@ -50,6 +50,15 @@ func (m *mockCgroupManager) Destroy() error { return nil } +func (m *mockCgroupManager) Exists() bool { + paths := m.GetPaths() + if paths != nil { + _, err := os.Lstat(paths["devices"]) + return err == nil + } + return false +} + func (m *mockCgroupManager) GetPaths() map[string]string { return m.paths } @@ -134,11 +143,27 @@ func (m *mockProcess) forwardChildLogs() { } func TestGetContainerPids(t *testing.T) { - container := &linuxContainer{ - id: "myid", - config: &configs.Config{}, - cgroupManager: &mockCgroupManager{allPids: []int{1, 2, 3}}, + pid := 1 + stat, err := system.Stat(pid) + if err != nil { + t.Fatalf("can't stat pid %d, got %v", pid, err) } + container := &linuxContainer{ + id: "myid", + config: &configs.Config{}, + cgroupManager: &mockCgroupManager{ + allPids: []int{1, 2, 3}, + paths: map[string]string{ + "device": "/proc/self/cgroups", + }, + }, + initProcess: &mockProcess{ + _pid: 1, + started: 10, + }, + initProcessStartTime: stat.StartTime, + } + container.state = &runningState{c: container} pids, err := container.Processes() if err != nil { t.Fatal(err)