diff --git a/pkg/apis/core/helper/qos/BUILD b/pkg/apis/core/helper/qos/BUILD index a029fdb5b8f..c2a1e9010b8 100644 --- a/pkg/apis/core/helper/qos/BUILD +++ b/pkg/apis/core/helper/qos/BUILD @@ -11,7 +11,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/apis/core/helper/qos", deps = [ "//pkg/apis/core:go_default_library", - "//pkg/apis/core/helper:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], diff --git a/pkg/apis/core/helper/qos/qos.go b/pkg/apis/core/helper/qos/qos.go index 18414322c82..fad6fb24074 100644 --- a/pkg/apis/core/helper/qos/qos.go +++ b/pkg/apis/core/helper/qos/qos.go @@ -22,12 +22,12 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/apis/core/helper" ) +var supportedQoSComputeResources = sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) + func isSupportedQoSComputeResource(name core.ResourceName) bool { - supportedQoSComputeResources := sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) - return supportedQoSComputeResources.Has(string(name)) || helper.IsHugePageResourceName(name) + return supportedQoSComputeResources.Has(string(name)) } // GetPodQOS returns the QoS class of a pod. diff --git a/pkg/apis/core/v1/helper/qos/BUILD b/pkg/apis/core/v1/helper/qos/BUILD index d29fcf09e93..7dd60de18c5 100644 --- a/pkg/apis/core/v1/helper/qos/BUILD +++ b/pkg/apis/core/v1/helper/qos/BUILD @@ -26,7 +26,7 @@ go_library( srcs = ["qos.go"], importpath = "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos", deps = [ - "//pkg/apis/core/v1/helper:go_default_library", + "//pkg/apis/core:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", diff --git a/pkg/apis/core/v1/helper/qos/qos.go b/pkg/apis/core/v1/helper/qos/qos.go index 5e9dbdd7462..426f054efa6 100644 --- a/pkg/apis/core/v1/helper/qos/qos.go +++ b/pkg/apis/core/v1/helper/qos/qos.go @@ -20,15 +20,16 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" + "k8s.io/kubernetes/pkg/apis/core" ) +var supportedQoSComputeResources = sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) + // QOSList is a set of (resource name, QoS class) pairs. type QOSList map[v1.ResourceName]v1.PodQOSClass func isSupportedQoSComputeResource(name v1.ResourceName) bool { - supportedQoSComputeResources := sets.NewString(string(v1.ResourceCPU), string(v1.ResourceMemory)) - return supportedQoSComputeResources.Has(string(name)) || v1helper.IsHugePageResourceName(name) + return supportedQoSComputeResources.Has(string(name)) } // GetPodQOS returns the QoS class of a pod. diff --git a/pkg/apis/core/v1/helper/qos/qos_test.go b/pkg/apis/core/v1/helper/qos/qos_test.go index 7d14b519e79..0685d4e6559 100644 --- a/pkg/apis/core/v1/helper/qos/qos_test.go +++ b/pkg/apis/core/v1/helper/qos/qos_test.go @@ -131,10 +131,10 @@ func TestGetPodQOS(t *testing.T) { expected: v1.PodQOSBurstable, }, { - pod: newPod("burstable-hugepages", []v1.Container{ - newContainer("burstable", addResource("hugepages-2Mi", "1Gi", getResourceList("0", "0")), addResource("hugepages-2Mi", "1Gi", getResourceList("0", "0"))), + pod: newPod("best-effort-hugepages", []v1.Container{ + newContainer("best-effort", addResource("hugepages-2Mi", "1Gi", getResourceList("0", "0")), addResource("hugepages-2Mi", "1Gi", getResourceList("0", "0"))), }), - expected: v1.PodQOSBurstable, + expected: v1.PodQOSBestEffort, }, } for id, testCase := range testCases { diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index aaccd4d2ca4..8d3110fe40c 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4252,7 +4252,13 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, fldPa allErrs := field.ErrorList{} limPath := fldPath.Child("limits") reqPath := fldPath.Child("requests") + limContainsCpuOrMemory := false + reqContainsCpuOrMemory := false + limContainsHugePages := false + reqContainsHugePages := false + supportedQoSComputeResources := sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) for resourceName, quantity := range requirements.Limits { + fldPath := limPath.Key(string(resourceName)) // Validate resource name. allErrs = append(allErrs, validateContainerResourceName(string(resourceName), fldPath)...) @@ -4263,10 +4269,17 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, fldPa if resourceName == core.ResourceEphemeralStorage && !utilfeature.DefaultFeatureGate.Enabled(features.LocalStorageCapacityIsolation) { allErrs = append(allErrs, field.Forbidden(limPath, "ResourceEphemeralStorage field disabled by feature-gate for ResourceRequirements")) } - if helper.IsHugePageResourceName(resourceName) && !utilfeature.DefaultFeatureGate.Enabled(features.HugePages) { - allErrs = append(allErrs, field.Forbidden(limPath, fmt.Sprintf("%s field disabled by feature-gate for ResourceRequirements", resourceName))) + if helper.IsHugePageResourceName(resourceName) { + if !utilfeature.DefaultFeatureGate.Enabled(features.HugePages) { + allErrs = append(allErrs, field.Forbidden(limPath, fmt.Sprintf("%s field disabled by feature-gate for ResourceRequirements", resourceName))) + } else { + limContainsHugePages = true + } } + if supportedQoSComputeResources.Has(string(resourceName)) { + limContainsCpuOrMemory = true + } } for resourceName, quantity := range requirements.Requests { fldPath := reqPath.Key(string(resourceName)) @@ -4287,6 +4300,16 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, fldPa } else if resourceName == core.ResourceNvidiaGPU { allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be equal to %s request", core.ResourceNvidiaGPU))) } + if helper.IsHugePageResourceName(resourceName) { + reqContainsHugePages = true + } + if supportedQoSComputeResources.Has(string(resourceName)) { + reqContainsCpuOrMemory = true + } + + } + if !limContainsCpuOrMemory && !reqContainsCpuOrMemory && (reqContainsHugePages || limContainsHugePages) { + allErrs = append(allErrs, field.Forbidden(fldPath, fmt.Sprintf("HugePages require cpu or memory"))) } return allErrs diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 5583eb9a857..6ba034b5e13 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -3316,6 +3316,32 @@ func TestAlphaHugePagesIsolation(t *testing.T) { successCases := []core.Pod{ { // Basic fields. ObjectMeta: metav1.ObjectMeta{Name: "123", Namespace: "ns"}, + Spec: core.PodSpec{ + Containers: []core.Container{ + { + Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + }, + Limits: core.ResourceList{ + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + }, + }, + }, + }, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSClusterFirst, + }, + }, + } + failureCases := []core.Pod{ + { // Basic fields. + ObjectMeta: metav1.ObjectMeta{Name: "hugepages-requireCpuOrMemory", Namespace: "ns"}, Spec: core.PodSpec{ Containers: []core.Container{ { @@ -3334,8 +3360,6 @@ func TestAlphaHugePagesIsolation(t *testing.T) { DNSPolicy: core.DNSClusterFirst, }, }, - } - failureCases := []core.Pod{ { // Basic fields. ObjectMeta: metav1.ObjectMeta{Name: "hugepages-shared", Namespace: "ns"}, Spec: core.PodSpec{ @@ -3344,10 +3368,14 @@ func TestAlphaHugePagesIsolation(t *testing.T) { Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", Resources: core.ResourceRequirements{ Requests: core.ResourceList{ - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), }, Limits: core.ResourceList{ - core.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), }, }, }, @@ -3364,12 +3392,15 @@ func TestAlphaHugePagesIsolation(t *testing.T) { Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", Resources: core.ResourceRequirements{ Requests: core.ResourceList{ - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), - core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), }, Limits: core.ResourceList{ - core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), - core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + core.ResourceName(core.ResourceCPU): resource.MustParse("10"), + core.ResourceName(core.ResourceMemory): resource.MustParse("10G"), + core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), }, }, },