From 26f11c458620751733250b35d1f60c9ed2a96e57 Mon Sep 17 00:00:00 2001 From: ndixita Date: Wed, 30 Oct 2024 01:24:36 +0000 Subject: [PATCH] QOS changes for Pod Level resources --- pkg/apis/core/helper/qos/qos.go | 111 +++++++++++------- pkg/apis/core/v1/helper/qos/qos.go | 110 +++++++++++------ pkg/apis/core/v1/helper/qos/qos_test.go | 90 +++++++++++++- .../src/k8s.io/kubectl/pkg/util/qos/qos.go | 105 +++++++++++------ 4 files changed, 294 insertions(+), 122 deletions(-) diff --git a/pkg/apis/core/helper/qos/qos.go b/pkg/apis/core/helper/qos/qos.go index b32fffa0e3f..2b0cdcfff6e 100644 --- a/pkg/apis/core/helper/qos/qos.go +++ b/pkg/apis/core/helper/qos/qos.go @@ -21,7 +21,9 @@ package qos import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" ) var supportedQoSComputeResources = sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) @@ -39,6 +41,45 @@ func GetPodQOS(pod *core.Pod) core.PodQOSClass { return ComputePodQOS(pod) } +// zeroQuantity represents a resource.Quantity with value "0", used as a baseline +// for resource comparisons. +var zeroQuantity = resource.MustParse("0") + +// processResourceList adds non-zero quantities for supported QoS compute resources +// quantities from newList to list. +func processResourceList(list, newList core.ResourceList) { + for name, quantity := range newList { + if !isSupportedQoSComputeResource(name) { + continue + } + if quantity.Cmp(zeroQuantity) == 1 { + delta := quantity.DeepCopy() + if _, exists := list[name]; !exists { + list[name] = delta + } else { + delta.Add(list[name]) + list[name] = delta + } + } + } +} + +// getQOSResources returns a set of resource names from the provided resource list that: +// 1. Are supported QoS compute resources +// 2. Have quantities greater than zero +func getQOSResources(list core.ResourceList) sets.Set[string] { + qosResources := sets.New[string]() + for name, quantity := range list { + if !isSupportedQoSComputeResource(name) { + continue + } + if quantity.Cmp(zeroQuantity) == 1 { + qosResources.Insert(string(name)) + } + } + return qosResources +} + // ComputePodQOS evaluates the list of containers to determine a pod's QoS class. This function is more // expensive than GetPodQOS which should be used for pods having a non-empty .Status.QOSClass. // A pod is besteffort if none of its containers have specified any requests or limits. @@ -48,54 +89,44 @@ func GetPodQOS(pod *core.Pod) core.PodQOSClass { func ComputePodQOS(pod *core.Pod) core.PodQOSClass { requests := core.ResourceList{} limits := core.ResourceList{} - zeroQuantity := resource.MustParse("0") isGuaranteed := true - // note, ephemeral containers are not considered for QoS as they cannot define resources - allContainers := []core.Container{} - allContainers = append(allContainers, pod.Spec.Containers...) - allContainers = append(allContainers, pod.Spec.InitContainers...) - for _, container := range allContainers { - // process requests - for name, quantity := range container.Resources.Requests { - if !isSupportedQoSComputeResource(name) { - continue - } - if quantity.Cmp(zeroQuantity) == 1 { - delta := quantity.DeepCopy() - if _, exists := requests[name]; !exists { - requests[name] = delta - } else { - delta.Add(requests[name]) - requests[name] = delta - } - } - } - // process limits - qosLimitsFound := sets.NewString() - for name, quantity := range container.Resources.Limits { - if !isSupportedQoSComputeResource(name) { - continue - } - if quantity.Cmp(zeroQuantity) == 1 { - qosLimitsFound.Insert(string(name)) - delta := quantity.DeepCopy() - if _, exists := limits[name]; !exists { - limits[name] = delta - } else { - delta.Add(limits[name]) - limits[name] = delta - } - } + // When pod-level resources are specified, we use them to determine QoS class. + if utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources) && + pod.Spec.Resources != nil { + if len(pod.Spec.Resources.Requests) > 0 { + // process requests + processResourceList(requests, pod.Spec.Resources.Requests) } - if !qosLimitsFound.HasAll(string(core.ResourceMemory), string(core.ResourceCPU)) { - isGuaranteed = false + if len(pod.Spec.Resources.Limits) > 0 { + // process limits + processResourceList(limits, pod.Spec.Resources.Limits) + qosLimitResources := getQOSResources(pod.Spec.Resources.Limits) + if !qosLimitResources.HasAll(string(core.ResourceMemory), string(core.ResourceCPU)) { + isGuaranteed = false + } + } + } else { + // note, ephemeral containers are not considered for QoS as they cannot define resources + allContainers := []core.Container{} + allContainers = append(allContainers, pod.Spec.Containers...) + allContainers = append(allContainers, pod.Spec.InitContainers...) + for _, container := range allContainers { + // process requests + processResourceList(requests, container.Resources.Requests) + // process limits + processResourceList(limits, container.Resources.Limits) + qosLimitResources := getQOSResources(container.Resources.Limits) + if !qosLimitResources.HasAll(string(core.ResourceMemory), string(core.ResourceCPU)) { + isGuaranteed = false + } } } + if len(requests) == 0 && len(limits) == 0 { return core.PodQOSBestEffort } - // Check is requests match limits for all resources. + // Check if requests match limits for all resources. if isGuaranteed { for name, req := range requests { if lim, exists := limits[name]; !exists || lim.Cmp(req) != 0 { diff --git a/pkg/apis/core/v1/helper/qos/qos.go b/pkg/apis/core/v1/helper/qos/qos.go index 79e2eb2abd2..66e512b16d0 100644 --- a/pkg/apis/core/v1/helper/qos/qos.go +++ b/pkg/apis/core/v1/helper/qos/qos.go @@ -20,7 +20,9 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" ) var supportedQoSComputeResources = sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) @@ -41,57 +43,89 @@ func GetPodQOS(pod *v1.Pod) v1.PodQOSClass { return ComputePodQOS(pod) } +// zeroQuantity represents a resource.Quantity with value "0", used as a baseline +// for resource comparisons. +var zeroQuantity = resource.MustParse("0") + +// processResourceList adds non-zero quantities for supported QoS compute resources +// quantities from newList to list. +func processResourceList(list, newList v1.ResourceList) { + for name, quantity := range newList { + if !isSupportedQoSComputeResource(name) { + continue + } + if quantity.Cmp(zeroQuantity) == 1 { + delta := quantity.DeepCopy() + if _, exists := list[name]; !exists { + list[name] = delta + } else { + delta.Add(list[name]) + list[name] = delta + } + } + } +} + +// getQOSResources returns a set of resource names from the provided resource list that: +// 1. Are supported QoS compute resources +// 2. Have quantities greater than zero +func getQOSResources(list v1.ResourceList) sets.Set[string] { + qosResources := sets.New[string]() + for name, quantity := range list { + if !isSupportedQoSComputeResource(name) { + continue + } + if quantity.Cmp(zeroQuantity) == 1 { + qosResources.Insert(string(name)) + } + } + return qosResources +} + // ComputePodQOS evaluates the list of containers to determine a pod's QoS class. This function is more // expensive than GetPodQOS which should be used for pods having a non-empty .Status.QOSClass. // A pod is besteffort if none of its containers have specified any requests or limits. // A pod is guaranteed only when requests and limits are specified for all the containers and they are equal. // A pod is burstable if limits and requests do not match across all containers. +// TODO(ndixita): Refactor ComputePodQOS into smaller functions to make it more +// readable and maintainable. func ComputePodQOS(pod *v1.Pod) v1.PodQOSClass { requests := v1.ResourceList{} limits := v1.ResourceList{} - zeroQuantity := resource.MustParse("0") isGuaranteed := true - allContainers := []v1.Container{} - allContainers = append(allContainers, pod.Spec.Containers...) - allContainers = append(allContainers, pod.Spec.InitContainers...) - for _, container := range allContainers { - // process requests - for name, quantity := range container.Resources.Requests { - if !isSupportedQoSComputeResource(name) { - continue - } - if quantity.Cmp(zeroQuantity) == 1 { - delta := quantity.DeepCopy() - if _, exists := requests[name]; !exists { - requests[name] = delta - } else { - delta.Add(requests[name]) - requests[name] = delta - } - } - } - // process limits - qosLimitsFound := sets.NewString() - for name, quantity := range container.Resources.Limits { - if !isSupportedQoSComputeResource(name) { - continue - } - if quantity.Cmp(zeroQuantity) == 1 { - qosLimitsFound.Insert(string(name)) - delta := quantity.DeepCopy() - if _, exists := limits[name]; !exists { - limits[name] = delta - } else { - delta.Add(limits[name]) - limits[name] = delta - } - } + // When pod-level resources are specified, we use them to determine QoS class. + if utilfeature.DefaultFeatureGate.Enabled(features.PodLevelResources) && + pod.Spec.Resources != nil { + if len(pod.Spec.Resources.Requests) > 0 { + // process requests + processResourceList(requests, pod.Spec.Resources.Requests) } - if !qosLimitsFound.HasAll(string(v1.ResourceMemory), string(v1.ResourceCPU)) { - isGuaranteed = false + if len(pod.Spec.Resources.Limits) > 0 { + // process limits + processResourceList(limits, pod.Spec.Resources.Limits) + qosLimitResources := getQOSResources(pod.Spec.Resources.Limits) + if !qosLimitResources.HasAll(string(v1.ResourceMemory), string(v1.ResourceCPU)) { + isGuaranteed = false + } + } + } else { + // note, ephemeral containers are not considered for QoS as they cannot define resources + allContainers := []v1.Container{} + allContainers = append(allContainers, pod.Spec.Containers...) + allContainers = append(allContainers, pod.Spec.InitContainers...) + for _, container := range allContainers { + // process requests + processResourceList(requests, container.Resources.Requests) + // process limits + processResourceList(limits, container.Resources.Limits) + qosLimitResources := getQOSResources(container.Resources.Limits) + if !qosLimitResources.HasAll(string(v1.ResourceMemory), string(v1.ResourceCPU)) { + isGuaranteed = false + } } } + if len(requests) == 0 && len(limits) == 0 { return v1.PodQOSBestEffort } diff --git a/pkg/apis/core/v1/helper/qos/qos_test.go b/pkg/apis/core/v1/helper/qos/qos_test.go index d16c17a14e7..29b9302c9cf 100644 --- a/pkg/apis/core/v1/helper/qos/qos_test.go +++ b/pkg/apis/core/v1/helper/qos/qos_test.go @@ -22,15 +22,19 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper/qos" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" + "k8s.io/kubernetes/pkg/features" ) func TestComputePodQOS(t *testing.T) { testCases := []struct { - pod *v1.Pod - expected v1.PodQOSClass + pod *v1.Pod + expected v1.PodQOSClass + podLevelResourcesEnabled bool }{ { pod: newPod("guaranteed", []v1.Container{ @@ -126,8 +130,76 @@ func TestComputePodQOS(t *testing.T) { }), expected: v1.PodQOSBurstable, }, + { + pod: newPodWithResources( + "guaranteed-with-pod-level-resources", + []v1.Container{ + newContainer("best-effort", getResourceList("", ""), getResourceList("", "")), + }, + getResourceRequirements(getResourceList("10m", "100Mi"), getResourceList("10m", "100Mi")), + ), + expected: v1.PodQOSGuaranteed, + podLevelResourcesEnabled: true, + }, + { + pod: newPodWithResources( + "guaranteed-with-pod-and-container-level-resources", + []v1.Container{ + newContainer("burstable", getResourceList("3m", "10Mi"), getResourceList("5m", "20Mi")), + }, + getResourceRequirements(getResourceList("10m", "100Mi"), getResourceList("10m", "100Mi")), + ), + expected: v1.PodQOSGuaranteed, + podLevelResourcesEnabled: true, + }, + { + pod: newPodWithResources( + "burstable-with-pod-level-resources", + []v1.Container{ + newContainer("best-effort", getResourceList("", ""), getResourceList("", "")), + }, + getResourceRequirements(getResourceList("10m", "10Mi"), getResourceList("20m", "50Mi")), + ), + expected: v1.PodQOSBurstable, + podLevelResourcesEnabled: true, + }, + { + pod: newPodWithResources( + "burstable-with-pod-and-container-level-resources", + []v1.Container{ + newContainer("burstable", getResourceList("5m", "10Mi"), getResourceList("5m", "10Mi")), + }, + getResourceRequirements(getResourceList("10m", "10Mi"), getResourceList("20m", "50Mi")), + ), + expected: v1.PodQOSBurstable, + podLevelResourcesEnabled: true, + }, + { + pod: newPodWithResources( + "burstable-with-pod-and-container-level-requests", + []v1.Container{ + newContainer("burstable", getResourceList("5m", "10Mi"), getResourceList("", "")), + }, + getResourceRequirements(getResourceList("10m", "10Mi"), getResourceList("", "")), + ), + expected: v1.PodQOSBurstable, + podLevelResourcesEnabled: true, + }, + { + pod: newPodWithResources( + "burstable-with-pod-and-container-level-resources-2", + []v1.Container{ + newContainer("burstable", getResourceList("5m", "10Mi"), getResourceList("", "")), + newContainer("guaranteed", getResourceList("5m", "10Mi"), getResourceList("5m", "10Mi")), + }, + getResourceRequirements(getResourceList("10m", "10Mi"), getResourceList("5m", "")), + ), + expected: v1.PodQOSBurstable, + podLevelResourcesEnabled: true, + }, } for id, testCase := range testCases { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodLevelResources, testCase.podLevelResourcesEnabled) if actual := ComputePodQOS(testCase.pod); testCase.expected != actual { t.Errorf("[%d]: invalid qos pod %s, expected: %s, actual: %s", id, testCase.pod.Name, testCase.expected, actual) } @@ -158,17 +230,17 @@ func addResource(rName, value string, rl v1.ResourceList) v1.ResourceList { return rl } -func getResourceRequirements(requests, limits v1.ResourceList) v1.ResourceRequirements { +func getResourceRequirements(requests, limits v1.ResourceList) *v1.ResourceRequirements { res := v1.ResourceRequirements{} res.Requests = requests res.Limits = limits - return res + return &res } func newContainer(name string, requests v1.ResourceList, limits v1.ResourceList) v1.Container { return v1.Container{ Name: name, - Resources: getResourceRequirements(requests, limits), + Resources: *(getResourceRequirements(requests, limits)), } } @@ -183,6 +255,14 @@ func newPod(name string, containers []v1.Container) *v1.Pod { } } +func newPodWithResources(name string, containers []v1.Container, podResources *v1.ResourceRequirements) *v1.Pod { + pod := newPod(name, containers) + if podResources != nil { + pod.Spec.Resources = podResources + } + return pod +} + func newPodWithInitContainers(name string, containers []v1.Container, initContainers []v1.Container) *v1.Pod { return &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/staging/src/k8s.io/kubectl/pkg/util/qos/qos.go b/staging/src/k8s.io/kubectl/pkg/util/qos/qos.go index 68b1b9072a9..1b102a0fe56 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/qos/qos.go +++ b/staging/src/k8s.io/kubectl/pkg/util/qos/qos.go @@ -37,6 +37,45 @@ func GetPodQOS(pod *core.Pod) core.PodQOSClass { return ComputePodQOS(pod) } +// zeroQuantity represents a resource.Quantity with value "0", used as a baseline +// for resource comparisons. +var zeroQuantity = resource.MustParse("0") + +// processResourceList adds non-zero quantities for supported QoS compute resources +// quantities from newList to list. +func processResourceList(list, newList core.ResourceList) { + for name, quantity := range newList { + if !isSupportedQoSComputeResource(name) { + continue + } + if quantity.Cmp(zeroQuantity) == 1 { + delta := quantity.DeepCopy() + if _, exists := list[name]; !exists { + list[name] = delta + } else { + delta.Add(list[name]) + list[name] = delta + } + } + } +} + +// getQOSResources returns a set of resource names from the provided resource list that: +// 1. Are supported QoS compute resources +// 2. Have quantities greater than zero +func getQOSResources(list core.ResourceList) sets.Set[string] { + qosResources := sets.New[string]() + for name, quantity := range list { + if !isSupportedQoSComputeResource(name) { + continue + } + if quantity.Cmp(zeroQuantity) == 1 { + qosResources.Insert(string(name)) + } + } + return qosResources +} + // ComputePodQOS evaluates the list of containers to determine a pod's QoS class. This function is more // expensive than GetPodQOS which should be used for pods having a non-empty .Status.QOSClass. // A pod is besteffort if none of its containers have specified any requests or limits. @@ -45,50 +84,38 @@ func GetPodQOS(pod *core.Pod) core.PodQOSClass { func ComputePodQOS(pod *core.Pod) core.PodQOSClass { requests := core.ResourceList{} limits := core.ResourceList{} - zeroQuantity := resource.MustParse("0") isGuaranteed := true - // note, ephemeral containers are not considered for QoS as they cannot define resources - allContainers := []core.Container{} - allContainers = append(allContainers, pod.Spec.Containers...) - allContainers = append(allContainers, pod.Spec.InitContainers...) - for _, container := range allContainers { - // process requests - for name, quantity := range container.Resources.Requests { - if !isSupportedQoSComputeResource(name) { - continue - } - if quantity.Cmp(zeroQuantity) == 1 { - delta := quantity.DeepCopy() - if _, exists := requests[name]; !exists { - requests[name] = delta - } else { - delta.Add(requests[name]) - requests[name] = delta - } - } - } - // process limits - qosLimitsFound := sets.NewString() - for name, quantity := range container.Resources.Limits { - if !isSupportedQoSComputeResource(name) { - continue - } - if quantity.Cmp(zeroQuantity) == 1 { - qosLimitsFound.Insert(string(name)) - delta := quantity.DeepCopy() - if _, exists := limits[name]; !exists { - limits[name] = delta - } else { - delta.Add(limits[name]) - limits[name] = delta - } - } + if pod.Spec.Resources != nil { + if pod.Spec.Resources.Requests != nil { + // process requests + processResourceList(requests, pod.Spec.Resources.Requests) } - if !qosLimitsFound.HasAll(string(core.ResourceMemory), string(core.ResourceCPU)) { - isGuaranteed = false + if pod.Spec.Resources.Limits != nil { + // process limits + processResourceList(limits, pod.Spec.Resources.Limits) + qosLimitResources := getQOSResources(pod.Spec.Resources.Limits) + if !qosLimitResources.HasAll(string(core.ResourceMemory), string(core.ResourceCPU)) { + isGuaranteed = false + } + } + } else { + // note, ephemeral containers are not considered for QoS as they cannot define resources + allContainers := []core.Container{} + allContainers = append(allContainers, pod.Spec.Containers...) + allContainers = append(allContainers, pod.Spec.InitContainers...) + for _, container := range allContainers { + // process requests + processResourceList(requests, container.Resources.Requests) + // process limits + processResourceList(limits, container.Resources.Limits) + qosLimitResources := getQOSResources(container.Resources.Limits) + if !qosLimitResources.HasAll(string(core.ResourceMemory), string(core.ResourceCPU)) { + isGuaranteed = false + } } } + if len(requests) == 0 && len(limits) == 0 { return core.PodQOSBestEffort }