From a24f2c11923078d3dcd0e1d661715634ac358c3e Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Thu, 15 Feb 2024 23:13:07 +0900 Subject: [PATCH] Validate restartable init container state transition This allows the state of restartable init containers to be transitioned from terminated to non-terminated even for pods with RestartPolicyNever or RestartPolicyOnFailure. --- pkg/apis/core/validation/validation.go | 42 ++- pkg/apis/core/validation/validation_test.go | 381 ++++++++++++++++++++ 2 files changed, 422 insertions(+), 1 deletion(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 61b543cc587..a7827cf1208 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -5137,6 +5137,46 @@ func ValidateContainerStateTransition(newStatuses, oldStatuses []core.ContainerS return allErrs } +// ValidateInitContainerStateTransition test to if any illegal init container state transitions are being attempted +func ValidateInitContainerStateTransition(newStatuses, oldStatuses []core.ContainerStatus, fldpath *field.Path, podSpec *core.PodSpec) field.ErrorList { + allErrs := field.ErrorList{} + // If we should always restart, containers are allowed to leave the terminated state + if podSpec.RestartPolicy == core.RestartPolicyAlways { + return allErrs + } + for i, oldStatus := range oldStatuses { + // Skip any container that is not terminated + if oldStatus.State.Terminated == nil { + continue + } + // Skip any container that failed but is allowed to restart + if oldStatus.State.Terminated.ExitCode != 0 && podSpec.RestartPolicy == core.RestartPolicyOnFailure { + continue + } + + // Skip any restartable init container that is allowed to restart + isRestartableInitContainer := false + for _, c := range podSpec.InitContainers { + if oldStatus.Name == c.Name { + if c.RestartPolicy != nil && *c.RestartPolicy == core.ContainerRestartPolicyAlways { + isRestartableInitContainer = true + } + break + } + } + if isRestartableInitContainer { + continue + } + + for _, newStatus := range newStatuses { + if oldStatus.Name == newStatus.Name && newStatus.State.Terminated == nil { + allErrs = append(allErrs, field.Forbidden(fldpath.Index(i).Child("state"), "may not be transitioned to non-terminated state")) + } + } + } + return allErrs +} + // 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") @@ -5158,7 +5198,7 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions // If pod should not restart, make sure the status update does not transition // 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)...) + allErrs = append(allErrs, ValidateInitContainerStateTransition(newPod.Status.InitContainerStatuses, oldPod.Status.InitContainerStatuses, fldPath.Child("initContainerStatuses"), &oldPod.Spec)...) // 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)...) allErrs = append(allErrs, validatePodResourceClaimStatuses(newPod.Status.ResourceClaimStatuses, newPod.Spec.ResourceClaims, fldPath.Child("resourceClaimStatuses"))...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 1ae129e8a4f..6dcac27985d 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -15248,6 +15248,387 @@ func TestValidatePodStatusUpdate(t *testing.T) { }, "", "ResourceClaimStatuses okay", + }, { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "init", + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "init", + Ready: true, + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + 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", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "init", + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + RestartPolicy: core.RestartPolicyNever, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "init", + Ready: false, + 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()), + }, + }, + }}, + }, + }, + `status.initContainerStatuses[0].state: Forbidden: may not be transitioned to non-terminated state`, + "init container cannot restart if RestartPolicyNever", + }, { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "restartable-init", + RestartPolicy: &containerRestartPolicyAlways, + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + RestartPolicy: core.RestartPolicyNever, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "restartable-init", + Ready: true, + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + 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", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "restartable-init", + RestartPolicy: &containerRestartPolicyAlways, + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + RestartPolicy: core.RestartPolicyNever, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "restartable-init", + Ready: false, + 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()), + }, + }, + }}, + }, + }, + "", + "restartable init container can restart if RestartPolicyNever", + }, { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "restartable-init", + RestartPolicy: &containerRestartPolicyAlways, + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + RestartPolicy: core.RestartPolicyOnFailure, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "restartable-init", + Ready: true, + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + 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", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "restartable-init", + RestartPolicy: &containerRestartPolicyAlways, + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + RestartPolicy: core.RestartPolicyOnFailure, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "restartable-init", + Ready: false, + 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()), + }, + }, + }}, + }, + }, + "", + "restartable init container can restart if RestartPolicyOnFailure", + }, { + core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "restartable-init", + RestartPolicy: &containerRestartPolicyAlways, + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + RestartPolicy: core.RestartPolicyAlways, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "restartable-init", + Ready: true, + State: core.ContainerState{ + Running: &core.ContainerStateRunning{ + StartedAt: metav1.NewTime(time.Now()), + }, + }, + }}, + 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", + }, + Spec: core.PodSpec{ + InitContainers: []core.Container{ + { + Name: "restartable-init", + RestartPolicy: &containerRestartPolicyAlways, + }, + }, + Containers: []core.Container{ + { + Name: "nginx", + }, + }, + RestartPolicy: core.RestartPolicyAlways, + }, + Status: core.PodStatus{ + InitContainerStatuses: []core.ContainerStatus{{ + ContainerID: "docker://numbers", + Image: "alpine", + ImageID: "docker-pullable://nginx@sha256:d0gf00d", + Name: "restartable-init", + Ready: false, + 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()), + }, + }, + }}, + }, + }, + "", + "restartable init container can restart if RestartPolicyAlways", }, }