Merge pull request #36551 from timstclair/cvm-system

Automatic merge from submit-queue

Fix getting cgroup pids

Fixes https://github.com/kubernetes/kubernetes/issues/35214, https://github.com/kubernetes/kubernetes/issues/33232

Verified manually, but I didn't have time to run all the e2e's yet (will check it in the morning).

This should be cherry-picked into 1.4, and merged into 1.5 (/cc @saad-ali )

```release-note
Fix fetching pids running in a cgroup, which caused problems with OOM score adjustments & setting the /system cgroup ("misc" in the summary API).
```

/cc @kubernetes/sig-node

pkg/kubelet/cm/BUILD

@@ -27,6 +27,7 @@ go_library(
         "//pkg/api:go_default_library",
         "//pkg/api/resource:go_default_library",
         "//pkg/kubelet/cadvisor:go_default_library",
+        "//pkg/kubelet/cm/util:go_default_library",
         "//pkg/kubelet/qos:go_default_library",
         "//pkg/types:go_default_library",
         "//pkg/util:go_default_library",

pkg/kubelet/cm/container_manager_linux.go

@@ -37,6 +37,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
 	"k8s.io/kubernetes/pkg/kubelet/cadvisor"
+	cmutil "k8s.io/kubernetes/pkg/kubelet/cm/util"
 	"k8s.io/kubernetes/pkg/kubelet/qos"
 	"k8s.io/kubernetes/pkg/util"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
@@ -443,14 +444,8 @@ func (cm *containerManagerImpl) setupNode() error {
 			return fmt.Errorf("system container cannot be root (\"/\")")
 		}
 		cont := newSystemCgroups(cm.SystemCgroupsName)
-		rootContainer := &fs.Manager{
-			Cgroups: &configs.Cgroup{
-				Parent: "/",
-				Name:   "/",
-			},
-		}
 		cont.ensureStateFunc = func(manager *fs.Manager) error {
-			return ensureSystemCgroups(rootContainer, manager)
+			return ensureSystemCgroups("/", manager)
 		}
 		systemContainers = append(systemContainers, cont)
 	}
@@ -747,7 +742,7 @@ func getContainer(pid int) (string, error) {
 // The reason of leaving kernel threads at root cgroup is that we don't want to tie the
 // execution of these threads with to-be defined /system quota and create priority inversions.
 //
-func ensureSystemCgroups(rootContainer *fs.Manager, manager *fs.Manager) error {
+func ensureSystemCgroups(rootCgroupPath string, manager *fs.Manager) error {
 	// Move non-kernel PIDs to the system container.
 	attemptsRemaining := 10
 	var errs []error
@@ -756,7 +751,7 @@ func ensureSystemCgroups(rootContainer *fs.Manager, manager *fs.Manager) error {
 		errs = []error{}
 		attemptsRemaining--
 
-		allPids, err := rootContainer.GetPids()
+		allPids, err := cmutil.GetPids(rootCgroupPath)
 		if err != nil {
 			errs = append(errs, fmt.Errorf("failed to list PIDs for root: %v", err))
 			continue

pkg/kubelet/cm/util/BUILD

@@ -0,0 +1,21 @@
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+load(
+    "@io_bazel_rules_go//go:def.bzl",
+    "go_binary",
+    "go_library",
+    "go_test",
+    "cgo_library",
+)
+
+go_library(
+    name = "go_default_library",
+    srcs = ["cgroups_linux.go"],
+    tags = ["automanaged"],
+    deps = [
+        "//vendor:github.com/opencontainers/runc/libcontainer/cgroups",
+        "//vendor:github.com/opencontainers/runc/libcontainer/utils",
+    ],
+)

pkg/kubelet/cm/util/cgroups_linux.go

@@ -0,0 +1,76 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"path/filepath"
+
+	libcontainercgroups "github.com/opencontainers/runc/libcontainer/cgroups"
+	libcontainerutils "github.com/opencontainers/runc/libcontainer/utils"
+)
+
+// Forked from opencontainers/runc/libcontainer/cgroup/fs.Manager.GetPids()
+func GetPids(cgroupPath string) ([]int, error) {
+	dir, err := getCgroupPath(cgroupPath)
+	if err != nil {
+		return nil, err
+	}
+	return libcontainercgroups.GetPids(dir)
+}
+
+// getCgroupPath gets the file path to the "devices" subsystem of the desired cgroup.
+// cgroupPath is the path in the cgroup hierarchy.
+func getCgroupPath(cgroupPath string) (string, error) {
+	cgroupPath = libcontainerutils.CleanPath(cgroupPath)
+
+	mnt, root, err := libcontainercgroups.FindCgroupMountpointAndRoot("devices")
+	// If we didn't mount the subsystem, there is no point we make the path.
+	if err != nil {
+		return "", err
+	}
+
+	// If the cgroup name/path is absolute do not look relative to the cgroup of the init process.
+	if filepath.IsAbs(cgroupPath) {
+		// Sometimes subsystems can be mounted togethger as 'cpu,cpuacct'.
+		return filepath.Join(root, mnt, cgroupPath), nil
+	}
+
+	parentPath, err := getCgroupParentPath(mnt, root)
+	if err != nil {
+		return "", err
+	}
+
+	return filepath.Join(parentPath, cgroupPath), nil
+}
+
+// getCgroupParentPath gets the parent filepath to this cgroup, for resolving relative cgroup paths.
+func getCgroupParentPath(mountpoint, root string) (string, error) {
+	// Use GetThisCgroupDir instead of GetInitCgroupDir, because the creating
+	// process could in container and shared pid namespace with host, and
+	// /proc/1/cgroup could point to whole other world of cgroups.
+	initPath, err := libcontainercgroups.GetThisCgroupDir("devices")
+	if err != nil {
+		return "", err
+	}
+	// This is needed for nested containers, because in /proc/self/cgroup we
+	// see paths from host, which don't exist in container.
+	relDir, err := filepath.Rel(root, initPath)
+	if err != nil {
+		return "", err
+	}
+	return filepath.Join(mountpoint, relDir), nil
+}

pkg/kubelet/cm/util/cgroups_unsupported.go

@@ -0,0 +1,23 @@
+// +build !linux
+
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+func GetPids(cgroupPath string) ([]int, error) {
+	return nil, nil
+}

pkg/util/oom/BUILD

@@ -20,9 +20,8 @@ go_library(
     ],
     tags = ["automanaged"],
     deps = [
+        "//pkg/kubelet/cm/util:go_default_library",
         "//vendor:github.com/golang/glog",
-        "//vendor:github.com/opencontainers/runc/libcontainer/cgroups/fs",
-        "//vendor:github.com/opencontainers/runc/libcontainer/configs",
     ],
 )
 

pkg/util/oom/oom_linux.go

@@ -23,11 +23,12 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
+	"path/filepath"
 	"strconv"
 
+	cmutil "k8s.io/kubernetes/pkg/kubelet/cm/util"
+
 	"github.com/golang/glog"
-	"github.com/opencontainers/runc/libcontainer/cgroups/fs"
-	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
 func NewOOMAdjuster() *OOMAdjuster {
@@ -40,13 +41,7 @@ func NewOOMAdjuster() *OOMAdjuster {
 }
 
 func getPids(cgroupName string) ([]int, error) {
-	fsManager := fs.Manager{
-		Cgroups: &configs.Cgroup{
-			Parent: "/",
-			Name:   cgroupName,
-		},
-	}
-	return fsManager.GetPids()
+	return cmutil.GetPids(filepath.Join("/", cgroupName))
 }
 
 // Writes 'value' to /proc/<pid>/oom_score_adj. PID = 0 means self