Skip to content

Commit 1cf4587

Browse files
vivzbansalvivzbansal
committed
Fix build error
1 parent 1eb966c commit 1cf4587

File tree

11 files changed

+97
-54
lines changed

11 files changed

+97
-54
lines changed

‎pkg/api/pod/util.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1314,7 +1314,7 @@ func MarkPodProposedForResize(oldPod, newPod *api.Pod) {
13141314
continue
13151315
}
13161316
newPod.Status.Resize = api.PodResizeStatusProposed
1317-
break
1317+
return
13181318
}
13191319
}
13201320
}

‎pkg/apis/core/v1/defaults.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,20 @@ func setDefaultResizePolicy(obj *v1.Container) {
245245
})
246246
}
247247
}
248-
if _, exists := obj.Resources.Requests[v1.ResourceCPU]; exists {
249-
defaultResizePolicy(v1.ResourceCPU)
248+
if !resizePolicySpecified[v1.ResourceCPU] {
249+
if _, exists := obj.Resources.Requests[v1.ResourceCPU]; exists {
250+
defaultResizePolicy(v1.ResourceCPU)
251+
} else if _, exists := obj.Resources.Limits[v1.ResourceCPU]; exists {
252+
defaultResizePolicy(v1.ResourceCPU)
253+
}
250254
}
251-
if _, exists := obj.Resources.Requests[v1.ResourceMemory]; exists {
252-
defaultResizePolicy(v1.ResourceMemory)
255+
256+
if !resizePolicySpecified[v1.ResourceMemory] {
257+
if _, exists := obj.Resources.Requests[v1.ResourceMemory]; exists {
258+
defaultResizePolicy(v1.ResourceMemory)
259+
} else if _, exists := obj.Resources.Limits[v1.ResourceMemory]; exists {
260+
defaultResizePolicy(v1.ResourceMemory)
261+
}
253262
}
254263
}
255264

‎pkg/apis/core/v1/defaults_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3195,13 +3195,12 @@ func TestSetDefaultResizePolicy(t *testing.T) {
31953195
}
31963196
output := roundTrip(t, runtime.Object(&testPod))
31973197
pod2 := output.(*v1.Pod)
3198-
if !cmp.Equal(pod2.Spec.Containers[0].ResizePolicy, tc.expectedResizePolicy) {
3199-
t.Errorf("expected resize policy %+v, but got %+v for normal containers", tc.expectedResizePolicy, pod2.Spec.Containers[0].ResizePolicy)
3200-
}
32013198
if isSidecarContainer {
32023199
if !cmp.Equal(pod2.Spec.InitContainers[0].ResizePolicy, tc.expectedResizePolicy) {
32033200
t.Errorf("expected resize policy %+v, but got %+v for restartable init containers", tc.expectedResizePolicy, pod2.Spec.InitContainers[0].ResizePolicy)
32043201
}
3202+
} else if !cmp.Equal(pod2.Spec.Containers[0].ResizePolicy, tc.expectedResizePolicy) {
3203+
t.Errorf("expected resize policy %+v, but got %+v for normal containers", tc.expectedResizePolicy, pod2.Spec.Containers[0].ResizePolicy)
32053204
}
32063205
})
32073206
}

‎pkg/apis/core/validation/validation.go‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5289,7 +5289,6 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
52895289
for ix, container := range mungedPodSpec.Containers {
52905290
container.Image = oldPod.Spec.Containers[ix].Image // +k8s:verify-mutation:reason=clone
52915291
newContainers = append(newContainers, container)
5292-
52935292
}
52945293
mungedPodSpec.Containers = newContainers
52955294
// munge spec.initContainers[*].image
@@ -5644,10 +5643,7 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
56445643
originalCPUMemPodSpec := *newPod.Spec.DeepCopy()
56455644
var newContainers []core.Container
56465645
for ix, container := range originalCPUMemPodSpec.Containers {
5647-
lim := dropCPUMemoryUpdates(container.Resources.Limits, oldPod.Spec.Containers[ix].Resources.Limits)
5648-
req := dropCPUMemoryUpdates(container.Resources.Requests, oldPod.Spec.Containers[ix].Resources.Requests)
5649-
container.Resources = core.ResourceRequirements{Limits: lim, Requests: req}
5650-
container.ResizePolicy = oldPod.Spec.Containers[ix].ResizePolicy // +k8s:verify-mutation:reason=clone
5646+
dropCPUMemoryResourcesFromContainer(&container, &oldPod.Spec.Containers[ix])
56515647
newContainers = append(newContainers, container)
56525648
}
56535649
originalCPUMemPodSpec.Containers = newContainers
@@ -5657,10 +5653,7 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
56575653
if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
56585654
for ix, container := range originalCPUMemPodSpec.InitContainers {
56595655
if container.RestartPolicy != nil && *container.RestartPolicy == core.ContainerRestartPolicyAlways { // restartable init container
5660-
lim := dropCPUMemoryUpdates(container.Resources.Limits, oldPod.Spec.InitContainers[ix].Resources.Limits)
5661-
req := dropCPUMemoryUpdates(container.Resources.Requests, oldPod.Spec.InitContainers[ix].Resources.Requests)
5662-
container.Resources = core.ResourceRequirements{Limits: lim, Requests: req}
5663-
container.ResizePolicy = oldPod.Spec.InitContainers[ix].ResizePolicy // +k8s:verify-mutation:reason=clone
5656+
dropCPUMemoryResourcesFromContainer(&container, &oldPod.Spec.InitContainers[ix])
56645657
}
56655658
newInitContainers = append(newInitContainers, container)
56665659
}
@@ -5683,25 +5676,32 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
56835676
return allErrs
56845677
}
56855678

5686-
func dropCPUMemoryUpdates(resourceList, oldResourceList core.ResourceList) core.ResourceList {
5687-
if oldResourceList == nil {
5688-
return nil
5689-
}
5690-
var mungedResourceList core.ResourceList
5691-
if resourceList == nil {
5692-
mungedResourceList = make(core.ResourceList)
5693-
} else {
5694-
mungedResourceList = resourceList.DeepCopy()
5695-
}
5696-
delete(mungedResourceList, core.ResourceCPU)
5697-
delete(mungedResourceList, core.ResourceMemory)
5698-
if cpu, found := oldResourceList[core.ResourceCPU]; found {
5699-
mungedResourceList[core.ResourceCPU] = cpu
5700-
}
5701-
if mem, found := oldResourceList[core.ResourceMemory]; found {
5702-
mungedResourceList[core.ResourceMemory] = mem
5679+
// dropCPUMemoryResourcesFromContainer deletes the cpu and memory resources from the container, and copies them from the old pod container resources if present.
5680+
func dropCPUMemoryResourcesFromContainer(container *core.Container, oldPodSpecContainer *core.Container) {
5681+
dropCPUMemoryUpdates := func(resourceList, oldResourceList core.ResourceList) core.ResourceList {
5682+
if oldResourceList == nil {
5683+
return nil
5684+
}
5685+
var mungedResourceList core.ResourceList
5686+
if resourceList == nil {
5687+
mungedResourceList = make(core.ResourceList)
5688+
} else {
5689+
mungedResourceList = resourceList.DeepCopy()
5690+
}
5691+
delete(mungedResourceList, core.ResourceCPU)
5692+
delete(mungedResourceList, core.ResourceMemory)
5693+
if cpu, found := oldResourceList[core.ResourceCPU]; found {
5694+
mungedResourceList[core.ResourceCPU] = cpu
5695+
}
5696+
if mem, found := oldResourceList[core.ResourceMemory]; found {
5697+
mungedResourceList[core.ResourceMemory] = mem
5698+
}
5699+
return mungedResourceList
57035700
}
5704-
return mungedResourceList
5701+
lim := dropCPUMemoryUpdates(container.Resources.Limits, oldPodSpecContainer.Resources.Limits)
5702+
req := dropCPUMemoryUpdates(container.Resources.Requests, oldPodSpecContainer.Resources.Requests)
5703+
container.Resources = core.ResourceRequirements{Limits: lim, Requests: req}
5704+
container.ResizePolicy = oldPodSpecContainer.ResizePolicy // +k8s:verify-mutation:reason=clone
57055705
}
57065706

57075707
// isPodResizeRequestSupported checks whether the pod is running on a node with InPlacePodVerticalScaling enabled.
@@ -8630,3 +8630,11 @@ func validateImageVolumeSource(imageVolume *core.ImageVolumeSource, fldPath *fie
86308630
allErrs = append(allErrs, validatePullPolicy(imageVolume.PullPolicy, fldPath.Child("pullPolicy"))...)
86318631
return allErrs
86328632
}
8633+
8634+
// isRestartableInitContainer returns true if the container has ContainerRestartPolicyAlways.
8635+
func isRestartableInitContainer(initContainer *core.Container) bool {
8636+
if initContainer == nil || initContainer.RestartPolicy == nil {
8637+
return false
8638+
}
8639+
return *initContainer.RestartPolicy == core.ContainerRestartPolicyAlways
8640+
}

‎pkg/apis/core/validation/validation_test.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26053,12 +26053,6 @@ func TestValidatePodResize(t *testing.T) {
2605326053
test.new.Namespace = "namespace"
2605426054
test.old.Namespace = "namespace"
2605526055
}
26056-
if test.isSideCarCtr {
26057-
if test.new.Spec.InitContainers == nil && test.old.Spec.InitContainers == nil {
26058-
test.new.Spec.InitContainers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}
26059-
test.old.Spec.InitContainers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}
26060-
}
26061-
}
2606226056
if test.new.Spec.Containers == nil && test.old.Spec.Containers == nil {
2606326057
test.new.Spec.Containers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}
2606426058
test.old.Spec.Containers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}

‎pkg/kubelet/apis/podresources/server_v1.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"k8s.io/kubernetes/pkg/kubelet/metrics"
2828

2929
podresourcesv1 "k8s.io/kubelet/pkg/apis/podresources/v1"
30-
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
3130
)
3231

3332
// v1PodResourcesServer implements PodResourcesListerServer

‎pkg/kubelet/kubelet_pods.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1780,6 +1780,7 @@ func allocatedResourcesMatchStatus(allocatedPod *v1.Pod, podStatus *kubecontaine
17801780
return true
17811781
}
17821782

1783+
// allocatedContainerResourcesMatchStatus returns true if the container resources matches with the container statuses resources.
17831784
func allocatedContainerResourcesMatchStatus(allocatedPod *v1.Pod, c *v1.Container, podStatus *kubecontainer.PodStatus) bool {
17841785
if cs := podStatus.FindContainerStatusByName(c.Name); cs != nil {
17851786
if cs.State != kubecontainer.ContainerStateRunning {

‎pkg/kubelet/kuberuntime/kuberuntime_container.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ import (
5151
remote "k8s.io/cri-client/pkg"
5252
kubelettypes "k8s.io/kubelet/pkg/types"
5353
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
54-
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
5554
"k8s.io/kubernetes/pkg/features"
5655
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
5756
"k8s.io/kubernetes/pkg/kubelet/events"
@@ -1072,14 +1071,12 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
10721071
// If the container is previously initialized but its status is not
10731072
// found, it means its last status is removed for some reason.
10741073
// Restart it if it is a restartable init container.
1075-
if isPreviouslyInitialized && podutil.IsRestartableInitContainer(container) {
10761074
if isPreviouslyInitialized && podutil.IsRestartableInitContainer(container) {
10771075
changes.InitContainersToStart = append(changes.InitContainersToStart, i)
10781076
}
10791077
continue
10801078
}
10811079

1082-
if isPreviouslyInitialized && !podutil.IsRestartableInitContainer(container) {
10831080
if isPreviouslyInitialized && !podutil.IsRestartableInitContainer(container) {
10841081
// after initialization, only restartable init containers need to be kept
10851082
// running
@@ -1096,7 +1093,6 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
10961093
changes.InitContainersToStart = append(changes.InitContainersToStart, i)
10971094

10981095
case kubecontainer.ContainerStateRunning:
1099-
if !podutil.IsRestartableInitContainer(container) {
11001096
if !podutil.IsRestartableInitContainer(container) {
11011097
break
11021098
}
@@ -1173,7 +1169,6 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
11731169
// If the init container failed and the restart policy is Never, the pod is terminal.
11741170
// Otherwise, restart the init container.
11751171
case kubecontainer.ContainerStateExited:
1176-
if podutil.IsRestartableInitContainer(container) {
11771172
if podutil.IsRestartableInitContainer(container) {
11781173
changes.InitContainersToStart = append(changes.InitContainersToStart, i)
11791174
} else { // init container
@@ -1197,7 +1192,6 @@ func (m *kubeGenericRuntimeManager) computeInitContainerActions(pod *v1.Pod, pod
11971192
}
11981193

11991194
default: // kubecontainer.ContainerStatusUnknown or other unknown states
1200-
if podutil.IsRestartableInitContainer(container) {
12011195
if podutil.IsRestartableInitContainer(container) {
12021196
// If the restartable init container is in unknown state, restart it.
12031197
changes.ContainersToKill[status.ID] = containerToKillInfo{

‎pkg/kubelet/kuberuntime/kuberuntime_manager.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -750,9 +750,12 @@ func (m *kubeGenericRuntimeManager) doPodResizeAction(pod *v1.Pod, podContainerC
750750
}
751751
}
752752
if len(podContainerChanges.InitContainersToUpdate[rName]) > 0 {
753-
if err = m.updatePodContainersResources(pod, rName, podContainerChanges.InitContainersToUpdate[rName], true); err != nil {
754-
klog.ErrorS(err, "updatePodContainersResources failed for init containers", "pod", format.Pod(pod), "resource", rName)
755-
return err
753+
updateContainerResults, errUpdate := m.updatePodContainerResources(ctx, pod, rName, podContainerChanges.InitContainersToUpdate[rName], true)
754+
for _, containerResult := range updateContainerResults {
755+
result.AddSyncResult(containerResult)
756+
}
757+
if errUpdate != nil {
758+
return errUpdate
756759
}
757760
}
758761

‎pkg/kubelet/kuberuntime/kuberuntime_manager_test.go‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2821,9 +2821,11 @@ func TestUpdatePodRestartableInitContainerResources(t *testing.T) {
28212821
apiSpecResources []v1.ResourceRequirements
28222822
apiStatusResources []v1.ResourceRequirements
28232823
requiresRestart []bool
2824+
injectedError error
28242825
invokeUpdateResources bool
28252826
expectedCurrentLimits []v1.ResourceList
28262827
expectedCurrentRequests []v1.ResourceList
2828+
expectedResults []*kubecontainer.SyncResult
28272829
}{
28282830
"Guaranteed QoS Pod - CPU & memory resize requested, update CPU": {
28292831
resourceName: v1.ResourceCPU,
@@ -2841,6 +2843,20 @@ func TestUpdatePodRestartableInitContainerResources(t *testing.T) {
28412843
invokeUpdateResources: true,
28422844
expectedCurrentLimits: []v1.ResourceList{res150m100Mi, res250m200Mi, res350m300Mi},
28432845
expectedCurrentRequests: []v1.ResourceList{res150m100Mi, res250m200Mi, res350m300Mi},
2846+
expectedResults: []*kubecontainer.SyncResult{
2847+
{
2848+
Action: kubecontainer.UpdateContainerCPU,
2849+
Target: pod.Spec.InitContainers[0].Name,
2850+
},
2851+
{
2852+
Action: kubecontainer.UpdateContainerCPU,
2853+
Target: pod.Spec.InitContainers[1].Name,
2854+
},
2855+
{
2856+
Action: kubecontainer.UpdateContainerCPU,
2857+
Target: pod.Spec.InitContainers[2].Name,
2858+
},
2859+
},
28442860
},
28452861
"Guaranteed QoS Pod - CPU & memory resize requested, update memory": {
28462862
resourceName: v1.ResourceMemory,
@@ -2858,12 +2874,27 @@ func TestUpdatePodRestartableInitContainerResources(t *testing.T) {
28582874
invokeUpdateResources: true,
28592875
expectedCurrentLimits: []v1.ResourceList{res100m150Mi, res200m250Mi, res300m350Mi},
28602876
expectedCurrentRequests: []v1.ResourceList{res100m150Mi, res200m250Mi, res300m350Mi},
2877+
expectedResults: []*kubecontainer.SyncResult{
2878+
{
2879+
Action: kubecontainer.UpdateContainerMemory,
2880+
Target: pod.Spec.InitContainers[0].Name,
2881+
},
2882+
{
2883+
Action: kubecontainer.UpdateContainerMemory,
2884+
Target: pod.Spec.InitContainers[1].Name,
2885+
},
2886+
{
2887+
Action: kubecontainer.UpdateContainerMemory,
2888+
Target: pod.Spec.InitContainers[2].Name,
2889+
},
2890+
},
28612891
},
28622892
} {
28632893
var initContainersToUpdate []containerToUpdateInfo
28642894
for idx := range pod.Spec.InitContainers {
28652895
// default resize policy when pod resize feature is enabled
28662896
pod.Spec.InitContainers[idx].Resources = tc.apiSpecResources[idx]
2897+
pod.Status.ContainerStatuses[idx].Resources = &tc.apiStatusResources[idx]
28672898
cInfo := containerToUpdateInfo{
28682899
apiContainerIdx: idx,
28692900
kubeContainerID: kubecontainer.ContainerID{},
@@ -2883,8 +2914,14 @@ func TestUpdatePodRestartableInitContainerResources(t *testing.T) {
28832914
initContainersToUpdate = append(initContainersToUpdate, cInfo)
28842915
}
28852916
fakeRuntime.Called = []string{}
2886-
err := m.updatePodContainersResources(pod, tc.resourceName, initContainersToUpdate, true)
2887-
require.NoError(t, err, dsc)
2917+
2918+
updateContainerResults, err := m.updatePodContainerResources(context.TODO(), pod, tc.resourceName, initContainersToUpdate, true)
2919+
assert.ElementsMatch(t, tc.expectedResults, updateContainerResults)
2920+
if tc.injectedError == nil {
2921+
require.NoError(t, err, dsc)
2922+
} else {
2923+
require.EqualError(t, err, tc.injectedError.Error(), dsc)
2924+
}
28882925

28892926
if tc.invokeUpdateResources {
28902927
assert.Contains(t, fakeRuntime.Called, "UpdateContainerResources", dsc)

0 commit comments

Comments
 (0)