github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-29 14:37:00 +00:00
Code Issues Projects Releases Wiki Activity

validate resize request

Browse Source 
We validate resize request by ensuring that pod QoS is unchanged and
only cpu and memory resources and resize policies are mutated.
This commit is contained in:
Anish Shah 2024-10-22 12:20:30 -07:00
parent 8f967c19b3
commit 2bf1f2349c
3 changed files with 369 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
pkg/apis/core/validation/validation.go
View File

@ -5135,16 +5135,6 @@ var updatablePodSpecFields = []string{
"`spec.activeDeadlineSeconds`",
"`spec.tolerations` (only additions to existing tolerations)",
"`spec.terminationGracePeriodSeconds` (allow it to be set to 1 if it was previously negative)",
"`spec.containers[*].resources` (for CPU/memory only)",
}
// TODO(vinaykul,InPlacePodVerticalScaling): Drop this var once InPlacePodVerticalScaling goes GA and featuregate is gone.
var updatablePodSpecFieldsNoResources = []string{
"`spec.containers[*].image`",
"`spec.initContainers[*].image`",
"`spec.activeDeadlineSeconds`",
"`spec.tolerations` (only additions to existing tolerations)",
"`spec.terminationGracePeriodSeconds` (allow it to be set to 1 if it was previously negative)",
}
// ValidatePodUpdate tests to see if the update is legal for an end user to make. newPod is updated with fields
@ -5206,45 +5196,12 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
return allErrs
}
if qos.GetPodQOS(oldPod) != qos.ComputePodQOS(newPod) {
allErrs = append(allErrs, field.Invalid(fldPath, newPod.Status.QOSClass, "Pod QoS is immutable"))
}
// handle updateable fields by munging those fields prior to deep equal comparison.
mungedPodSpec := *newPod.Spec.DeepCopy()
// munge spec.containers[*].image
var newContainers []core.Container
for ix, container := range mungedPodSpec.Containers {
container.Image = oldPod.Spec.Containers[ix].Image // +k8s:verify-mutation:reason=clone
// When the feature-gate is turned off, any new requests attempting to update CPU or memory
// resource values will result in validation failure.
if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) {
// Resources are mutable for CPU & memory only
// - user can now modify Resources to express new desired Resources
mungeCpuMemResources := func(resourceList, oldResourceList core.ResourceList) core.ResourceList {
if oldResourceList == nil {
return nil
}
var mungedResourceList core.ResourceList
if resourceList == nil {
mungedResourceList = make(core.ResourceList)
} else {
mungedResourceList = resourceList.DeepCopy()
}
delete(mungedResourceList, core.ResourceCPU)
delete(mungedResourceList, core.ResourceMemory)
if cpu, found := oldResourceList[core.ResourceCPU]; found {
mungedResourceList[core.ResourceCPU] = cpu
}
if mem, found := oldResourceList[core.ResourceMemory]; found {
mungedResourceList[core.ResourceMemory] = mem
}
return mungedResourceList
}
lim := mungeCpuMemResources(container.Resources.Limits, oldPod.Spec.Containers[ix].Resources.Limits)
req := mungeCpuMemResources(container.Resources.Requests, oldPod.Spec.Containers[ix].Resources.Requests)
container.Resources = core.ResourceRequirements{Limits: lim, Requests: req}
}
newContainers = append(newContainers, container)
}
mungedPodSpec.Containers = newContainers
@ -5321,10 +5278,7 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
// This diff isn't perfect, but it's a helluva lot better an "I'm not going to tell you what the difference is".
// TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff
specDiff := cmp.Diff(oldPod.Spec, mungedPodSpec)
errs := field.Forbidden(specPath, fmt.Sprintf("pod updates may not change fields other than %s\n%v", strings.Join(updatablePodSpecFieldsNoResources, ","), specDiff))
if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) {
errs = field.Forbidden(specPath, fmt.Sprintf("pod updates may not change fields other than %s\n%v", strings.Join(updatablePodSpecFields, ","), specDiff))
}
errs := field.Forbidden(specPath, fmt.Sprintf("pod updates may not change fields other than %s\n%v", strings.Join(updatablePodSpecFields, ","), specDiff))
allErrs = append(allErrs, errs)
}
return allErrs
@ -5531,6 +5485,65 @@ func ValidatePodEphemeralContainersUpdate(newPod, oldPod *core.Pod, opts PodVali
return allErrs
}
func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) field.ErrorList {
// Part 1: Validate newPod's spec and updates to metadata
fldPath := field.NewPath("metadata")
allErrs := ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath)
allErrs = append(allErrs, validatePodMetadataAndSpec(newPod, opts)...)
allErrs = append(allErrs, ValidatePodSpecificAnnotationUpdates(newPod, oldPod, fldPath.Child("annotations"), opts)...)
// static pods cannot be resized.
if _, ok := oldPod.Annotations[core.MirrorPodAnnotationKey]; ok {
return field.ErrorList{field.Forbidden(field.NewPath(""), "static pods cannot be resized")}
}
// Part 2: Validate that the changes between oldPod.Spec.Containers[].Resources and
// newPod.Spec.Containers[].Resources are allowed.
specPath := field.NewPath("spec")
if qos.GetPodQOS(oldPod) != qos.ComputePodQOS(newPod) {
allErrs = append(allErrs, field.Invalid(specPath, newPod.Status.QOSClass, "Pod QoS is immutable"))
}
// Ensure that only CPU and memory resources are mutable.
mungedPodSpec := *newPod.Spec.DeepCopy()
var newContainers []core.Container
for ix, container := range mungedPodSpec.Containers {
mungeCPUMemResources := func(resourceList, oldResourceList core.ResourceList) core.ResourceList {
if oldResourceList == nil {
return nil
}
var mungedResourceList core.ResourceList
if resourceList == nil {
mungedResourceList = make(core.ResourceList)
} else {
mungedResourceList = resourceList.DeepCopy()
}
delete(mungedResourceList, core.ResourceCPU)
delete(mungedResourceList, core.ResourceMemory)
if cpu, found := oldResourceList[core.ResourceCPU]; found {
mungedResourceList[core.ResourceCPU] = cpu
}
if mem, found := oldResourceList[core.ResourceMemory]; found {
mungedResourceList[core.ResourceMemory] = mem
}
return mungedResourceList
}
lim := mungeCPUMemResources(container.Resources.Limits, oldPod.Spec.Containers[ix].Resources.Limits)
req := mungeCPUMemResources(container.Resources.Requests, oldPod.Spec.Containers[ix].Resources.Requests)
container.Resources = core.ResourceRequirements{Limits: lim, Requests: req}
container.ResizePolicy = oldPod.Spec.Containers[ix].ResizePolicy // +k8s:verify-mutation:reason=clone
newContainers = append(newContainers, container)
}
mungedPodSpec.Containers = newContainers
if !apiequality.Semantic.DeepEqual(mungedPodSpec, oldPod.Spec) {
// This likely means that the user has made changes to CPU and Memory resources.
specDiff := cmp.Diff(oldPod.Spec, mungedPodSpec)
errs := field.Forbidden(specPath, fmt.Sprintf("pod resize may not change fields other than cpu and memory\n%v", specDiff))
allErrs = append(allErrs, errs)
}
return allErrs
}
// ValidatePodBinding tests if required fields in the pod binding are legal.
func ValidatePodBinding(binding *core.Binding) field.ErrorList {
allErrs := field.ErrorList{}

322
pkg/apis/core/validation/validation_test.go
View File

@ -12495,7 +12495,7 @@ func TestValidatePodUpdate(t *testing.T) {
Limits: getResources("100m", "0", "1Gi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "cpu limit change",
}, {
new: *podtest.MakePod("pod",
@ -12510,7 +12510,7 @@ func TestValidatePodUpdate(t *testing.T) {
Limits: getResourceLimits("100m", "200Mi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "memory limit change",
}, {
new: *podtest.MakePod("pod",
@ -12540,7 +12540,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("200m", "0"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "cpu request change",
}, {
new: *podtest.MakePod("pod",
@ -12555,7 +12555,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("0", "100Mi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "memory request change",
}, {
new: *podtest.MakePod("pod",
@ -12587,7 +12587,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResources("100m", "100Mi", "1Gi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS unchanged, guaranteed -> guaranteed",
}, {
new: *podtest.MakePod("pod",
@ -12604,7 +12604,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResources("200m", "200Mi", "1Gi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS unchanged, burstable -> burstable",
}, {
new: *podtest.MakePod("pod",
@ -12620,7 +12620,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS unchanged, burstable -> burstable, add limits",
}, {
new: *podtest.MakePod("pod",
@ -12636,7 +12636,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS unchanged, burstable -> burstable, remove limits",
}, {
new: *podtest.MakePod("pod",
@ -12652,7 +12652,7 @@ func TestValidatePodUpdate(t *testing.T) {
Limits: getResources("200m", "500Mi", "1Gi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS unchanged, burstable -> burstable, add requests",
}, {
new: *podtest.MakePod("pod",
@ -12668,7 +12668,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("100m", "200Mi"),
}))),
),
err: "",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS unchanged, burstable -> burstable, remove requests",
}, {
new: *podtest.MakePod("pod",
@ -12685,7 +12685,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "Pod QoS is immutable",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS change, guaranteed -> burstable",
}, {
new: *podtest.MakePod("pod",
@ -12701,7 +12701,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "Pod QoS is immutable",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS change, burstable -> guaranteed",
}, {
new: *podtest.MakePod("pod",
@ -12712,7 +12712,7 @@ func TestValidatePodUpdate(t *testing.T) {
}))),
),
old: *podtest.MakePod("pod"),
err: "Pod QoS is immutable",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS change, besteffort -> burstable",
}, {
new: *podtest.MakePod("pod"),
@ -12723,7 +12723,7 @@ func TestValidatePodUpdate(t *testing.T) {
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "Pod QoS is immutable",
err: "spec: Forbidden: pod updates may not change fields",
test: "Pod QoS change, burstable -> besteffort",
}, {
new: *podtest.MakePod("pod",
@ -25104,3 +25104,297 @@ func TestValidateSELinuxChangePolicy(t *testing.T) {
})
}
}
func TestValidatePodResize(t *testing.T) {
tests := []struct {
new core.Pod
old core.Pod
err string
test string
}{
{
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("200m", "0", "1Gi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("100m", "0", "1Gi"),
}))),
),
err: "",
test: "cpu limit change",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("100m", "100Mi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("100m", "200Mi"),
}))),
),
err: "",
test: "memory limit change",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("100m", "100Mi", "1Gi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("100m", "100Mi", "2Gi"),
}))),
),
err: "spec: Forbidden: pod resize may not change fields other than cpu and memory",
test: "storage limit change",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResourceLimits("100m", "0"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResourceLimits("200m", "0"),
}))),
),
err: "",
test: "cpu request change",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResourceLimits("0", "200Mi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResourceLimits("0", "100Mi"),
}))),
),
err: "",
test: "memory request change",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResources("100m", "0", "2Gi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResources("100m", "0", "1Gi"),
}))),
),
err: "spec: Forbidden: pod resize may not change fields other than cpu and memory",
test: "storage request change",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("200m", "400Mi", "1Gi"),
Requests: getResources("200m", "400Mi", "1Gi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("100m", "100Mi", "1Gi"),
Requests: getResources("100m", "100Mi", "1Gi"),
}))),
),
err: "",
test: "Pod QoS unchanged, guaranteed -> guaranteed",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("200m", "200Mi", "2Gi"),
Requests: getResources("100m", "100Mi", "1Gi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("400m", "400Mi", "2Gi"),
Requests: getResources("200m", "200Mi", "1Gi"),
}))),
),
err: "",
test: "Pod QoS unchanged, burstable -> burstable",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("200m", "200Mi"),
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "",
test: "Pod QoS unchanged, burstable -> burstable, add limits",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("200m", "200Mi"),
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "",
test: "Pod QoS unchanged, burstable -> burstable, remove limits",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("400m", "", "1Gi"),
Requests: getResources("300m", "", "1Gi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("200m", "500Mi", "1Gi"),
}))),
),
err: "",
test: "Pod QoS unchanged, burstable -> burstable, add requests",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("400m", "500Mi", "2Gi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResources("200m", "300Mi", "2Gi"),
Requests: getResourceLimits("100m", "200Mi"),
}))),
),
err: "",
test: "Pod QoS unchanged, burstable -> burstable, remove requests",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("200m", "200Mi"),
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("100m", "100Mi"),
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "Pod QoS is immutable",
test: "Pod QoS change, guaranteed -> burstable",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("100m", "100Mi"),
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "Pod QoS is immutable",
test: "Pod QoS change, burstable -> guaranteed",
}, {
new: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("200m", "200Mi"),
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
old: *podtest.MakePod("pod"),
err: "Pod QoS is immutable",
test: "Pod QoS change, besteffort -> burstable",
}, {
new: *podtest.MakePod("pod"),
old: *podtest.MakePod("pod",
podtest.SetContainers(podtest.MakeContainer("container",
podtest.SetContainerResources(core.ResourceRequirements{
Limits: getResourceLimits("200m", "200Mi"),
Requests: getResourceLimits("100m", "100Mi"),
}))),
),
err: "Pod QoS is immutable",
test: "Pod QoS change, burstable -> besteffort",
},
}
for _, test := range tests {
test.new.ObjectMeta.ResourceVersion = "1"
test.old.ObjectMeta.ResourceVersion = "1"
// set required fields if old and new match and have no opinion on the value
if test.new.Name == "" && test.old.Name == "" {
test.new.Name = "name"
test.old.Name = "name"
}
if test.new.Namespace == "" && test.old.Namespace == "" {
test.new.Namespace = "namespace"
test.old.Namespace = "namespace"
}
if test.new.Spec.Containers == nil && test.old.Spec.Containers == nil {
test.new.Spec.Containers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}
test.old.Spec.Containers = []core.Container{{Name: "autoadded", Image: "image", TerminationMessagePolicy: "File", ImagePullPolicy: "Always"}}
}
if len(test.new.Spec.DNSPolicy) == 0 && len(test.old.Spec.DNSPolicy) == 0 {
test.new.Spec.DNSPolicy = core.DNSClusterFirst
test.old.Spec.DNSPolicy = core.DNSClusterFirst
}
if len(test.new.Spec.RestartPolicy) == 0 && len(test.old.Spec.RestartPolicy) == 0 {
test.new.Spec.RestartPolicy = "Always"
test.old.Spec.RestartPolicy = "Always"
}
errs := ValidatePodResize(&test.new, &test.old, PodValidationOptions{})
if test.err == "" {
if len(errs) != 0 {
t.Errorf("unexpected invalid: %s (%+v)\nA: %+v\nB: %+v", test.test, errs, test.new, test.old)
}
} else {
if len(errs) == 0 {
t.Errorf("unexpected valid: %s\nA: %+v\nB: %+v", test.test, test.new, test.old)
} else if actualErr := errs.ToAggregate().Error(); !strings.Contains(actualErr, test.err) {
t.Errorf("unexpected error message: %s\nExpected error: %s\nActual error: %s", test.test, test.err, actualErr)
}
}
}
}

2
pkg/registry/core/pod/strategy.go
View File

@ -321,7 +321,7 @@ func (podResizeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Ob
oldPod := old.(*api.Pod)
opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&newPod.Spec, &oldPod.Spec, &newPod.ObjectMeta, &oldPod.ObjectMeta)
opts.ResourceIsPod = true
return nil
return corevalidation.ValidatePodResize(newPod, oldPod, opts)
}
// WarningsOnUpdate returns warnings for the given update.