Merge pull request kubernetes#130733 from natasha41575/pod-resize-conditions

[FG:InPlacePodVerticalScaling] Move pod resize status to pod conditions

Author: k8s-ci-robot

api/openapi-spec/swagger.json

@@ -336,12 +336,6 @@ func MakeContainerStatus(name string, allocatedResources api.ResourceList) api.C
 	return cs
 }
 
-func SetResizeStatus(resizeStatus api.PodResizeStatus) TweakPodStatus {
-	return func(podstatus *api.PodStatus) {
-		podstatus.Resize = resizeStatus
-	}
-}
-
 // TweakContainers applies the container tweaks to all containers (regular & init) in the pod.
 // Note: this should typically be added to pod tweaks after all containers have been added.
 func TweakContainers(tweaks ...TweakContainer) Tweak {

api/openapi-spec/v3/api__v1_openapi.json

@@ -791,7 +791,7 @@ func dropDisabledPodStatusFields(podStatus, oldPodStatus *api.PodStatus, podSpec
 	}
 
 	if !utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) && !inPlacePodVerticalScalingInUse(oldPodSpec) {
-		// Drop Resize and Resources fields
+		// Drop Resources fields
 		dropResourcesField := func(csl []api.ContainerStatus) {
 			for i := range csl {
 				csl[i].Resources = nil
@@ -800,7 +800,6 @@ func dropDisabledPodStatusFields(podStatus, oldPodStatus *api.PodStatus, podSpec
 		dropResourcesField(podStatus.ContainerStatuses)
 		dropResourcesField(podStatus.InitContainerStatuses)
 		dropResourcesField(podStatus.EphemeralContainerStatuses)
-		podStatus.Resize = ""
 	}
 	if !utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) ||
 		!utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScalingAllocatedStatus) {

pkg/api/pod/testing/make.go

@@ -2675,7 +2675,6 @@ func TestDropInPlacePodVerticalScaling(t *testing.T) {
 				},
 			},
 			Status: api.PodStatus{
-				Resize: api.PodResizeStatusInProgress,
 				ContainerStatuses: []api.ContainerStatus{
 					{
 						Name:               "c1",

pkg/api/pod/util.go

@@ -2951,6 +2951,17 @@ const (
 	// DisruptionTarget indicates the pod is about to be terminated due to a
 	// disruption (such as preemption, eviction API or garbage-collection).
 	DisruptionTarget PodConditionType = "DisruptionTarget"
+	// PodResizePending indicates that the pod has been resized, but kubelet has not
+	// yet allocated the resources. If both PodResizePending and PodResizeInProgress
+	// are set, it means that a new resize was requested in the middle of a previous
+	// pod resize that is still in progress.
+	PodResizePending PodConditionType = "PodResizePending"
+	// PodResizeInProgress indicates that a resize is in progress, and is present whenever
+	// the Kubelet has allocated resources for the resize, but has not yet actuated all of
+	// the required changes.
+	// If both PodResizePending and PodResizeInProgress are set, it means that a new resize was
+	// requested in the middle of a previous pod resize that is still in progress.
+	PodResizeInProgress PodConditionType = "PodResizeInProgress"
 )
 
 // PodCondition represents pod's condition
@@ -2970,7 +2981,7 @@ type PodCondition struct {
 	Message string
 }
 
-// PodResizeStatus shows status of desired resize of a pod's containers.
+// Deprecated: PodResizeStatus shows status of desired resize of a pod's containers.
 type PodResizeStatus string
 
 const (
@@ -4251,6 +4262,9 @@ type PodStatus struct {
 	// Status of resources resize desired for pod's containers.
 	// It is empty if no resources resize is pending.
 	// Any changes to container resources will automatically set this to "Proposed"
+	// Deprecated: Resize status is moved to two pod conditions PodResizePending and PodResizeInProgress.
+	// PodResizePending will track states where the spec has been resized, but the Kubelet has not yet allocated the resources.
+	// PodResizeInProgress will track in-progress resizes, and should be present whenever allocated resources != acknowledged resources.
 	// +featureGate=InPlacePodVerticalScaling
 	// +optional
 	Resize PodResizeStatus

pkg/api/pod/util_test.go

@@ -2863,21 +2863,23 @@ func (kl *Kubelet) HandlePodSyncs(pods []*v1.Pod) {
 
 // canResizePod determines if the requested resize is currently feasible.
 // pod should hold the desired (pre-allocated) spec.
-// Returns true if the resize can proceed.
-func (kl *Kubelet) canResizePod(pod *v1.Pod) (bool, v1.PodResizeStatus, string) {
+// Returns true if the resize can proceed; returns a reason and message
+// otherwise.
+func (kl *Kubelet) canResizePod(pod *v1.Pod) (bool, string, string) {
 	if v1qos.GetPodQOS(pod) == v1.PodQOSGuaranteed && !utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScalingExclusiveCPUs) {
 		if utilfeature.DefaultFeatureGate.Enabled(features.CPUManager) {
 			if kl.containerManager.GetNodeConfig().CPUManagerPolicy == "static" {
 				msg := "Resize is infeasible for Guaranteed Pods alongside CPU Manager static policy"
 				klog.V(3).InfoS(msg, "pod", format.Pod(pod))
-				return false, v1.PodResizeStatusInfeasible, msg
+				return false, v1.PodReasonInfeasible, msg
 			}
 		}
 		if utilfeature.DefaultFeatureGate.Enabled(features.MemoryManager) {
 			if kl.containerManager.GetNodeConfig().MemoryManagerPolicy == "Static" {
 				msg := "Resize is infeasible for Guaranteed Pods alongside Memory Manager static policy"
 				klog.V(3).InfoS(msg, "pod", format.Pod(pod))
-				return false, v1.PodResizeStatusInfeasible, msg
+				return false, v1.PodReasonInfeasible, msg
+
 			}
 		}
 	}
@@ -2900,7 +2902,8 @@ func (kl *Kubelet) canResizePod(pod *v1.Pod) (bool, v1.PodResizeStatus, string)
 		}
 		msg = "Node didn't have enough capacity: " + msg
 		klog.V(3).InfoS(msg, "pod", klog.KObj(pod))
-		return false, v1.PodResizeStatusInfeasible, msg
+		return false, v1.PodReasonInfeasible, msg
+
 	}
 
 	// Treat the existing pod needing resize as a new pod with desired resources seeking admit.
@@ -2911,83 +2914,75 @@ func (kl *Kubelet) canResizePod(pod *v1.Pod) (bool, v1.PodResizeStatus, string)
 	if ok, failReason, failMessage := kl.canAdmitPod(allocatedPods, pod); !ok {
 		// Log reason and return. Let the next sync iteration retry the resize
 		klog.V(3).InfoS("Resize cannot be accommodated", "pod", klog.KObj(pod), "reason", failReason, "message", failMessage)
-		return false, v1.PodResizeStatusDeferred, failMessage
+		return false, v1.PodReasonDeferred, failMessage
 	}
 
-	return true, v1.PodResizeStatusInProgress, ""
+	return true, "", ""
 }
 
 // handlePodResourcesResize returns the "allocated pod", which should be used for all resource
 // calculations after this function is called. It also updates the cached ResizeStatus according to
 // the allocation decision and pod status.
-func (kl *Kubelet) handlePodResourcesResize(pod *v1.Pod, podStatus *kubecontainer.PodStatus) (*v1.Pod, error) {
-	allocatedPod, updated := kl.allocationManager.UpdatePodFromAllocation(pod)
-
-	if !updated {
-		// Desired resources == allocated resources. Check whether a resize is in progress.
+func (kl *Kubelet) handlePodResourcesResize(pod *v1.Pod, podStatus *kubecontainer.PodStatus) (allocatedPod *v1.Pod, err error) {
+	// Always check whether a resize is in progress so we can set the PodResizeInProgressCondition
+	// accordingly.
+	defer func() {
+		if err != nil {
+			return
+		}
 		if kl.isPodResizeInProgress(allocatedPod, podStatus) {
 			// If a resize is in progress, make sure the cache has the correct state in case the Kubelet restarted.
-			kl.statusManager.SetPodResizeStatus(pod.UID, v1.PodResizeStatusInProgress)
+			kl.statusManager.SetPodResizeInProgressCondition(pod.UID, "", "")
 		} else {
-			// (Desired == Allocated == Actual) => clear the resize status.
-			kl.statusManager.SetPodResizeStatus(pod.UID, "")
+			// (Allocated == Actual) => clear the resize in-progress status.
+			kl.statusManager.ClearPodResizeInProgressCondition(pod.UID)
 		}
-		// Pod allocation does not need to be updated.
-		return allocatedPod, nil
+	}()
+
+	podFromAllocation, updated := kl.allocationManager.UpdatePodFromAllocation(pod)
+	if !updated {
+		// Desired resources == allocated resources. Pod allocation does not need to be updated.
+		kl.statusManager.ClearPodResizePendingCondition(pod.UID)
+		return podFromAllocation, nil
+
 	} else if resizable, msg := kuberuntime.IsInPlacePodVerticalScalingAllowed(pod); !resizable {
 		// If there is a pending resize but the resize is not allowed, always use the allocated resources.
-		kl.recorder.Eventf(pod, v1.EventTypeWarning, events.ResizeInfeasible, msg)
-		kl.statusManager.SetPodResizeStatus(pod.UID, v1.PodResizeStatusInfeasible)
-		return allocatedPod, nil
+		kl.statusManager.SetPodResizePendingCondition(pod.UID, v1.PodReasonInfeasible, msg)
+		return podFromAllocation, nil
 	}
 
 	kl.podResizeMutex.Lock()
 	defer kl.podResizeMutex.Unlock()
 	// Desired resources != allocated resources. Can we update the allocation to the desired resources?
-	fit, resizeStatus, resizeMsg := kl.canResizePod(pod)
+	fit, reason, message := kl.canResizePod(pod)
 	if fit {
 		// Update pod resource allocation checkpoint
 		if err := kl.allocationManager.SetAllocatedResources(pod); err != nil {
 			return nil, err
 		}
+		kl.statusManager.ClearPodResizePendingCondition(pod.UID)
 		for i, container := range pod.Spec.Containers {
-			if !apiequality.Semantic.DeepEqual(container.Resources, allocatedPod.Spec.Containers[i].Resources) {
+			if !apiequality.Semantic.DeepEqual(container.Resources, podFromAllocation.Spec.Containers[i].Resources) {
 				key := kuberuntime.GetStableKey(pod, &container)
 				kl.crashLoopBackOff.Reset(key)
 			}
 		}
 		for i, container := range pod.Spec.InitContainers {
 			if podutil.IsRestartableInitContainer(&container) {
-				if !apiequality.Semantic.DeepEqual(container.Resources, allocatedPod.Spec.InitContainers[i].Resources) {
+				if !apiequality.Semantic.DeepEqual(container.Resources, podFromAllocation.Spec.InitContainers[i].Resources) {
 					key := kuberuntime.GetStableKey(pod, &container)
 					kl.crashLoopBackOff.Reset(key)
 				}
 			}
 		}
-		allocatedPod = pod
-
-		// Special case when the updated allocation matches the actuated resources. This can occur
-		// when reverting a resize that hasn't been actuated, or when making an equivalent change
-		// (such as CPU requests below MinShares). This is an optimization to clear the resize
-		// status immediately, rather than waiting for the next SyncPod iteration.
-		if !kl.isPodResizeInProgress(allocatedPod, podStatus) {
-			// In this case, consider the resize complete.
-			kl.statusManager.SetPodResizeStatus(pod.UID, "")
-			return allocatedPod, nil
-		}
-	}
-	if resizeStatus != "" {
-		kl.statusManager.SetPodResizeStatus(pod.UID, resizeStatus)
-		if resizeMsg != "" {
-			switch resizeStatus {
-			case v1.PodResizeStatusDeferred:
-				kl.recorder.Eventf(pod, v1.EventTypeWarning, events.ResizeDeferred, resizeMsg)
-			case v1.PodResizeStatusInfeasible:
-				kl.recorder.Eventf(pod, v1.EventTypeWarning, events.ResizeInfeasible, resizeMsg)
-			}
-		}
+		return pod, nil
+	}
+
+	if reason != "" {
+		kl.statusManager.SetPodResizePendingCondition(pod.UID, reason, message)
 	}
-	return allocatedPod, nil
+
+	return podFromAllocation, nil
 }
 
 // isPodResizingInProgress checks whether the actuated resizable resources differ from the allocated resources

pkg/apis/core/types.go

@@ -1738,14 +1738,15 @@ func getPhase(pod *v1.Pod, info []v1.ContainerStatus, podIsTerminal bool) v1.Pod
 	}
 }
 
-func (kl *Kubelet) determinePodResizeStatus(allocatedPod *v1.Pod, podStatus *kubecontainer.PodStatus, podIsTerminal bool) v1.PodResizeStatus {
+func (kl *Kubelet) determinePodResizeStatus(allocatedPod *v1.Pod, podIsTerminal bool) []*v1.PodCondition {
 	// If pod is terminal, clear the resize status.
 	if podIsTerminal {
-		kl.statusManager.SetPodResizeStatus(allocatedPod.UID, "")
-		return ""
+		kl.statusManager.ClearPodResizeInProgressCondition(allocatedPod.UID)
+		kl.statusManager.ClearPodResizePendingCondition(allocatedPod.UID)
+		return nil
 	}
 
-	resizeStatus := kl.statusManager.GetPodResizeStatus(allocatedPod.UID)
+	resizeStatus := kl.statusManager.GetPodResizeConditions(allocatedPod.UID)
 	return resizeStatus
 }
 
@@ -1759,9 +1760,6 @@ func (kl *Kubelet) generateAPIPodStatus(pod *v1.Pod, podStatus *kubecontainer.Po
 		oldPodStatus = pod.Status
 	}
 	s := kl.convertStatusToAPIStatus(pod, podStatus, oldPodStatus)
-	if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) {
-		s.Resize = kl.determinePodResizeStatus(pod, podStatus, podIsTerminal)
-	}
 	// calculate the next phase and preserve reason
 	allStatus := append(append([]v1.ContainerStatus{}, s.ContainerStatuses...), s.InitContainerStatuses...)
 	s.Phase = getPhase(pod, allStatus, podIsTerminal)
@@ -1827,6 +1825,12 @@ func (kl *Kubelet) generateAPIPodStatus(pod *v1.Pod, podStatus *kubecontainer.Po
 			s.Conditions = append(s.Conditions, c)
 		}
 	}
+	if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) {
+		resizeStatus := kl.determinePodResizeStatus(pod, podIsTerminal)
+		for _, c := range resizeStatus {
+			s.Conditions = append(s.Conditions, *c)
+		}
+	}
 
 	// copy over the pod disruption conditions from state which is already
 	// updated during the eviciton (due to either node resource pressure or

pkg/generated/openapi/zz_generated.openapi.go

@@ -3881,7 +3881,6 @@ func Test_generateAPIPodStatusForInPlaceVPAEnabled(t *testing.T) {
 							AllocatedResources: CPU1AndMem1GAndStorage2GAndCustomResource,
 						},
 					},
-					Resize: "InProgress",
 				},
 			},
 		},
@@ -3912,7 +3911,6 @@ func Test_generateAPIPodStatusForInPlaceVPAEnabled(t *testing.T) {
 							AllocatedResources: CPU1AndMem1GAndStorage2G,
 						},
 					},
-					Resize: "InProgress",
 				},
 			},
 		},
@@ -3926,9 +3924,10 @@ func Test_generateAPIPodStatusForInPlaceVPAEnabled(t *testing.T) {
 			oldStatus := test.pod.Status
 			kl.statusManager.SetPodStatus(test.pod, oldStatus)
 			actual := kl.generateAPIPodStatus(test.pod, &testKubecontainerPodStatus /* criStatus */, false /* test.isPodTerminal */)
-
-			if actual.Resize != "" {
-				t.Fatalf("Unexpected Resize status: %s", actual.Resize)
+			for _, c := range actual.Conditions {
+				if c.Type == v1.PodResizePending || c.Type == v1.PodResizeInProgress {
+					t.Fatalf("unexpected resize status: %v", c)
+				}
 			}
 		})
 	}