diff --git a/pkg/kubelet/dockershim/helpers_linux.go b/pkg/kubelet/dockershim/helpers_linux.go index 431150c31d9..57e6840b99d 100644 --- a/pkg/kubelet/dockershim/helpers_linux.go +++ b/pkg/kubelet/dockershim/helpers_linux.go @@ -118,7 +118,7 @@ func (ds *dockerService) updateCreateConfig( if err := applyContainerSecurityContext(lc, podSandboxID, createConfig.Config, createConfig.HostConfig, securityOptSep); err != nil { return fmt.Errorf("failed to apply container security context for container %q: %v", config.Metadata.Name, err) } - modifyPIDNamespaceOverrides(ds.disableSharedPID, apiVersion, createConfig.HostConfig) + modifyContainerPIDNamespaceOverrides(ds.disableSharedPID, apiVersion, createConfig.HostConfig, podSandboxID) } // Apply cgroupsParent derived from the sandbox config. diff --git a/pkg/kubelet/dockershim/security_context.go b/pkg/kubelet/dockershim/security_context.go index 51a58e7be88..a23f884ee42 100644 --- a/pkg/kubelet/dockershim/security_context.go +++ b/pkg/kubelet/dockershim/security_context.go @@ -122,13 +122,16 @@ func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig * // modifySandboxNamespaceOptions apply namespace options for sandbox func modifySandboxNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, hostConfig *dockercontainer.HostConfig, network *knetwork.PluginManager) { + // The sandbox's PID namespace is the one that's shared, so CONTAINER and POD are equivalent for it modifyCommonNamespaceOptions(nsOpts, hostConfig) modifyHostOptionsForSandbox(nsOpts, network, hostConfig) } // modifyContainerNamespaceOptions apply namespace options for container func modifyContainerNamespaceOptions(nsOpts *runtimeapi.NamespaceOption, podSandboxID string, hostConfig *dockercontainer.HostConfig) { - hostConfig.PidMode = dockercontainer.PidMode(fmt.Sprintf("container:%v", podSandboxID)) + if nsOpts.GetPid() == runtimeapi.NamespaceMode_POD { + hostConfig.PidMode = dockercontainer.PidMode(fmt.Sprintf("container:%v", podSandboxID)) + } modifyCommonNamespaceOptions(nsOpts, hostConfig) modifyHostOptionsForContainer(nsOpts, podSandboxID, hostConfig) } @@ -181,14 +184,16 @@ func modifyHostOptionsForContainer(nsOpts *runtimeapi.NamespaceOption, podSandbo // 1. Docker engine prior to API Version 1.24 doesn't support attaching to another container's // PID namespace, and it didn't stabilize until 1.26. This check can be removed when Kubernetes' // minimum Docker version is at least 1.13.1 (API version 1.26). -// 2. The administrator has overridden the default behavior by means of a kubelet flag. This is an -// "escape hatch" to return to previous behavior of isolated namespaces and should be removed once -// no longer needed. -func modifyPIDNamespaceOverrides(disableSharedPID bool, version *semver.Version, hc *dockercontainer.HostConfig) { - if !strings.HasPrefix(string(hc.PidMode), "container:") { - return - } - if disableSharedPID || version.LT(semver.Version{Major: 1, Minor: 26}) { - hc.PidMode = "" +// 2. The administrator can override the API behavior by using the deprecated --docker-disable-shared-pid=false +// flag. Until this flag is removed, this causes pods to use NamespaceMode_POD instead of +// NamespaceMode_CONTAINER regardless of pod configuration. +// TODO(verb): remove entirely once these two conditions are satisfied +func modifyContainerPIDNamespaceOverrides(disableSharedPID bool, version *semver.Version, hc *dockercontainer.HostConfig, podSandboxID string) { + if version.LT(semver.Version{Major: 1, Minor: 26}) { + if strings.HasPrefix(string(hc.PidMode), "container:") { + hc.PidMode = "" + } + } else if !disableSharedPID && hc.PidMode == "" { + hc.PidMode = dockercontainer.PidMode(fmt.Sprintf("container:%v", podSandboxID)) } } diff --git a/pkg/kubelet/dockershim/security_context_test.go b/pkg/kubelet/dockershim/security_context_test.go index 382f004acaf..cf2fbdba474 100644 --- a/pkg/kubelet/dockershim/security_context_test.go +++ b/pkg/kubelet/dockershim/security_context_test.go @@ -328,51 +328,93 @@ func TestModifyContainerNamespacePIDOverride(t *testing.T) { input, expected dockercontainer.PidMode }{ { - name: "SharedPID.Enable", - disable: false, - version: &semver.Version{Major: 1, Minor: 26}, - input: "container:sandbox", - expected: "container:sandbox", - }, - { - name: "SharedPID.Disable", + name: "mode:CONTAINER docker:NEW flag:UNSET", disable: true, version: &semver.Version{Major: 1, Minor: 26}, - input: "container:sandbox", + input: "", expected: "", }, { - name: "SharedPID.OldDocker", + name: "mode:CONTAINER docker:NEW flag:SET", disable: false, - version: &semver.Version{Major: 1, Minor: 25}, - input: "container:sandbox", - expected: "", - }, - { - name: "SharedPID.HostPid", - disable: true, - version: &semver.Version{Major: 1, Minor: 27}, - input: "host", - expected: "host", - }, - { - name: "SharedPID.DistantFuture", - disable: false, - version: &semver.Version{Major: 2, Minor: 10}, - input: "container:sandbox", + version: &semver.Version{Major: 1, Minor: 26}, + input: "", expected: "container:sandbox", }, { - name: "SharedPID.EmptyPidMode", + name: "mode:CONTAINER docker:OLD flag:UNSET", disable: true, version: &semver.Version{Major: 1, Minor: 25}, input: "", expected: "", }, + { + name: "mode:CONTAINER docker:OLD flag:SET", + disable: false, + version: &semver.Version{Major: 1, Minor: 25}, + input: "", + expected: "", + }, + { + name: "mode:HOST docker:NEW flag:UNSET", + disable: true, + version: &semver.Version{Major: 1, Minor: 26}, + input: "host", + expected: "host", + }, + { + name: "mode:HOST docker:NEW flag:SET", + disable: false, + version: &semver.Version{Major: 1, Minor: 26}, + input: "host", + expected: "host", + }, + { + name: "mode:HOST docker:OLD flag:UNSET", + disable: true, + version: &semver.Version{Major: 1, Minor: 25}, + input: "host", + expected: "host", + }, + { + name: "mode:HOST docker:OLD flag:SET", + disable: false, + version: &semver.Version{Major: 1, Minor: 25}, + input: "host", + expected: "host", + }, + { + name: "mode:POD docker:NEW flag:UNSET", + disable: true, + version: &semver.Version{Major: 1, Minor: 26}, + input: "container:sandbox", + expected: "container:sandbox", + }, + { + name: "mode:POD docker:NEW flag:SET", + disable: false, + version: &semver.Version{Major: 1, Minor: 26}, + input: "container:sandbox", + expected: "container:sandbox", + }, + { + name: "mode:POD docker:OLD flag:UNSET", + disable: true, + version: &semver.Version{Major: 1, Minor: 25}, + input: "container:sandbox", + expected: "", + }, + { + name: "mode:POD docker:OLD flag:SET", + disable: false, + version: &semver.Version{Major: 1, Minor: 25}, + input: "container:sandbox", + expected: "", + }, } for _, tc := range cases { dockerCfg := &dockercontainer.HostConfig{PidMode: tc.input} - modifyPIDNamespaceOverrides(tc.disable, tc.version, dockerCfg) + modifyContainerPIDNamespaceOverrides(tc.disable, tc.version, dockerCfg, "sandbox") assert.Equal(t, tc.expected, dockerCfg.PidMode, "[Test case %q]", tc.name) } } diff --git a/pkg/kubelet/kuberuntime/BUILD b/pkg/kubelet/kuberuntime/BUILD index e17005eb3da..b52b1ffaebb 100644 --- a/pkg/kubelet/kuberuntime/BUILD +++ b/pkg/kubelet/kuberuntime/BUILD @@ -84,6 +84,7 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/credentialprovider:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubelet/apis/cri/runtime/v1alpha2:go_default_library", "//pkg/kubelet/apis/cri/testing:go_default_library", "//pkg/kubelet/container:go_default_library", @@ -101,6 +102,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature/testing:go_default_library", "//vendor/k8s.io/client-go/util/flowcontrol:go_default_library", ], ) diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index ef087171b32..91131e7b13d 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -294,10 +294,13 @@ func networkNamespaceForPod(pod *v1.Pod) runtimeapi.NamespaceMode { } func pidNamespaceForPod(pod *v1.Pod) runtimeapi.NamespaceMode { - if pod != nil && pod.Spec.HostPID { - return runtimeapi.NamespaceMode_NODE + if pod != nil { + if pod.Spec.HostPID { + return runtimeapi.NamespaceMode_NODE + } + // TODO(verb): set NamespaceMode_POD based on ShareProcessNamespace after #58716 is merged } - // Note that PID does not default to the zero value + // Note that PID does not default to the zero value for v1.Pod return runtimeapi.NamespaceMode_CONTAINER } diff --git a/pkg/kubelet/kuberuntime/helpers_test.go b/pkg/kubelet/kuberuntime/helpers_test.go index d0c0d8e4c94..7eaf377dee0 100644 --- a/pkg/kubelet/kuberuntime/helpers_test.go +++ b/pkg/kubelet/kuberuntime/helpers_test.go @@ -25,6 +25,9 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" + "k8s.io/kubernetes/pkg/features" runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" runtimetesting "k8s.io/kubernetes/pkg/kubelet/apis/cri/testing" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" @@ -307,6 +310,8 @@ func TestGetSeccompProfileFromAnnotations(t *testing.T) { } func TestNamespacesForPod(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodShareProcessNamespace, true)() + for desc, test := range map[string]struct { input *v1.Pod expected *runtimeapi.NamespaceOption @@ -341,6 +346,7 @@ func TestNamespacesForPod(t *testing.T) { Pid: runtimeapi.NamespaceMode_NODE, }, }, + // TODO(verb): add test cases for ShareProcessNamespace true (after #58716 is merged) } { t.Logf("TestCase: %s", desc) actual := namespacesForPod(test.input)