Merge pull request #119665 from vinaykul/getpodqos-optimization

Perf optimization: GetPodQOS() returns persisted value of PodStatus.QOSClass, if set.

pkg/apis/core/helper/qos/qos.go

@@ -30,12 +30,22 @@ func isSupportedQoSComputeResource(name core.ResourceName) bool {
 	return supportedQoSComputeResources.Has(string(name))
 }
 
-// GetPodQOS returns the QoS class of a pod.
+// GetPodQOS returns the QoS class of a pod persisted in the PodStatus.QOSClass field.
+// If PodStatus.QOSClass is empty, it returns value of ComputePodQOS() which evaluates pod's QoS class.
+func GetPodQOS(pod *core.Pod) core.PodQOSClass {
+	if pod.Status.QOSClass != "" {
+		return pod.Status.QOSClass
+	}
+	return ComputePodQOS(pod)
+}
+
+// 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.
 // When this function is updated please also update staging/src/k8s.io/kubectl/pkg/util/qos/qos.go
-func GetPodQOS(pod *core.Pod) core.PodQOSClass {
+func ComputePodQOS(pod *core.Pod) core.PodQOSClass {
 	requests := core.ResourceList{}
 	limits := core.ResourceList{}
 	zeroQuantity := resource.MustParse("0")

pkg/apis/core/v1/helper/qos/qos.go

@@ -32,11 +32,21 @@ func isSupportedQoSComputeResource(name v1.ResourceName) bool {
 	return supportedQoSComputeResources.Has(string(name))
 }
 
-// GetPodQOS returns the QoS class of a pod.
+// GetPodQOS returns the QoS class of a pod persisted in the PodStatus.QOSClass field.
+// If PodStatus.QOSClass is empty, it returns value of ComputePodQOS() which evaluates pod's QoS class.
+func GetPodQOS(pod *v1.Pod) v1.PodQOSClass {
+	if pod.Status.QOSClass != "" {
+		return pod.Status.QOSClass
+	}
+	return ComputePodQOS(pod)
+}
+
+// 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.
-func GetPodQOS(pod *v1.Pod) v1.PodQOSClass {
+func ComputePodQOS(pod *v1.Pod) v1.PodQOSClass {
 	requests := v1.ResourceList{}
 	limits := v1.ResourceList{}
 	zeroQuantity := resource.MustParse("0")

pkg/apis/core/v1/helper/qos/qos_test.go

@@ -27,7 +27,7 @@ import (
 	corev1 "k8s.io/kubernetes/pkg/apis/core/v1"
 )
 
-func TestGetPodQOS(t *testing.T) {
+func TestComputePodQOS(t *testing.T) {
 	testCases := []struct {
 		pod      *v1.Pod
 		expected v1.PodQOSClass
@@ -128,15 +128,15 @@ func TestGetPodQOS(t *testing.T) {
 		},
 	}
 	for id, testCase := range testCases {
-		if actual := GetPodQOS(testCase.pod); testCase.expected != actual {
+		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)
 		}
 
-		// Convert v1.Pod to core.Pod, and then check against `core.helper.GetPodQOS`.
+		// Convert v1.Pod to core.Pod, and then check against `core.helper.ComputePodQOS`.
 		pod := core.Pod{}
 		corev1.Convert_v1_Pod_To_core_Pod(testCase.pod, &pod, nil)
 
-		if actual := qos.GetPodQOS(&pod); core.PodQOSClass(testCase.expected) != actual {
+		if actual := qos.ComputePodQOS(&pod); core.PodQOSClass(testCase.expected) != actual {
 			t.Errorf("[%d]: conversion invalid qos pod %s, expected: %s, actual: %s", id, testCase.pod.Name, testCase.expected, actual)
 		}
 	}

pkg/apis/core/validation/validation.go

@@ -4812,19 +4812,8 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
 		return allErrs
 	}
 
-	//TODO(vinaykul,InPlacePodVerticalScaling): With KEP 2527, we can rely on persistence of PodStatus.QOSClass
+	if qos.GetPodQOS(oldPod) != qos.ComputePodQOS(newPod) {
-	// We can use PodStatus.QOSClass instead of GetPodQOS here, in kubelet, and elsewhere, as PodStatus.QOSClass
+		allErrs = append(allErrs, field.Invalid(fldPath, newPod.Status.QOSClass, "Pod QoS is immutable"))
-	// does not change once it is bootstrapped in podCreate. This needs to be addressed before beta as a
-	// separate PR covering all uses of GetPodQOS. With that change, we can drop the below block.
-	// Ref: https://github.com/kubernetes/kubernetes/pull/102884#discussion_r1093790446
-	// Ref: https://github.com/kubernetes/kubernetes/pull/102884/#discussion_r663280487
-	if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) {
-		// reject attempts to change pod qos
-		oldQoS := qos.GetPodQOS(oldPod)
-		newQoS := qos.GetPodQOS(newPod)
-		if newQoS != oldQoS {
-			allErrs = append(allErrs, field.Invalid(fldPath, newQoS, "Pod QoS is immutable"))
-		}
 	}
 
 	// handle updateable fields by munging those fields prior to deep equal comparison.

staging/src/k8s.io/kubectl/pkg/describe/describe.go

@@ -871,11 +871,7 @@ func describePod(pod *corev1.Pod, events *corev1.EventList) (string, error) {
 			}
 		}
 		describeVolumes(pod.Spec.Volumes, w, "")
-		if pod.Status.QOSClass != "" {
-			w.Write(LEVEL_0, "QoS Class:\t%s\n", pod.Status.QOSClass)
-		} else {
 		w.Write(LEVEL_0, "QoS Class:\t%s\n", qos.GetPodQOS(pod))
-		}
 		printLabelsMultiline(w, "Node-Selectors", pod.Spec.NodeSelector)
 		printPodTolerationsMultiline(w, "Tolerations", pod.Spec.Tolerations)
 		describeTopologySpreadConstraints(pod.Spec.TopologySpreadConstraints, w, "")

staging/src/k8s.io/kubectl/pkg/util/qos/qos.go

@@ -28,11 +28,21 @@ func isSupportedQoSComputeResource(name core.ResourceName) bool {
 	return supportedQoSComputeResources.Has(string(name))
 }
 
-// GetPodQOS returns the QoS class of a pod.
+// GetPodQOS returns the QoS class of a pod persisted in the PodStatus.QOSClass field.
+// If PodStatus.QOSClass is empty, it returns value of ComputePodQOS() which evaluates pod's QoS class.
+func GetPodQOS(pod *core.Pod) core.PodQOSClass {
+	if pod.Status.QOSClass != "" {
+		return pod.Status.QOSClass
+	}
+	return ComputePodQOS(pod)
+}
+
+// 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.
-func GetPodQOS(pod *core.Pod) core.PodQOSClass {
+func ComputePodQOS(pod *core.Pod) core.PodQOSClass {
 	requests := core.ResourceList{}
 	limits := core.ResourceList{}
 	zeroQuantity := resource.MustParse("0")