Skip to content

Commit

Permalink
api: reject removing requsets & limits for Burstable pods.
Browse files Browse the repository at this point in the history
  • Loading branch information
@AnishShah
AnishShah committed Nov 7, 2024
1 parent 6e25c2a commit 85ad550
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 11 deletions.
20 changes: 20 additions & 0 deletions pkg/apis/core/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5503,6 +5503,26 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
allErrs = append(allErrs, field.Invalid(specPath, newPod.Status.QOSClass, "Pod QOS Class may not change as a result of resizing"))
}

// TODO(#128675): Do not allow removing resource requests/limits for Burstable Pods.
// TODO: Check for sidecar containers too.
for ix, ctr := range oldPod.Spec.Containers {
resourcesRemoved := func(resourceList, oldResourceList core.ResourceList) bool {
if oldResourceList != nil && resourceList == nil {
return true
}
for name := range oldResourceList {
if _, ok := resourceList[name]; !ok {
return true
}
}

return false
}
if resourcesRemoved(newPod.Spec.Containers[ix].Resources.Requests, ctr.Resources.Requests) || resourcesRemoved(newPod.Spec.Containers[ix].Resources.Limits, ctr.Resources.Limits) {
allErrs = append(allErrs, field.Forbidden(specPath, "resource requests and limits cannot be removed"))
}
}

// Ensure that only CPU and memory resources are mutable.
originalCPUMemPodSpec := *newPod.Spec.DeepCopy()
var newContainers []core.Container
Expand Down
42 changes: 31 additions & 11 deletions pkg/apis/core/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25157,20 +25157,10 @@ func TestValidatePodResize(t *testing.T) {
old: mkPod(getResources("100m", "100Mi", "", ""), core.ResourceList{}),
new: mkPod(getResources("100m", "100Mi", "", ""), getResources("200m", "200Mi", "", "")),
err: "",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove limits",
old: mkPod(getResources("100m", "100Mi", "", ""), getResources("200m", "200Mi", "", "")),
new: mkPod(getResources("100m", "100Mi", "", ""), core.ResourceList{}),
err: "",
}, {
test: "Pod QoS unchanged, burstable -> burstable, add requests",
old: mkPod(core.ResourceList{}, getResources("200m", "500Mi", "1Gi", "")),
new: mkPod(getResources("300m", "", "", ""), getResources("400m", "", "1Gi", "")),
err: "",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove requests",
old: mkPod(getResources("100m", "200Mi", "", ""), getResources("200m", "300Mi", "2Gi", "")),
new: mkPod(core.ResourceList{}, getResources("400m", "500Mi", "2Gi", "")),
new: mkPod(getResources("300m", "", "", ""), getResources("400m", "500Mi", "1Gi", "")),
err: "",
}, {
test: "Pod QoS change, guaranteed -> burstable",
Expand Down Expand Up @@ -25202,6 +25192,36 @@ func TestValidatePodResize(t *testing.T) {
old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetOS(core.Windows)),
new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", ""), podtest.SetOS(core.Windows)),
err: "Forbidden: windows pods cannot be resized",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove cpu limit",
old: mkPod(core.ResourceList{}, getResources("100m", "100Mi", "", "")),
new: mkPod(core.ResourceList{}, getResources("", "100Mi", "", "")),
err: "resource requests and limits cannot be removed",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove memory limit",
old: mkPod(core.ResourceList{}, getResources("100m", "100Mi", "", "")),
new: mkPod(core.ResourceList{}, getResources("100m", "", "", "")),
err: "resource requests and limits cannot be removed",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove cpu request",
old: mkPod(getResources("100m", "100Mi", "", ""), core.ResourceList{}),
new: mkPod(getResources("", "100Mi", "", ""), core.ResourceList{}),
err: "resource requests and limits cannot be removed",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove memory request",
old: mkPod(getResources("100m", "100Mi", "", ""), core.ResourceList{}),
new: mkPod(getResources("100m", "", "", ""), core.ResourceList{}),
err: "resource requests and limits cannot be removed",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove cpu and memory limits",
old: mkPod(core.ResourceList{}, getResources("100m", "100Mi", "", "")),
new: mkPod(core.ResourceList{}, getResources("", "", "", "")),
err: "resource requests and limits cannot be removed",
}, {
test: "Pod QoS unchanged, burstable -> burstable, remove cpu and memory requests",
old: mkPod(getResources("100m", "100Mi", "", ""), core.ResourceList{}),
new: mkPod(getResources("", "", "", ""), core.ResourceList{}),
err: "resource requests and limits cannot be removed",
},
}

Expand Down
76 changes: 76 additions & 0 deletions test/e2e/common/node/pod_resize.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,82 @@ func doPodResizeErrorTests(f *framework.Framework) {
},
},
},
{
name: "Burstable QoS pod, one container with cpu & memory requests + limits - remove memory requests",
containers: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
patchString: `{"spec":{"containers":[
{"name":"c1", "resources":{"requests":{"memory":null}}}
]}}`,
patchError: "resource requests and limits cannot be removed",
expected: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
},
{
name: "Burstable QoS pod, one container with cpu & memory requests + limits - remove memory limits",
containers: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
patchString: `{"spec":{"containers":[
{"name":"c1", "resources":{"limits":{"memory":null}}}
]}}`,
patchError: "resource requests and limits cannot be removed",
expected: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
},
{
name: "Burstable QoS pod, one container with cpu & memory requests + limits - remove CPU requests",
containers: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
patchString: `{"spec":{"containers":[
{"name":"c1", "resources":{"requests":{"cpu":null}}}
]}}`,
patchError: "resource requests and limits cannot be removed",
expected: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
},
{
name: "Burstable QoS pod, one container with cpu & memory requests + limits - remove CPU limits",
containers: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
patchString: `{"spec":{"containers":[
{"name":"c1", "resources":{"limits":{"cpu":null}}}
]}}`,
patchError: "resource requests and limits cannot be removed",
expected: []e2epod.ResizableContainerInfo{
{
Name: "c1",
Resources: &e2epod.ContainerResources{CPUReq: "200m", CPULim: "400m", MemReq: "250Mi", MemLim: "500Mi"},
},
},
},
}

timeouts := f.Timeouts
Expand Down

0 comments on commit 85ad550

Please sign in to comment.