Repository: kubernetes/kubernetes (language: go)

Issue:
[FG:InPlacePodVerticalScaling] Implement version skew handling for in-place pod resize
### What would you like to be added?

This is a tracking bug to implement two changes to better handle version skew as documented in https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1287-in-place-update-pod-resources#version-skew-strategy
1. kubelet: When feature-gate is disabled, if kubelet sees a Proposed resize, it rejects the resize as Infeasible.
2. scheduler: If PodStatus.Resize field is not empty, scheduler uses max(ResourcesAllocated, Requests) even if the feature-gate is disabled.


### Why is this needed?

This is needed to gracefully handle component version skews that can occur during upgrades.


Discussion / hints:
/sig node
/good-first-issue
/assign
Hello @infdahai infdahai, are you working on this? 
According to description of pr. The changes are documented but implementation is still remaining, right?
@Sajiyah-Salat yeah, I just quit this. 
can I work on this issue?
> can I work on this issue?

you can just use `/assign` command
/assign
@carlory , seem like you have experienced in contributing upstream, do you want to give the chance to others for `good-first-issue`? 
/unassign
/assign
/assign

/assign


Hey everyone. Just saw this issue assigned to multiple people. Was curious to know if someone is working on it or is it up for grabs?
/unassign
/assign
@vinaykul  can you explain how to tackle with this issue?
/assign
/assign
/assign
/assign
/assign
/assign

I don't understand the description (sorry for that, I'm a new contributor) (@vinaykul for visibility)
> kubelet: When feature-gate is disabled, if kubelet sees a Proposed resize, it rejects the resize as Infeasible.

Is it a desired state description or a current state? If a current state, then what should be a desired behavior here?  
I've fixed (I guess) the first part in `kubelet` - @vinaykul please take a look when you're free. 
If it is not what was expected here, please guide me 🙇 
@vinaykul hi, could you take a look? Or point out somebody who can take it over from you if you're not available?
/assign

@Shrihariii dunno if it makes sense for you to work on it - there is already an open pull request and discussion in it is going on.
/assign
/assign
@SequoiaGod: GitHub didn't allow me to assign the following users: SequoiaGod.

Note that only [kubernetes members](https://github.com/orgs/kubernetes/people) with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see [the contributor guide](https://git.k8s.io/community/contributors/guide/first-contribution.md#issue-assignment-in-github)

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/117767#issuecomment-2137595697):

>/assign


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
To anyone still working on this issue,
This comment (https://github.com/kubernetes/kubernetes/pull/118843#issuecomment-1637343160) looks important.

@userAdityaa: GitHub didn't allow me to assign the following users: userAdityaa.

Note that only [kubernetes members](https://github.com/orgs/kubernetes/people) with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see [the contributor guide](https://git.k8s.io/community/contributors/guide/first-contribution.md#issue-assignment-in-github)

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/117767#issuecomment-2182828758):

>/assign
>


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
/assign
@suraniyakunal: GitHub didn't allow me to assign the following users: suraniyakunal.

Note that only [kubernetes members](https://github.com/orgs/kubernetes/people) with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see [the contributor guide](https://git.k8s.io/community/contributors/guide/first-contribution.md#issue-assignment-in-github)

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/117767#issuecomment-2198068885):

>/assign


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
/assign

@LB540: GitHub didn't allow me to assign the following users: LB540.

Note that only [kubernetes members](https://github.com/orgs/kubernetes/people) with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see [the contributor guide](https://git.k8s.io/community/contributors/guide/first-contribution.md#issue-assignment-in-github)

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/117767#issuecomment-2237116524):

>/assign
>


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
/assign

@Bibekkshetri055: GitHub didn't allow me to assign the following users: Bibekkshetri055.

Note that only [kubernetes members](https://github.com/orgs/kubernetes/people) with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see [the contributor guide](https://git.k8s.io/community/contributors/guide/first-contribution.md#issue-assignment-in-github)

<details>

In response to [this](https://github.com/kubernetes/kubernetes/issues/117767#issuecomment-2268363715):

>/assign
>


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
</details>
if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) {
		// Handle pod resize here instead of doing it in HandlePodUpdates because
		// this conveniently retries any Deferred resize requests
		// TODO(vinaykul,InPlacePodVerticalScaling): Investigate doing this in HandlePodUpdates + periodic SyncLoop scan

		//     See: https://github.com/kubernetes/kubernetes/pull/102884#discussion_r663160060
		if kl.podWorkers.CouldHaveRunningContainers(pod.UID) && !kubetypes.IsStaticPod(pod) {
			pod = kl.handlePodResourcesResize(pod)
		}

	} else {
		if kl.podWorkers.CouldHaveRunningContainers(pod.UID) && !kubetypes.IsStaticPod(pod) {
			pod.Status.Conditions = append(pod.Status.Conditions, v1.PodCondition{
				Type:   v1.PodConditionType("ResizeInfeasible"),
				Status: v1.ConditionTrue,
				Reason: "InPlacePodVerticalScaling feature gate is disabled",
			})
		}
	}

If the feature gate is disabled simply we can set the pod status conditions to be infeasible ? 
	
	
	
This is blocked by the decision of how to handle version skew. See https://github.com/kubernetes/kubernetes/issues/126616.

TL;DR - at this point, any version skew solution that depends on Kubelet will push out the release another ~3 releases, which is unacceptable.
/remove-good-first-issue
/remove-help

This isn't a good first issue, since the logic is still up for debate. See https://github.com/kubernetes/enhancements/pull/4704 for the discussion.

I don't think anyone is actively working on this, please add yourself back and reach out to me if you are.

/unassign @amomsi
/unassign @pbialon
/unassign @itiwarishubham
/unassign @archits581
/unassign @devilcrises
/unassign @wlp1153468871
/unassign @Shrihariii
/unassign @EtherSol
/unassign @Bhanuteja01
/unassign @MeenuyD
/triage accepted
/hold
The version skew strategy has been updated from the original design:

https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/1287-in-place-update-pod-resources/README.md#version-skew-strategy
I'm interested in taking this up. https://github.com/kubernetes/kubernetes/issues/125394 seems to be related to how we handle the version skew policy, how do we go about that?
The version skew strategy is based on the resources field, not the resize policy, so I think https://github.com/kubernetes/kubernetes/issues/125394 is not tied to this, unless I'm missing something?
Oh, you were referring to the side note about version skew? I think that's out of date, and can be ignored.
/assign

Fix patch:
diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go
index 5aa310e223dea..35514722b6505 100644
--- a/pkg/apis/core/validation/validation.go
+++ b/pkg/apis/core/validation/validation.go
@@ -5515,6 +5515,10 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
 		allErrs = append(allErrs, field.Invalid(specPath, newPod.Status.QOSClass, "Pod QOS Class may not change as a result of resizing"))
 	}
 
+	if !isPodResizeRequestSupported(*oldPod) {
+		allErrs = append(allErrs, field.Forbidden(specPath, "Pod running on node without support for resize"))
+	}
+
 	// Ensure that only CPU and memory resources are mutable.
 	originalCPUMemPodSpec := *newPod.Spec.DeepCopy()
 	var newContainers []core.Container
@@ -5555,6 +5559,23 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
 	return allErrs
 }
 
+// isPodResizeRequestSupported checks whether the pod is running on a node with InPlacePodVerticalScaling enabled.
+func isPodResizeRequestSupported(pod core.Pod) bool {
+	// TODO: Remove this after GA+3 releases of InPlacePodVerticalScaling
+	// This code handles the version skew as described in the KEP.
+	// For handling version skew we're only allowing to update the Pod's Resources
+	// if the Pod already has Pod.Status.ContainerStatuses[i].Resources. This means
+	// that the apiserver would only allow updates to Pods running on Nodes with
+	// the InPlacePodVerticalScaling feature gate enabled.
+	for _, c := range pod.Status.ContainerStatuses {
+		if c.State.Running != nil {
+			return c.Resources != nil
+		}
+	}
+	// No running containers. We cannot tell whether the node supports resize at this point, so we assume it does.
+	return true
+}
+
 // ValidatePodBinding tests if required fields in the pod binding are legal.
 func ValidatePodBinding(binding *core.Binding) field.ErrorList {
 	allErrs := field.ErrorList{}


Test patch:
diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go
index bd457b40adf60..412b411695821 100644
--- a/pkg/apis/core/validation/validation_test.go
+++ b/pkg/apis/core/validation/validation_test.go
@@ -25188,6 +25188,74 @@ func TestValidatePodResize(t *testing.T) {
 			new:  mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", ""), podtest.SetOS(core.Windows)),
 			err:  "Forbidden: windows pods cannot be resized",
 		},
+		{
+			test: "Pod with nil Resource field in Status",
+			old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
+				ContainerStatuses: []core.ContainerStatus{{
+					ContainerID: "docker://numbers",
+					Image:       "nginx:alpine",
+					Name:        "main",
+					Ready:       true,
+					Started:     proto.Bool(true),
+					Resources:   nil,
+					State: core.ContainerState{
+						Running: &core.ContainerStateRunning{
+							StartedAt: metav1.NewTime(time.Now()),
+						},
+					},
+				}},
+			})),
+			new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
+			err: "Forbidden: Pod running on node without support for resize",
+		},
+		{
+			test: "Pod with non-nil Resources field in Status",
+			old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
+				ContainerStatuses: []core.ContainerStatus{{
+					ContainerID: "docker://numbers",
+					Image:       "nginx:alpine",
+					Name:        "main",
+					Ready:       true,
+					Started:     proto.Bool(true),
+					Resources:   &core.ResourceRequirements{},
+					State: core.ContainerState{
+						Running: &core.ContainerStateRunning{
+							StartedAt: metav1.NewTime(time.Now()),
+						},
+					},
+				}},
+			})),
+			new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
+			err: "",
+		},
+		{
+			test: "Pod without running containers",
+			old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
+				ContainerStatuses: []core.ContainerStatus{},
+			})),
+			new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
+			err: "",
+		},
+		{
+			test: "Pod with containers which are not running yet",
+			old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
+				ContainerStatuses: []core.ContainerStatus{{
+					ContainerID: "docker://numbers",
+					Image:       "nginx:alpine",
+					Name:        "main",
+					Ready:       true,
+					Started:     proto.Bool(true),
+					Resources:   &core.ResourceRequirements{},
+					State: core.ContainerState{
+						Waiting: &core.ContainerStateWaiting{
+							Reason: "PodInitializing",
+						},
+					},
+				}},
+			})),
+			new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
+			err: "",
+		},
 	}
 
 	for _, test := range tests {