Skip to content

Commit 8a8dc27

Browse files
committed
Adding the logic to validate pod-level resources as following:
1. The effective container requests cannot be greater than pod-level requests 2. Inidividual container limits cannot be greater than pod-level limits 3. Only CPU & Memory are supported at pod-level 4. Inplace container resources updates are not supported if pod-level resources are set Note: effective container requests cannot be greater than pod-level limits is supported by transitivity. Effective container requests <= pod-level requests && pod-level requests <= pod-level limits; Therefore effective container requests <= pod-level limits Signed-off-by: ndixita <[email protected]>
1 parent a2ddde8 commit 8a8dc27

File tree

5 files changed

+651
-16
lines changed

5 files changed

+651
-16
lines changed

pkg/api/pod/testing/make.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ func SetResourceVersion(rv string) Tweak {
7272
}
7373
}
7474

75+
func SetPodResources(resources *api.ResourceRequirements) Tweak {
76+
return func(pod *api.Pod) {
77+
pod.Spec.Resources = resources
78+
}
79+
}
80+
7581
func SetContainers(containers ...api.Container) Tweak {
7682
return func(pod *api.Pod) {
7783
pod.Spec.Containers = containers

pkg/api/pod/util.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
384384
AllowNamespacedSysctlsForHostNetAndHostIPC: false,
385385
AllowNonLocalProjectedTokenPath: false,
386386
AllowPodLifecycleSleepActionZeroValue: utilfeature.DefaultFeatureGate.Enabled(features.PodLifecycleSleepActionAllowZero),
387+
PodLevelResourcesEnabled: utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources),
387388
}
388389

389390
// If old spec uses relaxed validation or enabled the RelaxedEnvironmentVariableValidation feature gate,

pkg/apis/core/validation/validation.go

Lines changed: 119 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import (
4747
"k8s.io/apimachinery/pkg/util/validation/field"
4848
utilfeature "k8s.io/apiserver/pkg/util/feature"
4949
utilsysctl "k8s.io/component-helpers/node/util/sysctl"
50+
resourcehelper "k8s.io/component-helpers/resource"
5051
schedulinghelper "k8s.io/component-helpers/scheduling/corev1"
5152
kubeletapis "k8s.io/kubelet/pkg/apis"
5253

@@ -333,7 +334,7 @@ func ValidateRuntimeClassName(name string, fldPath *field.Path) field.ErrorList
333334
// validateOverhead can be used to check whether the given Overhead is valid.
334335
func validateOverhead(overhead core.ResourceList, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
335336
// reuse the ResourceRequirements validation logic
336-
return ValidateResourceRequirements(&core.ResourceRequirements{Limits: overhead}, nil, fldPath, opts)
337+
return ValidateContainerResourceRequirements(&core.ResourceRequirements{Limits: overhead}, nil, fldPath, opts)
337338
}
338339

339340
// Validates that given value is not negative.
@@ -3589,7 +3590,7 @@ func validateContainerCommon(ctr *core.Container, volumes map[string]core.Volume
35893590
allErrs = append(allErrs, ValidateVolumeMounts(ctr.VolumeMounts, volDevices, volumes, ctr, path.Child("volumeMounts"), opts)...)
35903591
allErrs = append(allErrs, ValidateVolumeDevices(ctr.VolumeDevices, volMounts, volumes, path.Child("volumeDevices"))...)
35913592
allErrs = append(allErrs, validatePullPolicy(ctr.ImagePullPolicy, path.Child("imagePullPolicy"))...)
3592-
allErrs = append(allErrs, ValidateResourceRequirements(&ctr.Resources, podClaimNames, path.Child("resources"), opts)...)
3593+
allErrs = append(allErrs, ValidateContainerResourceRequirements(&ctr.Resources, podClaimNames, path.Child("resources"), opts)...)
35933594
allErrs = append(allErrs, validateResizePolicy(ctr.ResizePolicy, path.Child("resizePolicy"), podRestartPolicy)...)
35943595
allErrs = append(allErrs, ValidateSecurityContext(ctr.SecurityContext, path.Child("securityContext"), hostUsers)...)
35953596
return allErrs
@@ -4048,6 +4049,8 @@ type PodValidationOptions struct {
40484049
AllowPodLifecycleSleepActionZeroValue bool
40494050
// Allow only Recursive value of SELinuxChangePolicy.
40504051
AllowOnlyRecursiveSELinuxChangePolicy bool
4052+
// Indicates whether PodLevelResources feature is enabled or disabled.
4053+
PodLevelResourcesEnabled bool
40514054
}
40524055

40534056
// validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set,
@@ -4202,6 +4205,11 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi
42024205
allErrs = append(allErrs, validateContainers(spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("containers"), opts, &spec.RestartPolicy, hostUsers)...)
42034206
allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("initContainers"), opts, &spec.RestartPolicy, hostUsers)...)
42044207
allErrs = append(allErrs, validateEphemeralContainers(spec.EphemeralContainers, spec.Containers, spec.InitContainers, vols, podClaimNames, fldPath.Child("ephemeralContainers"), opts, &spec.RestartPolicy, hostUsers)...)
4208+
4209+
if opts.PodLevelResourcesEnabled {
4210+
allErrs = append(allErrs, validatePodResources(spec, podClaimNames, fldPath.Child("resources"), opts)...)
4211+
}
4212+
42054213
allErrs = append(allErrs, validatePodHostNetworkDeps(spec, fldPath, opts)...)
42064214
allErrs = append(allErrs, validateRestartPolicy(&spec.RestartPolicy, fldPath.Child("restartPolicy"))...)
42074215
allErrs = append(allErrs, validateDNSPolicy(&spec.DNSPolicy, fldPath.Child("dnsPolicy"))...)
@@ -4282,6 +4290,77 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi
42824290
return allErrs
42834291
}
42844292

4293+
func validatePodResources(spec *core.PodSpec, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
4294+
if spec.Resources == nil {
4295+
return nil
4296+
}
4297+
4298+
allErrs := field.ErrorList{}
4299+
4300+
if spec.Resources.Claims != nil {
4301+
allErrs = append(allErrs, field.Forbidden(fldPath.Child("claims"), "claims cannot be set for Resources at pod-level"))
4302+
}
4303+
4304+
// validatePodResourceRequirements checks if resource names and quantities are
4305+
// valid, and requests are less than limits.
4306+
allErrs = append(allErrs, validatePodResourceRequirements(spec.Resources, podClaimNames, fldPath, opts)...)
4307+
allErrs = append(allErrs, validatePodResourceConsistency(spec, fldPath)...)
4308+
return allErrs
4309+
}
4310+
4311+
// validatePodResourceConsistency checks if aggregate container-level requests are
4312+
// less than or equal to pod-level requests, and individual container-level limits
4313+
// are less than or equal to pod-level limits.
4314+
func validatePodResourceConsistency(spec *core.PodSpec, fldPath *field.Path) field.ErrorList {
4315+
allErrs := field.ErrorList{}
4316+
4317+
// Convert the *core.PodSpec to *v1.PodSpec to satisfy the call to
4318+
// resourcehelper.PodRequests method, in the subsequent lines,
4319+
// which requires a *v1.Pod object (containing a *v1.PodSpec).
4320+
v1PodSpec := &v1.PodSpec{}
4321+
// TODO(ndixita): Convert_core_PodSpec_To_v1_PodSpec is risky. Add a copy of
4322+
// AggregateContainerRequests against internal core.Pod type for beta release of
4323+
// PodLevelResources feature.
4324+
if err := corev1.Convert_core_PodSpec_To_v1_PodSpec(spec, v1PodSpec, nil); err != nil {
4325+
allErrs = append(allErrs, field.InternalError(fldPath, fmt.Errorf("invalid %q: %v", fldPath, err.Error())))
4326+
}
4327+
4328+
reqPath := fldPath.Child("requests")
4329+
// resourcehelper.AggregateContainerRequests method requires a Pod object to
4330+
// calculate the total requests requirements of a pod. Hence a Pod object using
4331+
// v1PodSpec i.e. (&v1.Pod{Spec: *v1PodSpec}, is created on the fly, and passed
4332+
// to the AggregateContainerRequests method to facilitate proper resource
4333+
// calculation without modifying AggregateContainerRequests method.
4334+
aggrContainerReqs := resourcehelper.AggregateContainerRequests(&v1.Pod{Spec: *v1PodSpec}, resourcehelper.PodResourcesOptions{})
4335+
4336+
// Pod-level requests must be >= aggregate requests of all containers in a pod.
4337+
for resourceName, ctrReqs := range aggrContainerReqs {
4338+
key := resourceName.String()
4339+
podSpecRequests := spec.Resources.Requests[core.ResourceName(key)]
4340+
4341+
fldPath := reqPath.Key(key)
4342+
if ctrReqs.Cmp(podSpecRequests) > 0 {
4343+
allErrs = append(allErrs, field.Invalid(fldPath, podSpecRequests.String(), fmt.Sprintf("must be greater than or equal to aggregate container requests of %s", ctrReqs.String())))
4344+
}
4345+
}
4346+
4347+
// Individual Container limits must be <= Pod-level limits.
4348+
for i, ctr := range spec.Containers {
4349+
for resourceName, ctrLimit := range ctr.Resources.Limits {
4350+
podSpecLimits, exists := spec.Resources.Limits[core.ResourceName(resourceName.String())]
4351+
if !exists {
4352+
continue
4353+
}
4354+
4355+
if ctrLimit.Cmp(podSpecLimits) > 0 {
4356+
fldPath := fldPath.Child("containers").Index(i).Key(resourceName.String()).Child("limits")
4357+
allErrs = append(allErrs, field.Invalid(fldPath, ctrLimit.String(), fmt.Sprintf("must be less than or equal to pod limits of %s", podSpecLimits.String())))
4358+
}
4359+
}
4360+
}
4361+
return allErrs
4362+
}
4363+
42854364
func validateLinux(spec *core.PodSpec, fldPath *field.Path) field.ErrorList {
42864365
allErrs := field.ErrorList{}
42874366
securityContext := spec.SecurityContext
@@ -5486,6 +5565,16 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
54865565
allErrs := ValidateImmutableField(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath)
54875566
allErrs = append(allErrs, validatePodMetadataAndSpec(newPod, opts)...)
54885567

5568+
// pods with pod-level resources cannot be resized
5569+
isPodLevelResourcesSet := func(pod *core.Pod) bool {
5570+
return pod.Spec.Resources != nil &&
5571+
(len(pod.Spec.Resources.Requests)+len(pod.Spec.Resources.Limits) > 0)
5572+
}
5573+
5574+
if isPodLevelResourcesSet(oldPod) || isPodLevelResourcesSet(newPod) {
5575+
return field.ErrorList{field.Forbidden(field.NewPath(""), "pods with pod-level resources cannot be resized")}
5576+
}
5577+
54895578
// static pods cannot be resized.
54905579
if _, ok := oldPod.Annotations[core.MirrorPodAnnotationKey]; ok {
54915580
return field.ErrorList{field.Forbidden(field.NewPath(""), "static pods cannot be resized")}
@@ -6403,6 +6492,22 @@ func validateContainerResourceName(value core.ResourceName, fldPath *field.Path)
64036492
return allErrs
64046493
}
64056494

6495+
// validatePodResourceName verifies that:
6496+
// 1. The resource name is a valid compute resource name for pod-level specification.
6497+
// 2. The resource is supported by the PodLevelResources feature.
6498+
func validatePodResourceName(resourceName core.ResourceName, fldPath *field.Path) field.ErrorList {
6499+
allErrs := validateResourceName(resourceName, fldPath)
6500+
if len(allErrs) != 0 {
6501+
return allErrs
6502+
}
6503+
6504+
if !resourcehelper.IsSupportedPodLevelResource(v1.ResourceName(resourceName)) {
6505+
return append(allErrs, field.NotSupported(fldPath, resourceName, sets.List(resourcehelper.SupportedPodLevelResources())))
6506+
}
6507+
6508+
return allErrs
6509+
}
6510+
64066511
// Validate resource names that can go in a resource quota
64076512
// Refer to docs/design/resources.md for more details.
64086513
func ValidateResourceQuotaResourceName(value core.ResourceName, fldPath *field.Path) field.ErrorList {
@@ -6750,8 +6855,16 @@ func validateBasicResource(quantity resource.Quantity, fldPath *field.Path) fiel
67506855
return field.ErrorList{}
67516856
}
67526857

6858+
func validatePodResourceRequirements(requirements *core.ResourceRequirements, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
6859+
return validateResourceRequirements(requirements, validatePodResourceName, podClaimNames, fldPath, opts)
6860+
}
6861+
6862+
func ValidateContainerResourceRequirements(requirements *core.ResourceRequirements, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
6863+
return validateResourceRequirements(requirements, validateContainerResourceName, podClaimNames, fldPath, opts)
6864+
}
6865+
67536866
// Validates resource requirement spec.
6754-
func ValidateResourceRequirements(requirements *core.ResourceRequirements, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
6867+
func validateResourceRequirements(requirements *core.ResourceRequirements, resourceNameFn func(core.ResourceName, *field.Path) field.ErrorList, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
67556868
allErrs := field.ErrorList{}
67566869
limPath := fldPath.Child("limits")
67576870
reqPath := fldPath.Child("requests")
@@ -6764,7 +6877,7 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, podCl
67646877

67656878
fldPath := limPath.Key(string(resourceName))
67666879
// Validate resource name.
6767-
allErrs = append(allErrs, validateContainerResourceName(resourceName, fldPath)...)
6880+
allErrs = append(allErrs, resourceNameFn(resourceName, fldPath)...)
67686881

67696882
// Validate resource quantity.
67706883
allErrs = append(allErrs, ValidateResourceQuantityValue(resourceName, quantity, fldPath)...)
@@ -6783,7 +6896,8 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, podCl
67836896
for resourceName, quantity := range requirements.Requests {
67846897
fldPath := reqPath.Key(string(resourceName))
67856898
// Validate resource name.
6786-
allErrs = append(allErrs, validateContainerResourceName(resourceName, fldPath)...)
6899+
allErrs = append(allErrs, resourceNameFn(resourceName, fldPath)...)
6900+
67876901
// Validate resource quantity.
67886902
allErrs = append(allErrs, ValidateResourceQuantityValue(resourceName, quantity, fldPath)...)
67896903

0 commit comments

Comments
 (0)