From 6a40bcb426861447aee861866027637d7bc1d0e1 Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Wed, 9 Jul 2025 21:12:09 -0700 Subject: [PATCH 1/5] Retry pending resizes if a status update leads to aggregate requests shrinking --- pkg/kubelet/allocation/allocation_manager.go | 24 ++++++++--------- pkg/kubelet/kubelet.go | 27 ++++++++++++++++++-- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/pkg/kubelet/allocation/allocation_manager.go b/pkg/kubelet/allocation/allocation_manager.go index c703310f918..b44c5158141 100644 --- a/pkg/kubelet/allocation/allocation_manager.go +++ b/pkg/kubelet/allocation/allocation_manager.go @@ -117,6 +117,9 @@ type Manager interface { // Returns true if the pending resize was added to the queue, false if it was already present. PushPendingResize(uid types.UID) bool + // HasPendingResizes returns whether there are currently any pending resizes. + HasPendingResizes() bool + // RetryPendingResizes retries all pending resizes. RetryPendingResizes(trigger string) @@ -451,6 +454,13 @@ func (m *manager) isResizeIncreasingRequests(pod *v1.Pod) bool { newRequest.Cpu().Cmp(*oldRequest.Cpu()) > 0 } +func (m *manager) HasPendingResizes() bool { + m.allocationMutex.Lock() + defer m.allocationMutex.Unlock() + + return len(m.podsWithPendingResizes) > 0 +} + // GetContainerResourceAllocation returns the last checkpointed AllocatedResources values // If checkpoint manager has not been initialized, it returns nil, false func (m *manager) GetContainerResourceAllocation(podUID types.UID, containerName string) (v1.ResourceRequirements, bool) { @@ -739,18 +749,6 @@ func (m *manager) canResizePod(allocatedPods []*v1.Pod, pod *v1.Pod) (bool, stri return false, v1.PodReasonInfeasible, msg } - for i := range allocatedPods { - for j, c := range allocatedPods[i].Status.ContainerStatuses { - actuatedResources, exists := m.GetActuatedResources(allocatedPods[i].UID, c.Name) - if exists { - // Overwrite the actual resources in the status with the actuated resources. - // This lets us reuse the existing scheduler libraries without having to wait - // for the actual resources in the status to be updated. - allocatedPods[i].Status.ContainerStatuses[j].Resources = &actuatedResources - } - } - } - if ok, failReason, failMessage := m.canAdmitPod(allocatedPods, pod); !ok { // Log reason and return. klog.V(3).InfoS("Resize cannot be accommodated", "pod", klog.KObj(pod), "reason", failReason, "message", failMessage) @@ -772,8 +770,6 @@ func (m *manager) CheckPodResizeInProgress(allocatedPod *v1.Pod, podStatus *kube if m.recorder != nil { m.recorder.Eventf(allocatedPod, v1.EventTypeNormal, events.ResizeCompleted, podResizeCompletedEventMsg) } - // TODO(natasha41575): We only need to make this call if any of the resources were decreased. - m.RetryPendingResizes(TriggerReasonPodResized) } } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index ba966e35624..08a1ee355cb 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -70,6 +70,7 @@ import ( cloudprovider "k8s.io/cloud-provider" "k8s.io/component-base/zpages/flagz" "k8s.io/component-helpers/apimachinery/lease" + resourcehelper "k8s.io/component-helpers/resource" internalapi "k8s.io/cri-api/pkg/apis" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" remote "k8s.io/cri-client/pkg" @@ -1900,10 +1901,9 @@ func (kl *Kubelet) SyncPod(ctx context.Context, updateType kubetypes.SyncPodType if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { // Check whether a resize is in progress so we can set the PodResizeInProgressCondition accordingly. kl.allocationManager.CheckPodResizeInProgress(pod, podStatus) - // TODO(natasha41575): There is a race condition here, where the goroutine in the + // TODO(#132851): There is a race condition here, where the goroutine in the // allocation manager may allocate a new resize and unconditionally set the // PodResizeInProgressCondition before we set the status below. - // See https://github.com/kubernetes/kubernetes/issues/132851. } // Generate final API pod status with pod and status manager status @@ -2799,9 +2799,12 @@ func (kl *Kubelet) HandlePodRemoves(pods []*v1.Pod) { // pod is updated in the API. func (kl *Kubelet) HandlePodReconcile(pods []*v1.Pod) { start := kl.clock.Now() + retryPendingResizes := false + hasPendingResizes := kl.allocationManager.HasPendingResizes() for _, pod := range pods { // Update the pod in pod manager, status manager will do periodically reconcile according // to the pod manager. + oldPod, _ := kl.podManager.GetPodByUID(pod.UID) kl.podManager.UpdatePod(pod) pod, mirrorPod, wasMirror := kl.podManager.GetPodAndMirrorPod(pod) @@ -2813,6 +2816,22 @@ func (kl *Kubelet) HandlePodReconcile(pods []*v1.Pod) { // Static pods should be reconciled the same way as regular pods } + // If there are pending resizes, check whether the requests shrank as a result of the status + // resources changing. + if hasPendingResizes && !retryPendingResizes && oldPod != nil && + utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { + opts := resourcehelper.PodResourcesOptions{ + UseStatusResources: true, + SkipPodLevelResources: !utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources), + } + oldRequest := resourcehelper.PodRequests(oldPod, opts) + newRequest := resourcehelper.PodRequests(pod, opts) + + // If memory requests or limits shrank, then retry the pending resizes. + retryPendingResizes = newRequest.Memory().Cmp(*oldRequest.Memory()) < 1 || + newRequest.Cpu().Cmp(*oldRequest.Cpu()) < 1 + } + // TODO: reconcile being calculated in the config manager is questionable, and avoiding // extra syncs may no longer be necessary. Reevaluate whether Reconcile and Sync can be // merged (after resolving the next two TODOs). @@ -2841,6 +2860,10 @@ func (kl *Kubelet) HandlePodReconcile(pods []*v1.Pod) { } } } + + if retryPendingResizes { + kl.allocationManager.RetryPendingResizes(allocation.TriggerReasonPodResized) + } } // HandlePodSyncs is the callback in the syncHandler interface for pods From 6e86af48ccbcdcff275c91d9191b56522c094a4c Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 18 Jul 2025 00:45:28 +0000 Subject: [PATCH 2/5] fix check if requests have shrunk --- pkg/kubelet/kubelet.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 08a1ee355cb..9c963017102 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -2816,20 +2816,21 @@ func (kl *Kubelet) HandlePodReconcile(pods []*v1.Pod) { // Static pods should be reconciled the same way as regular pods } - // If there are pending resizes, check whether the requests shrank as a result of the status - // resources changing. - if hasPendingResizes && !retryPendingResizes && oldPod != nil && - utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { - opts := resourcehelper.PodResourcesOptions{ - UseStatusResources: true, - SkipPodLevelResources: !utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources), - } - oldRequest := resourcehelper.PodRequests(oldPod, opts) - newRequest := resourcehelper.PodRequests(pod, opts) + if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { + // If there are pending resizes, check whether the requests shrank as a result of the status + // resources changing. + if hasPendingResizes && !retryPendingResizes && oldPod != nil { + opts := resourcehelper.PodResourcesOptions{ + UseStatusResources: true, + SkipPodLevelResources: !utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources), + } + oldRequest := resourcehelper.PodRequests(oldPod, opts) + newRequest := resourcehelper.PodRequests(pod, opts) - // If memory requests or limits shrank, then retry the pending resizes. - retryPendingResizes = newRequest.Memory().Cmp(*oldRequest.Memory()) < 1 || - newRequest.Cpu().Cmp(*oldRequest.Cpu()) < 1 + // If cpu or memory requests shrank, then retry the pending resizes. + retryPendingResizes = newRequest.Memory().Cmp(*oldRequest.Memory()) < 0 || + newRequest.Cpu().Cmp(*oldRequest.Cpu()) < 0 + } } // TODO: reconcile being calculated in the config manager is questionable, and avoiding @@ -2861,8 +2862,10 @@ func (kl *Kubelet) HandlePodReconcile(pods []*v1.Pod) { } } - if retryPendingResizes { - kl.allocationManager.RetryPendingResizes(allocation.TriggerReasonPodResized) + if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { + if retryPendingResizes { + kl.allocationManager.RetryPendingResizes(allocation.TriggerReasonPodResized) + } } } From 0d24c3b57f05a347f4e2b050a73910bb40018c8f Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 18 Jul 2025 00:46:30 +0000 Subject: [PATCH 3/5] add sourcesReady parameter to fakeKubelet constructor --- pkg/kubelet/kubelet_node_status_test.go | 4 ++-- pkg/kubelet/kubelet_test.go | 11 ++++++----- pkg/kubelet/kubelet_volumes_test.go | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index a6f587d55ea..b423d7e8fc1 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/pkg/kubelet/kubelet_node_status_test.go @@ -213,7 +213,7 @@ func TestUpdateNewNodeStatus(t *testing.T) { } inputImageList, expectedImageList := generateTestingImageLists(numTestImages, int(tc.nodeStatusMaxImages)) testKubelet := newTestKubeletWithImageList( - t, inputImageList, false /* controllerAttachDetachEnabled */, true /*initFakeVolumePlugin*/, true /* localStorageCapacityIsolation */, false /*excludePodAdmitHandlers*/) + t, inputImageList, false /* controllerAttachDetachEnabled */, true /*initFakeVolumePlugin*/, true /* localStorageCapacityIsolation */, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet kubelet.nodeStatusMaxImages = tc.nodeStatusMaxImages @@ -1588,7 +1588,7 @@ func TestUpdateNewNodeStatusTooLargeReservation(t *testing.T) { // generate one more in inputImageList than we configure the Kubelet to report inputImageList, _ := generateTestingImageLists(nodeStatusMaxImages+1, nodeStatusMaxImages) testKubelet := newTestKubeletWithImageList( - t, inputImageList, false /* controllerAttachDetachEnabled */, true /* initFakeVolumePlugin */, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/) + t, inputImageList, false /* controllerAttachDetachEnabled */, true /* initFakeVolumePlugin */, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet kubelet.nodeStatusMaxImages = nodeStatusMaxImages diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 9a9fe986b6e..837eb1b24da 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -177,10 +177,10 @@ func newTestKubelet(t *testing.T, controllerAttachDetachEnabled bool) *TestKubel Size: 456, }, } - return newTestKubeletWithImageList(t, imageList, controllerAttachDetachEnabled, true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/) + return newTestKubeletWithImageList(t, imageList, controllerAttachDetachEnabled, true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) } -func newTestKubeletExcludeAdmitHandlers(t *testing.T, controllerAttachDetachEnabled bool) *TestKubelet { +func newTestKubeletExcludeAdmitHandlers(t *testing.T, controllerAttachDetachEnabled, sourcesReady bool) *TestKubelet { imageList := []kubecontainer.Image{ { ID: "abc", @@ -193,7 +193,7 @@ func newTestKubeletExcludeAdmitHandlers(t *testing.T, controllerAttachDetachEnab Size: 456, }, } - return newTestKubeletWithImageList(t, imageList, controllerAttachDetachEnabled, true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, true /*excludePodAdmitHandlers*/) + return newTestKubeletWithImageList(t, imageList, controllerAttachDetachEnabled, true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, true /*excludePodAdmitHandlers*/, sourcesReady) } func newTestKubeletWithImageList( @@ -203,6 +203,7 @@ func newTestKubeletWithImageList( initFakeVolumePlugin bool, localStorageCapacityIsolation bool, excludeAdmitHandlers bool, + sourcesReady bool, ) *TestKubelet { logger, _ := ktesting.NewTestContext(t) @@ -328,7 +329,7 @@ func newTestKubeletWithImageList( func(pod *v1.Pod) { kubelet.HandlePodSyncs([]*v1.Pod{pod}) }, kubelet.GetActivePods, kubelet.podManager.GetPodByUID, - config.NewSourcesReady(func(_ sets.Set[string]) bool { return false }), + config.NewSourcesReady(func(_ sets.Set[string]) bool { return sourcesReady }), ) kubelet.allocationManager.SetContainerRuntime(fakeRuntime) volumeStatsAggPeriod := time.Second * 10 @@ -1073,7 +1074,7 @@ func TestHandleMemExceeded(t *testing.T) { // Tests that we handle result of interface UpdatePluginResources correctly // by setting corresponding status in status map. func TestHandlePluginResources(t *testing.T) { - testKubelet := newTestKubeletExcludeAdmitHandlers(t, false /* controllerAttachDetachEnabled */) + testKubelet := newTestKubeletExcludeAdmitHandlers(t, false /* controllerAttachDetachEnabled */, false /*sourcesReady*/) defer testKubelet.Cleanup() kl := testKubelet.kubelet diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index 774223606bb..ef4bae385df 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -220,7 +220,7 @@ func TestPodVolumeDeadlineAttachAndMount(t *testing.T) { } testKubelet := newTestKubeletWithImageList(t, nil /*imageList*/, false, /* controllerAttachDetachEnabled */ - false /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/) + false /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -282,7 +282,7 @@ func TestPodVolumeDeadlineUnmount(t *testing.T) { } testKubelet := newTestKubeletWithImageList(t, nil /*imageList*/, false, /* controllerAttachDetachEnabled */ - true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/) + true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet From 65951744564064dd8fca631340285ef3cc35f75f Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 18 Jul 2025 00:47:05 +0000 Subject: [PATCH 4/5] unit test for HandlePodReconcile retrying pending resizes --- pkg/kubelet/kubelet_test.go | 87 +++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 837eb1b24da..f56d3ec428a 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -4225,3 +4225,90 @@ func TestHandlePodUpdates_RecordContainerRequestedResizes(t *testing.T) { }) } } + +func TestHandlePodReconcile_RetryPendingResizes(t *testing.T) { + if goruntime.GOOS == "windows" { + t.Skip("InPlacePodVerticalScaling is not currently supported for Windows") + } + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, true) + + testKubelet := newTestKubeletExcludeAdmitHandlers(t, false /* controllerAttachDetachEnabled */, true /*sourcesReady*/) + defer testKubelet.Cleanup() + kubelet := testKubelet.kubelet + + cpu500m := resource.MustParse("500m") + cpu1000m := resource.MustParse("1") + mem500M := resource.MustParse("500Mi") + mem1000M := resource.MustParse("1Gi") + + makePodWithRequests := func(name string, requests v1.ResourceList) *v1.Pod { + return &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + UID: types.UID(name), + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "c1", + Image: "i1", + Resources: v1.ResourceRequirements{ + Requests: requests, + }, + }, + }, + }, + } + } + + pendingResizeAllocated := makePodWithRequests("pod-pending-resize", v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}) + pendingResizeDesired := makePodWithRequests("pod-pending-resize", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}) + + testCases := []struct { + name string + oldPod *v1.Pod + newPod *v1.Pod + shouldRetryPendingResize bool + }{ + { + name: "requests are increasing", + oldPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), + newPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}), + shouldRetryPendingResize: false, + }, + { + name: "requests are unchanged", + oldPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), + newPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), + shouldRetryPendingResize: false, + }, + { + name: "requests are decreasing", + oldPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}), + newPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), + shouldRetryPendingResize: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // For the sake of this test, just reject all resize requests. + handler := &testPodAdmitHandler{podsToReject: []*v1.Pod{pendingResizeAllocated}} + kubelet.allocationManager.AddPodAdmitHandlers(lifecycle.PodAdmitHandlers{handler}) + + require.NoError(t, kubelet.allocationManager.SetAllocatedResources(pendingResizeAllocated)) + require.NoError(t, kubelet.allocationManager.SetActuatedResources(pendingResizeAllocated, nil)) + kubelet.podManager.AddPod(pendingResizeDesired) + kubelet.podManager.AddPod(tc.oldPod) + kubelet.allocationManager.PushPendingResize(pendingResizeDesired.UID) + + kubelet.statusManager.ClearPodResizePendingCondition(pendingResizeDesired.UID) + kubelet.HandlePodReconcile([]*v1.Pod{tc.newPod}) + require.Equal(t, tc.shouldRetryPendingResize, kubelet.statusManager.IsPodResizeDeferred(pendingResizeDesired.UID)) + + kubelet.allocationManager.RemovePod(pendingResizeDesired.UID) + kubelet.podManager.RemovePod((pendingResizeDesired)) + kubelet.podManager.RemovePod(tc.oldPod) + }) + } +} From ae5247afc18d4bbdf65ea7b0b0bca14bb04431ec Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 18 Jul 2025 21:22:41 +0000 Subject: [PATCH 5/5] address feedback --- pkg/kubelet/kubelet.go | 9 ++- pkg/kubelet/kubelet_node_status_test.go | 4 +- pkg/kubelet/kubelet_test.go | 89 +++++++++++++++++++------ pkg/kubelet/kubelet_volumes_test.go | 4 +- 4 files changed, 81 insertions(+), 25 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 9c963017102..a82bbfd68f5 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -2824,8 +2824,13 @@ func (kl *Kubelet) HandlePodReconcile(pods []*v1.Pod) { UseStatusResources: true, SkipPodLevelResources: !utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources), } - oldRequest := resourcehelper.PodRequests(oldPod, opts) - newRequest := resourcehelper.PodRequests(pod, opts) + + // Ignore desired resources when aggregating the resources. + allocatedOldPod, _ := kl.allocationManager.UpdatePodFromAllocation(oldPod) + allocatedPod, _ := kl.allocationManager.UpdatePodFromAllocation(pod) + + oldRequest := resourcehelper.PodRequests(allocatedOldPod, opts) + newRequest := resourcehelper.PodRequests(allocatedPod, opts) // If cpu or memory requests shrank, then retry the pending resizes. retryPendingResizes = newRequest.Memory().Cmp(*oldRequest.Memory()) < 0 || diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index b423d7e8fc1..86f03304701 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/pkg/kubelet/kubelet_node_status_test.go @@ -213,7 +213,7 @@ func TestUpdateNewNodeStatus(t *testing.T) { } inputImageList, expectedImageList := generateTestingImageLists(numTestImages, int(tc.nodeStatusMaxImages)) testKubelet := newTestKubeletWithImageList( - t, inputImageList, false /* controllerAttachDetachEnabled */, true /*initFakeVolumePlugin*/, true /* localStorageCapacityIsolation */, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) + t, inputImageList, false /* controllerAttachDetachEnabled */, true /*initFakeVolumePlugin*/, true /* localStorageCapacityIsolation */, false /*excludePodAdmitHandlers*/, false /*enableResizing*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet kubelet.nodeStatusMaxImages = tc.nodeStatusMaxImages @@ -1588,7 +1588,7 @@ func TestUpdateNewNodeStatusTooLargeReservation(t *testing.T) { // generate one more in inputImageList than we configure the Kubelet to report inputImageList, _ := generateTestingImageLists(nodeStatusMaxImages+1, nodeStatusMaxImages) testKubelet := newTestKubeletWithImageList( - t, inputImageList, false /* controllerAttachDetachEnabled */, true /* initFakeVolumePlugin */, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) + t, inputImageList, false /* controllerAttachDetachEnabled */, true /* initFakeVolumePlugin */, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*enableResizing*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet kubelet.nodeStatusMaxImages = nodeStatusMaxImages diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index f56d3ec428a..b10cbfbe988 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -177,7 +177,7 @@ func newTestKubelet(t *testing.T, controllerAttachDetachEnabled bool) *TestKubel Size: 456, }, } - return newTestKubeletWithImageList(t, imageList, controllerAttachDetachEnabled, true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) + return newTestKubeletWithImageList(t, imageList, controllerAttachDetachEnabled, true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*enableResizing*/) } func newTestKubeletExcludeAdmitHandlers(t *testing.T, controllerAttachDetachEnabled, sourcesReady bool) *TestKubelet { @@ -203,7 +203,7 @@ func newTestKubeletWithImageList( initFakeVolumePlugin bool, localStorageCapacityIsolation bool, excludeAdmitHandlers bool, - sourcesReady bool, + enableResizing bool, ) *TestKubelet { logger, _ := ktesting.NewTestContext(t) @@ -329,7 +329,7 @@ func newTestKubeletWithImageList( func(pod *v1.Pod) { kubelet.HandlePodSyncs([]*v1.Pod{pod}) }, kubelet.GetActivePods, kubelet.podManager.GetPodByUID, - config.NewSourcesReady(func(_ sets.Set[string]) bool { return sourcesReady }), + config.NewSourcesReady(func(_ sets.Set[string]) bool { return enableResizing }), ) kubelet.allocationManager.SetContainerRuntime(fakeRuntime) volumeStatsAggPeriod := time.Second * 10 @@ -1074,7 +1074,7 @@ func TestHandleMemExceeded(t *testing.T) { // Tests that we handle result of interface UpdatePluginResources correctly // by setting corresponding status in status map. func TestHandlePluginResources(t *testing.T) { - testKubelet := newTestKubeletExcludeAdmitHandlers(t, false /* controllerAttachDetachEnabled */, false /*sourcesReady*/) + testKubelet := newTestKubeletExcludeAdmitHandlers(t, false /* controllerAttachDetachEnabled */, false /*enableResizing*/) defer testKubelet.Cleanup() kl := testKubelet.kubelet @@ -4232,16 +4232,20 @@ func TestHandlePodReconcile_RetryPendingResizes(t *testing.T) { } featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, true) - testKubelet := newTestKubeletExcludeAdmitHandlers(t, false /* controllerAttachDetachEnabled */, true /*sourcesReady*/) + testKubelet := newTestKubeletExcludeAdmitHandlers(t, false /* controllerAttachDetachEnabled */, true /*enableResizing*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet - cpu500m := resource.MustParse("500m") - cpu1000m := resource.MustParse("1") - mem500M := resource.MustParse("500Mi") - mem1000M := resource.MustParse("1Gi") + lowCPU := resource.MustParse("500m") + highCPU := resource.MustParse("1") + lowMem := resource.MustParse("500Mi") + highMem := resource.MustParse("1Gi") - makePodWithRequests := func(name string, requests v1.ResourceList) *v1.Pod { + // Set desired resources to some huge value to verify that they are being ignored in the aggregate check. + enormousCPU := resource.MustParse("2000m") + enormousMem := resource.MustParse("2000Mi") + + makePodWithResources := func(name string, requests v1.ResourceList, status v1.ResourceList) *v1.Pod { return &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -4258,11 +4262,54 @@ func TestHandlePodReconcile_RetryPendingResizes(t *testing.T) { }, }, }, + Status: v1.PodStatus{ + ContainerStatuses: []v1.ContainerStatus{ + { + Name: "c1", + Resources: &v1.ResourceRequirements{ + Requests: status, + }, + }, + }, + }, } } - pendingResizeAllocated := makePodWithRequests("pod-pending-resize", v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}) - pendingResizeDesired := makePodWithRequests("pod-pending-resize", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}) + pendingResizeAllocated := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-pending-resize", + UID: types.UID("pod-pending-resize"), + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "c1", + Image: "i1", + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{v1.ResourceCPU: highCPU, v1.ResourceMemory: highMem}, + }, + }, + }, + }, + } + + pendingResizeDesired := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-pending-resize", + UID: types.UID("pod-pending-resize"), + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "c1", + Image: "i1", + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{v1.ResourceCPU: lowCPU, v1.ResourceMemory: lowMem}, + }, + }, + }, + }, + } testCases := []struct { name string @@ -4272,20 +4319,20 @@ func TestHandlePodReconcile_RetryPendingResizes(t *testing.T) { }{ { name: "requests are increasing", - oldPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), - newPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}), + oldPod: makePodWithResources("updated-pod", v1.ResourceList{v1.ResourceCPU: highCPU, v1.ResourceMemory: highMem}, v1.ResourceList{v1.ResourceCPU: lowCPU, v1.ResourceMemory: lowMem}), + newPod: makePodWithResources("updated-pod", v1.ResourceList{v1.ResourceCPU: enormousCPU, v1.ResourceMemory: enormousMem}, v1.ResourceList{v1.ResourceCPU: highCPU, v1.ResourceMemory: highMem}), shouldRetryPendingResize: false, }, { name: "requests are unchanged", - oldPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), - newPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), + oldPod: makePodWithResources("updated-pod", v1.ResourceList{v1.ResourceCPU: lowCPU, v1.ResourceMemory: lowMem}, v1.ResourceList{v1.ResourceCPU: lowCPU, v1.ResourceMemory: lowMem}), + newPod: makePodWithResources("updated-pod", v1.ResourceList{v1.ResourceCPU: enormousCPU, v1.ResourceMemory: enormousMem}, v1.ResourceList{v1.ResourceCPU: lowCPU, v1.ResourceMemory: lowMem}), shouldRetryPendingResize: false, }, { name: "requests are decreasing", - oldPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}), - newPod: makePodWithRequests("updated-pod", v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}), + oldPod: makePodWithResources("updated-pod", v1.ResourceList{v1.ResourceCPU: lowCPU, v1.ResourceMemory: lowMem}, v1.ResourceList{v1.ResourceCPU: highCPU, v1.ResourceMemory: highMem}), + newPod: makePodWithResources("updated-pod", v1.ResourceList{v1.ResourceCPU: enormousCPU, v1.ResourceMemory: enormousMem}, v1.ResourceList{v1.ResourceCPU: lowCPU, v1.ResourceMemory: lowMem}), shouldRetryPendingResize: true, }, } @@ -4297,7 +4344,11 @@ func TestHandlePodReconcile_RetryPendingResizes(t *testing.T) { kubelet.allocationManager.AddPodAdmitHandlers(lifecycle.PodAdmitHandlers{handler}) require.NoError(t, kubelet.allocationManager.SetAllocatedResources(pendingResizeAllocated)) - require.NoError(t, kubelet.allocationManager.SetActuatedResources(pendingResizeAllocated, nil)) + require.NoError(t, kubelet.allocationManager.SetAllocatedResources(tc.oldPod)) + + // We only expect status resources to change in HandlePodReconcile. + tc.oldPod.Spec = tc.newPod.Spec + kubelet.podManager.AddPod(pendingResizeDesired) kubelet.podManager.AddPod(tc.oldPod) kubelet.allocationManager.PushPendingResize(pendingResizeDesired.UID) diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index ef4bae385df..9ba12c2f31a 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -220,7 +220,7 @@ func TestPodVolumeDeadlineAttachAndMount(t *testing.T) { } testKubelet := newTestKubeletWithImageList(t, nil /*imageList*/, false, /* controllerAttachDetachEnabled */ - false /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) + false /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*enableResizing*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet @@ -282,7 +282,7 @@ func TestPodVolumeDeadlineUnmount(t *testing.T) { } testKubelet := newTestKubeletWithImageList(t, nil /*imageList*/, false, /* controllerAttachDetachEnabled */ - true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*sourcesReady*/) + true /*initFakeVolumePlugin*/, true /*localStorageCapacityIsolation*/, false /*excludePodAdmitHandlers*/, false /*enableResizing*/) defer testKubelet.Cleanup() kubelet := testKubelet.kubelet