Invoke UpdateContainerResources or trigger container restarts (for RestartContainer policy) when memory requests are resized

pkg/kubelet/kuberuntime/kuberuntime_manager.go

@@ -634,8 +634,8 @@ func (m *kubeGenericRuntimeManager) computePodResizeAction(pod *v1.Pod, containe
 		return true
 	}
 
-	determineContainerResize := func(rName v1.ResourceName, specValue, statusValue int64) (resize, restart bool) {
+	determineContainerResize := func(rName v1.ResourceName, desiredValue, currentValue int64) (resize, restart bool) {
-		if specValue == statusValue {
+		if desiredValue == currentValue {
 			return false, false
 		}
 		for _, policy := range container.ResizePolicy {
@@ -646,7 +646,7 @@ func (m *kubeGenericRuntimeManager) computePodResizeAction(pod *v1.Pod, containe
 		// If a resource policy isn't set, the implicit default is NotRequired.
 		return true, false
 	}
-	markContainerForUpdate := func(rName v1.ResourceName, specValue, statusValue int64) {
+	markContainerForUpdate := func(rName v1.ResourceName, desiredValue, currentValue int64) {
 		cUpdateInfo := containerToUpdateInfo{
 			container:                 &container,
 			kubeContainerID:           kubeContainerStatus.ID,
@@ -655,18 +655,19 @@ func (m *kubeGenericRuntimeManager) computePodResizeAction(pod *v1.Pod, containe
 		}
 		// Order the container updates such that resource decreases are applied before increases
 		switch {
-		case specValue > statusValue: // append
+		case desiredValue > currentValue: // append
 			changes.ContainersToUpdate[rName] = append(changes.ContainersToUpdate[rName], cUpdateInfo)
-		case specValue < statusValue: // prepend
+		case desiredValue < currentValue: // prepend
 			changes.ContainersToUpdate[rName] = append(changes.ContainersToUpdate[rName], containerToUpdateInfo{})
 			copy(changes.ContainersToUpdate[rName][1:], changes.ContainersToUpdate[rName])
 			changes.ContainersToUpdate[rName][0] = cUpdateInfo
 		}
 	}
 	resizeMemLim, restartMemLim := determineContainerResize(v1.ResourceMemory, desiredResources.memoryLimit, currentResources.memoryLimit)
+	resizeMemReq, restartMemReq := determineContainerResize(v1.ResourceMemory, desiredResources.memoryRequest, currentResources.memoryRequest)
 	resizeCPULim, restartCPULim := determineContainerResize(v1.ResourceCPU, desiredResources.cpuLimit, currentResources.cpuLimit)
 	resizeCPUReq, restartCPUReq := determineContainerResize(v1.ResourceCPU, desiredResources.cpuRequest, currentResources.cpuRequest)
-	if restartCPULim || restartCPUReq || restartMemLim {
+	if restartCPULim || restartCPUReq || restartMemLim || restartMemReq {
 		// resize policy requires this container to restart
 		changes.ContainersToKill[kubeContainerStatus.ID] = containerToKillInfo{
 			name:      kubeContainerStatus.Name,
@@ -683,6 +684,8 @@ func (m *kubeGenericRuntimeManager) computePodResizeAction(pod *v1.Pod, containe
 	} else {
 		if resizeMemLim {
 			markContainerForUpdate(v1.ResourceMemory, desiredResources.memoryLimit, currentResources.memoryLimit)
+		} else if resizeMemReq {
+			markContainerForUpdate(v1.ResourceMemory, desiredResources.memoryRequest, currentResources.memoryRequest)
 		}
 		if resizeCPULim {
 			markContainerForUpdate(v1.ResourceCPU, desiredResources.cpuLimit, currentResources.cpuLimit)

pkg/kubelet/kuberuntime/kuberuntime_manager_test.go

@@ -2890,6 +2890,96 @@ func TestComputePodActionsForPodResize(t *testing.T) {
 				return &pa
 			},
 		},
+		"Update container memory (requests only) with RestartContainer policy for memory": {
+			setupFn: func(pod *v1.Pod) {
+				c := &pod.Spec.Containers[2]
+				c.ResizePolicy = []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartRequired}
+				c.Resources = v1.ResourceRequirements{
+					Limits:   v1.ResourceList{v1.ResourceCPU: cpu200m, v1.ResourceMemory: mem200M},
+					Requests: v1.ResourceList{v1.ResourceCPU: cpu100m, v1.ResourceMemory: mem100M},
+				}
+				setupActuatedResources(pod, c, v1.ResourceRequirements{
+					Limits: v1.ResourceList{
+						v1.ResourceCPU:    cpu200m.DeepCopy(),
+						v1.ResourceMemory: mem200M.DeepCopy(),
+					},
+					Requests: v1.ResourceList{
+						v1.ResourceCPU:    cpu100m.DeepCopy(),
+						v1.ResourceMemory: mem200M.DeepCopy(),
+					},
+				})
+			},
+			getExpectedPodActionsFn: func(pod *v1.Pod, podStatus *kubecontainer.PodStatus) *podActions {
+				kcs := podStatus.FindContainerStatusByName(pod.Spec.Containers[2].Name)
+				killMap := make(map[kubecontainer.ContainerID]containerToKillInfo)
+				killMap[kcs.ID] = containerToKillInfo{
+					container: &pod.Spec.Containers[2],
+					name:      pod.Spec.Containers[2].Name,
+				}
+				pa := podActions{
+					SandboxID:          podStatus.SandboxStatuses[0].Id,
+					ContainersToStart:  []int{2},
+					ContainersToKill:   killMap,
+					ContainersToUpdate: map[v1.ResourceName][]containerToUpdateInfo{},
+					UpdatePodResources: true,
+				}
+				return &pa
+			},
+		},
+		"Update container memory (requests only) with RestartNotRequired policy for memory": {
+			setupFn: func(pod *v1.Pod) {
+				c := &pod.Spec.Containers[2]
+				c.ResizePolicy = []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartNotRequired}
+				c.Resources = v1.ResourceRequirements{
+					Limits:   v1.ResourceList{v1.ResourceCPU: cpu200m, v1.ResourceMemory: mem200M},
+					Requests: v1.ResourceList{v1.ResourceCPU: cpu100m, v1.ResourceMemory: mem100M},
+				}
+				setupActuatedResources(pod, c, v1.ResourceRequirements{
+					Limits: v1.ResourceList{
+						v1.ResourceCPU:    cpu200m.DeepCopy(),
+						v1.ResourceMemory: mem200M.DeepCopy(),
+					},
+					Requests: v1.ResourceList{
+						v1.ResourceCPU:    cpu100m.DeepCopy(),
+						v1.ResourceMemory: mem200M.DeepCopy(),
+					},
+				})
+			},
+			getExpectedPodActionsFn: func(pod *v1.Pod, podStatus *kubecontainer.PodStatus) *podActions {
+				kcs := podStatus.FindContainerStatusByName(pod.Spec.Containers[2].Name)
+				killMap := make(map[kubecontainer.ContainerID]containerToKillInfo)
+				killMap[kcs.ID] = containerToKillInfo{
+					container: &pod.Spec.Containers[2],
+					name:      pod.Spec.Containers[2].Name,
+				}
+				pa := podActions{
+					SandboxID:         podStatus.SandboxStatuses[0].Id,
+					ContainersToStart: []int{},
+					ContainersToKill:  getKillMap(pod, podStatus, []int{}),
+					ContainersToUpdate: map[v1.ResourceName][]containerToUpdateInfo{
+						v1.ResourceMemory: {
+							{
+								container:       &pod.Spec.Containers[2],
+								kubeContainerID: kcs.ID,
+								desiredContainerResources: containerResources{
+									memoryLimit:   mem200M.Value(),
+									memoryRequest: mem100M.Value(),
+									cpuLimit:      cpu200m.MilliValue(),
+									cpuRequest:    cpu100m.MilliValue(),
+								},
+								currentContainerResources: &containerResources{
+									memoryLimit:   mem200M.Value(),
+									memoryRequest: mem200M.Value(),
+									cpuLimit:      cpu200m.MilliValue(),
+									cpuRequest:    cpu100m.MilliValue(),
+								},
+							},
+						},
+					},
+				}
+				return &pa
+			},
+		},
 	} {
 		t.Run(desc, func(t *testing.T) {
 			pod, status := makeBasePodAndStatus()

test/e2e/common/node/pod_resize.go

@@ -106,7 +106,6 @@ func doPodResizeTests() {
 		},
 		{
 			name: "Guaranteed QoS pod, one container - decrease CPU only",
-			testRollback: true,
 			containers: []e2epod.ResizableContainerInfo{
 				{
 					Name:      "c1",
@@ -126,6 +125,7 @@ func doPodResizeTests() {
 					MemPolicy: &noRestart,
 				},
 			},
+			testRollback: true,
 		},
 		{
 			name: "Guaranteed QoS pod, three containers (c1, c2, c3) - increase: CPU (c1,c3), memory (c2, c3) ; decrease: CPU (c2)",
@@ -180,7 +180,6 @@ func doPodResizeTests() {
 		},
 		{
 			name: "Burstable QoS pod, one container with cpu & memory requests + limits - decrease memory requests only",
-			testRollback: true,
 			containers: []e2epod.ResizableContainerInfo{
 				{
 					Name:      "c1",
@@ -196,6 +195,7 @@ func doPodResizeTests() {
 					Resources: &e2epod.ContainerResources{CPUReq: originalCPU, CPULim: originalCPULimit, MemReq: reducedMem, MemLim: originalMemLimit},
 				},
 			},
+			testRollback: true,
 		},
 		{
 			name: "Burstable QoS pod, one container with cpu & memory requests + limits - increase memory requests only",
@@ -235,7 +235,6 @@ func doPodResizeTests() {
 		},
 		{
 			name: "Burstable QoS pod, one container with cpu & memory requests + limits - decrease CPU requests only",
-			testRollback: true,
 			containers: []e2epod.ResizableContainerInfo{
 				{
 					Name:      "c1",
@@ -251,10 +250,10 @@ func doPodResizeTests() {
 					Resources: &e2epod.ContainerResources{CPUReq: reducedCPU, CPULim: originalCPULimit, MemReq: originalMem, MemLim: originalMemLimit},
 				},
 			},
+			testRollback: true,
 		},
 		{
 			name: "Burstable QoS pod, one container with cpu & memory requests + limits - decrease CPU limits only",
-			testRollback: true,
 			containers: []e2epod.ResizableContainerInfo{
 				{
 					Name:      "c1",
@@ -270,6 +269,7 @@ func doPodResizeTests() {
 					Resources: &e2epod.ContainerResources{CPUReq: originalCPU, CPULim: reducedCPULimit, MemReq: originalMem, MemLim: originalMemLimit},
 				},
 			},
+			testRollback: true,
 		},
 		{
 			name: "Burstable QoS pod, one container with cpu & memory requests + limits - increase CPU requests only",
@@ -525,7 +525,6 @@ func doPodResizeTests() {
 		},
 		{
 			name: "Guaranteed QoS pod, one container - increase CPU (NotRequired) & memory (RestartContainer)",
-			testRollback: true,
 			containers: []e2epod.ResizableContainerInfo{
 				{
 					Name:      "c1",
@@ -546,10 +545,10 @@ func doPodResizeTests() {
 					RestartCount: 1,
 				},
 			},
+			testRollback: true,
 		},
 		{
 			name: "Burstable QoS pod, one container - decrease CPU (NotRequired) & memory (RestartContainer)",
-			testRollback: true,
 			containers: []e2epod.ResizableContainerInfo{
 				{
 					Name:      "c1",
@@ -570,6 +569,53 @@ func doPodResizeTests() {
 					RestartCount: 1,
 				},
 			},
+			testRollback: true,
+		},
+		{
+			name: "Burstable QoS pod, one container - decrease memory request (RestartContainer memory resize policy)",
+			containers: []e2epod.ResizableContainerInfo{
+				{
+					Name:      "c1",
+					Resources: &e2epod.ContainerResources{CPUReq: originalCPU, CPULim: originalCPULimit, MemReq: originalMem, MemLim: originalMemLimit},
+					CPUPolicy: &noRestart,
+					MemPolicy: &doRestart,
+				},
+			},
+			patchString: fmt.Sprintf(`{"spec":{"containers":[
+						{"name":"c1", "resources":{"requests":{"cpu":"%s","memory":"%s"},"limits":{"cpu":"%s","memory":"%s"}}}
+					]}}`, originalCPU, reducedMem, originalCPULimit, originalMemLimit),
+			expected: []e2epod.ResizableContainerInfo{
+				{
+					Name:         "c1",
+					Resources:    &e2epod.ContainerResources{CPUReq: originalCPU, CPULim: originalCPULimit, MemReq: reducedMem, MemLim: originalMemLimit},
+					CPUPolicy:    &noRestart,
+					MemPolicy:    &doRestart,
+					RestartCount: 1,
+				},
+			},
+		},
+		{
+			name: "Burstable QoS pod, one container - increase memory request (NoRestart memory resize policy)",
+			containers: []e2epod.ResizableContainerInfo{
+				{
+					Name:      "c1",
+					Resources: &e2epod.ContainerResources{CPUReq: originalCPU, CPULim: originalCPULimit, MemReq: originalMem, MemLim: originalMemLimit},
+					CPUPolicy: &noRestart,
+					MemPolicy: &noRestart,
+				},
+			},
+			patchString: fmt.Sprintf(`{"spec":{"containers":[
+							{"name":"c1", "resources":{"requests":{"cpu":"%s","memory":"%s"},"limits":{"cpu":"%s","memory":"%s"}}}
+						]}}`, originalCPU, increasedMem, originalCPULimit, originalMemLimit),
+			expected: []e2epod.ResizableContainerInfo{
+				{
+					Name:         "c1",
+					Resources:    &e2epod.ContainerResources{CPUReq: originalCPU, CPULim: originalCPULimit, MemReq: increasedMem, MemLim: originalMemLimit},
+					CPUPolicy:    &noRestart,
+					MemPolicy:    &noRestart,
+					RestartCount: 0,
+				},
+			},
 		},
 		{
 			name: "Burstable QoS pod, three containers - increase c1 resources, no change for c2, decrease c3 resources (no net change for pod)",
@@ -1394,13 +1440,13 @@ func doPodResizeErrorTests() {
 //       Above tests are performed by doSheduletTests() and doPodResizeResourceQuotaTests()
 //       in test/e2e/node/pod_resize.go
 
-var _ = SIGDescribe("Pod InPlace Resize Container", feature.InPlacePodVerticalScaling, func() {
+var _ = SIGDescribe("[InPlacePodResize] Pod InPlace Resize Container", feature.InPlacePodVerticalScaling, func() {
 	f := framework.NewDefaultFramework("pod-resize-tests")
 
 	ginkgo.BeforeEach(func(ctx context.Context) {
-		node, err := e2enode.GetRandomReadySchedulableNode(ctx, f.ClientSet)
+		_, err := e2enode.GetRandomReadySchedulableNode(ctx, f.ClientSet)
 		framework.ExpectNoError(err)
-		if framework.NodeOSDistroIs("windows") || e2enode.IsARM64(node) {
+		if framework.NodeOSDistroIs("windows") {
 			e2eskipper.Skipf("runtime does not support InPlacePodVerticalScaling -- skipping")
 		}
 	})

test/e2e/framework/pod/resize.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"time"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -48,6 +49,7 @@ const (
 	Cgroupv2CPURequest         string = "/sys/fs/cgroup/cpu.weight"
 	CPUPeriod                  string = "100000"
 	MinContainerRuntimeVersion string = "1.6.9"
+	MinimumGracePeriodSeconds  int64  = 2
 )
 
 var (
@@ -167,6 +169,7 @@ func makeResizableContainer(tcInfo ResizableContainerInfo) v1.Container {
 func MakePodWithResizableContainers(ns, name, timeStamp string, tcInfo []ResizableContainerInfo) *v1.Pod {
 	testInitContainers, testContainers := separateContainers(tcInfo)
 
+	var minGracePeriodSeconds int64 = MinimumGracePeriodSeconds
 	pod := &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -180,6 +183,7 @@ func MakePodWithResizableContainers(ns, name, timeStamp string, tcInfo []Resizab
 			InitContainers:                testInitContainers,
 			Containers:                    testContainers,
 			RestartPolicy:                 v1.RestartPolicyOnFailure,
+			TerminationGracePeriodSeconds: &minGracePeriodSeconds,
 		},
 	}
 	return pod
@@ -350,6 +354,7 @@ func VerifyPodContainersCgroupValues(ctx context.Context, f *framework.Framework
 			}
 			errs = append(errs, VerifyCgroupValue(f, pod, ci.Name, cgroupCPULimit, expectedCPULimitString))
 			errs = append(errs, VerifyCgroupValue(f, pod, ci.Name, cgroupCPURequest, strconv.FormatInt(expectedCPUShares, 10)))
+			//TODO(vinaykul,InPlacePodVerticalScaling): Verify oom_score_adj when runc adds support for updating it
 		}
 	}
 	return utilerrors.NewAggregate(errs)
@@ -420,6 +425,8 @@ func WaitForPodResizeActuation(ctx context.Context, f *framework.Framework, podC
 		})),
 	)
 
+	// Wait 2x termination grace period to catch any restarts - expected or not
+	time.Sleep(time.Duration(*pod.Spec.TerminationGracePeriodSeconds*2) * time.Second)
 	resizedPod, err := framework.GetObject(podClient.Get, pod.Name, metav1.GetOptions{})(ctx)
 	framework.ExpectNoError(err, "failed to get resized pod")
 	return resizedPod

test/e2e/framework/pod/utils.go

@@ -294,6 +294,25 @@ func VerifyCgroupValue(f *framework.Framework, pod *v1.Pod, cName, cgPath, expec
 	return nil
 }
 
+// VerifyOomScoreAdjValue verifies that oom_score_adj for pid 1 (pidof init/systemd -> app)
+// has the expected value in specified container of the pod. It execs into the container,
+// reads the oom_score_adj value from procfs, and compares it against the expected value.
+func VerifyOomScoreAdjValue(f *framework.Framework, pod *v1.Pod, cName, expectedOomScoreAdj string) error {
+	cmd := fmt.Sprintf("cat /proc/1/oom_score_adj")
+	framework.Logf("Namespace %s Pod %s Container %s - looking for oom_score_adj value %s",
+		pod.Namespace, pod.Name, cName, expectedOomScoreAdj)
+	oomScoreAdj, _, err := ExecCommandInContainerWithFullOutput(f, pod.Name, cName, "/bin/sh", "-c", cmd)
+	if err != nil {
+		return fmt.Errorf("failed to find expected value %s for container app process", expectedOomScoreAdj)
+	}
+	oomScoreAdj = strings.Trim(oomScoreAdj, "\n")
+	if oomScoreAdj != expectedOomScoreAdj {
+		return fmt.Errorf("oom_score_adj value %s not equal to expected %s", oomScoreAdj, expectedOomScoreAdj)
+	}
+	fmt.Printf("VDBG: POD: %s EXPECTED_OOM_ADJ %s ACTUAL_OOM_ADJ %s\n", pod.Name, expectedOomScoreAdj, oomScoreAdj)
+	return nil
+}
+
 // IsPodOnCgroupv2Node checks whether the pod is running on cgroupv2 node.
 // TODO: Deduplicate this function with NPD cluster e2e test:
 // https://github.com/kubernetes/kubernetes/blob/2049360379bcc5d6467769cef112e6e492d3d2f0/test/e2e/node/node_problem_detector.go#L369