diff --git a/pkg/api/pod/testing/make.go b/pkg/api/pod/testing/make.go index b73de5d1a11..71c29614a70 100644 --- a/pkg/api/pod/testing/make.go +++ b/pkg/api/pod/testing/make.go @@ -72,6 +72,12 @@ func SetResourceVersion(rv string) Tweak { } } +func SetPodResources(resources *api.ResourceRequirements) Tweak { + return func(pod *api.Pod) { + pod.Spec.Resources = resources + } +} + func SetContainers(containers ...api.Container) Tweak { return func(pod *api.Pod) { pod.Spec.Containers = containers diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index b472d00fe10..152a4ebe37f 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -384,6 +384,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po AllowNamespacedSysctlsForHostNetAndHostIPC: false, AllowNonLocalProjectedTokenPath: false, AllowPodLifecycleSleepActionZeroValue: utilfeature.DefaultFeatureGate.Enabled(features.PodLifecycleSleepActionAllowZero), + PodLevelResourcesEnabled: utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources), } // If old spec uses relaxed validation or enabled the RelaxedEnvironmentVariableValidation feature gate, diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 9e2868f683e..da98ab06f0d 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -47,6 +47,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" utilsysctl "k8s.io/component-helpers/node/util/sysctl" + resourcehelper "k8s.io/component-helpers/resource" schedulinghelper "k8s.io/component-helpers/scheduling/corev1" kubeletapis "k8s.io/kubelet/pkg/apis" @@ -333,7 +334,7 @@ func ValidateRuntimeClassName(name string, fldPath *field.Path) field.ErrorList // validateOverhead can be used to check whether the given Overhead is valid. func validateOverhead(overhead core.ResourceList, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { // reuse the ResourceRequirements validation logic - return ValidateResourceRequirements(&core.ResourceRequirements{Limits: overhead}, nil, fldPath, opts) + return ValidateContainerResourceRequirements(&core.ResourceRequirements{Limits: overhead}, nil, fldPath, opts) } // Validates that given value is not negative. @@ -3589,7 +3590,7 @@ func validateContainerCommon(ctr *core.Container, volumes map[string]core.Volume allErrs = append(allErrs, ValidateVolumeMounts(ctr.VolumeMounts, volDevices, volumes, ctr, path.Child("volumeMounts"), opts)...) allErrs = append(allErrs, ValidateVolumeDevices(ctr.VolumeDevices, volMounts, volumes, path.Child("volumeDevices"))...) allErrs = append(allErrs, validatePullPolicy(ctr.ImagePullPolicy, path.Child("imagePullPolicy"))...) - allErrs = append(allErrs, ValidateResourceRequirements(&ctr.Resources, podClaimNames, path.Child("resources"), opts)...) + allErrs = append(allErrs, ValidateContainerResourceRequirements(&ctr.Resources, podClaimNames, path.Child("resources"), opts)...) allErrs = append(allErrs, validateResizePolicy(ctr.ResizePolicy, path.Child("resizePolicy"), podRestartPolicy)...) allErrs = append(allErrs, ValidateSecurityContext(ctr.SecurityContext, path.Child("securityContext"), hostUsers)...) return allErrs @@ -4048,6 +4049,8 @@ type PodValidationOptions struct { AllowPodLifecycleSleepActionZeroValue bool // Allow only Recursive value of SELinuxChangePolicy. AllowOnlyRecursiveSELinuxChangePolicy bool + // Indicates whether PodLevelResources feature is enabled or disabled. + PodLevelResourcesEnabled bool } // 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 allErrs = append(allErrs, validateContainers(spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("containers"), opts, &spec.RestartPolicy, hostUsers)...) allErrs = append(allErrs, validateInitContainers(spec.InitContainers, spec.Containers, vols, podClaimNames, gracePeriod, fldPath.Child("initContainers"), opts, &spec.RestartPolicy, hostUsers)...) allErrs = append(allErrs, validateEphemeralContainers(spec.EphemeralContainers, spec.Containers, spec.InitContainers, vols, podClaimNames, fldPath.Child("ephemeralContainers"), opts, &spec.RestartPolicy, hostUsers)...) + + if opts.PodLevelResourcesEnabled { + allErrs = append(allErrs, validatePodResources(spec, podClaimNames, fldPath.Child("resources"), opts)...) + } + allErrs = append(allErrs, validatePodHostNetworkDeps(spec, fldPath, opts)...) allErrs = append(allErrs, validateRestartPolicy(&spec.RestartPolicy, fldPath.Child("restartPolicy"))...) allErrs = append(allErrs, validateDNSPolicy(&spec.DNSPolicy, fldPath.Child("dnsPolicy"))...) @@ -4282,6 +4290,77 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi return allErrs } +func validatePodResources(spec *core.PodSpec, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList { + if spec.Resources == nil { + return nil + } + + allErrs := field.ErrorList{} + + if spec.Resources.Claims != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("claims"), "claims cannot be set for Resources at pod-level")) + } + + // validatePodResourceRequirements checks if resource names and quantities are + // valid, and requests are less than limits. + allErrs = append(allErrs, validatePodResourceRequirements(spec.Resources, podClaimNames, fldPath, opts)...) + allErrs = append(allErrs, validatePodResourceConsistency(spec, fldPath)...) + return allErrs +} + +// validatePodResourceConsistency checks if aggregate container-level requests are +// less than or equal to pod-level requests, and individual container-level limits +// are less than or equal to pod-level limits. +func validatePodResourceConsistency(spec *core.PodSpec, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + // Convert the *core.PodSpec to *v1.PodSpec to satisfy the call to + // resourcehelper.PodRequests method, in the subsequent lines, + // which requires a *v1.Pod object (containing a *v1.PodSpec). + v1PodSpec := &v1.PodSpec{} + // TODO(ndixita): Convert_core_PodSpec_To_v1_PodSpec is risky. Add a copy of + // AggregateContainerRequests against internal core.Pod type for beta release of + // PodLevelResources feature. + if err := corev1.Convert_core_PodSpec_To_v1_PodSpec(spec, v1PodSpec, nil); err != nil { + allErrs = append(allErrs, field.InternalError(fldPath, fmt.Errorf("invalid %q: %v", fldPath, err.Error()))) + } + + reqPath := fldPath.Child("requests") + // resourcehelper.AggregateContainerRequests method requires a Pod object to + // calculate the total requests requirements of a pod. Hence a Pod object using + // v1PodSpec i.e. (&v1.Pod{Spec: *v1PodSpec}, is created on the fly, and passed + // to the AggregateContainerRequests method to facilitate proper resource + // calculation without modifying AggregateContainerRequests method. + aggrContainerReqs := resourcehelper.AggregateContainerRequests(&v1.Pod{Spec: *v1PodSpec}, resourcehelper.PodResourcesOptions{}) + + // Pod-level requests must be >= aggregate requests of all containers in a pod. + for resourceName, ctrReqs := range aggrContainerReqs { + key := resourceName.String() + podSpecRequests := spec.Resources.Requests[core.ResourceName(key)] + + fldPath := reqPath.Key(key) + if ctrReqs.Cmp(podSpecRequests) > 0 { + allErrs = append(allErrs, field.Invalid(fldPath, podSpecRequests.String(), fmt.Sprintf("must be greater than or equal to aggregate container requests of %s", ctrReqs.String()))) + } + } + + // Individual Container limits must be <= Pod-level limits. + for i, ctr := range spec.Containers { + for resourceName, ctrLimit := range ctr.Resources.Limits { + podSpecLimits, exists := spec.Resources.Limits[core.ResourceName(resourceName.String())] + if !exists { + continue + } + + if ctrLimit.Cmp(podSpecLimits) > 0 { + fldPath := fldPath.Child("containers").Index(i).Key(resourceName.String()).Child("limits") + allErrs = append(allErrs, field.Invalid(fldPath, ctrLimit.String(), fmt.Sprintf("must be less than or equal to pod limits of %s", podSpecLimits.String()))) + } + } + } + return allErrs +} + func validateLinux(spec *core.PodSpec, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} securityContext := spec.SecurityContext @@ -5486,6 +5565,16 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel allErrs := ValidateImmutableField(&newPod.ObjectMeta, &oldPod.ObjectMeta, fldPath) allErrs = append(allErrs, validatePodMetadataAndSpec(newPod, opts)...) + // pods with pod-level resources cannot be resized + isPodLevelResourcesSet := func(pod *core.Pod) bool { + return pod.Spec.Resources != nil && + (len(pod.Spec.Resources.Requests)+len(pod.Spec.Resources.Limits) > 0) + } + + if isPodLevelResourcesSet(oldPod) || isPodLevelResourcesSet(newPod) { + return field.ErrorList{field.Forbidden(field.NewPath(""), "pods with pod-level resources cannot be resized")} + } + // static pods cannot be resized. if _, ok := oldPod.Annotations[core.MirrorPodAnnotationKey]; ok { return field.ErrorList{field.Forbidden(field.NewPath(""), "static pods cannot be resized")} @@ -6403,6 +6492,22 @@ func validateContainerResourceName(value core.ResourceName, fldPath *field.Path) return allErrs } +// validatePodResourceName verifies that: +// 1. The resource name is a valid compute resource name for pod-level specification. +// 2. The resource is supported by the PodLevelResources feature. +func validatePodResourceName(resourceName core.ResourceName, fldPath *field.Path) field.ErrorList { + allErrs := validateResourceName(resourceName, fldPath) + if len(allErrs) != 0 { + return allErrs + } + + if !resourcehelper.IsSupportedPodLevelResource(v1.ResourceName(resourceName)) { + return append(allErrs, field.NotSupported(fldPath, resourceName, sets.List(resourcehelper.SupportedPodLevelResources()))) + } + + return allErrs +} + // Validate resource names that can go in a resource quota // Refer to docs/design/resources.md for more details. func ValidateResourceQuotaResourceName(value core.ResourceName, fldPath *field.Path) field.ErrorList { @@ -6750,8 +6855,16 @@ func validateBasicResource(quantity resource.Quantity, fldPath *field.Path) fiel return field.ErrorList{} } +func validatePodResourceRequirements(requirements *core.ResourceRequirements, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList { + return validateResourceRequirements(requirements, validatePodResourceName, podClaimNames, fldPath, opts) +} + +func ValidateContainerResourceRequirements(requirements *core.ResourceRequirements, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList { + return validateResourceRequirements(requirements, validateContainerResourceName, podClaimNames, fldPath, opts) +} + // Validates resource requirement spec. -func ValidateResourceRequirements(requirements *core.ResourceRequirements, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList { +func validateResourceRequirements(requirements *core.ResourceRequirements, resourceNameFn func(core.ResourceName, *field.Path) field.ErrorList, podClaimNames sets.Set[string], fldPath *field.Path, opts PodValidationOptions) field.ErrorList { allErrs := field.ErrorList{} limPath := fldPath.Child("limits") reqPath := fldPath.Child("requests") @@ -6764,7 +6877,7 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, podCl fldPath := limPath.Key(string(resourceName)) // Validate resource name. - allErrs = append(allErrs, validateContainerResourceName(resourceName, fldPath)...) + allErrs = append(allErrs, resourceNameFn(resourceName, fldPath)...) // Validate resource quantity. allErrs = append(allErrs, ValidateResourceQuantityValue(resourceName, quantity, fldPath)...) @@ -6783,7 +6896,8 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, podCl for resourceName, quantity := range requirements.Requests { fldPath := reqPath.Key(string(resourceName)) // Validate resource name. - allErrs = append(allErrs, validateContainerResourceName(resourceName, fldPath)...) + allErrs = append(allErrs, resourceNameFn(resourceName, fldPath)...) + // Validate resource quantity. allErrs = append(allErrs, ValidateResourceQuantityValue(resourceName, quantity, fldPath)...) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 23d500e48fe..b1890997e18 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -5758,7 +5758,7 @@ func TestAlphaLocalStorageCapacityIsolation(t *testing.T) { resource.BinarySI), }, } - if errs := ValidateResourceRequirements(&containerLimitCase, nil, field.NewPath("resources"), PodValidationOptions{}); len(errs) != 0 { + if errs := ValidateContainerResourceRequirements(&containerLimitCase, nil, field.NewPath("resources"), PodValidationOptions{}); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } @@ -12299,7 +12299,6 @@ func TestValidatePodCreateWithSchedulingGates(t *testing.T) { } func TestValidatePodUpdate(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, true) var ( activeDeadlineSecondsZero = int64(0) activeDeadlineSecondsNegative = int64(-30) @@ -13670,7 +13669,41 @@ func TestValidatePodUpdate(t *testing.T) { err: "pod updates may not change fields other than", test: "the podAntiAffinity cannot be updated on gated pods", }, + { + new: *podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("200m", "0", "1Gi", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + old: *podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("100m", "0", "1Gi", ""), + }))), + ), + err: "pod updates may not change fields other than", + test: "cpu limit change with pod-level resources", + }, { + new: *podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("100m", "100Mi", "", ""), + }))), + ), + old: *podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("100m", "200Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + err: "pod updates may not change fields other than", + test: "memory limit change with pod-level resources", + }, } + for _, test := range tests { test.new.ObjectMeta.ResourceVersion = "1" test.old.ObjectMeta.ResourceVersion = "1" @@ -18643,6 +18676,260 @@ func TestValidateServiceUpdate(t *testing.T) { } } +func TestValidatePodResourceConsistency(t *testing.T) { + path := field.NewPath("resources") + tests := []struct { + name string + podResources core.ResourceRequirements + containers []core.Container + expectedErrors []string + }{{ + name: "aggregate container requests less than pod requests", + podResources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + core.ResourceMemory: resource.MustParse("10Mi"), + }, + }, + containers: []core.Container{ + { + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("5"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, { + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("4"), + core.ResourceMemory: resource.MustParse("3Mi"), + }, + }, + }, + }, + }, { + name: "aggregate container requests equal to pod requests", + podResources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + core.ResourceMemory: resource.MustParse("10Mi"), + }, + }, + containers: []core.Container{ + { + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("5"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, { + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("5"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, + }, + }, { + name: "aggregate container requests greater than pod requests", + podResources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + core.ResourceMemory: resource.MustParse("10Mi"), + }, + }, + containers: []core.Container{ + { + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("6"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, { + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("8"), + core.ResourceMemory: resource.MustParse("3Mi"), + }, + }, + }, + }, + expectedErrors: []string{"must be greater than or equal to aggregate container requests"}, + }, { + name: "aggregate container limits less than pod limits", + podResources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + core.ResourceMemory: resource.MustParse("10Mi"), + }, + }, + containers: []core.Container{ + { + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("5"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, { + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("4"), + core.ResourceMemory: resource.MustParse("3Mi"), + }, + }, + }, + }, + }, { + name: "aggregate container limits equal to pod limits", + podResources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + core.ResourceMemory: resource.MustParse("10Mi"), + }, + }, + containers: []core.Container{ + { + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("5"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, { + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("5"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, + }, + }, { + name: "aggregate container limits greater than pod limits", + podResources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + core.ResourceMemory: resource.MustParse("10Mi"), + }, + }, + containers: []core.Container{ + { + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("5"), + core.ResourceMemory: resource.MustParse("5Mi"), + }, + }, + }, { + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("6"), + core.ResourceMemory: resource.MustParse("9Mi"), + }, + }, + }, + }, + }, { + name: "indivdual container limits greater than pod limits", + podResources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + core.ResourceMemory: resource.MustParse("10Mi"), + }, + }, + containers: []core.Container{ + { + Resources: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("11"), + core.ResourceMemory: resource.MustParse("12Mi"), + }, + }, + }, + }, + expectedErrors: []string{ + "must be less than or equal to pod limits", + "must be less than or equal to pod limits", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + spec := core.PodSpec{ + Resources: &tc.podResources, + Containers: tc.containers, + } + errs := validatePodResourceConsistency(&spec, path) + if len(errs) != len(tc.expectedErrors) { + t.Errorf("expected %d errors, got %d errors, got errors: %v", len(tc.expectedErrors), len(errs), errs) + } + + for _, expectedErr := range tc.expectedErrors { + expectedErrExists := false + for _, gotErr := range errs { + if strings.Contains(gotErr.Error(), expectedErr) { + expectedErrExists = true + break + } + } + + if !expectedErrExists { + t.Errorf("expected: %v, got errors: %v", expectedErr, errs) + } + } + }) + } +} + +func TestValidatePodResourceNames(t *testing.T) { + table := []struct { + input core.ResourceName + expectedFailure bool + }{ + {"memory", false}, + {"cpu", false}, + {"storage", true}, + {"requests.cpu", true}, + {"requests.memory", true}, + {"requests.storage", true}, + {"limits.cpu", true}, + {"limits.memory", true}, + {"limits.storage", true}, + {"network", true}, + {"disk", true}, + {"", true}, + {".", true}, + {"..", true}, + {"my.favorite.app.co/12345", true}, + {"my.favorite.app.co/_12345", true}, + {"my.favorite.app.co/12345_", true}, + {"kubernetes.io/..", true}, + {core.ResourceName("kubernetes.io/" + strings.Repeat("a", 64)), true}, + {core.ResourceName("kubernetes.io/" + strings.Repeat("a", 64)), true}, + {core.ResourceName("kubernetes.io/" + core.ResourceCPU), true}, + {core.ResourceName("kubernetes.io/" + core.ResourceMemory), true}, + {"kubernetes.io//", true}, + {"kubernetes.io", true}, + {"kubernetes.io/will/not/work/", true}, + } + for _, item := range table { + errs := validatePodResourceName(item.input, field.NewPath("field")) + if len(errs) != 0 && !item.expectedFailure { + t.Errorf("expected no failure for input %q, got: %v", item.input, errs) + } + + if len(errs) == 0 && item.expectedFailure { + t.Errorf("expected failure for input %q", item.input) + } + } +} + func TestValidateResourceNames(t *testing.T) { table := []struct { input core.ResourceName @@ -23083,10 +23370,14 @@ func TestValidatePodTemplateSpecSeccomp(t *testing.T) { func TestValidateResourceRequirements(t *testing.T) { path := field.NewPath("resources") + // TODO(ndixita): refactor the tests to check the expected errors are equal to + // got errors. tests := []struct { name string requirements core.ResourceRequirements - opts PodValidationOptions + validateFn func(requirements *core.ResourceRequirements, + podClaimNames sets.Set[string], fldPath *field.Path, + opts PodValidationOptions) field.ErrorList }{{ name: "limits and requests of hugepage resource are equal", requirements: core.ResourceRequirements{ @@ -23099,7 +23390,7 @@ func TestValidateResourceRequirements(t *testing.T) { core.ResourceName(core.ResourceHugePagesPrefix + "2Mi"): resource.MustParse("2Mi"), }, }, - opts: PodValidationOptions{}, + validateFn: ValidateContainerResourceRequirements, }, { name: "limits and requests of memory resource are equal", requirements: core.ResourceRequirements{ @@ -23110,7 +23401,7 @@ func TestValidateResourceRequirements(t *testing.T) { core.ResourceMemory: resource.MustParse("2Mi"), }, }, - opts: PodValidationOptions{}, + validateFn: ValidateContainerResourceRequirements, }, { name: "limits and requests of cpu resource are equal", requirements: core.ResourceRequirements{ @@ -23121,13 +23412,36 @@ func TestValidateResourceRequirements(t *testing.T) { core.ResourceCPU: resource.MustParse("10"), }, }, - opts: PodValidationOptions{}, + validateFn: ValidateContainerResourceRequirements, }, + { + name: "limits and requests of memory resource are equal", + requirements: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceMemory: resource.MustParse("2Mi"), + }, + Requests: core.ResourceList{ + core.ResourceMemory: resource.MustParse("2Mi"), + }, + }, + validateFn: validatePodResourceRequirements, + }, { + name: "limits and requests of cpu resource are equal", + requirements: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + }, + Requests: core.ResourceList{ + core.ResourceCPU: resource.MustParse("10"), + }, + }, + validateFn: validatePodResourceRequirements, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - if errs := ValidateResourceRequirements(&tc.requirements, nil, path, tc.opts); len(errs) != 0 { + if errs := tc.validateFn(&tc.requirements, nil, path, PodValidationOptions{}); len(errs) != 0 { t.Errorf("unexpected errors: %v", errs) } }) @@ -23136,7 +23450,9 @@ func TestValidateResourceRequirements(t *testing.T) { errTests := []struct { name string requirements core.ResourceRequirements - opts PodValidationOptions + validateFn func(requirements *core.ResourceRequirements, + podClaimNames sets.Set[string], fldPath *field.Path, + opts PodValidationOptions) field.ErrorList }{{ name: "hugepage resource without cpu or memory", requirements: core.ResourceRequirements{ @@ -23147,13 +23463,69 @@ func TestValidateResourceRequirements(t *testing.T) { core.ResourceName(core.ResourceHugePagesPrefix + "2Mi"): resource.MustParse("2Mi"), }, }, - opts: PodValidationOptions{}, + validateFn: ValidateContainerResourceRequirements, + }, { + name: "pod resource with hugepages", + requirements: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceName(core.ResourceHugePagesPrefix + "2Mi"): resource.MustParse("2Mi"), + }, + Requests: core.ResourceList{ + core.ResourceName(core.ResourceHugePagesPrefix + "2Mi"): resource.MustParse("2Mi"), + }, + }, + validateFn: validatePodResourceRequirements, + }, { + name: "pod resource with ephemeral-storage", + requirements: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceName(core.ResourceEphemeralStorage): resource.MustParse("2Mi"), + }, + Requests: core.ResourceList{ + core.ResourceName(core.ResourceEphemeralStorage + "2Mi"): resource.MustParse("2Mi"), + }, + }, + validateFn: validatePodResourceRequirements, + }, { + name: "pod resource with unsupported prefixed resources", + requirements: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceName("kubernetesio/" + core.ResourceCPU): resource.MustParse("2"), + }, + Requests: core.ResourceList{ + core.ResourceName("kubernetesio/" + core.ResourceMemory): resource.MustParse("2"), + }, + }, + validateFn: validatePodResourceRequirements, + }, { + name: "pod resource with unsupported native resources", + requirements: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceName("kubernetes.io/" + strings.Repeat("a", 63)): resource.MustParse("2"), + }, + Requests: core.ResourceList{ + core.ResourceName("kubernetes.io/" + strings.Repeat("a", 63)): resource.MustParse("2"), + }, + }, + validateFn: validatePodResourceRequirements, }, + { + name: "pod resource with unsupported empty native resource name", + requirements: core.ResourceRequirements{ + Limits: core.ResourceList{ + core.ResourceName("kubernetes.io/"): resource.MustParse("2"), + }, + Requests: core.ResourceList{ + core.ResourceName("kubernetes.io"): resource.MustParse("2"), + }, + }, + validateFn: validatePodResourceRequirements, + }, } for _, tc := range errTests { t.Run(tc.name, func(t *testing.T) { - if errs := ValidateResourceRequirements(&tc.requirements, nil, path, tc.opts); len(errs) == 0 { + if errs := tc.validateFn(&tc.requirements, nil, path, PodValidationOptions{}); len(errs) == 0 { t.Error("expected errors") } }) @@ -25113,6 +25485,148 @@ func TestValidatePodResize(t *testing.T) { new *core.Pod err string }{ + { + test: "pod-level resources with container cpu limit change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("200m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, { + test: "pod-level resources with container memory limit change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Limits: getResources("100m", "200Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, + { + test: "pod-level resources with container cpu request change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "200Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("200m", "200Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, { + test: "pod-level resources with container memory request change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "200Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, + { + test: "pod-level resources with pod-level memory limit change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("200m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("200m", "100Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, + { + test: "pod-level resources with pod-level memory request change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Requests: getResources("200m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Requests: getResources("200m", "100Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, + { + test: "pod-level resources with pod-level cpu limit change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("200m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Limits: getResources("100m", "200Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, + { + test: "pod-level resources with pod-level cpu request change", + new: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Requests: getResources("100m", "200Mi", "", "")}), + ), + old: podtest.MakePod("pod", + podtest.SetContainers(podtest.MakeContainer("container", + podtest.SetContainerResources(core.ResourceRequirements{ + Requests: getResources("100m", "100Mi", "", ""), + }))), + podtest.SetPodResources(&core.ResourceRequirements{Requests: getResources("200m", "200Mi", "", "")}), + ), + err: "pods with pod-level resources cannot be resized", + }, { test: "cpu limit change", old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", "")), diff --git a/pkg/apis/node/validation/validation.go b/pkg/apis/node/validation/validation.go index 4e0a20aaf1e..d66e82b9382 100644 --- a/pkg/apis/node/validation/validation.go +++ b/pkg/apis/node/validation/validation.go @@ -54,7 +54,7 @@ func ValidateRuntimeClassUpdate(new, old *node.RuntimeClass) field.ErrorList { func validateOverhead(overhead *node.Overhead, fldPath *field.Path) field.ErrorList { // reuse the ResourceRequirements validation logic - return corevalidation.ValidateResourceRequirements(&core.ResourceRequirements{Limits: overhead.PodFixed}, nil, fldPath, + return corevalidation.ValidateContainerResourceRequirements(&core.ResourceRequirements{Limits: overhead.PodFixed}, nil, fldPath, corevalidation.PodValidationOptions{}) }