Deprecate IPPVSAllocatedStatus: always set allocatedResources with InPlacePodVerticalScaling

This commit is contained in:
Tim Allclair 2025-03-17 20:55:55 -07:00
parent 67bdb110b4
commit aba588cd14
7 changed files with 57 additions and 77 deletions

View File

@ -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 {

View File

@ -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
}
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()))
}
}
})
}
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 !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()))
}
}
})
}
}
})
}
}

View File

@ -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: {

View File

@ -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
}
}

View File

@ -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])
})
}
}

View File

@ -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)

View File

@ -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