From 702ab3ee3a402fdf08aebbd39ddc51291828bb57 Mon Sep 17 00:00:00 2001 From: Wei Wei Date: Fri, 10 Nov 2017 11:24:34 +0800 Subject: [PATCH 1/3] share pid namespace for Pod container Signed-off-by: Wei Wei --- cmd/crio/config.go | 3 ++ cmd/crio/main.go | 7 ++++ docs/crio.8.md | 3 ++ docs/crio.conf.5.md | 3 ++ libkpod/config.go | 3 ++ server/container_create.go | 8 ++++- test/helpers.bash | 4 ++- test/namespaces.bats | 67 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 test/namespaces.bats diff --git a/cmd/crio/config.go b/cmd/crio/config.go index 76e3361d..6d602824 100644 --- a/cmd/crio/config.go +++ b/cmd/crio/config.go @@ -115,6 +115,9 @@ default_mounts = [ # pids_limit is the number of processes allowed in a container pids_limit = {{ .PidsLimit }} +# disable using a shared PID namespace for containers in a pod +disable_shared_pid_namespace = {{ .DisableSharedPIDNamespace }} + # log_size_max is the max limit for the container log size in bytes. # Negative values indicate that no limit is imposed. log_size_max = {{ .LogSizeMax }} diff --git a/cmd/crio/main.go b/cmd/crio/main.go index 1f211d63..96d4a261 100644 --- a/cmd/crio/main.go +++ b/cmd/crio/main.go @@ -131,6 +131,9 @@ func mergeConfig(config *server.Config, ctx *cli.Context) error { if ctx.GlobalIsSet("pids-limit") { config.PidsLimit = ctx.GlobalInt64("pids-limit") } + if ctx.GlobalIsSet("disable-shared-pid-namespace") { + config.DisableSharedPIDNamespace = ctx.GlobalBool("disable-shared-pid-namespace") + } if ctx.GlobalIsSet("log-size-max") { config.LogSizeMax = ctx.GlobalInt64("log-size-max") } @@ -296,6 +299,10 @@ func main() { Value: libkpod.DefaultPidsLimit, Usage: "maximum number of processes allowed in a container", }, + cli.BoolFlag{ + Name: "disable-shared-pid-namespace", + Usage: "disable using a shared PID namespace for containers in a pod", + }, cli.Int64Flag{ Name: "log-size-max", Value: libkpod.DefaultLogSizeMax, diff --git a/docs/crio.8.md b/docs/crio.8.md index 2c9d4857..f349ad5b 100644 --- a/docs/crio.8.md +++ b/docs/crio.8.md @@ -118,6 +118,9 @@ set the CPU profile file path **--pids-limit**="" Maximum number of processes allowed in a container (default: 1024) +**--disable-shared-pid-namespace**="" + Disable using a shared PID namespace for containers in a pod (default: false) + **--root**="" The crio root dir (default: "/var/lib/containers/storage") diff --git a/docs/crio.conf.5.md b/docs/crio.conf.5.md index 32cac7a4..017cf025 100644 --- a/docs/crio.conf.5.md +++ b/docs/crio.conf.5.md @@ -87,6 +87,9 @@ Example: **pids_limit**="" Maximum number of processes allowed in a container (default: 1024) +**disable_shared_pid_namespace**="" + Disable using a shared PID namespace for containers in a pod (default: false) + **runtime**="" OCI runtime path (default: "/usr/bin/runc") diff --git a/libkpod/config.go b/libkpod/config.go index 687b4b38..ee5f2116 100644 --- a/libkpod/config.go +++ b/libkpod/config.go @@ -121,6 +121,9 @@ type RuntimeConfig struct { // NoPivot instructs the runtime to not use `pivot_root`, but instead use `MS_MOVE` NoPivot bool `toml:"no_pivot"` + // DisableSharePidNamespace instructs the runtime to disable share pid namespace + DisableSharedPIDNamespace bool `toml:"disable_shared_pid_namespace"` + // Conmon is the path to conmon binary, used for managing the runtime. Conmon string `toml:"conmon"` diff --git a/server/container_create.go b/server/container_create.go index 540ff891..564ed54f 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -921,9 +921,15 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, return nil, err } - // Do not share pid ns for now if containerConfig.GetLinux().GetSecurityContext().GetNamespaceOptions().GetHostPid() { + // kubernetes PodSpec specify to use Host PID namespace specgen.RemoveLinuxNamespace(string(rspec.PIDNamespace)) + } else if !s.config.DisableSharedPIDNamespace { + // share Pod PID namespace + pidNsPath := fmt.Sprintf("/proc/%d/ns/pid", podInfraState.Pid) + if err := specgen.AddOrReplaceLinuxNamespace(string(rspec.PIDNamespace), pidNsPath); err != nil { + return nil, err + } } netNsPath := sb.NetNsPath() diff --git a/test/helpers.bash b/test/helpers.bash index 21488e1f..90bf5157 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -56,6 +56,8 @@ IMAGE_VOLUMES=${IMAGE_VOLUMES:-mkdir} PIDS_LIMIT=${PIDS_LIMIT:-1024} # Log size max limit LOG_SIZE_MAX_LIMIT=${LOG_SIZE_MAX_LIMIT:--1} +# enable share container pid namespace +DISABLE_SHARED_PID_NAMESPACE=${DISABLE_SHARED_PID_NAMESPACE:-false} TESTDIR=$(mktemp -d) @@ -240,7 +242,7 @@ function start_crio() { "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --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_OPTIONS --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_OPTIONS --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" ${DEFAULT_MOUNTS_OPTS} ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTIONS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --cni-plugin-dir "$CRIO_CNI_PLUGIN" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --log-size-max "$LOG_SIZE_MAX_LIMIT" --config /dev/null config >$CRIO_CONFIG + "$CRIO_BINARY" ${DEFAULT_MOUNTS_OPTS} ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTIONS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --cni-plugin-dir "$CRIO_CNI_PLUGIN" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --disable-shared-pid-namespace=${DISABLE_SHARED_PID_NAMESPACE} --log-size-max "$LOG_SIZE_MAX_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 diff --git a/test/namespaces.bats b/test/namespaces.bats new file mode 100644 index 00000000..b9eb47e4 --- /dev/null +++ b/test/namespaces.bats @@ -0,0 +1,67 @@ +#!/usr/bin/env bats + +load helpers + +function teardown() { + cleanup_test +} + +@test "pod disable shared pid namespace" { + DISABLE_SHARED_PID_NAMESPACE="true" start_crio + + run crictl runs "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run crictl create "$pod_id" "$TESTDATA"/container_redis.json "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run crictl start "$ctr_id" + [ "$status" -eq 0 ] + + run crictl exec --sync "$ctr_id" cat /proc/1/cmdline + echo "$output" + [ "$status" -eq 0 ] + [[ "$output" =~ "redis" ]] + + run crictl stops "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + run crictl rms "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + cleanup_ctrs + cleanup_pods + stop_crio +} + +@test "pod enable shared pid namespace" { + DISABLE_SHARED_PID_NAMESPACE="false" start_crio + + run crictl runs "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + pod_id="$output" + run crictl create "$pod_id" "$TESTDATA"/container_redis.json "$TESTDATA"/sandbox_config.json + echo "$output" + [ "$status" -eq 0 ] + ctr_id="$output" + run crictl start "$ctr_id" + [ "$status" -eq 0 ] + + run crictl exec --sync "$ctr_id" cat /proc/1/cmdline + echo "$output" + [ "$status" -eq 0 ] + [[ "$output" =~ "pause" ]] + + run crictl stops "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + run crictl rms "$pod_id" + echo "$output" + [ "$status" -eq 0 ] + cleanup_ctrs + cleanup_pods + stop_crio +} From bbd9a6528ccc1fe5c73aead8f4fd60cc0df097b4 Mon Sep 17 00:00:00 2001 From: Chris Evich Date: Fri, 17 Nov 2017 11:49:18 -0500 Subject: [PATCH 2/3] Remove disused contrib/rpm I don't believe the files in this dir are actually used anymore. Remove them so content can be added to this directory in the future w/o clashing. Signed-off-by: Chris Evich --- contrib/rpm/Makefile | 14 -------- contrib/rpm/crio.spec | 76 ------------------------------------------- 2 files changed, 90 deletions(-) delete mode 100644 contrib/rpm/Makefile delete mode 100644 contrib/rpm/crio.spec diff --git a/contrib/rpm/Makefile b/contrib/rpm/Makefile deleted file mode 100644 index 24bbca28..00000000 --- a/contrib/rpm/Makefile +++ /dev/null @@ -1,14 +0,0 @@ -.PHONY: dist -dist: crio.spec - spectool -g crio.spec - -.PHONY: rpm -rpm: dist - rpmbuild --define "_sourcedir `pwd`" --define "_specdir `pwd`" \ - --define "_rpmdir `pwd`" --define "_srcrpmdir `pwd`" -ba crio.spec - -all: rpm - -clean: - rm -f *rpm *gz - rm -rf x86_64 diff --git a/contrib/rpm/crio.spec b/contrib/rpm/crio.spec deleted file mode 100644 index 3485fe37..00000000 --- a/contrib/rpm/crio.spec +++ /dev/null @@ -1,76 +0,0 @@ -%define debug_package %{nil} -%global provider github -%global provider_tld com -%global project kubernetes-incubator -%global repo cri-o -%global Name crio -# https://github.com/kubernetes-incubator/cri-o -%global provider_prefix %{provider}.%{provider_tld}/%{project}/%{repo} -%global import_path %{provider_prefix} -%global commit 8ba639952a95f2e24cc98987689138b67545576c -%global shortcommit %(c=%{commit}; echo ${c:0:7}) - -Name: %{Name} -Version: 0.0.1 -Release: 1.git%{shortcommit}%{?dist} -Summary: Kubelet Container Runtime Interface (CRI) for OCI runtimes. -Group: Applications/Text -License: Apache 2.0 -URL: https://%{provider_prefix} -Source0: https://%{provider_prefix}/archive/%{commit}/%{repo}-%{shortcommit}.tar.gz -Provides: %{repo} - -BuildRequires: golang-github-cpuguy83-go-md2man - -%description -The crio package provides an implementation of the -Kubelet Container Runtime Interface (CRI) using OCI conformant runtimes. - -crio provides following functionalities: - - Support multiple image formats including the existing Docker image format - Support for multiple means to download images including trust & image verification - Container image management (managing image layers, overlay filesystems, etc) - Container process lifecycle management - Monitoring and logging required to satisfy the CRI - Resource isolation as required by the CRI - -%prep -%setup -q -n %{repo}-%{commit} - -%build -make all - -%install -%make_install -%make_install install.systemd - -#define license tag if not already defined -%{!?_licensedir:%global license %doc} -%files -%{_bindir}/crio -%{_bindir}/crioctl -%{_mandir}/man5/crio.conf.5* -%{_mandir}/man8/crio.8* -%{_sysconfdir}/crio.conf -%{_sysconfdir}/seccomp.json -%dir /%{_libexecdir}/crio -/%{_libexecdir}/crio/conmon -/%{_libexecdir}/crio/pause -%{_unitdir}/crio.service -%doc README.md -%license LICENSE -%dir /usr/share/oci-umount/oci-umount.d -/usr/share/oci-umount/oci-umount.d/cri-umount.conf - - -%preun -%systemd_preun %{Name} - -%postun -%systemd_postun_with_restart %{Name} - -%changelog -* Mon Oct 31 2016 Dan Walsh - 0.0.1 -- Initial RPM release - From 946307e5c2d04a6670aaf1a02844d90e0425b255 Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Fri, 17 Nov 2017 16:52:06 -0800 Subject: [PATCH 3/3] Make pid namespace sharing optional and disabled by default We reverse the logic so that pid ns sharing is disabled by default. Signed-off-by: Mrunal Patel --- cmd/crio/config.go | 4 ++-- cmd/crio/main.go | 8 ++++---- docs/crio.8.md | 4 ++-- docs/crio.conf.5.md | 4 ++-- libkpod/config.go | 4 ++-- server/container_create.go | 2 +- test/helpers.bash | 4 ++-- test/namespaces.bats | 4 ++-- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cmd/crio/config.go b/cmd/crio/config.go index 6d602824..7d26059f 100644 --- a/cmd/crio/config.go +++ b/cmd/crio/config.go @@ -115,8 +115,8 @@ default_mounts = [ # pids_limit is the number of processes allowed in a container pids_limit = {{ .PidsLimit }} -# disable using a shared PID namespace for containers in a pod -disable_shared_pid_namespace = {{ .DisableSharedPIDNamespace }} +# enable using a shared PID namespace for containers in a pod +enable_shared_pid_namespace = {{ .EnableSharedPIDNamespace }} # log_size_max is the max limit for the container log size in bytes. # Negative values indicate that no limit is imposed. diff --git a/cmd/crio/main.go b/cmd/crio/main.go index 06a01cb0..2f91c115 100644 --- a/cmd/crio/main.go +++ b/cmd/crio/main.go @@ -132,8 +132,8 @@ func mergeConfig(config *server.Config, ctx *cli.Context) error { if ctx.GlobalIsSet("pids-limit") { config.PidsLimit = ctx.GlobalInt64("pids-limit") } - if ctx.GlobalIsSet("disable-shared-pid-namespace") { - config.DisableSharedPIDNamespace = ctx.GlobalBool("disable-shared-pid-namespace") + if ctx.GlobalIsSet("enable-shared-pid-namespace") { + config.EnableSharedPIDNamespace = ctx.GlobalBool("enable-shared-pid-namespace") } if ctx.GlobalIsSet("log-size-max") { config.LogSizeMax = ctx.GlobalInt64("log-size-max") @@ -301,8 +301,8 @@ func main() { Usage: "maximum number of processes allowed in a container", }, cli.BoolFlag{ - Name: "disable-shared-pid-namespace", - Usage: "disable using a shared PID namespace for containers in a pod", + Name: "enable-shared-pid-namespace", + Usage: "enable using a shared PID namespace for containers in a pod", }, cli.Int64Flag{ Name: "log-size-max", diff --git a/docs/crio.8.md b/docs/crio.8.md index 0f75d3f4..0de67375 100644 --- a/docs/crio.8.md +++ b/docs/crio.8.md @@ -118,8 +118,8 @@ set the CPU profile file path **--pids-limit**="" Maximum number of processes allowed in a container (default: 1024) -**--disable-shared-pid-namespace**="" - Disable using a shared PID namespace for containers in a pod (default: false) +**--enable-shared-pid-namespace**="" + Enable using a shared PID namespace for containers in a pod (default: false) **--root**="" The crio root dir (default: "/var/lib/containers/storage") diff --git a/docs/crio.conf.5.md b/docs/crio.conf.5.md index 55c2517f..708f26e7 100644 --- a/docs/crio.conf.5.md +++ b/docs/crio.conf.5.md @@ -87,8 +87,8 @@ Example: **pids_limit**="" Maximum number of processes allowed in a container (default: 1024) -**disable_shared_pid_namespace**="" - Disable using a shared PID namespace for containers in a pod (default: false) +**enable_shared_pid_namespace**="" + Enable using a shared PID namespace for containers in a pod (default: false) **runtime**="" OCI runtime path (default: "/usr/bin/runc") diff --git a/libkpod/config.go b/libkpod/config.go index ee5f2116..9007f35d 100644 --- a/libkpod/config.go +++ b/libkpod/config.go @@ -121,8 +121,8 @@ type RuntimeConfig struct { // NoPivot instructs the runtime to not use `pivot_root`, but instead use `MS_MOVE` NoPivot bool `toml:"no_pivot"` - // DisableSharePidNamespace instructs the runtime to disable share pid namespace - DisableSharedPIDNamespace bool `toml:"disable_shared_pid_namespace"` + // EnableSharePidNamespace instructs the runtime to enable share pid namespace + EnableSharedPIDNamespace bool `toml:"enable_shared_pid_namespace"` // Conmon is the path to conmon binary, used for managing the runtime. Conmon string `toml:"conmon"` diff --git a/server/container_create.go b/server/container_create.go index 564ed54f..c04bfa69 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -924,7 +924,7 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, if containerConfig.GetLinux().GetSecurityContext().GetNamespaceOptions().GetHostPid() { // kubernetes PodSpec specify to use Host PID namespace specgen.RemoveLinuxNamespace(string(rspec.PIDNamespace)) - } else if !s.config.DisableSharedPIDNamespace { + } else if s.config.EnableSharedPIDNamespace { // share Pod PID namespace pidNsPath := fmt.Sprintf("/proc/%d/ns/pid", podInfraState.Pid) if err := specgen.AddOrReplaceLinuxNamespace(string(rspec.PIDNamespace), pidNsPath); err != nil { diff --git a/test/helpers.bash b/test/helpers.bash index 90bf5157..f3b2b087 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -57,7 +57,7 @@ PIDS_LIMIT=${PIDS_LIMIT:-1024} # Log size max limit LOG_SIZE_MAX_LIMIT=${LOG_SIZE_MAX_LIMIT:--1} # enable share container pid namespace -DISABLE_SHARED_PID_NAMESPACE=${DISABLE_SHARED_PID_NAMESPACE:-false} +ENABLE_SHARED_PID_NAMESPACE=${ENABLE_SHARED_PID_NAMESPACE:-false} TESTDIR=$(mktemp -d) @@ -242,7 +242,7 @@ function start_crio() { "$COPYIMG_BINARY" --root "$TESTDIR/crio" $STORAGE_OPTIONS --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_OPTIONS --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_OPTIONS --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" ${DEFAULT_MOUNTS_OPTS} ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTIONS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --cni-plugin-dir "$CRIO_CNI_PLUGIN" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --disable-shared-pid-namespace=${DISABLE_SHARED_PID_NAMESPACE} --log-size-max "$LOG_SIZE_MAX_LIMIT" --config /dev/null config >$CRIO_CONFIG + "$CRIO_BINARY" ${DEFAULT_MOUNTS_OPTS} ${HOOKS_OPTS} --conmon "$CONMON_BINARY" --listen "$CRIO_SOCKET" --cgroup-manager "$CGROUP_MANAGER" --registry "docker.io" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/crio" --runroot "$TESTDIR/crio-run" $STORAGE_OPTIONS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$CRIO_CNI_CONFIG" --cni-plugin-dir "$CRIO_CNI_PLUGIN" --signature-policy "$INTEGRATION_ROOT"/policy.json --image-volumes "$IMAGE_VOLUMES" --pids-limit "$PIDS_LIMIT" --enable-shared-pid-namespace=${ENABLE_SHARED_PID_NAMESPACE} --log-size-max "$LOG_SIZE_MAX_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 diff --git a/test/namespaces.bats b/test/namespaces.bats index b9eb47e4..ff9787a9 100644 --- a/test/namespaces.bats +++ b/test/namespaces.bats @@ -7,7 +7,7 @@ function teardown() { } @test "pod disable shared pid namespace" { - DISABLE_SHARED_PID_NAMESPACE="true" start_crio + ENABLE_SHARED_PID_NAMESPACE="false" start_crio run crictl runs "$TESTDATA"/sandbox_config.json echo "$output" @@ -37,7 +37,7 @@ function teardown() { } @test "pod enable shared pid namespace" { - DISABLE_SHARED_PID_NAMESPACE="false" start_crio + ENABLE_SHARED_PID_NAMESPACE="true" start_crio run crictl runs "$TESTDATA"/sandbox_config.json echo "$output"