Skip to content

Commit

Permalink
UPSTREAM: <carry>: add management workload check for guaranteed qos
Browse files Browse the repository at this point in the history
when static pods have workload partitioning enabled we should not alter their resources if they are Guaranteed QoS, this change adds a check for Guaranteed QoS

Signed-off-by: ehila <ehila@redhat.com>

test: add unit tests for error states

Signed-off-by: ehila <ehila@redhat.com>
  • Loading branch information
eggfoobar authored and dinhxuanvu committed Apr 15, 2024
1 parent 5aacc69 commit 47d3e19
Show file tree
Hide file tree
Showing 2 changed files with 463 additions and 12 deletions.
65 changes: 61 additions & 4 deletions pkg/kubelet/managed/managed.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,16 @@ import (
)

var (
pinnedManagementEnabled bool
pinnedManagementFilename = "/etc/kubernetes/openshift-workload-pinning"
pinnedManagementEnabled bool
pinnedManagementFilename = "/etc/kubernetes/openshift-workload-pinning"
)

const (
qosWarning = "skipping pod CPUs requests modifications because it has guaranteed QoS class"
WorkloadsAnnotationPrefix = "target.workload.openshift.io/"
WorkloadsCapacitySuffix = "workload.openshift.io/cores"
ContainerAnnotationFormat = "resources.workload.openshift.io/%v"
ContainerAnnotationPrefix = "resources.workload.openshift.io/"
WorkloadAnnotationWarning = "workload.openshift.io/warning"
)

type WorkloadContainerAnnotation struct {
Expand Down Expand Up @@ -92,6 +97,11 @@ func ModifyStaticPodForPinnedManagement(pod *v1.Pod) (*v1.Pod, string, error) {
if pod.Annotations == nil {
pod.Annotations = make(map[string]string)
}
if isPodGuaranteed(pod) {
stripWorkloadAnnotations(pod.Annotations)
pod.Annotations[WorkloadAnnotationWarning] = qosWarning
return pod, "", nil
}
pod.Annotations[fmt.Sprintf("%v%v", WorkloadsAnnotationPrefix, workloadName)] = value
if err := updateContainers(workloadName, pod); err != nil {
return nil, "", err
Expand Down Expand Up @@ -122,7 +132,7 @@ func updateContainers(workloadName string, pod *v1.Pod) error {

containerAnnotation := NewWorkloadContainerAnnotation(MilliCPUToShares(cpuRequestInMilli))
jsonAnnotation, _ := containerAnnotation.Serialize()
containerNameKey := fmt.Sprintf(ContainerAnnotationFormat, container.Name)
containerNameKey := fmt.Sprintf("%v%v", ContainerAnnotationPrefix, container.Name)

newCPURequest := resource.NewMilliQuantity(cpuRequestInMilli*1000, cpuRequest.Format)

Expand All @@ -145,3 +155,50 @@ func updateContainers(workloadName string, pod *v1.Pod) error {
}
return nil
}

// isPodGuaranteed checks if the pod has a guaranteed QoS.
// This QoS check is different from the library versions that check QoS,
// this is because of the order at which changes get observed.
// (i.e. the library assumes the defaulter has ran on the pod resource before calculating QoS).
//
// The files will get interpreted before they reach the API server and before the defaulter applies changes,
// this function takes into account the case where only `limits.cpu` are provided but no `requests.cpu` are since that
// counts as a Guaranteed QoS after the defaulter runs.
func isPodGuaranteed(pod *v1.Pod) bool {
isGuaranteed := func(containers []v1.Container) bool {
for _, c := range containers {
// only memory and CPU resources are relevant to decide pod QoS class
for _, r := range []v1.ResourceName{v1.ResourceMemory, v1.ResourceCPU} {
limit := c.Resources.Limits[r]
request, requestExist := c.Resources.Requests[r]
if limit.IsZero() {
return false
}
if !requestExist {
continue
}
// In some corner case, when you set CPU requests to 0 the k8s defaulter will change it to the value
// specified under the limit.
if r == v1.ResourceCPU && request.IsZero() {
continue
}
if !limit.Equal(request) {
return false
}
}
}
return true
}
return isGuaranteed(pod.Spec.InitContainers) && isGuaranteed(pod.Spec.Containers)
}

func stripWorkloadAnnotations(annotations map[string]string) {
for k := range annotations {
if strings.HasPrefix(k, WorkloadsAnnotationPrefix) {
delete(annotations, k)
}
if strings.HasPrefix(k, ContainerAnnotationPrefix) {
delete(annotations, k)
}
}
}

0 comments on commit 47d3e19

Please sign in to comment.