From 7dee4a262f1b93df4a3abac6d6cf9309cd96b73d Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Sun, 8 Mar 2026 20:44:10 +0000 Subject: [PATCH] allow nri plugins to block pod-level resizes --- .../kuberuntime/kuberuntime_container.go | 7 +- .../kuberuntime/kuberuntime_manager.go | 21 +++-- .../kuberuntime/kuberuntime_manager_test.go | 93 +++++++++++++++++-- 3 files changed, 104 insertions(+), 17 deletions(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index 9a47ed6d3b4..ebda5f340a4 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -436,7 +436,12 @@ func (m *kubeGenericRuntimeManager) updatePodSandboxResources(ctx context.Contex _, err := m.runtimeService.UpdatePodSandboxResources(ctx, podResourcesRequest) if err != nil { stat, _ := grpcstatus.FromError(err) - if stat.Code() == codes.Unimplemented { + // Containerd 2.2 returns an error with code 'Unknown' and message 'not implemented yet' instead of + // an error with code 'Unimplemented'. + // This is being fixed in https://github.com/containerd/containerd/pull/13023, so this hardcoded + // string check can be removed once containerd 2.2 is no longer supported. + unimplementedMsg := "not implemented" + if stat.Code() == codes.Unimplemented || (stat.Code() == codes.Unknown && strings.Contains(stat.Message(), unimplementedMsg)) { logger.V(3).Info("updatePodSandboxResources failed: unimplemented; this call is best-effort: proceeding with resize", "sandboxID", sandboxID) return nil } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index 6a6b69b728c..35511094198 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -907,21 +907,30 @@ func (m *kubeGenericRuntimeManager) doPodResizeAction(ctx context.Context, pod * } resizedResources.Memory = podResources.Memory } + + // Notify the runtime first. If this fails, the runtime has rejected the resize. + mergedPodResources := mergeResourceConfig(currentPodResources, resizedResources) + if err = m.updatePodSandboxResources(ctx, podContainerChanges.SandboxID, pod, mergedPodResources); err != nil { + return fmt.Errorf("failed to notify runtime for UpdatePodSandboxResources (resource=%s); resize rejected: %w", rName, err) + } + + // Actuate the pod-level cgroup change. err = pcm.SetPodCgroupConfig(logger, pod, resizedResources) if err != nil { logger.Error(err, "Failed to set cgroup config", "resource", rName, "pod", klog.KObj(pod)) + // If cgroup adjustment fails, notify the runtime to revert to the previous resources. + if rollbackErr := m.updatePodSandboxResources(ctx, podContainerChanges.SandboxID, pod, currentPodResources); rollbackErr != nil { + logger.Error(rollbackErr, "Failed to notify runtime to rollback UpdatePodSandboxResources", "resource", rName, "pod", klog.KObj(pod)) + } return err } - currentPodResources = mergeResourceConfig(currentPodResources, resizedResources) - if err = m.updatePodSandboxResources(ctx, podContainerChanges.SandboxID, pod, currentPodResources); err != nil { - logger.Error(err, "Failed to notify runtime for UpdatePodSandboxResources", "resource", rName, "pod", klog.KObj(pod)) - // Don't propagate the error since the updatePodSandboxResources call is best-effort. - } + + // Update our tracking of the current state. + currentPodResources = mergedPodResources if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodLevelResourcesVerticalScaling) { if err = updateActuatedPodLevelResources(rName); err != nil { logger.Error(err, "Failed to update pod-level actuated resources", "resource", rName, "pod", klog.KObj(pod)) - } } return nil diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index c27479af182..a61756a22ad 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -3916,15 +3916,16 @@ func TestDoPodResizeAction(t *testing.T) { metrics.PodResizeDurationMilliseconds.Reset() for i, tc := range []struct { - testName string - currentResources resourceRequirements - desiredResources resourceRequirements - updatedResources []v1.ResourceName - otherContainersHaveLimits bool - runtimeErrors map[string][]error - expectedError string - expectedErrorMessage string - expectPodCgroupUpdates int + testName string + currentResources resourceRequirements + desiredResources resourceRequirements + updatedResources []v1.ResourceName + otherContainersHaveLimits bool + runtimeErrors map[string][]error + expectedError string + expectedErrorMessage string + expectPodCgroupUpdates int + injectPodUpdateCgroupsError error }{ { testName: "Increase cpu and memory requests and limits, with computed pod limits", @@ -4038,6 +4039,72 @@ func TestDoPodResizeAction(t *testing.T) { updatedResources: []v1.ResourceName{v1.ResourceCPU, v1.ResourceMemory}, expectPodCgroupUpdates: 2, // cpu lim, memory lim }, + { + testName: "Fail updatePodSandboxResources blocks resize", + currentResources: resourceRequirements{ + cpuRequest: 100, cpuLimit: 100, + }, + desiredResources: resourceRequirements{ + cpuRequest: 200, cpuLimit: 200, + }, + updatedResources: []v1.ResourceName{v1.ResourceCPU}, + expectPodCgroupUpdates: 0, + runtimeErrors: map[string][]error{ + "UpdatePodSandboxResources": {fmt.Errorf("runtime sandbox update failed")}, + }, + expectedError: "ResizePodInPlaceError", + expectedErrorMessage: "failed to notify runtime for UpdatePodSandboxResources (resource=cpu); resize rejected: updatePodSandboxResources failed for sanboxID \"sandbox-id\": runtime sandbox update failed", + }, + { + testName: "Fail SetPodCgroupConfig triggers rollback of Sandbox resources", + currentResources: resourceRequirements{ + cpuRequest: 100, cpuLimit: 100, + }, + desiredResources: resourceRequirements{ + cpuRequest: 200, cpuLimit: 200, + }, + updatedResources: []v1.ResourceName{v1.ResourceCPU}, + expectPodCgroupUpdates: 1, + injectPodUpdateCgroupsError: fmt.Errorf("cgroup update failed"), + expectedError: "ResizePodInPlaceError", + expectedErrorMessage: "cgroup update failed", + }, + { + testName: "Ignore Unimplemented error from updatePodSandboxResources", + currentResources: resourceRequirements{ + cpuRequest: 100, cpuLimit: 100, + }, + desiredResources: resourceRequirements{ + cpuRequest: 200, cpuLimit: 200, + }, + updatedResources: []v1.ResourceName{v1.ResourceCPU}, + expectPodCgroupUpdates: 1, // Should proceed to cgroups despite the error + runtimeErrors: map[string][]error{ + // Use a GRPC Unimplemented error + "UpdatePodSandboxResources": {status.Error(codes.Unimplemented, "not supported")}, + }, + // No error expected because we swallow Unimplemented + expectedError: "", + expectedErrorMessage: "", + }, + { + testName: "Ignore Unimplemented message from updatePodSandboxResources", + currentResources: resourceRequirements{ + cpuRequest: 100, cpuLimit: 100, + }, + desiredResources: resourceRequirements{ + cpuRequest: 200, cpuLimit: 200, + }, + updatedResources: []v1.ResourceName{v1.ResourceCPU}, + expectPodCgroupUpdates: 1, // Should proceed to cgroups despite the error + runtimeErrors: map[string][]error{ + // Use a GRPC Unimplemented error + "UpdatePodSandboxResources": {status.Error(codes.Unknown, "not implemented yet")}, + }, + // No error expected because we swallow Unimplemented + expectedError: "", + expectedErrorMessage: "", + }, } { t.Run(tc.testName, func(t *testing.T) { _, _, m, err := createTestRuntimeManagerWithErrors(tCtx, tc.runtimeErrors) @@ -4068,7 +4135,12 @@ func TestDoPodResizeAction(t *testing.T) { }, nil).Maybe() if tc.expectPodCgroupUpdates > 0 { // TODO: Update to use proper logger once contextual logging migration is complete - mockPCM.EXPECT().SetPodCgroupConfig(klog.TODO(), mock.Anything, mock.Anything).Return(nil).Times(tc.expectPodCgroupUpdates) + call := mockPCM.EXPECT().SetPodCgroupConfig(klog.TODO(), mock.Anything, mock.Anything) + if tc.injectPodUpdateCgroupsError != nil { + call.Return(tc.injectPodUpdateCgroupsError).Times(1) + } else { + call.Return(nil).Times(tc.expectPodCgroupUpdates) + } } pod, kps := makeBasePodAndStatus() @@ -4130,6 +4202,7 @@ func TestDoPodResizeAction(t *testing.T) { actions := podActions{ ContainersToUpdate: containersToUpdate, + SandboxID: "sandbox-id", } resizeResult := m.doPodResizeAction(tCtx, pod, kps, actions)