Merge pull request #2338 from lifubang/systemdcgroupv2

fix path error in systemd when stopped

LGTMs: @mrunalp @AkihiroSuda

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 {

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"])
+}

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)
+}

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
+}

libcontainer/cgroups/systemd/v1.go

@@ -463,3 +463,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"])
+}

libcontainer/cgroups/systemd/v2.go

@@ -352,3 +352,7 @@ func (m *unifiedManager) GetFreezerState() (configs.FreezerState, error) {
 	}
 	return fsMgr.GetFreezerState()
 }
+
+func (m *unifiedManager) Exists() bool {
+	return cgroups.PathExists(m.path)
+}

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 {

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)

tests/integration/kill.bats

@@ -25,6 +25,10 @@ function teardown() {
 
   retry 10 1 eval "__runc state test_busybox | grep -q 'stopped'"
 
+  # we should ensure kill work after the container stopped
+  runc kill -a test_busybox 0
+  [ "$status" -eq 0 ]
+
   runc delete test_busybox
   [ "$status" -eq 0 ]
 }

tests/integration/ps.bats

@@ -60,3 +60,27 @@ function teardown() {
   [[ ${lines[0]} =~ \ +PID\ +TTY\ +STAT\ +TIME\ +COMMAND+ ]]
   [[ "${lines[1]}" =~ [0-9]+ ]]
 }
+
+@test "ps after the container stopped" {
+  # ps requires cgroups
+  [[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
+  set_cgroups_path "$BUSYBOX_BUNDLE"
+
+  # start busybox detached
+  runc run -d --console-socket $CONSOLE_SOCKET test_busybox
+  [ "$status" -eq 0 ]
+
+  # check state
+  testcontainer test_busybox running
+
+  runc ps test_busybox
+  [ "$status" -eq 0 ]
+
+  runc kill test_busybox KILL
+  [ "$status" -eq 0 ]
+
+  retry 10 1 eval "__runc state test_busybox | grep -q 'stopped'"
+
+  runc ps test_busybox
+  [ "$status" -eq 0 ]
+}