From e49dd346577a3511727236335e7ff6eb63be6264 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Fri, 7 Jul 2017 14:43:35 -0700 Subject: [PATCH 1/4] Add support for container pids limit We add a daemon level setting and will add a container override once it is supported in CRI. Signed-off-by: Mrunal Patel --- cmd/crio/config.go | 3 +++ cmd/crio/main.go | 8 ++++++++ server/config.go | 11 +++++++++++ server/container_create.go | 7 +++++++ 4 files changed, 29 insertions(+) diff --git a/cmd/crio/config.go b/cmd/crio/config.go index 5d61a02e..bcd41042 100644 --- a/cmd/crio/config.go +++ b/cmd/crio/config.go @@ -98,6 +98,9 @@ apparmor_profile = "{{ .ApparmorProfile }}" # for the runtime. cgroup_manager = "{{ .CgroupManager }}" +# pids_limit is the number of processes allowed in a container +pids_limit = {{ .PidsLimit }} + # The "crio.image" table contains settings pertaining to the # management of OCI images. [crio.image] diff --git a/cmd/crio/main.go b/cmd/crio/main.go index 4f2460c0..09bac519 100644 --- a/cmd/crio/main.go +++ b/cmd/crio/main.go @@ -103,6 +103,9 @@ func mergeConfig(config *server.Config, ctx *cli.Context) error { if ctx.GlobalIsSet("cgroup-manager") { config.CgroupManager = ctx.GlobalString("cgroup-manager") } + if ctx.GlobalIsSet("pids-limit") { + config.PidsLimit = ctx.GlobalInt64("pids-limit") + } if ctx.GlobalIsSet("cni-config-dir") { config.NetworkDir = ctx.GlobalString("cni-config-dir") } @@ -239,6 +242,11 @@ func main() { Name: "cgroup-manager", Usage: "cgroup manager (cgroupfs or systemd)", }, + cli.Int64Flag{ + Name: "pids-limit", + Value: server.DefaultPidsLimit, + Usage: "maximum number of processes allowed in a container", + }, cli.StringFlag{ Name: "cni-config-dir", Usage: "CNI configuration files directory", diff --git a/server/config.go b/server/config.go index 081dacdd..86c0a380 100644 --- a/server/config.go +++ b/server/config.go @@ -43,6 +43,12 @@ const ( ImageVolumesIgnore ImageVolumesType = "ignore" ) +const ( + // DefaultPidsLimit is the default value for maximum number of processes + // allowed inside a container + DefaultPidsLimit = 1024 +) + // This structure is necessary to fake the TOML tables when parsing, // while also not requiring a bunch of layered structs for no good // reason. @@ -133,6 +139,10 @@ type RuntimeConfig struct { // CgroupManager is the manager implementation name which is used to // handle cgroups for containers. CgroupManager string `toml:"cgroup_manager"` + + // PidsLimit is the number of processes each container is restricted to + // by the cgroup process number controller. + PidsLimit int64 `toml:"pids_limit"` } // ImageConfig represents the "crio.image" TOML config table. @@ -261,6 +271,7 @@ func DefaultConfig() *Config { SeccompProfile: seccompProfilePath, ApparmorProfile: apparmorProfileName, CgroupManager: cgroupManager, + PidsLimit: DefaultPidsLimit, }, ImageConfig: ImageConfig{ DefaultTransport: defaultTransport, diff --git a/server/container_create.go b/server/container_create.go index ed7dd126..be0e4987 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -19,6 +19,7 @@ import ( "github.com/kubernetes-incubator/cri-o/server/apparmor" "github.com/kubernetes-incubator/cri-o/server/seccomp" "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/devices" "github.com/opencontainers/runc/libcontainer/user" rspec "github.com/opencontainers/runtime-spec/specs-go" @@ -673,6 +674,12 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, } } + // Set up pids limit if pids cgroup is mounted + _, err = cgroups.FindCgroupMountpoint("pids") + if err == nil { + specgen.SetLinuxResourcesPidsLimit(s.config.PidsLimit) + } + // by default, the root path is an empty string. set it now. specgen.SetRootPath(mountPoint) From c58bcc4ccfb67e49068cc77f567d7831b177a7a1 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Fri, 7 Jul 2017 14:44:41 -0700 Subject: [PATCH 2/4] docs: Document pids limit for crio Signed-off-by: Mrunal Patel --- docs/crio.8.md | 3 +++ docs/crio.conf.5.md | 3 +++ 2 files changed, 6 insertions(+) diff --git a/docs/crio.8.md b/docs/crio.8.md index 0fb0131f..904064b8 100644 --- a/docs/crio.8.md +++ b/docs/crio.8.md @@ -91,6 +91,9 @@ set the CPU profile file path **--pause-image**="" Image which contains the pause executable (default: "kubernetes/pause") +**--pids-limit**="" + Maximum number of processes allowed in a container (default: 1024) + **--root**="" CRIO root dir (default: "/var/lib/containers/storage") diff --git a/docs/crio.conf.5.md b/docs/crio.conf.5.md index 07950d70..93c30ebf 100644 --- a/docs/crio.conf.5.md +++ b/docs/crio.conf.5.md @@ -54,6 +54,9 @@ The `crio` table supports the following options: **conmon_env**=[] Environment variable list for conmon process (default: ["PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",]) +**pids_limit**="" + Maximum number of processes allowed in a container (default: 1024) + **runtime**="" OCI runtime path (default: "/usr/bin/runc") From ed9d49f247f7348e081ab788deb4e14929dfcf1d Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Fri, 7 Jul 2017 16:32:37 -0700 Subject: [PATCH 3/4] container: Add cgroup mount for introspection Signed-off-by: Mrunal Patel --- server/container_create.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/container_create.go b/server/container_create.go index be0e4987..d2136a0c 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -324,6 +324,9 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, return nil, err } + // Add cgroup mount so container process can introspect its own limits + specgen.AddCgroupsMount("ro") + if err := addDevices(sb, containerConfig, &specgen); err != nil { return nil, err } From 288415d31d49be0f2e11a2c67b43fb86822f30de Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Mon, 10 Jul 2017 16:43:40 -0700 Subject: [PATCH 4/4] test: Add test for pids limit Signed-off-by: Mrunal Patel --- test/cgroups.bats | 40 ++++++++++++++++++++++++++++++++++++++++ test/helpers.bash | 4 +++- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 test/cgroups.bats diff --git a/test/cgroups.bats b/test/cgroups.bats new file mode 100644 index 00000000..44d1acfc --- /dev/null +++ b/test/cgroups.bats @@ -0,0 +1,40 @@ +#!/usr/bin/env bats + +load helpers + +function teardown() { + cleanup_test +} + +@test "pids limit" { + if ! grep pids /proc/self/cgroup; then + skip "pids cgroup controller is not mounted" + fi + PIDS_LIMIT=1234 start_crio + run crioctl pod run --config "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + pids_limit_config=$(cat "$TESTDATA"/container_config.json | python -c 'import json,sys;obj=json.load(sys.stdin); obj["command"] = ["/bin/sleep", "600"]; json.dump(obj, sys.stdout)') + echo "$pids_limit_config" > "$TESTDIR"/container_pids_limit.json + run crioctl ctr create --config "$TESTDIR"/container_pids_limit.json --pod "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run crioctl ctr start --id "$ctr_id" + echo "$output" + [ "$status" -eq 0 ] + run crioctl ctr execsync --id "$ctr_id" cat /sys/fs/cgroup/pids/pids.max + echo "$output" + [ "$status" -eq 0 ] + [[ "$output" =~ "1234" ]] + run crioctl pod stop --id "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + run crioctl pod remove --id "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + cleanup_ctrs + cleanup_pods + stop_crio +} \ No newline at end of file diff --git a/test/helpers.bash b/test/helpers.bash index f7703cea..8bf17c62 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -53,6 +53,8 @@ DEFAULT_LOG_PATH=/var/log/crio/pods CGROUP_MANAGER=${CGROUP_MANAGER:-cgroupfs} # Image volumes handling IMAGE_VOLUMES=${IMAGE_VOLUMES:-mkdir} +# Container pids limit +PIDS_LIMIT=${PIDS_LIMIT:-1024} TESTDIR=$(mktemp -d) if [ -e /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then @@ -203,7 +205,7 @@ function start_crio() { "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=mrunalp/image-volume-test --import-from=dir:"$ARTIFACTS_PATH"/image-volume-test-image --add-name=docker.io/library/mrunalp/image-volume-test --signature-policy="$INTEGRATION_ROOT"/policy.json "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=busybox:latest --import-from=dir:"$ARTIFACTS_PATH"/busybox-image --add-name=docker.io/library/busybox:latest --signature-policy="$INTEGRATION_ROOT"/policy.json "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTS --runroot "$TESTDIR/crio-run" --image-name=runcom/stderr-test:latest --import-from=dir:"$ARTIFACTS_PATH"/stderr-test --add-name=docker.io/runcom/stderr-test:latest --signature-policy="$INTEGRATION_ROOT"/policy.json - "$CRIO_BINARY" --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --config /dev/null config >$CRIO_CONFIG + "$CRIO_BINARY" --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --config /dev/null config >$CRIO_CONFIG # Prepare the CNI configuration files, we're running with non host networking by default if [[ -n "$4" ]]; then