From ea1af57eab335ec78c2bc3205a816967ee3bbf68 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 25 Oct 2019 12:35:17 +0000 Subject: [PATCH 1/3] Promote feature PodShareProcessNamespace to GA --- pkg/apis/core/types.go | 1 - pkg/features/kube_features.go | 4 +++- staging/src/k8s.io/api/core/v1/types.go | 1 - 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 7da9bc87272..92dfc317a18 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -2812,7 +2812,6 @@ type PodSecurityContext struct { // in the same pod, and the first process in each container will not be assigned PID 1. // HostPID and ShareProcessNamespace cannot both be set. // Optional: Default to false. - // This field is beta-level and may be disabled with the PodShareProcessNamespace feature. // +k8s:conversion-gen=false // +optional ShareProcessNamespace *bool diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index f5098d9bf99..7b089a124ac 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -112,7 +112,9 @@ const ( EphemeralContainers featuregate.Feature = "EphemeralContainers" // owner: @verb + // alpha: v1.10 // beta: v1.12 + // GA: v1.17 // // Allows all containers in a pod to share a process namespace. PodShareProcessNamespace featuregate.Feature = "PodShareProcessNamespace" @@ -519,7 +521,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS LocalStorageCapacityIsolation: {Default: true, PreRelease: featuregate.Beta}, Sysctls: {Default: true, PreRelease: featuregate.Beta}, EphemeralContainers: {Default: false, PreRelease: featuregate.Alpha}, - PodShareProcessNamespace: {Default: true, PreRelease: featuregate.Beta}, + PodShareProcessNamespace: {Default: true, PreRelease: featuregate.GA}, // remove in 1.19 PodPriority: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 TaintNodesByCondition: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 QOSReserved: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index a284d627148..445eb30ef54 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -2940,7 +2940,6 @@ type PodSpec struct { // in the same pod, and the first process in each container will not be assigned PID 1. // HostPID and ShareProcessNamespace cannot both be set. // Optional: Default to false. - // This field is beta-level and may be disabled with the PodShareProcessNamespace feature. // +k8s:conversion-gen=false // +optional ShareProcessNamespace *bool `json:"shareProcessNamespace,omitempty" protobuf:"varint,27,opt,name=shareProcessNamespace"` From 494b90130c8a4664ac4bed4b57ec90bf3f167e28 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 25 Oct 2019 12:50:52 +0000 Subject: [PATCH 2/3] Generated code for PodShareProcessNamespace GA --- api/openapi-spec/swagger.json | 2 +- staging/src/k8s.io/api/core/v1/generated.proto | 1 - staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 753f68e5f57..e76da396902 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -10086,7 +10086,7 @@ "type": "string" }, "shareProcessNamespace": { - "description": "Share a single process namespace between all of the containers in a pod. When this is set containers will be able to view and signal processes from other containers in the same pod, and the first process in each container will not be assigned PID 1. HostPID and ShareProcessNamespace cannot both be set. Optional: Default to false. This field is beta-level and may be disabled with the PodShareProcessNamespace feature.", + "description": "Share a single process namespace between all of the containers in a pod. When this is set containers will be able to view and signal processes from other containers in the same pod, and the first process in each container will not be assigned PID 1. HostPID and ShareProcessNamespace cannot both be set. Optional: Default to false.", "type": "boolean" }, "subdomain": { diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index 94e21274064..6a365acf3e8 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -3384,7 +3384,6 @@ message PodSpec { // in the same pod, and the first process in each container will not be assigned PID 1. // HostPID and ShareProcessNamespace cannot both be set. // Optional: Default to false. - // This field is beta-level and may be disabled with the PodShareProcessNamespace feature. // +k8s:conversion-gen=false // +optional optional bool shareProcessNamespace = 27; diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index b2c448002f7..13481f51e5e 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -1614,7 +1614,7 @@ var map_PodSpec = map[string]string{ "hostNetwork": "Host networking requested for this pod. Use the host's network namespace. If this option is set, the ports that will be used must be specified. Default to false.", "hostPID": "Use the host's pid namespace. Optional: Default to false.", "hostIPC": "Use the host's ipc namespace. Optional: Default to false.", - "shareProcessNamespace": "Share a single process namespace between all of the containers in a pod. When this is set containers will be able to view and signal processes from other containers in the same pod, and the first process in each container will not be assigned PID 1. HostPID and ShareProcessNamespace cannot both be set. Optional: Default to false. This field is beta-level and may be disabled with the PodShareProcessNamespace feature.", + "shareProcessNamespace": "Share a single process namespace between all of the containers in a pod. When this is set containers will be able to view and signal processes from other containers in the same pod, and the first process in each container will not be assigned PID 1. HostPID and ShareProcessNamespace cannot both be set. Optional: Default to false.", "securityContext": "SecurityContext holds pod-level security attributes and common container settings. Optional: Defaults to empty. See type description for default values of each field.", "imagePullSecrets": "ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. If specified, these secrets will be passed to individual puller implementations for them to use. For example, in the case of docker, only DockerConfig type secrets are honored. More info: https://kubernetes.io/docs/concepts/containers/images#specifying-imagepullsecrets-on-a-pod", "hostname": "Specifies the hostname of the Pod If not specified, the pod's hostname will be set to a system-defined value.", From cbbe7d1bb91d3e59a1f51e4f729a059b4564e009 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Thu, 31 Oct 2019 17:15:23 +0000 Subject: [PATCH 3/3] Remove checks for PodShareProcessNamespace feature gate --- pkg/api/pod/util.go | 16 ---- pkg/api/pod/util_test.go | 100 ------------------------ pkg/features/kube_features.go | 2 +- pkg/kubelet/kuberuntime/helpers.go | 4 +- pkg/kubelet/kuberuntime/helpers_test.go | 49 ------------ test/e2e_node/BUILD | 1 - test/e2e_node/security_context_test.go | 6 -- 7 files changed, 2 insertions(+), 176 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 30bdcd8e83c..4e9fc774a08 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -340,12 +340,6 @@ func dropDisabledFields( } } - if !utilfeature.DefaultFeatureGate.Enabled(features.PodShareProcessNamespace) && !shareProcessNamespaceInUse(oldPodSpec) { - if podSpec.SecurityContext != nil { - podSpec.SecurityContext.ShareProcessNamespace = nil - } - } - if !utilfeature.DefaultFeatureGate.Enabled(features.Sysctls) && !sysctlsInUse(oldPodSpec) { if podSpec.SecurityContext != nil { podSpec.SecurityContext.Sysctls = nil @@ -633,16 +627,6 @@ func appArmorInUse(podAnnotations map[string]string) bool { return false } -func shareProcessNamespaceInUse(podSpec *api.PodSpec) bool { - if podSpec == nil { - return false - } - if podSpec.SecurityContext != nil && podSpec.SecurityContext.ShareProcessNamespace != nil { - return true - } - return false -} - func tokenRequestProjectionInUse(podSpec *api.PodSpec) bool { if podSpec == nil { return false diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 93952681008..12485e681f2 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -980,106 +980,6 @@ func TestDropEmptyDirSizeLimit(t *testing.T) { } } -func TestDropPodShareProcessNamespace(t *testing.T) { - podWithShareProcessNamespace := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - SecurityContext: &api.PodSecurityContext{ - ShareProcessNamespace: &[]bool{true}[0], - }, - }, - } - } - podWithoutShareProcessNamespace := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - SecurityContext: &api.PodSecurityContext{}, - }, - } - } - podWithoutSecurityContext := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{}, - } - } - - podInfo := []struct { - description string - hasShareProcessNamespace bool - pod func() *api.Pod - }{ - { - description: "has ShareProcessNamespace", - hasShareProcessNamespace: true, - pod: podWithShareProcessNamespace, - }, - { - description: "does not have ShareProcessNamespace", - hasShareProcessNamespace: false, - pod: podWithoutShareProcessNamespace, - }, - { - description: "does not have SecurityContext", - hasShareProcessNamespace: false, - pod: podWithoutSecurityContext, - }, - { - description: "is nil", - hasShareProcessNamespace: false, - pod: func() *api.Pod { return nil }, - }, - } - - for _, enabled := range []bool{true, false} { - for _, oldPodInfo := range podInfo { - for _, newPodInfo := range podInfo { - oldPodHasShareProcessNamespace, oldPod := oldPodInfo.hasShareProcessNamespace, oldPodInfo.pod() - newPodHasShareProcessNamespace, newPod := newPodInfo.hasShareProcessNamespace, newPodInfo.pod() - if newPod == nil { - continue - } - - t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodShareProcessNamespace, enabled)() - - var oldPodSpec *api.PodSpec - if oldPod != nil { - oldPodSpec = &oldPod.Spec - } - dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) - - // old pod should never be changed - if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { - t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) - } - - switch { - case enabled || oldPodHasShareProcessNamespace: - // new pod should not be changed if the feature is enabled, or if the old pod had ShareProcessNamespace set - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) - } - case newPodHasShareProcessNamespace: - // new pod should be changed - if reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod was not changed") - } - // new pod should not have ShareProcessNamespace - if !reflect.DeepEqual(newPod, podWithoutShareProcessNamespace()) { - t.Errorf("new pod had ShareProcessNamespace: %v", diff.ObjectReflectDiff(newPod, podWithoutShareProcessNamespace())) - } - default: - // new pod should not need to be changed - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) - } - } - }) - } - } - } -} - func TestDropAppArmor(t *testing.T) { podWithAppArmor := func() *api.Pod { return &api.Pod{ diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 7b089a124ac..a17ef453840 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -521,7 +521,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS LocalStorageCapacityIsolation: {Default: true, PreRelease: featuregate.Beta}, Sysctls: {Default: true, PreRelease: featuregate.Beta}, EphemeralContainers: {Default: false, PreRelease: featuregate.Alpha}, - PodShareProcessNamespace: {Default: true, PreRelease: featuregate.GA}, // remove in 1.19 + PodShareProcessNamespace: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.19 PodPriority: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 TaintNodesByCondition: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 QOSReserved: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index 1d7c4e2c12d..2d970333b3b 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -24,10 +24,8 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" - utilfeature "k8s.io/apiserver/pkg/util/feature" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" "k8s.io/klog" - "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) @@ -250,7 +248,7 @@ func pidNamespaceForPod(pod *v1.Pod) runtimeapi.NamespaceMode { if pod.Spec.HostPID { return runtimeapi.NamespaceMode_NODE } - if utilfeature.DefaultFeatureGate.Enabled(features.PodShareProcessNamespace) && pod.Spec.ShareProcessNamespace != nil && *pod.Spec.ShareProcessNamespace { + if pod.Spec.ShareProcessNamespace != nil && *pod.Spec.ShareProcessNamespace { return runtimeapi.NamespaceMode_POD } } diff --git a/pkg/kubelet/kuberuntime/helpers_test.go b/pkg/kubelet/kuberuntime/helpers_test.go index a48b9591291..e8f26d99bd7 100644 --- a/pkg/kubelet/kuberuntime/helpers_test.go +++ b/pkg/kubelet/kuberuntime/helpers_test.go @@ -25,11 +25,8 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" runtimetesting "k8s.io/cri-api/pkg/apis/testing" - "k8s.io/kubernetes/pkg/features" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" ) @@ -285,8 +282,6 @@ func TestGetSeccompProfileFromAnnotations(t *testing.T) { } func TestNamespacesForPod(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodShareProcessNamespace, true)() - for desc, test := range map[string]struct { input *v1.Pod expected *runtimeapi.NamespaceOption @@ -350,48 +345,4 @@ func TestNamespacesForPod(t *testing.T) { actual := namespacesForPod(test.input) assert.Equal(t, test.expected, actual) } - - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodShareProcessNamespace, false)() - - for desc, test := range map[string]struct { - input *v1.Pod - expected *runtimeapi.NamespaceOption - }{ - "v1.Pod default namespaces": { - &v1.Pod{}, - &runtimeapi.NamespaceOption{ - Ipc: runtimeapi.NamespaceMode_POD, - Network: runtimeapi.NamespaceMode_POD, - Pid: runtimeapi.NamespaceMode_CONTAINER, - }, - }, - "Shared Process Namespace (feature disabled)": { - &v1.Pod{ - Spec: v1.PodSpec{ - ShareProcessNamespace: &[]bool{true}[0], - }, - }, - &runtimeapi.NamespaceOption{ - Ipc: runtimeapi.NamespaceMode_POD, - Network: runtimeapi.NamespaceMode_POD, - Pid: runtimeapi.NamespaceMode_CONTAINER, - }, - }, - "Shared Process Namespace, redundant flag (feature disabled)": { - &v1.Pod{ - Spec: v1.PodSpec{ - ShareProcessNamespace: &[]bool{false}[0], - }, - }, - &runtimeapi.NamespaceOption{ - Ipc: runtimeapi.NamespaceMode_POD, - Network: runtimeapi.NamespaceMode_POD, - Pid: runtimeapi.NamespaceMode_CONTAINER, - }, - }, - } { - t.Logf("TestCase: %s", desc) - actual := namespacesForPod(test.input) - assert.Equal(t, test.expected, actual) - } } diff --git a/test/e2e_node/BUILD b/test/e2e_node/BUILD index 92f21edf97f..c6d2c2a05bf 100644 --- a/test/e2e_node/BUILD +++ b/test/e2e_node/BUILD @@ -161,7 +161,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/tools/watch:go_default_library", "//staging/src/k8s.io/cri-api/pkg/apis:go_default_library", diff --git a/test/e2e_node/security_context_test.go b/test/e2e_node/security_context_test.go index 9b823f80d62..b87f5047462 100644 --- a/test/e2e_node/security_context_test.go +++ b/test/e2e_node/security_context_test.go @@ -26,8 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/test/e2e/framework" e2epod "k8s.io/kubernetes/test/e2e/framework/pod" imageutils "k8s.io/kubernetes/test/utils/image" @@ -79,10 +77,6 @@ var _ = framework.KubeDescribe("Security Context", func() { if !isEnabled { framework.Skipf("Skipped because shared PID namespace is not supported by this docker version.") } - // It's not enough to set this flag in the kubelet because the apiserver needs it too - if !utilfeature.DefaultFeatureGate.Enabled(features.PodShareProcessNamespace) { - framework.Skipf("run test with --feature-gates=PodShareProcessNamespace=true to test PID namespace sharing") - } ginkgo.By("Create a pod with shared PID namespace.") f.PodClient().CreateSync(&v1.Pod{