allow nri plugins to block pod-level resizes

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
 		}

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

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)