From eab9197d1a8799203deb37159ba951aed1c8ebb2 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 21 Feb 2025 18:46:17 +0000 Subject: [PATCH] Add observedGeneration and validation to pod status and conditions --- pkg/apis/core/types.go | 12 +++++- pkg/apis/core/validation/validation.go | 13 +++++- pkg/apis/core/validation/validation_test.go | 44 +++++++++++++++++++++ staging/src/k8s.io/api/core/v1/types.go | 10 +++++ test/e2e/framework/pod/resource.go | 2 +- test/e2e/storage/podlogs/podlogs.go | 2 +- test/e2e/windows/node_shutdown.go | 2 +- test/e2e_node/node_shutdown_linux_test.go | 2 +- 8 files changed, 79 insertions(+), 8 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 25b5885bb3a..c4ea9e27329 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -2955,8 +2955,11 @@ const ( // PodCondition represents pod's condition type PodCondition struct { - Type PodConditionType - Status ConditionStatus + Type PodConditionType + // +featureGate=PodObservedGenerationTracking + // +optional + ObservedGeneration int64 + Status ConditionStatus // +optional LastProbeTime metav1.Time // +optional @@ -4164,6 +4167,11 @@ type EphemeralContainer struct { // PodStatus represents information about the status of a pod. Status may trail the actual // state of a system. type PodStatus struct { + // If set, this represents the .metadata.generation that the pod status was set based upon. + // This is an alpha field. Enable PodObservedGenerationTracking to be able to use this field. + // +featureGate=PodObservedGenerationTracking + // +optional + ObservedGeneration int64 // +optional Phase PodPhase // +optional diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index ce84c41b96d..1687c6b51be 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -5449,9 +5449,10 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions fldPath := field.NewPath("metadata") allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath) allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"), opts)...) - allErrs = append(allErrs, validatePodConditions(newPod.Status.Conditions, fldPath.Child("conditions"))...) fldPath = field.NewPath("status") + allErrs = append(allErrs, validatePodConditions(newPod.Status.Conditions, fldPath.Child("conditions"))...) + if newPod.Spec.NodeName != oldPod.Spec.NodeName { allErrs = append(allErrs, field.Forbidden(fldPath.Child("nodeName"), "may not be changed directly")) } @@ -5462,6 +5463,10 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions } } + if newPod.Status.ObservedGeneration < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("observedGeneration"), newPod.Status.ObservedGeneration, "must be a non-negative integer")) + } + // Pod QoS is immutable allErrs = append(allErrs, ValidateImmutableField(newPod.Status.QOSClass, oldPod.Status.QOSClass, fldPath.Child("qosClass"))...) @@ -5497,7 +5502,8 @@ func ValidatePodStatusUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions return allErrs } -// validatePodConditions tests if the custom pod conditions are valid. +// validatePodConditions tests if the custom pod conditions are valid, and that the observedGeneration +// is a non-negative integer. func validatePodConditions(conditions []core.PodCondition, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} systemConditions := sets.New( @@ -5505,6 +5511,9 @@ func validatePodConditions(conditions []core.PodCondition, fldPath *field.Path) core.PodReady, core.PodInitialized) for i, condition := range conditions { + if condition.ObservedGeneration < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("observedGeneration"), condition.ObservedGeneration, "must be a non-negative integer")) + } if systemConditions.Has(condition.Type) { continue } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 23b4f417f5e..0a739399e4e 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -13897,6 +13897,50 @@ func TestValidatePodStatusUpdate(t *testing.T) { err string test string }{{ + *podtest.MakePod("foo", + podtest.SetStatus(core.PodStatus{ + ObservedGeneration: 1, + }), + ), + *podtest.MakePod("foo"), + "", + "set valid status.observedGeneration", + }, { + *podtest.MakePod("foo", + podtest.SetStatus(core.PodStatus{ + Conditions: []core.PodCondition{{ + Type: core.PodScheduled, + Status: core.ConditionTrue, + ObservedGeneration: 1, + }}, + }), + ), + *podtest.MakePod("foo"), + "", + "set valid condition.observedGeneration", + }, { + *podtest.MakePod("foo", + podtest.SetStatus(core.PodStatus{ + ObservedGeneration: -1, + }), + ), + *podtest.MakePod("foo"), + "status.observedGeneration: Invalid value: -1: must be a non-negative integer", + "set invalid status.observedGeneration", + }, { + *podtest.MakePod("foo", + podtest.SetStatus(core.PodStatus{ + Conditions: []core.PodCondition{{ + Type: core.PodScheduled, + Status: core.ConditionTrue, + ObservedGeneration: -1, + }}, + }), + ), + *podtest.MakePod("foo"), + "status.conditions[0].observedGeneration: Invalid value: -1: must be a non-negative integer", + "set invalid condition.observedGeneration", + }, { *podtest.MakePod("foo", podtest.SetNodeName("node1"), podtest.SetStatus(core.PodStatus{ diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 2fc4c3a76e8..4c5b4b2781c 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3308,6 +3308,11 @@ type PodCondition struct { // Type is the type of the condition. // More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#pod-conditions Type PodConditionType `json:"type" protobuf:"bytes,1,opt,name=type,casttype=PodConditionType"` + // If set, this represents the .metadata.generation that the pod condition was set based upon. + // This is an alpha field. Enable PodObservedGenerationTracking to be able to use this field. + // +featureGate=PodObservedGenerationTracking + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` // Status is the status of the condition. // Can be True, False, Unknown. // More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#pod-conditions @@ -4841,6 +4846,11 @@ type EphemeralContainer struct { // state of a system, especially if the node that hosts the pod cannot contact the control // plane. type PodStatus struct { + // If set, this represents the .metadata.generation that the pod status was set based upon. + // This is an alpha field. Enable PodObservedGenerationTracking to be able to use this field. + // +featureGate=PodObservedGenerationTracking + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` // The phase of a Pod is a simple, high-level summary of where the Pod is in its lifecycle. // The conditions array, the reason and message fields, and the individual container status // arrays contain more detail about the pod's status. diff --git a/test/e2e/framework/pod/resource.go b/test/e2e/framework/pod/resource.go index c8119890512..6dc039587e5 100644 --- a/test/e2e/framework/pod/resource.go +++ b/test/e2e/framework/pod/resource.go @@ -172,7 +172,7 @@ func LogPodStates(pods []v1.Pod) { if pod.DeletionGracePeriodSeconds != nil { grace = fmt.Sprintf("%ds", *pod.DeletionGracePeriodSeconds) } - framework.Logf("%-[1]*[2]s %-[3]*[4]s %-[5]*[6]s %-[7]*[8]s %[9]s", + framework.Logf("%-[1]*[2]s %-[3]*[4]s %-[5]*[6]s %-[7]*[8]s %[9]v", maxPodW, pod.ObjectMeta.Name, maxNodeW, pod.Spec.NodeName, maxPhaseW, pod.Status.Phase, maxGraceW, grace, pod.Status.Conditions) } framework.Logf("") // Final empty line helps for readability. diff --git a/test/e2e/storage/podlogs/podlogs.go b/test/e2e/storage/podlogs/podlogs.go index 692078b8362..da1c35fcc00 100644 --- a/test/e2e/storage/podlogs/podlogs.go +++ b/test/e2e/storage/podlogs/podlogs.go @@ -313,7 +313,7 @@ func WatchPods(ctx context.Context, cs clientset.Interface, ns string, to io.Wri } buffer := new(bytes.Buffer) fmt.Fprintf(buffer, - "%s pod: %s: %s/%s %s: %s %s\n", + "%s pod: %s: %s/%s %s: %s %v\n", time.Now().Format(timeFormat), e.Type, pod.Namespace, diff --git a/test/e2e/windows/node_shutdown.go b/test/e2e/windows/node_shutdown.go index f6ea463d274..3d366c09014 100644 --- a/test/e2e/windows/node_shutdown.go +++ b/test/e2e/windows/node_shutdown.go @@ -221,7 +221,7 @@ var _ = sigDescribe(feature.Windows, "GracefulNodeShutdown", framework.WithSeria if !isPodReadyToStartConditionSetToFalse(&pod) { framework.Logf("Expecting pod (%v/%v) 's ready to start condition set to false, "+ "but it's not currently: Pod Condition %+v", pod.Namespace, pod.Name, pod.Status.Conditions) - return fmt.Errorf("pod (%v/%v) 's ready to start condition should be false, condition: %s, phase: %s", + return fmt.Errorf("pod (%v/%v) 's ready to start condition should be false, condition: %v, phase: %s", pod.Namespace, pod.Name, pod.Status.Conditions, pod.Status.Phase) } } diff --git a/test/e2e_node/node_shutdown_linux_test.go b/test/e2e_node/node_shutdown_linux_test.go index 2281e6b275e..ebfa1875211 100644 --- a/test/e2e_node/node_shutdown_linux_test.go +++ b/test/e2e_node/node_shutdown_linux_test.go @@ -341,7 +341,7 @@ var _ = SIGDescribe("GracefulNodeShutdown", framework.WithSerial(), feature.Grac if !isPodReadyToStartConditionSetToFalse(&pod) { framework.Logf("Expecting pod (%v/%v) 's ready to start condition set to false, "+ "but it's not currently: Pod Condition %+v", pod.Namespace, pod.Name, pod.Status.Conditions) - return fmt.Errorf("pod (%v/%v) 's ready to start condition should be false, condition: %s, phase: %s", + return fmt.Errorf("pod (%v/%v) 's ready to start condition should be false, condition: %v, phase: %s", pod.Namespace, pod.Name, pod.Status.Conditions, pod.Status.Phase) } }