Deprecate IPPVSAllocatedStatus: always set allocatedResources with InPlacePodVerticalScaling

Author: tallclair

pkg/api/pod/util.go

@@ -800,9 +800,7 @@ func dropDisabledPodStatusFields(podStatus, oldPodStatus *api.PodStatus, podSpec
 		dropResourcesField(podStatus.ContainerStatuses)
 		dropResourcesField(podStatus.InitContainerStatuses)
 		dropResourcesField(podStatus.EphemeralContainerStatuses)
-	}
-	if !utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) ||
-		!utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScalingAllocatedStatus) {
+
 		// Drop AllocatedResources field
 		dropAllocatedResourcesField := func(csl []api.ContainerStatus) {
 			for i := range csl {

pkg/api/pod/util_test.go

@@ -2743,64 +2743,55 @@ func TestDropInPlacePodVerticalScaling(t *testing.T) {
 		t.Run(fmt.Sprintf("InPlacePodVerticalScaling=%t", ippvsEnabled), func(t *testing.T) {
 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, ippvsEnabled)
 
-			for _, allocatedStatusEnabled := range []bool{true, false} {
-				t.Run(fmt.Sprintf("AllocatedStatus=%t", allocatedStatusEnabled), func(t *testing.T) {
-					featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScalingAllocatedStatus, allocatedStatusEnabled)
-
-					for _, oldPodInfo := range podInfo {
-						for _, newPodInfo := range podInfo {
-							oldPodHasInPlaceVerticalScaling, oldPod := oldPodInfo.hasInPlaceVerticalScaling, oldPodInfo.pod()
-							newPodHasInPlaceVerticalScaling, newPod := newPodInfo.hasInPlaceVerticalScaling, newPodInfo.pod()
-							if newPod == nil {
-								continue
-							}
+			for _, oldPodInfo := range podInfo {
+				for _, newPodInfo := range podInfo {
+					oldPodHasInPlaceVerticalScaling, oldPod := oldPodInfo.hasInPlaceVerticalScaling, oldPodInfo.pod()
+					newPodHasInPlaceVerticalScaling, newPod := newPodInfo.hasInPlaceVerticalScaling, newPodInfo.pod()
+					if newPod == nil {
+						continue
+					}
 
-							t.Run(fmt.Sprintf("old pod %v, new pod %v", oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
-								var oldPodSpec *api.PodSpec
-								var oldPodStatus *api.PodStatus
-								if oldPod != nil {
-									oldPodSpec = &oldPod.Spec
-									oldPodStatus = &oldPod.Status
-								}
-								dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)
-								dropDisabledPodStatusFields(&newPod.Status, oldPodStatus, &newPod.Spec, oldPodSpec)
-
-								// old pod should never be changed
-								if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
-									t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod()))
-								}
-
-								switch {
-								case ippvsEnabled || oldPodHasInPlaceVerticalScaling:
-									// new pod shouldn't change if feature enabled or if old pod has ResizePolicy set
-									expected := newPodInfo.pod()
-									if !ippvsEnabled || !allocatedStatusEnabled {
-										expected.Status.ContainerStatuses[0].AllocatedResources = nil
-									}
-									if !reflect.DeepEqual(newPod, expected) {
-										t.Errorf("new pod changed: %v", cmp.Diff(newPod, expected))
-									}
-								case newPodHasInPlaceVerticalScaling:
-									// new pod should be changed
-									if reflect.DeepEqual(newPod, newPodInfo.pod()) {
-										t.Errorf("new pod was not changed")
-									}
-									// new pod should not have ResizePolicy
-									if !reflect.DeepEqual(newPod, podWithoutInPlaceVerticalScaling()) {
-										t.Errorf("new pod has ResizePolicy: %v", cmp.Diff(newPod, podWithoutInPlaceVerticalScaling()))
-									}
-								default:
-									// new pod should not need to be changed
-									if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
-										t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
-									}
-								}
-							})
+					t.Run(fmt.Sprintf("old pod %v, new pod %v", oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
+						var oldPodSpec *api.PodSpec
+						var oldPodStatus *api.PodStatus
+						if oldPod != nil {
+							oldPodSpec = &oldPod.Spec
+							oldPodStatus = &oldPod.Status
 						}
-					}
+						dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)
+						dropDisabledPodStatusFields(&newPod.Status, oldPodStatus, &newPod.Spec, oldPodSpec)
 
-				})
+						// old pod should never be changed
+						if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
+							t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod()))
+						}
+
+						switch {
+						case ippvsEnabled || oldPodHasInPlaceVerticalScaling:
+							// new pod shouldn't change if feature enabled or if old pod has ResizePolicy set
+							expected := newPodInfo.pod()
+							if !reflect.DeepEqual(newPod, expected) {
+								t.Errorf("new pod changed: %v", cmp.Diff(newPod, expected))
+							}
+						case newPodHasInPlaceVerticalScaling:
+							// new pod should be changed
+							if reflect.DeepEqual(newPod, newPodInfo.pod()) {
+								t.Errorf("new pod was not changed")
+							}
+							// new pod should not have ResizePolicy
+							if !reflect.DeepEqual(newPod, podWithoutInPlaceVerticalScaling()) {
+								t.Errorf("new pod has ResizePolicy: %v", cmp.Diff(newPod, podWithoutInPlaceVerticalScaling()))
+							}
+						default:
+							// new pod should not need to be changed
+							if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+								t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
+							}
+						}
+					})
+				}
 			}
+
 		})
 	}
 }

pkg/features/kube_features.go

@@ -311,7 +311,8 @@ const (
 	// owner: @tallclair
 	// kep: http://kep.k8s.io/1287
 	//
-	// Enables the AllocatedResources field in container status. This feature requires
+	// Deprecated: This feature gate is no longer used.
+	// Was: Enables the AllocatedResources field in container status. This feature requires
 	// InPlacePodVerticalScaling also be enabled.
 	InPlacePodVerticalScalingAllocatedStatus featuregate.Feature = "InPlacePodVerticalScalingAllocatedStatus"
 
@@ -1344,6 +1345,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 
 	InPlacePodVerticalScalingAllocatedStatus: {
 		{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
+		{Version: version.MustParse("1.33"), Default: false, PreRelease: featuregate.Deprecated}, // remove in 1.36
 	},
 
 	InPlacePodVerticalScalingExclusiveCPUs: {

pkg/kubelet/kubelet_pods.go

@@ -2279,9 +2279,7 @@ func (kl *Kubelet) convertToAPIContainerStatuses(pod *v1.Pod, podStatus *kubecon
 			allocatedContainer := kubecontainer.GetContainerSpec(pod, cName)
 			if allocatedContainer != nil {
 				status.Resources = convertContainerStatusResources(allocatedContainer, status, cStatus, oldStatuses)
-				if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScalingAllocatedStatus) {
-					status.AllocatedResources = allocatedContainer.Resources.Requests
-				}
+				status.AllocatedResources = allocatedContainer.Resources.Requests
 			}
 		}
 

pkg/kubelet/kubelet_pods_test.go

@@ -5104,20 +5104,8 @@ func TestConvertToAPIContainerStatusesForResources(t *testing.T) {
 			}
 			podStatus := testPodStatus(state, resources)
 
-			for _, enableAllocatedStatus := range []bool{true, false} {
-				t.Run(fmt.Sprintf("AllocatedStatus=%t", enableAllocatedStatus), func(t *testing.T) {
-					featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScalingAllocatedStatus, enableAllocatedStatus)
-
-					expected := tc.Expected
-					if !enableAllocatedStatus {
-						expected = *expected.DeepCopy()
-						expected.AllocatedResources = nil
-					}
-
-					cStatuses := kubelet.convertToAPIContainerStatuses(tPod, podStatus, []v1.ContainerStatus{tc.OldStatus}, tPod.Spec.Containers, false, false)
-					assert.Equal(t, expected, cStatuses[0])
-				})
-			}
+			cStatuses := kubelet.convertToAPIContainerStatuses(tPod, podStatus, []v1.ContainerStatus{tc.OldStatus}, tPod.Spec.Containers, false, false)
+			assert.Equal(t, tc.Expected, cStatuses[0])
 		})
 	}
 }

pkg/registry/core/pod/strategy_test.go

@@ -3092,7 +3092,6 @@ func TestPodResizePrepareForUpdate(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, true)
-			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScalingAllocatedStatus, true)
 			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, true)
 			ctx := context.Background()
 			ResizeStrategy.PrepareForUpdate(ctx, tc.newPod, tc.oldPod)

test/compatibility_lifecycle/reference/versioned_feature_list.yaml

@@ -581,6 +581,10 @@
     lockToDefault: false
     preRelease: Alpha
     version: "1.32"
+  - default: false
+    lockToDefault: false
+    preRelease: Deprecated
+    version: "1.33"
 - name: InPlacePodVerticalScalingExclusiveCPUs
   versionedSpecs:
   - default: false