From 83a4ec90aef490d7f05f63abaf43d3ff4ae9a8d7 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Mon, 4 Oct 2021 11:44:19 +0200 Subject: [PATCH 01/11] Always set feature gates for ContainerVisitor test This fixes a bug where the test was dependent on the current set of feature gates. Since AllFeatureEnabledContainers() depends on the feature gates it must be evaluated after the test case is initialized. --- pkg/api/pod/util_test.go | 10 ++++++---- pkg/api/v1/pod/util_test.go | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 93f987ff7039..e9b5c4945547 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -36,6 +36,7 @@ import ( ) func TestVisitContainers(t *testing.T) { + setAllFeatureEnabledContainersDuringTest := ContainerType(0) testCases := []struct { desc string spec *api.PodSpec @@ -142,7 +143,7 @@ func TestVisitContainers(t *testing.T) { }, }, wantContainers: []string{"i1", "i2", "c1", "c2"}, - mask: AllFeatureEnabledContainers(), + mask: setAllFeatureEnabledContainersDuringTest, }, { desc: "all feature enabled container types with ephemeral containers enabled", @@ -161,7 +162,7 @@ func TestVisitContainers(t *testing.T) { }, }, wantContainers: []string{"i1", "i2", "c1", "c2", "e1", "e2"}, - mask: AllFeatureEnabledContainers(), + mask: setAllFeatureEnabledContainersDuringTest, ephemeralContainersEnabled: true, }, { @@ -187,8 +188,9 @@ func TestVisitContainers(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - if tc.ephemeralContainersEnabled { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, tc.ephemeralContainersEnabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, tc.ephemeralContainersEnabled)() + + if tc.mask == setAllFeatureEnabledContainersDuringTest { tc.mask = AllFeatureEnabledContainers() } diff --git a/pkg/api/v1/pod/util_test.go b/pkg/api/v1/pod/util_test.go index 961f801f7380..eb0425cf44a0 100644 --- a/pkg/api/v1/pod/util_test.go +++ b/pkg/api/v1/pod/util_test.go @@ -203,6 +203,7 @@ func TestFindPort(t *testing.T) { } func TestVisitContainers(t *testing.T) { + setAllFeatureEnabledContainersDuringTest := ContainerType(0) testCases := []struct { desc string spec *v1.PodSpec @@ -309,7 +310,7 @@ func TestVisitContainers(t *testing.T) { }, }, wantContainers: []string{"i1", "i2", "c1", "c2"}, - mask: AllFeatureEnabledContainers(), + mask: setAllFeatureEnabledContainersDuringTest, }, { desc: "all feature enabled container types with ephemeral containers enabled", @@ -328,7 +329,7 @@ func TestVisitContainers(t *testing.T) { }, }, wantContainers: []string{"i1", "i2", "c1", "c2", "e1", "e2"}, - mask: AllFeatureEnabledContainers(), + mask: setAllFeatureEnabledContainersDuringTest, ephemeralContainersEnabled: true, }, { @@ -354,8 +355,9 @@ func TestVisitContainers(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - if tc.ephemeralContainersEnabled { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, tc.ephemeralContainersEnabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, tc.ephemeralContainersEnabled)() + + if tc.mask == setAllFeatureEnabledContainersDuringTest { tc.mask = AllFeatureEnabledContainers() } From 6f4b8da9a30869b3216a7ee525cbdf6e23316dd5 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 1 Oct 2021 17:22:23 +0200 Subject: [PATCH 02/11] Promote EphemeralContainers feature to beta --- pkg/apis/core/types.go | 6 +++--- pkg/features/kube_features.go | 3 ++- staging/src/k8s.io/api/core/v1/types.go | 6 +++--- test/e2e_node/ephemeral_containers_test.go | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 62ea4c0c0dff..b3283c98313e 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -2749,7 +2749,7 @@ type PodSpec struct { // pod to perform user-initiated actions such as debugging. This list cannot be specified when // creating a pod, and it cannot be modified by updating the pod spec. In order to add an // ephemeral container to an existing pod, use the pod's ephemeralcontainers subresource. - // This field is alpha-level and is only honored by servers that enable the EphemeralContainers feature. + // This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate. // +optional EphemeralContainers []EphemeralContainer // +optional @@ -3188,7 +3188,7 @@ var _ = Container(EphemeralContainerCommon{}) // Ephemeral containers may not be added by directly updating the pod spec. They must be added // via the pod's ephemeralcontainers subresource, and they will appear in the pod spec // once added. -// This is an alpha feature enabled by the EphemeralContainers feature flag. +// This is a beta feature available on clusters that haven't disabled the EphemeralContainers feature gate. type EphemeralContainer struct { // Ephemeral containers have all of the fields of Container, plus additional fields // specific to ephemeral containers. Fields in common with Container are in the @@ -3253,7 +3253,7 @@ type PodStatus struct { ContainerStatuses []ContainerStatus // Status for any ephemeral containers that have run in this pod. - // This field is alpha-level and is only honored by servers that enable the EphemeralContainers feature. + // This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate. // +optional EphemeralContainerStatuses []ContainerStatus } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 3fdf0752ddef..ffca75ad1027 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -94,6 +94,7 @@ const ( // owner: @verb // alpha: v1.16 + // beta: v1.23 // // Allows running an ephemeral container in pod namespaces to troubleshoot a running pod. EphemeralContainers featuregate.Feature = "EphemeralContainers" @@ -791,7 +792,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS DevicePlugins: {Default: true, PreRelease: featuregate.Beta}, RotateKubeletServerCertificate: {Default: true, PreRelease: featuregate.Beta}, LocalStorageCapacityIsolation: {Default: true, PreRelease: featuregate.Beta}, - EphemeralContainers: {Default: false, PreRelease: featuregate.Alpha}, + EphemeralContainers: {Default: true, PreRelease: featuregate.Beta}, QOSReserved: {Default: false, PreRelease: featuregate.Alpha}, ExpandPersistentVolumes: {Default: true, PreRelease: featuregate.Beta}, ExpandInUsePersistentVolumes: {Default: true, PreRelease: featuregate.Beta}, diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index f0fa732281b9..2f6f80ce418f 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -2995,7 +2995,7 @@ type PodSpec struct { // pod to perform user-initiated actions such as debugging. This list cannot be specified when // creating a pod, and it cannot be modified by updating the pod spec. In order to add an // ephemeral container to an existing pod, use the pod's ephemeralcontainers subresource. - // This field is alpha-level and is only honored by servers that enable the EphemeralContainers feature. + // This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate. // +optional // +patchMergeKey=name // +patchStrategy=merge @@ -3582,7 +3582,7 @@ var _ = Container(EphemeralContainerCommon{}) // Ephemeral containers may not be added by directly updating the pod spec. They must be added // via the pod's ephemeralcontainers subresource, and they will appear in the pod spec // once added. -// This is an alpha feature enabled by the EphemeralContainers feature flag. +// This is a beta feature available on clusters that haven't disabled the EphemeralContainers feature gate. type EphemeralContainer struct { // Ephemeral containers have all of the fields of Container, plus additional fields // specific to ephemeral containers. Fields in common with Container are in the @@ -3683,7 +3683,7 @@ type PodStatus struct { // +optional QOSClass PodQOSClass `json:"qosClass,omitempty" protobuf:"bytes,9,rep,name=qosClass"` // Status for any ephemeral containers that have run in this pod. - // This field is alpha-level and is only populated by servers that enable the EphemeralContainers feature. + // This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate. // +optional EphemeralContainerStatuses []ContainerStatus `json:"ephemeralContainerStatuses,omitempty" protobuf:"bytes,13,rep,name=ephemeralContainerStatuses"` } diff --git a/test/e2e_node/ephemeral_containers_test.go b/test/e2e_node/ephemeral_containers_test.go index bf3d756f1d7e..db52e96667f1 100644 --- a/test/e2e_node/ephemeral_containers_test.go +++ b/test/e2e_node/ephemeral_containers_test.go @@ -27,7 +27,7 @@ import ( "github.com/onsi/ginkgo" ) -var _ = SIGDescribe("Ephemeral Containers [Feature:EphemeralContainers][NodeAlphaFeature:EphemeralContainers]", func() { +var _ = SIGDescribe("Ephemeral Containers", func() { f := framework.NewDefaultFramework("ephemeral-containers-test") var podClient *framework.PodClient ginkgo.BeforeEach(func() { From 764859081a315f2406e86a7fca48081aac96bf46 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Thu, 14 Oct 2021 15:27:38 -0700 Subject: [PATCH 03/11] Validate ephemeralContainerStatuses during update --- pkg/apis/core/validation/validation.go | 4 +- pkg/apis/core/validation/validation_test.go | 401 ++++++++++++++++++++ 2 files changed, 404 insertions(+), 1 deletion(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 781e9787e8e6..62bd05a68e1c 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4165,7 +4165,7 @@ func ValidateContainerStateTransition(newStatuses, oldStatuses []core.ContainerS return allErrs } -// ValidatePodStatusUpdate tests to see if the update is legal for an end user to make. +// ValidatePodStatusUpdate checks for changes to status that shouldn't occur in normal operation. func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) field.ErrorList { fldPath := field.NewPath("metadata") allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath) @@ -4187,6 +4187,8 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions // any terminated containers to a non-terminated state. allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.ContainerStatuses, oldPod.Status.ContainerStatuses, fldPath.Child("containerStatuses"), oldPod.Spec.RestartPolicy)...) allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.InitContainerStatuses, oldPod.Status.InitContainerStatuses, fldPath.Child("initContainerStatuses"), oldPod.Spec.RestartPolicy)...) + // The kubelet will never restart ephemeral containers, so treat them like they have an implicit RestartPolicyNever. + allErrs = append(allErrs, ValidateContainerStateTransition(newPod.Status.EphemeralContainerStatuses, oldPod.Status.EphemeralContainerStatuses, fldPath.Child("ephemeralContainerStatuses"), core.RestartPolicyNever)...) if newIPErrs := validatePodIPs(newPod); len(newIPErrs) > 0 { allErrs = append(allErrs, newIPErrs...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 76c995ed2fb8..0da7ec7896a8 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -22,6 +22,7 @@ import ( "reflect" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" asserttestify "github.com/stretchr/testify/assert" @@ -10332,6 +10333,406 @@ func TestValidatePodStatusUpdate(t *testing.T) { "", "Update nominatedNodeName", }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + Name: "init", + Ready: false, + Started: proto.Bool(false), + State: core.ContainerState{ + Waiting: &core.ContainerStateWaiting{ + Reason: "PodInitializing", + }, + }, + }}, + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + Name: "main", + Ready: false, + Started: proto.Bool(false), + State: core.ContainerState{ + Waiting: &core.ContainerStateWaiting{ + Reason: "PodInitializing", + }, + }, + }}, + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + "", + "Container statuses pending", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "init", + Ready: true, + State: core.ContainerState{ + Terminated: &core.ContainerStateTerminated{ + ContainerID: "docker://numbers", + Reason: "Completed", + }, + }, + }}, + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + Name: "init", + Ready: false, + State: core.ContainerState{ + Waiting: &core.ContainerStateWaiting{ + Reason: "PodInitializing", + }, + }, + }}, + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + Name: "main", + Ready: false, + Started: proto.Bool(false), + State: core.ContainerState{ + Waiting: &core.ContainerStateWaiting{ + Reason: "PodInitializing", + }, + }, + }}, + }, + }, + "", + "Container statuses running", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + EphemeralContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "busybox", + Name: "debug", + Ready: false, + State: core.ContainerState{ + Waiting: &core.ContainerStateWaiting{ + Reason: "PodInitializing", + }, + }, + }}, + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + }, + }, + "", + "Container statuses add ephemeral container", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + EphemeralContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "busybox", + ImageID: "docker-pullable://busybox@sha256:d0gf00d", + Name: "debug", + Ready: false, + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + EphemeralContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "busybox", + Name: "debug", + Ready: false, + State: core.ContainerState{ + Waiting: &core.ContainerStateWaiting{ + Reason: "PodInitializing", + }, + }, + }}, + }, + }, + "", + "Container statuses ephemeral container running", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + EphemeralContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "busybox", + ImageID: "docker-pullable://busybox@sha256:d0gf00d", + Name: "debug", + Ready: false, + State: core.ContainerState{ + Terminated: &core.ContainerStateTerminated{ + ContainerID: "docker://numbers", + Reason: "Completed", + StartedAt: metav1.NewTime(time.Now()), + FinishedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + EphemeralContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "busybox", + ImageID: "docker-pullable://busybox@sha256:d0gf00d", + Name: "debug", + Ready: false, + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + }, + }, + "", + "Container statuses ephemeral container exited", + }, + { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "init", + Ready: true, + State: core.ContainerState{ + Terminated: &core.ContainerStateTerminated{ + ContainerID: "docker://numbers", + Reason: "Completed", + }, + }, + }}, + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Terminated: &core.ContainerStateTerminated{ + ContainerID: "docker://numbers", + Reason: "Completed", + StartedAt: metav1.NewTime(time.Now()), + FinishedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + EphemeralContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "busybox", + ImageID: "docker-pullable://busybox@sha256:d0gf00d", + Name: "debug", + Ready: false, + State: core.ContainerState{ + Terminated: &core.ContainerStateTerminated{ + ContainerID: "docker://numbers", + Reason: "Completed", + StartedAt: metav1.NewTime(time.Now()), + FinishedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + }, + }, + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "init", + Ready: true, + State: core.ContainerState{ + Terminated: &core.ContainerStateTerminated{ + ContainerID: "docker://numbers", + Reason: "Completed", + }, + }, + }}, + ContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "nginx:alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "nginx", + Ready: true, + Started: proto.Bool(true), + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + EphemeralContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "busybox", + ImageID: "docker-pullable://busybox@sha256:d0gf00d", + Name: "debug", + Ready: false, + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + }, + }, + "", + "Container statuses all containers terminated", + }, } for _, test := range tests { From 26e183b9d9c741b564ca125846d6076981a2be08 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 15 Oct 2021 12:40:53 -0700 Subject: [PATCH 04/11] Clarify EphemeralContainer behavior in docs - Apply doc style guide - Specify behavior when namespace targeting isn't supported by runtime --- pkg/apis/core/types.go | 21 ++++++++++++--------- staging/src/k8s.io/api/core/v1/types.go | 21 ++++++++++++--------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index b3283c98313e..d0a0ce571257 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3180,14 +3180,15 @@ type EphemeralContainerCommon struct { // these two types. var _ = Container(EphemeralContainerCommon{}) -// An EphemeralContainer is a temporary container that may be added to an existing pod for +// An EphemeralContainer is a temporary container that you may add to an existing Pod for // user-initiated activities such as debugging. Ephemeral containers have no resource or -// scheduling guarantees, and they will not be restarted when they exit or when a pod is -// removed or restarted. If an ephemeral container causes a pod to exceed its resource -// allocation, the pod may be evicted. -// Ephemeral containers may not be added by directly updating the pod spec. They must be added -// via the pod's ephemeralcontainers subresource, and they will appear in the pod spec -// once added. +// scheduling guarantees, and they will not be restarted when they exit or when a Pod is +// removed or restarted. The kubelet may evict a Pod if an ephemeral container causes the +// Pod to exceed its resource allocation. +// +// To add an ephemeral container, use the ephemeralcontainers subresource of an existing +// Pod. Ephemeral containers may not be removed or restarted. +// // This is a beta feature available on clusters that haven't disabled the EphemeralContainers feature gate. type EphemeralContainer struct { // Ephemeral containers have all of the fields of Container, plus additional fields @@ -3198,8 +3199,10 @@ type EphemeralContainer struct { // If set, the name of the container from PodSpec that this ephemeral container targets. // The ephemeral container will be run in the namespaces (IPC, PID, etc) of this container. - // If not set then the ephemeral container is run in whatever namespaces are shared - // for the pod. Note that the container runtime must support this feature. + // If not set then the ephemeral container uses the namespaces configured in the Pod spec. + // + // The container runtime must implement support for this feature. If the runtime does not + // support namespace targeting then the result of setting this field is undefined. // +optional TargetContainerName string } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 2f6f80ce418f..eece7073612d 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3574,14 +3574,15 @@ type EphemeralContainerCommon struct { // these two types. var _ = Container(EphemeralContainerCommon{}) -// An EphemeralContainer is a container that may be added temporarily to an existing pod for +// An EphemeralContainer is a temporary container that you may add to an existing Pod for // user-initiated activities such as debugging. Ephemeral containers have no resource or -// scheduling guarantees, and they will not be restarted when they exit or when a pod is -// removed or restarted. If an ephemeral container causes a pod to exceed its resource -// allocation, the pod may be evicted. -// Ephemeral containers may not be added by directly updating the pod spec. They must be added -// via the pod's ephemeralcontainers subresource, and they will appear in the pod spec -// once added. +// scheduling guarantees, and they will not be restarted when they exit or when a Pod is +// removed or restarted. The kubelet may evict a Pod if an ephemeral container causes the +// Pod to exceed its resource allocation. +// +// To add an ephemeral container, use the ephemeralcontainers subresource of an existing +// Pod. Ephemeral containers may not be removed or restarted. +// // This is a beta feature available on clusters that haven't disabled the EphemeralContainers feature gate. type EphemeralContainer struct { // Ephemeral containers have all of the fields of Container, plus additional fields @@ -3592,8 +3593,10 @@ type EphemeralContainer struct { // If set, the name of the container from PodSpec that this ephemeral container targets. // The ephemeral container will be run in the namespaces (IPC, PID, etc) of this container. - // If not set then the ephemeral container is run in whatever namespaces are shared - // for the pod. Note that the container runtime must support this feature. + // If not set then the ephemeral container uses the namespaces configured in the Pod spec. + // + // The container runtime must implement support for this feature. If the runtime does not + // support namespace targeting then the result of setting this field is undefined. // +optional TargetContainerName string `json:"targetContainerName,omitempty" protobuf:"bytes,2,opt,name=targetContainerName"` } From d1d7882186b6ff51da1e0979f8294c1bc42b3f69 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 15 Oct 2021 17:50:37 -0700 Subject: [PATCH 05/11] Add test for EphemeralContainerCommon struct tags The tags for type EphemeralContainerCommon should be kept in sync with those of type Container. Co-authored-by: Jordan Liggitt --- api/api-rules/violation_exceptions.list | 1 - pkg/apis/core/types_test.go | 41 ++++++++++++++++++++ staging/src/k8s.io/api/core/v1/types.go | 8 +++- staging/src/k8s.io/api/core/v1/types_test.go | 19 +++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 pkg/apis/core/types_test.go diff --git a/api/api-rules/violation_exceptions.list b/api/api-rules/violation_exceptions.list index ebb0bd6f40fa..0e6a3e730574 100644 --- a/api/api-rules/violation_exceptions.list +++ b/api/api-rules/violation_exceptions.list @@ -90,7 +90,6 @@ API rule violation: list_type_missing,k8s.io/api/core/v1,EphemeralContainerCommo API rule violation: list_type_missing,k8s.io/api/core/v1,EphemeralContainerCommon,Command API rule violation: list_type_missing,k8s.io/api/core/v1,EphemeralContainerCommon,Env API rule violation: list_type_missing,k8s.io/api/core/v1,EphemeralContainerCommon,EnvFrom -API rule violation: list_type_missing,k8s.io/api/core/v1,EphemeralContainerCommon,Ports API rule violation: list_type_missing,k8s.io/api/core/v1,EphemeralContainerCommon,VolumeDevices API rule violation: list_type_missing,k8s.io/api/core/v1,EphemeralContainerCommon,VolumeMounts API rule violation: list_type_missing,k8s.io/api/core/v1,ExecAction,Command diff --git a/pkg/apis/core/types_test.go b/pkg/apis/core/types_test.go new file mode 100644 index 000000000000..813036d13a08 --- /dev/null +++ b/pkg/apis/core/types_test.go @@ -0,0 +1,41 @@ +/* +Copyright 2021 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 core + +import ( + "reflect" + "testing" +) + +// TestEphemeralContainer ensures that the tags of Container and EphemeralContainerCommon are kept in sync. +func TestEphemeralContainer(t *testing.T) { + ephemeralType := reflect.TypeOf(EphemeralContainerCommon{}) + containerType := reflect.TypeOf(Container{}) + + ephemeralFields := ephemeralType.NumField() + containerFields := containerType.NumField() + if containerFields != ephemeralFields { + t.Fatalf("%v has %d fields, %v has %d fields", ephemeralType, ephemeralFields, containerType, containerFields) + } + for i := 0; i < ephemeralFields; i++ { + ephemeralField := ephemeralType.Field(i) + containerField := containerType.Field(i) + if !reflect.DeepEqual(ephemeralField, containerField) { + t.Errorf("field %v differs:\n\t%#v\n\t%#v", ephemeralField.Name, ephemeralField, containerField) + } + } +} diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index eece7073612d..c5325d71b21b 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3475,7 +3475,13 @@ type EphemeralContainerCommon struct { // +optional WorkingDir string `json:"workingDir,omitempty" protobuf:"bytes,5,opt,name=workingDir"` // Ports are not allowed for ephemeral containers. - Ports []ContainerPort `json:"ports,omitempty" protobuf:"bytes,6,rep,name=ports"` + // +optional + // +patchMergeKey=containerPort + // +patchStrategy=merge + // +listType=map + // +listMapKey=containerPort + // +listMapKey=protocol + Ports []ContainerPort `json:"ports,omitempty" patchStrategy:"merge" patchMergeKey:"containerPort" protobuf:"bytes,6,rep,name=ports"` // List of sources to populate environment variables in the container. // The keys defined within a source must be a C_IDENTIFIER. All invalid keys // will be reported as an event when the container is starting. When a key exists in multiple diff --git a/staging/src/k8s.io/api/core/v1/types_test.go b/staging/src/k8s.io/api/core/v1/types_test.go index 6c602403befa..923c3b868b5d 100644 --- a/staging/src/k8s.io/api/core/v1/types_test.go +++ b/staging/src/k8s.io/api/core/v1/types_test.go @@ -39,3 +39,22 @@ func Test_ServiceSpecRemovedFieldProtobufNumberReservation(t *testing.T) { } } } + +// TestEphemeralContainer ensures that the tags of Container and EphemeralContainerCommon are kept in sync. +func TestEphemeralContainer(t *testing.T) { + ephemeralType := reflect.TypeOf(EphemeralContainerCommon{}) + containerType := reflect.TypeOf(Container{}) + + ephemeralFields := ephemeralType.NumField() + containerFields := containerType.NumField() + if containerFields != ephemeralFields { + t.Fatalf("%v has %d fields, %v has %d fields", ephemeralType, ephemeralFields, containerType, containerFields) + } + for i := 0; i < ephemeralFields; i++ { + ephemeralField := ephemeralType.Field(i) + containerField := containerType.Field(i) + if !reflect.DeepEqual(ephemeralField, containerField) { + t.Errorf("field %v differs:\n\t%#v\n\t%#v", ephemeralField.Name, ephemeralField, containerField) + } + } +} From f81c48cd0ae86da76720f4746802e5708823cdf9 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Sat, 16 Oct 2021 07:02:10 -0700 Subject: [PATCH 06/11] Disallow subpath for ephemeral container mounts --- pkg/apis/core/types.go | 1 + pkg/apis/core/validation/validation.go | 12 +++++++ pkg/apis/core/validation/validation_test.go | 36 +++++++++++++++++++++ staging/src/k8s.io/api/core/v1/types.go | 2 +- 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index d0a0ce571257..7a690f64b197 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3137,6 +3137,7 @@ type EphemeralContainerCommon struct { // already allocated to the pod. // +optional Resources ResourceRequirements + // Pod volumes to mount into the container's filesystem. Subpath mounts are not allowed for ephemeral containers. // +optional VolumeMounts []VolumeMount // volumeDevices is the list of block devices to be used by the container. diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 62bd05a68e1c..11fcb32908b6 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2873,6 +2873,18 @@ func validateEphemeralContainers(ephemeralContainers []core.EphemeralContainer, // Lifecycle, probes, resources and ports should be disallowed. This is implemented as a list // of allowed fields so that new fields will be given consideration prior to inclusion in Ephemeral Containers. allErrs = append(allErrs, validateFieldAllowList(ec.EphemeralContainerCommon, allowedEphemeralContainerFields, "cannot be set for an Ephemeral Container", idxPath)...) + + // VolumeMount subpaths have the potential to leak resources since they're implemented with bind mounts + // that aren't cleaned up until the pod exits. Since they also imply that the container is being used + // as part of the workload, they're disallowed entirely. + for i, vm := range ec.VolumeMounts { + if vm.SubPath != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPath"), "cannot be set for an Ephemeral Container")) + } + if vm.SubPathExpr != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPathExpr"), "cannot be set for an Ephemeral Container")) + } + } } return allErrs diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 0da7ec7896a8..91c4e05acb43 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -6360,6 +6360,42 @@ func TestValidateEphemeralContainers(t *testing.T) { }, field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].resources"}, }, + { + "Container uses disallowed field: VolumeMount.SubPath", + []core.EphemeralContainer{ + { + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + VolumeMounts: []core.VolumeMount{ + {Name: "vol", MountPath: "/vol"}, + {Name: "vol", MountPath: "/volsub", SubPath: "foo"}, + }, + }, + }, + }, + field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPath"}, + }, + { + "Container uses disallowed field: VolumeMount.SubPathExpr", + []core.EphemeralContainer{ + { + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + VolumeMounts: []core.VolumeMount{ + {Name: "vol", MountPath: "/vol"}, + {Name: "vol", MountPath: "/volsub", SubPathExpr: "$(POD_NAME)"}, + }, + }, + }, + }, + field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPathExpr"}, + }, } for _, tc := range tcs { diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index c5325d71b21b..b7fba95f4462 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3500,7 +3500,7 @@ type EphemeralContainerCommon struct { // already allocated to the pod. // +optional Resources ResourceRequirements `json:"resources,omitempty" protobuf:"bytes,8,opt,name=resources"` - // Pod volumes to mount into the container's filesystem. + // Pod volumes to mount into the container's filesystem. Subpath mounts are not allowed for ephemeral containers. // Cannot be updated. // +optional // +patchMergeKey=mountPath From b34e710972969a388e4691bb67be0a2e4d34676f Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 1 Oct 2021 17:48:21 +0200 Subject: [PATCH 07/11] Generated code for EphemeralContainers beta --- api/openapi-spec/swagger.json | 19 ++++++---- .../src/k8s.io/api/core/v1/generated.proto | 35 ++++++++++++------- .../core/v1/types_swagger_doc_generated.go | 10 +++--- .../applyconfigurations/internal/internal.go | 5 ++- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 17fc53a7374f..5d833a38fe50 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -5437,7 +5437,7 @@ "type": "object" }, "io.k8s.api.core.v1.EphemeralContainer": { - "description": "An EphemeralContainer is a container that may be added temporarily to an existing pod for user-initiated activities such as debugging. Ephemeral containers have no resource or scheduling guarantees, and they will not be restarted when they exit or when a pod is removed or restarted. If an ephemeral container causes a pod to exceed its resource allocation, the pod may be evicted. Ephemeral containers may not be added by directly updating the pod spec. They must be added via the pod's ephemeralcontainers subresource, and they will appear in the pod spec once added. This is an alpha feature enabled by the EphemeralContainers feature flag.", + "description": "An EphemeralContainer is a temporary container that you may add to an existing Pod for user-initiated activities such as debugging. Ephemeral containers have no resource or scheduling guarantees, and they will not be restarted when they exit or when a Pod is removed or restarted. The kubelet may evict a Pod if an ephemeral container causes the Pod to exceed its resource allocation.\n\nTo add an ephemeral container, use the ephemeralcontainers subresource of an existing Pod. Ephemeral containers may not be removed or restarted.\n\nThis is a beta feature available on clusters that haven't disabled the EphemeralContainers feature gate.", "properties": { "args": { "description": "Arguments to the entrypoint. The docker image's CMD is used if this is not provided. Variable references $(VAR_NAME) are expanded using the container's environment. If a variable cannot be resolved, the reference in the input string will be unchanged. Double $$ are reduced to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e. \"$$(VAR_NAME)\" will produce the string literal \"$(VAR_NAME)\". Escaped references will never be expanded, regardless of whether the variable exists or not. Cannot be updated. More info: https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/#running-a-command-in-a-shell", @@ -5494,7 +5494,14 @@ "items": { "$ref": "#/definitions/io.k8s.api.core.v1.ContainerPort" }, - "type": "array" + "type": "array", + "x-kubernetes-list-map-keys": [ + "containerPort", + "protocol" + ], + "x-kubernetes-list-type": "map", + "x-kubernetes-patch-merge-key": "containerPort", + "x-kubernetes-patch-strategy": "merge" }, "readinessProbe": { "$ref": "#/definitions/io.k8s.api.core.v1.Probe", @@ -5521,7 +5528,7 @@ "type": "boolean" }, "targetContainerName": { - "description": "If set, the name of the container from PodSpec that this ephemeral container targets. The ephemeral container will be run in the namespaces (IPC, PID, etc) of this container. If not set then the ephemeral container is run in whatever namespaces are shared for the pod. Note that the container runtime must support this feature.", + "description": "If set, the name of the container from PodSpec that this ephemeral container targets. The ephemeral container will be run in the namespaces (IPC, PID, etc) of this container. If not set then the ephemeral container uses the namespaces configured in the Pod spec.\n\nThe container runtime must implement support for this feature. If the runtime does not support namespace targeting then the result of setting this field is undefined.", "type": "string" }, "terminationMessagePath": { @@ -5546,7 +5553,7 @@ "x-kubernetes-patch-strategy": "merge" }, "volumeMounts": { - "description": "Pod volumes to mount into the container's filesystem. Cannot be updated.", + "description": "Pod volumes to mount into the container's filesystem. Subpath mounts are not allowed for ephemeral containers. Cannot be updated.", "items": { "$ref": "#/definitions/io.k8s.api.core.v1.VolumeMount" }, @@ -7782,7 +7789,7 @@ "type": "boolean" }, "ephemeralContainers": { - "description": "List of ephemeral containers run in this pod. Ephemeral containers may be run in an existing pod to perform user-initiated actions such as debugging. This list cannot be specified when creating a pod, and it cannot be modified by updating the pod spec. In order to add an ephemeral container to an existing pod, use the pod's ephemeralcontainers subresource. This field is alpha-level and is only honored by servers that enable the EphemeralContainers feature.", + "description": "List of ephemeral containers run in this pod. Ephemeral containers may be run in an existing pod to perform user-initiated actions such as debugging. This list cannot be specified when creating a pod, and it cannot be modified by updating the pod spec. In order to add an ephemeral container to an existing pod, use the pod's ephemeralcontainers subresource. This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate.", "items": { "$ref": "#/definitions/io.k8s.api.core.v1.EphemeralContainer" }, @@ -7969,7 +7976,7 @@ "type": "array" }, "ephemeralContainerStatuses": { - "description": "Status for any ephemeral containers that have run in this pod. This field is alpha-level and is only populated by servers that enable the EphemeralContainers feature.", + "description": "Status for any ephemeral containers that have run in this pod. This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate.", "items": { "$ref": "#/definitions/io.k8s.api.core.v1.ContainerStatus" }, diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index b53d63b642ed..574ed9799c3c 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -1186,15 +1186,16 @@ message EnvVarSource { optional SecretKeySelector secretKeyRef = 4; } -// An EphemeralContainer is a container that may be added temporarily to an existing pod for +// An EphemeralContainer is a temporary container that you may add to an existing Pod for // user-initiated activities such as debugging. Ephemeral containers have no resource or -// scheduling guarantees, and they will not be restarted when they exit or when a pod is -// removed or restarted. If an ephemeral container causes a pod to exceed its resource -// allocation, the pod may be evicted. -// Ephemeral containers may not be added by directly updating the pod spec. They must be added -// via the pod's ephemeralcontainers subresource, and they will appear in the pod spec -// once added. -// This is an alpha feature enabled by the EphemeralContainers feature flag. +// scheduling guarantees, and they will not be restarted when they exit or when a Pod is +// removed or restarted. The kubelet may evict a Pod if an ephemeral container causes the +// Pod to exceed its resource allocation. +// +// To add an ephemeral container, use the ephemeralcontainers subresource of an existing +// Pod. Ephemeral containers may not be removed or restarted. +// +// This is a beta feature available on clusters that haven't disabled the EphemeralContainers feature gate. message EphemeralContainer { // Ephemeral containers have all of the fields of Container, plus additional fields // specific to ephemeral containers. Fields in common with Container are in the @@ -1204,8 +1205,10 @@ message EphemeralContainer { // If set, the name of the container from PodSpec that this ephemeral container targets. // The ephemeral container will be run in the namespaces (IPC, PID, etc) of this container. - // If not set then the ephemeral container is run in whatever namespaces are shared - // for the pod. Note that the container runtime must support this feature. + // If not set then the ephemeral container uses the namespaces configured in the Pod spec. + // + // The container runtime must implement support for this feature. If the runtime does not + // support namespace targeting then the result of setting this field is undefined. // +optional optional string targetContainerName = 2; } @@ -1253,6 +1256,12 @@ message EphemeralContainerCommon { optional string workingDir = 5; // Ports are not allowed for ephemeral containers. + // +optional + // +patchMergeKey=containerPort + // +patchStrategy=merge + // +listType=map + // +listMapKey=containerPort + // +listMapKey=protocol repeated ContainerPort ports = 6; // List of sources to populate environment variables in the container. @@ -1276,7 +1285,7 @@ message EphemeralContainerCommon { // +optional optional ResourceRequirements resources = 8; - // Pod volumes to mount into the container's filesystem. + // Pod volumes to mount into the container's filesystem. Subpath mounts are not allowed for ephemeral containers. // Cannot be updated. // +optional // +patchMergeKey=mountPath @@ -3421,7 +3430,7 @@ message PodSpec { // pod to perform user-initiated actions such as debugging. This list cannot be specified when // creating a pod, and it cannot be modified by updating the pod spec. In order to add an // ephemeral container to an existing pod, use the pod's ephemeralcontainers subresource. - // This field is alpha-level and is only honored by servers that enable the EphemeralContainers feature. + // This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate. // +optional // +patchMergeKey=name // +patchStrategy=merge @@ -3735,7 +3744,7 @@ message PodStatus { optional string qosClass = 9; // Status for any ephemeral containers that have run in this pod. - // This field is alpha-level and is only populated by servers that enable the EphemeralContainers feature. + // This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate. // +optional repeated ContainerStatus ephemeralContainerStatuses = 13; } 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 f315b1627cbf..d45ab1d6be82 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 @@ -579,8 +579,8 @@ func (EnvVarSource) SwaggerDoc() map[string]string { } var map_EphemeralContainer = map[string]string{ - "": "An EphemeralContainer is a container that may be added temporarily to an existing pod for user-initiated activities such as debugging. Ephemeral containers have no resource or scheduling guarantees, and they will not be restarted when they exit or when a pod is removed or restarted. If an ephemeral container causes a pod to exceed its resource allocation, the pod may be evicted. Ephemeral containers may not be added by directly updating the pod spec. They must be added via the pod's ephemeralcontainers subresource, and they will appear in the pod spec once added. This is an alpha feature enabled by the EphemeralContainers feature flag.", - "targetContainerName": "If set, the name of the container from PodSpec that this ephemeral container targets. The ephemeral container will be run in the namespaces (IPC, PID, etc) of this container. If not set then the ephemeral container is run in whatever namespaces are shared for the pod. Note that the container runtime must support this feature.", + "": "An EphemeralContainer is a temporary container that you may add to an existing Pod for user-initiated activities such as debugging. Ephemeral containers have no resource or scheduling guarantees, and they will not be restarted when they exit or when a Pod is removed or restarted. The kubelet may evict a Pod if an ephemeral container causes the Pod to exceed its resource allocation.\n\nTo add an ephemeral container, use the ephemeralcontainers subresource of an existing Pod. Ephemeral containers may not be removed or restarted.\n\nThis is a beta feature available on clusters that haven't disabled the EphemeralContainers feature gate.", + "targetContainerName": "If set, the name of the container from PodSpec that this ephemeral container targets. The ephemeral container will be run in the namespaces (IPC, PID, etc) of this container. If not set then the ephemeral container uses the namespaces configured in the Pod spec.\n\nThe container runtime must implement support for this feature. If the runtime does not support namespace targeting then the result of setting this field is undefined.", } func (EphemeralContainer) SwaggerDoc() map[string]string { @@ -598,7 +598,7 @@ var map_EphemeralContainerCommon = map[string]string{ "envFrom": "List of sources to populate environment variables in the container. The keys defined within a source must be a C_IDENTIFIER. All invalid keys will be reported as an event when the container is starting. When a key exists in multiple sources, the value associated with the last source will take precedence. Values defined by an Env with a duplicate key will take precedence. Cannot be updated.", "env": "List of environment variables to set in the container. Cannot be updated.", "resources": "Resources are not allowed for ephemeral containers. Ephemeral containers use spare resources already allocated to the pod.", - "volumeMounts": "Pod volumes to mount into the container's filesystem. Cannot be updated.", + "volumeMounts": "Pod volumes to mount into the container's filesystem. Subpath mounts are not allowed for ephemeral containers. Cannot be updated.", "volumeDevices": "volumeDevices is the list of block devices to be used by the container.", "livenessProbe": "Probes are not allowed for ephemeral containers.", "readinessProbe": "Probes are not allowed for ephemeral containers.", @@ -1617,7 +1617,7 @@ var map_PodSpec = map[string]string{ "volumes": "List of volumes that can be mounted by containers belonging to the pod. More info: https://kubernetes.io/docs/concepts/storage/volumes", "initContainers": "List of initialization containers belonging to the pod. Init containers are executed in order prior to containers being started. If any init container fails, the pod is considered to have failed and is handled according to its restartPolicy. The name for an init container or normal container must be unique among all containers. Init containers may not have Lifecycle actions, Readiness probes, Liveness probes, or Startup probes. The resourceRequirements of an init container are taken into account during scheduling by finding the highest request/limit for each resource type, and then using the max of of that value or the sum of the normal containers. Limits are applied to init containers in a similar fashion. Init containers cannot currently be added or removed. Cannot be updated. More info: https://kubernetes.io/docs/concepts/workloads/pods/init-containers/", "containers": "List of containers belonging to the pod. Containers cannot currently be added or removed. There must be at least one container in a Pod. Cannot be updated.", - "ephemeralContainers": "List of ephemeral containers run in this pod. Ephemeral containers may be run in an existing pod to perform user-initiated actions such as debugging. This list cannot be specified when creating a pod, and it cannot be modified by updating the pod spec. In order to add an ephemeral container to an existing pod, use the pod's ephemeralcontainers subresource. This field is alpha-level and is only honored by servers that enable the EphemeralContainers feature.", + "ephemeralContainers": "List of ephemeral containers run in this pod. Ephemeral containers may be run in an existing pod to perform user-initiated actions such as debugging. This list cannot be specified when creating a pod, and it cannot be modified by updating the pod spec. In order to add an ephemeral container to an existing pod, use the pod's ephemeralcontainers subresource. This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate.", "restartPolicy": "Restart policy for all containers within the pod. One of Always, OnFailure, Never. Default to Always. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#restart-policy", "terminationGracePeriodSeconds": "Optional duration in seconds the pod needs to terminate gracefully. May be decreased in delete request. Value must be non-negative integer. The value zero indicates stop immediately via the kill signal (no opportunity to shut down). If this value is nil, the default grace period will be used instead. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process. Defaults to 30 seconds.", "activeDeadlineSeconds": "Optional duration in seconds the pod may be active on the node relative to StartTime before the system will actively try to mark it failed and kill associated containers. Value must be a positive integer.", @@ -1669,7 +1669,7 @@ var map_PodStatus = map[string]string{ "initContainerStatuses": "The list has one entry per init container in the manifest. The most recent successful init container will have ready = true, the most recently started container will have startTime set. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#pod-and-container-status", "containerStatuses": "The list has one entry per container in the manifest. Each entry is currently the output of `docker inspect`. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#pod-and-container-status", "qosClass": "The Quality of Service (QOS) classification assigned to the pod based on resource requirements See PodQOSClass type for available QOS classes More info: https://git.k8s.io/community/contributors/design-proposals/node/resource-qos.md", - "ephemeralContainerStatuses": "Status for any ephemeral containers that have run in this pod. This field is alpha-level and is only populated by servers that enable the EphemeralContainers feature.", + "ephemeralContainerStatuses": "Status for any ephemeral containers that have run in this pod. This field is beta-level and available on clusters that haven't disabled the EphemeralContainers feature gate.", } func (PodStatus) SwaggerDoc() map[string]string { diff --git a/staging/src/k8s.io/client-go/applyconfigurations/internal/internal.go b/staging/src/k8s.io/client-go/applyconfigurations/internal/internal.go index c47942f47e4a..2f57aae36f25 100644 --- a/staging/src/k8s.io/client-go/applyconfigurations/internal/internal.go +++ b/staging/src/k8s.io/client-go/applyconfigurations/internal/internal.go @@ -3878,7 +3878,10 @@ var schemaYAML = typed.YAMLObject(`types: list: elementType: namedType: io.k8s.api.core.v1.ContainerPort - elementRelationship: atomic + elementRelationship: associative + keys: + - containerPort + - protocol - name: readinessProbe type: namedType: io.k8s.api.core.v1.Probe From ba649b97b7334fd0ecaaf64fd2382b5236976f70 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Tue, 19 Oct 2021 08:37:28 -0400 Subject: [PATCH 08/11] Add ephemeral container checks to volume e2e tests --- test/e2e/framework/pods.go | 9 ++++--- test/e2e/framework/volume/fixtures.go | 30 +++++++++++++++++----- test/e2e_node/ephemeral_containers_test.go | 4 ++- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/test/e2e/framework/pods.go b/test/e2e/framework/pods.go index fb5aea162f1f..65e8859ed196 100644 --- a/test/e2e/framework/pods.go +++ b/test/e2e/framework/pods.go @@ -151,7 +151,7 @@ func (c *PodClient) Update(name string, updateFn func(pod *v1.Pod)) { } // AddEphemeralContainerSync adds an EphemeralContainer to a pod and waits for it to be running. -func (c *PodClient) AddEphemeralContainerSync(pod *v1.Pod, ec *v1.EphemeralContainer, timeout time.Duration) { +func (c *PodClient) AddEphemeralContainerSync(pod *v1.Pod, ec *v1.EphemeralContainer, timeout time.Duration) error { namespace := c.f.Namespace.Name podJS, err := json.Marshal(pod) @@ -165,10 +165,13 @@ func (c *PodClient) AddEphemeralContainerSync(pod *v1.Pod, ec *v1.EphemeralConta patch, err := strategicpatch.CreateTwoWayMergePatch(podJS, ecJS, pod) ExpectNoError(err, "error creating patch to add ephemeral container %q", format.Pod(pod)) - _, err = c.Patch(context.TODO(), pod.Name, types.StrategicMergePatchType, patch, metav1.PatchOptions{}, "ephemeralcontainers") - ExpectNoError(err, "Failed to patch ephemeral containers in pod %q", format.Pod(pod)) + // Clients may optimistically attempt to add an ephemeral container to determine whether the EphemeralContainers feature is enabled. + if _, err := c.Patch(context.TODO(), pod.Name, types.StrategicMergePatchType, patch, metav1.PatchOptions{}, "ephemeralcontainers"); err != nil { + return err + } ExpectNoError(e2epod.WaitForContainerRunning(c.f.ClientSet, namespace, pod.Name, ec.Name, timeout)) + return nil } // DeleteSync deletes the pod and wait for the pod to disappear for `timeout`. If the pod doesn't diff --git a/test/e2e/framework/volume/fixtures.go b/test/e2e/framework/volume/fixtures.go index 978ad8bd6db3..577d74edb4c8 100644 --- a/test/e2e/framework/volume/fixtures.go +++ b/test/e2e/framework/volume/fixtures.go @@ -448,14 +448,14 @@ func runVolumeTesterPod(client clientset.Interface, timeouts *framework.TimeoutC return clientPod, nil } -func testVolumeContent(f *framework.Framework, pod *v1.Pod, fsGroup *int64, fsType string, tests []Test) { +func testVolumeContent(f *framework.Framework, pod *v1.Pod, containerName string, fsGroup *int64, fsType string, tests []Test) { ginkgo.By("Checking that text file contents are perfect.") for i, test := range tests { if test.Mode == v1.PersistentVolumeBlock { // Block: check content deviceName := fmt.Sprintf("/opt/%d", i) commands := GenerateReadBlockCmd(deviceName, len(test.ExpectedContent)) - _, err := framework.LookForStringInPodExec(pod.Namespace, pod.Name, commands, test.ExpectedContent, time.Minute) + _, err := framework.LookForStringInPodExecToContainer(pod.Namespace, pod.Name, containerName, commands, test.ExpectedContent, time.Minute) framework.ExpectNoError(err, "failed: finding the contents of the block device %s.", deviceName) // Check that it's a real block device @@ -464,7 +464,7 @@ func testVolumeContent(f *framework.Framework, pod *v1.Pod, fsGroup *int64, fsTy // Filesystem: check content fileName := fmt.Sprintf("/opt/%d/%s", i, test.File) commands := GenerateReadFileCmd(fileName) - _, err := framework.LookForStringInPodExec(pod.Namespace, pod.Name, commands, test.ExpectedContent, time.Minute) + _, err := framework.LookForStringInPodExecToContainer(pod.Namespace, pod.Name, containerName, commands, test.ExpectedContent, time.Minute) framework.ExpectNoError(err, "failed: finding the contents of the mounted file %s.", fileName) // Check that a directory has been mounted @@ -475,14 +475,14 @@ func testVolumeContent(f *framework.Framework, pod *v1.Pod, fsGroup *int64, fsTy // Filesystem: check fsgroup if fsGroup != nil { ginkgo.By("Checking fsGroup is correct.") - _, err = framework.LookForStringInPodExec(pod.Namespace, pod.Name, []string{"ls", "-ld", dirName}, strconv.Itoa(int(*fsGroup)), time.Minute) + _, err = framework.LookForStringInPodExecToContainer(pod.Namespace, pod.Name, containerName, []string{"ls", "-ld", dirName}, strconv.Itoa(int(*fsGroup)), time.Minute) framework.ExpectNoError(err, "failed: getting the right privileges in the file %v", int(*fsGroup)) } // Filesystem: check fsType if fsType != "" { ginkgo.By("Checking fsType is correct.") - _, err = framework.LookForStringInPodExec(pod.Namespace, pod.Name, []string{"grep", " " + dirName + " ", "/proc/mounts"}, fsType, time.Minute) + _, err = framework.LookForStringInPodExecToContainer(pod.Namespace, pod.Name, containerName, []string{"grep", " " + dirName + " ", "/proc/mounts"}, fsType, time.Minute) framework.ExpectNoError(err, "failed: getting the right fsType %s", fsType) } } @@ -521,7 +521,23 @@ func testVolumeClient(f *framework.Framework, config TestConfig, fsGroup *int64, e2epod.WaitForPodToDisappear(f.ClientSet, clientPod.Namespace, clientPod.Name, labels.Everything(), framework.Poll, timeouts.PodDelete) }() - testVolumeContent(f, clientPod, fsGroup, fsType, tests) + testVolumeContent(f, clientPod, "", fsGroup, fsType, tests) + + ginkgo.By("Repeating the test on an ephemeral container (if enabled)") + ec := &v1.EphemeralContainer{ + EphemeralContainerCommon: v1.EphemeralContainerCommon(clientPod.Spec.Containers[0]), + } + ec.Name = "volume-ephemeral-container" + err = f.PodClient().AddEphemeralContainerSync(clientPod, ec, timeouts.PodStart) + // The API server will return NotFound for the subresource when the feature is disabled + // BEGIN TODO: remove after EphemeralContainers feature gate is retired + if apierrors.IsNotFound(err) { + framework.Logf("Skipping ephemeral container re-test because feature is disabled (error: %q)", err) + return + } + // END TODO: remove after EphemeralContainers feature gate is retired + framework.ExpectNoError(err, "failed to add ephemeral container for re-test") + testVolumeContent(f, clientPod, ec.Name, fsGroup, fsType, tests) } // InjectContent inserts index.html with given content into given volume. It does so by @@ -562,7 +578,7 @@ func InjectContent(f *framework.Framework, config TestConfig, fsGroup *int64, fs // Check that the data have been really written in this pod. // This tests non-persistent volume types - testVolumeContent(f, injectorPod, fsGroup, fsType, tests) + testVolumeContent(f, injectorPod, "", fsGroup, fsType, tests) } // generateWriteCmd is used by generateWriteBlockCmd and generateWriteFileCmd diff --git a/test/e2e_node/ephemeral_containers_test.go b/test/e2e_node/ephemeral_containers_test.go index db52e96667f1..17c5d03ce545 100644 --- a/test/e2e_node/ephemeral_containers_test.go +++ b/test/e2e_node/ephemeral_containers_test.go @@ -21,6 +21,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/kubernetes/pkg/kubelet/util/format" "k8s.io/kubernetes/test/e2e/framework" imageutils "k8s.io/kubernetes/test/utils/image" @@ -60,7 +61,8 @@ var _ = SIGDescribe("Ephemeral Containers", func() { TTY: true, }, } - podClient.AddEphemeralContainerSync(pod, ec, time.Minute) + err := podClient.AddEphemeralContainerSync(pod, ec, time.Minute) + framework.ExpectNoError(err, "Failed to patch ephemeral containers in pod %q", format.Pod(pod)) ginkgo.By("confirm that the container is really running") marco := f.ExecCommandInContainer(pod.Name, "debugger", "/bin/echo", "polo") From 40e7689f0e9dcff6f45bb5eea423500cd4fcafd2 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Tue, 19 Oct 2021 16:41:39 -0400 Subject: [PATCH 09/11] Move ephemeral container e2e to common --- .../common/node/ephemeral_containers.go} | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) rename test/{e2e_node/ephemeral_containers_test.go => e2e/common/node/ephemeral_containers.go} (64%) diff --git a/test/e2e_node/ephemeral_containers_test.go b/test/e2e/common/node/ephemeral_containers.go similarity index 64% rename from test/e2e_node/ephemeral_containers_test.go rename to test/e2e/common/node/ephemeral_containers.go index 17c5d03ce545..106b85f690ab 100644 --- a/test/e2e_node/ephemeral_containers_test.go +++ b/test/e2e/common/node/ephemeral_containers.go @@ -14,15 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -package e2enode +package node import ( "time" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/kubernetes/pkg/kubelet/util/format" "k8s.io/kubernetes/test/e2e/framework" + e2epod "k8s.io/kubernetes/test/e2e/framework/pod" + e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" imageutils "k8s.io/kubernetes/test/utils/image" "github.com/onsi/ginkgo" @@ -52,20 +55,28 @@ var _ = SIGDescribe("Ephemeral Containers", func() { }) ginkgo.By("adding an ephemeral container") + ecName := "debugger" ec := &v1.EphemeralContainer{ EphemeralContainerCommon: v1.EphemeralContainerCommon{ - Name: "debugger", + Name: ecName, Image: imageutils.GetE2EImage(imageutils.BusyBox), - Command: []string{"/bin/sh"}, + Command: e2epod.GenerateScriptCmd("while true; do sleep 2; echo polo; done"), Stdin: true, TTY: true, }, } err := podClient.AddEphemeralContainerSync(pod, ec, time.Minute) + // BEGIN TODO: Remove when EphemeralContainers feature gate is retired. + if apierrors.IsNotFound(err) { + e2eskipper.Skipf("Skipping test because EphemeralContainers feature disabled (error: %q)", err) + } + // END TODO: Remove when EphemeralContainers feature gate is retired. framework.ExpectNoError(err, "Failed to patch ephemeral containers in pod %q", format.Pod(pod)) - ginkgo.By("confirm that the container is really running") - marco := f.ExecCommandInContainer(pod.Name, "debugger", "/bin/echo", "polo") - framework.ExpectEqual(marco, "polo") + ginkgo.By("checking pod container endpoints") + _, err = framework.LookForStringInPodExecToContainer(pod.Namespace, pod.Name, ecName, []string{"/bin/echo", "marco"}, "marco", time.Minute) + framework.ExpectNoError(err, "Failed to exec in pod %q ephemeral container %q", format.Pod(pod), ecName) + _, err = framework.LookForStringInLog(pod.Namespace, pod.Name, ecName, "polo", time.Minute) + framework.ExpectNoError(err, "Failed to find logs in pod %q ephemeral container %q", format.Pod(pod), ecName) }) }) From d874cf8ffd888642d74d54662670aec1d6b72813 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Tue, 19 Oct 2021 17:27:26 -0400 Subject: [PATCH 10/11] List disallowed ephemeral container fields Listing these explicitly makes it easier to determine whether a new Container field has been evaluated for use with ephemeral containers. This does not change the behavior of ephemeral containers. --- pkg/apis/core/validation/validation.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 11fcb32908b6..54dd7d9062c5 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -79,9 +79,15 @@ var allowedEphemeralContainerFields = map[string]bool{ "Command": true, "Args": true, "WorkingDir": true, + "Ports": false, "EnvFrom": true, "Env": true, + "Resources": false, "VolumeMounts": true, + "LivenessProbe": false, + "ReadinessProbe": false, + "StartupProbe": false, + "Lifecycle": false, "TerminationMessagePath": true, "TerminationMessagePolicy": true, "ImagePullPolicy": true, From d33bbb8940fb2164dd6419b8bd295920322813f0 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Tue, 19 Oct 2021 17:35:48 -0400 Subject: [PATCH 11/11] Allow volumeDevices in ephemeral containers --- pkg/apis/core/validation/validation.go | 1 + pkg/apis/core/validation/validation_test.go | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 54dd7d9062c5..8bfdf9dd0b8c 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -84,6 +84,7 @@ var allowedEphemeralContainerFields = map[string]bool{ "Env": true, "Resources": false, "VolumeMounts": true, + "VolumeDevices": true, "LivenessProbe": false, "ReadinessProbe": false, "StartupProbe": false, diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 91c4e05acb43..fc1d2b0aea44 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -6143,7 +6143,10 @@ func getResourceLimits(cpu, memory string) core.ResourceList { func TestValidateEphemeralContainers(t *testing.T) { containers := []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}} initContainers := []core.Container{{Name: "ictr", Image: "iimage", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}} - vols := map[string]core.VolumeSource{"vol": {EmptyDir: &core.EmptyDirVolumeSource{}}} + vols := map[string]core.VolumeSource{ + "blk": {PersistentVolumeClaim: &core.PersistentVolumeClaimVolumeSource{ClaimName: "pvc"}}, + "vol": {EmptyDir: &core.EmptyDirVolumeSource{}}, + } // Success Cases for title, ephemeralContainers := range map[string][]core.EphemeralContainer{ @@ -6161,7 +6164,7 @@ func TestValidateEphemeralContainers(t *testing.T) { TargetContainerName: "ctr", }, }, - "All allowed Fields": { + "All allowed fields": { { EphemeralContainerCommon: core.EphemeralContainerCommon{ @@ -6184,6 +6187,9 @@ func TestValidateEphemeralContainers(t *testing.T) { VolumeMounts: []core.VolumeMount{ {Name: "vol", MountPath: "/vol"}, }, + VolumeDevices: []core.VolumeDevice{ + {Name: "blk", DevicePath: "/dev/block"}, + }, TerminationMessagePath: "/dev/termination-log", TerminationMessagePolicy: "File", ImagePullPolicy: "IfNotPresent",