Repository: kubernetes/kubernetes (language: go)

Issue:
[FG:InPlacePodVerticalScaling] Container resize policy feature introduced in v1.27 doesn't interrupt CrashLoopBackOff
### What happened?

Hi, does someone know how to “break” the CrashLoopBackOff container status? I don’t want to wait all the backoff time nor delete the whole pod. I’m using the new features of restarting the container after resizing introduced in Kubernetes v1.27, but it seems that after I resize the container using the Python client, it still waits for the back-off time to finish before trying to run the container again.

Deployment info:

```spec:
      containers:
        - name: ...
          image: systemautoscaler/prime-numbers:dev
          imagePullPolicy: Always
          ports:
            - containerPort: ...
          resizePolicy:
            - resourceName: cpu
              restartPolicy: RestartContainer
            - resourceName: memory
              restartPolicy: RestartContainer
```

### What did you expect to happen?

No matter the state of the container, when I resize it, it should be restarted, even if waiting for Back Off.

### How can we reproduce it (as minimally and precisely as possible)?

1. Create a deployment with the spec field as specified above.
2. Scale down container resources to 1m CPU and to 1Mi memory (both limits and requests) using the Kubernetes API e.g. `kubernetes.client.CoreV1Api().patch_namespaced_pod(name=pod_name, namespace=namespace, body=new_pod_data)`
3. Wait till the container is in CrashLoopBackOff and then try to scale up the container resources with enough CPU and memory to run successfully.

### Anything else we need to know?

_No response_

### Kubernetes version

<details>

```console
$ kubectl version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.4", GitCommit:"fa3d7990104d7c1f16943a67f11b154b71f6a132", GitTreeState:"clean", BuildDate:"2023-07-19T12:20:54Z", GoVersion:"go1.20.6", Compiler:"gc", Platform:"windows/amd64"}
Kustomize Version: v5.0.1
Server Version: version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.3", GitCommit:"25b4e43193bcda6c7328a6d147b1fb73a33f1598", GitTreeState:"clean", BuildDate:"2023-06-15T00:36:28Z", GoVersion:"go1.20.5", Compiler:"gc", Platform:"linux/amd64"}

```

</details>


### Cloud provider

<details>
Working locally on KinD cluster
</details>


### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# BuildNumber  Caption                                    OSArchitecture  Version
19045        Microsoft Windows 10 Home Single Language  64 bits         10.0.19045
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
/sig node
Not sure who worked on that feature but I see @vinaykul on the latest with this.  
/retitle Container resize policy feature introduced in v1.27 doesn't interrupt CrashLoopBackOff
xref to the enhancement: #1287
/triage accepted
/assign @haircommander 
> xref to the enhancement: #1287

I had no idea how this was related to the issue.  I realize that you meant to include [#1287](https://github.com/kubernetes/enhancements/issues/1287) rather than k/k.

Fix patch:
diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go
index 94f2a9ded9544..4fdcef9fbebde 100644
--- a/pkg/kubelet/kubelet.go
+++ b/pkg/kubelet/kubelet.go
@@ -50,6 +50,7 @@ import (
 	"k8s.io/utils/ptr"
 
 	v1 "k8s.io/api/core/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
@@ -2898,6 +2899,12 @@ func (kl *Kubelet) handlePodResourcesResize(pod *v1.Pod, podStatus *kubecontaine
 		if err := kl.statusManager.SetPodAllocation(pod); err != nil {
 			return nil, err
 		}
+		for i, container := range pod.Spec.Containers {
+			if !apiequality.Semantic.DeepEqual(container.Resources, allocatedPod.Spec.Containers[i].Resources) {
+				key := kuberuntime.GetStableKey(pod, &container)
+				kl.backOff.Reset(key)
+			}
+		}
 		allocatedPod = pod
 	}
 	if resizeStatus != "" {
diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go
index 7787402bff701..127a187f22850 100644
--- a/pkg/kubelet/kuberuntime/helpers.go
+++ b/pkg/kubelet/kuberuntime/helpers.go
@@ -172,10 +172,10 @@ func isInitContainerFailed(status *kubecontainer.Status) bool {
 	return false
 }
 
-// getStableKey generates a key (string) to uniquely identify a
+// GetStableKey generates a key (string) to uniquely identify a
 // (pod, container) tuple. The key should include the content of the
 // container, so that any change to the container generates a new key.
-func getStableKey(pod *v1.Pod, container *v1.Container) string {
+func GetStableKey(pod *v1.Pod, container *v1.Container) string {
 	hash := strconv.FormatUint(kubecontainer.HashContainer(container), 16)
 	return fmt.Sprintf("%s_%s_%s_%s_%s", pod.Name, pod.Namespace, string(pod.UID), container.Name, hash)
 }
diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go
index f7902bd47d880..cea016af3d46a 100644
--- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go
+++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go
@@ -1433,7 +1433,7 @@ func (m *kubeGenericRuntimeManager) doBackOff(pod *v1.Pod, container *v1.Contain
 	// Use the finished time of the latest exited container as the start point to calculate whether to do back-off.
 	ts := cStatus.FinishedAt
 	// backOff requires a unique key to identify the container.
-	key := getStableKey(pod, container)
+	key := GetStableKey(pod, container)
 	if backOff.IsInBackOffSince(key, ts) {
 		if containerRef, err := kubecontainer.GenerateContainerRef(pod, container); err == nil {
 			m.recorder.Eventf(containerRef, v1.EventTypeWarning, events.BackOffStartContainer,


Test patch:
diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go
index 71903ea5db92a..b8bb03aa70bec 100644
--- a/pkg/kubelet/kubelet_test.go
+++ b/pkg/kubelet/kubelet_test.go
@@ -2676,6 +2676,7 @@ func TestHandlePodResourcesResize(t *testing.T) {
 		newRequestsAllocated bool // Whether the new requests have already been allocated (but not actuated)
 		expectedAllocations  v1.ResourceList
 		expectedResize       v1.PodResizeStatus
+		expectBackoffReset   bool
 		goos                 string
 	}{
 		{
@@ -2684,6 +2685,7 @@ func TestHandlePodResourcesResize(t *testing.T) {
 			newRequests:         v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M},
 			expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M},
 			expectedResize:      v1.PodResizeStatusInProgress,
+			expectBackoffReset:  true,
 		},
 		{
 			name:                "Request CPU increase, memory decrease - expect InProgress",
@@ -2691,6 +2693,7 @@ func TestHandlePodResourcesResize(t *testing.T) {
 			newRequests:         v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem500M},
 			expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem500M},
 			expectedResize:      v1.PodResizeStatusInProgress,
+			expectBackoffReset:  true,
 		},
 		{
 			name:                "Request CPU decrease, memory increase - expect InProgress",
@@ -2698,6 +2701,7 @@ func TestHandlePodResourcesResize(t *testing.T) {
 			newRequests:         v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem1500M},
 			expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem1500M},
 			expectedResize:      v1.PodResizeStatusInProgress,
+			expectBackoffReset:  true,
 		},
 		{
 			name:                "Request CPU and memory increase beyond current capacity - expect Deferred",
@@ -2788,6 +2792,11 @@ func TestHandlePodResourcesResize(t *testing.T) {
 				}
 			}
 
+			now := kubelet.clock.Now()
+			// Put the container in backoff so we can confirm backoff is reset.
+			backoffKey := kuberuntime.GetStableKey(tt.pod, &tt.pod.Spec.Containers[0])
+			kubelet.backOff.Next(backoffKey, now)
+
 			updatedPod, err := kubelet.handlePodResourcesResize(newPod, podStatus)
 			require.NoError(t, err)
 			assert.Equal(t, tt.expectedAllocations, updatedPod.Spec.Containers[0].Resources.Requests, "updated pod spec resources")
@@ -2798,6 +2807,13 @@ func TestHandlePodResourcesResize(t *testing.T) {
 
 			resizeStatus := kubelet.statusManager.GetPodResizeStatus(newPod.UID)
 			assert.Equal(t, tt.expectedResize, resizeStatus)
+
+			isInBackoff := kubelet.backOff.IsInBackOffSince(backoffKey, now)
+			if tt.expectBackoffReset {
+				assert.False(t, isInBackoff, "container backoff should be reset")
+			} else {
+				assert.True(t, isInBackoff, "container backoff should not be reset")
+			}
 		})
 	}
 }
diff --git a/pkg/kubelet/kuberuntime/helpers_test.go b/pkg/kubelet/kuberuntime/helpers_test.go
index e479496059c76..8cc9c2956e43d 100644
--- a/pkg/kubelet/kuberuntime/helpers_test.go
+++ b/pkg/kubelet/kuberuntime/helpers_test.go
@@ -113,11 +113,11 @@ func TestStableKey(t *testing.T) {
 			Containers: []v1.Container{*container},
 		},
 	}
-	oldKey := getStableKey(pod, container)
+	oldKey := GetStableKey(pod, container)
 
 	// Updating the container image should change the key.
 	container.Image = "foo/image:v2"
-	newKey := getStableKey(pod, container)
+	newKey := GetStableKey(pod, container)
 	assert.NotEqual(t, oldKey, newKey)
 }