From 01eba8450ebfb8917f40b1aae2f82d46834c9615 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Mon, 27 Apr 2020 18:25:51 +0300 Subject: [PATCH] cache: never try to calculate memory requests. Do not try to estimate memory requests, even if we don't have resource requests/limits annotated by a webhook. The achievable accuracy for Burstable QoS containers is unusable. For Guaranteed QoS class pods copy the memory limit to memory request (and also prefer copying the CPU request to CPU limit instead of estimating). --- pkg/cri/resource-manager/cache/container.go | 34 ++++++----- pkg/cri/resource-manager/cache/utils.go | 65 ++++++--------------- 2 files changed, 38 insertions(+), 61 deletions(-) diff --git a/pkg/cri/resource-manager/cache/container.go b/pkg/cri/resource-manager/cache/container.go index d8a962082..54056a6cf 100644 --- a/pkg/cri/resource-manager/cache/container.go +++ b/pkg/cri/resource-manager/cache/container.go @@ -31,6 +31,11 @@ import ( func (c *container) fromCreateRequest(req *cri.CreateContainerRequest) error { c.PodID = req.PodSandboxId + pod, ok := c.cache.Pods[c.PodID] + if !ok { + return cacheError("can't find cached pod %s for container to create", c.PodID) + } + cfg := req.Config if cfg == nil { return cacheError("container of pod %s has no config", c.PodID) @@ -90,23 +95,18 @@ func (c *container) fromCreateRequest(req *cri.CreateContainerRequest) error { c.Tags = make(map[string]string) - // if we get more than one hint, check that there are no duplicates - // if len(c.TopologyHints) > 1 { - // c.TopologyHints = topology.DeDuplicateTopologyHints(c.TopologyHints) - // } - c.LinuxReq = cfg.GetLinux().GetResources() - if p, _ := c.cache.Pods[c.PodID]; p != nil && p.Resources != nil { - if r, ok := p.Resources.InitContainers[c.Name]; ok { + if pod.Resources != nil { + if r, ok := pod.Resources.InitContainers[c.Name]; ok { c.Resources = r - } else if r, ok := p.Resources.Containers[c.Name]; ok { + } else if r, ok := pod.Resources.Containers[c.Name]; ok { c.Resources = r } } if len(c.Resources.Requests) == 0 && len(c.Resources.Limits) == 0 { - c.Resources = estimateComputeResources(c.LinuxReq) + c.Resources = estimateComputeResources(c.LinuxReq, pod.CgroupParent) } c.TopologyHints = topology.MergeTopologyHints(c.TopologyHints, getKubeletHint(c.GetCpusetCpus(), c.GetCpusetMems())) @@ -118,8 +118,8 @@ func (c *container) fromCreateRequest(req *cri.CreateContainerRequest) error { func (c *container) fromListResponse(lrc *cri.Container) error { c.PodID = lrc.PodSandboxId - p, _ := c.cache.Pods[c.PodID] - if p == nil { + pod, ok := c.cache.Pods[c.PodID] + if !ok { return cacheError("can't find cached pod %s for listed container", c.PodID) } @@ -130,21 +130,25 @@ func (c *container) fromListResponse(lrc *cri.Container) error { c.ID = lrc.Id c.Name = meta.Name - c.Namespace = p.Namespace + c.Namespace = pod.Namespace c.State = ContainerState(int32(lrc.State)) c.Image = lrc.GetImage().GetImage() c.Labels = lrc.Labels c.Annotations = lrc.Annotations c.Tags = make(map[string]string) - if p.Resources != nil { - if r, ok := p.Resources.InitContainers[c.Name]; ok { + if pod.Resources != nil { + if r, ok := pod.Resources.InitContainers[c.Name]; ok { c.Resources = r - } else if r, ok := p.Resources.Containers[c.Name]; ok { + } else if r, ok := pod.Resources.Containers[c.Name]; ok { c.Resources = r } } + if len(c.Resources.Requests) == 0 && len(c.Resources.Limits) == 0 { + c.Resources = estimateComputeResources(c.LinuxReq, pod.CgroupParent) + } + return nil } diff --git a/pkg/cri/resource-manager/cache/utils.go b/pkg/cri/resource-manager/cache/utils.go index 6e05f6cce..51eddeb71 100644 --- a/pkg/cri/resource-manager/cache/utils.go +++ b/pkg/cri/resource-manager/cache/utils.go @@ -33,16 +33,14 @@ const ( milliCPUToCPU = 1000 QuotaPeriod = 100000 minQuotaPeriod = 1000 - - // memory limit to OOM score adjustement - guaranteedOOMScoreAdj int = -998 - besteffortOOMScoreAdj int = 1000 ) var memoryCapacity int64 // estimateComputeResources calculates resource requests/limits from a CRI request. -func estimateComputeResources(lnx *cri.LinuxContainerResources) corev1.ResourceRequirements { +func estimateComputeResources(lnx *cri.LinuxContainerResources, cgroupParent string) corev1.ResourceRequirements { + var qos corev1.PodQOSClass + resources := corev1.ResourceRequirements{ Requests: corev1.ResourceList{}, Limits: corev1.ResourceList{}, @@ -52,30 +50,33 @@ func estimateComputeResources(lnx *cri.LinuxContainerResources) corev1.ResourceR return resources } + if cgroupParent != "" { + qos = cgroupParentToQOS(cgroupParent) + } + // calculate CPU request if value := SharesToMilliCPU(lnx.CpuShares); value > 0 { qty := resapi.NewMilliQuantity(value, resapi.DecimalSI) resources.Requests[corev1.ResourceCPU] = *qty } - // calculate CPU limit - if value := QuotaToMilliCPU(lnx.CpuQuota, lnx.CpuPeriod); value > 0 { - qty := resapi.NewMilliQuantity(value, resapi.DecimalSI) - resources.Limits[corev1.ResourceCPU] = *qty - } - - // calculate memory request - if value := OomScoreAdjToMemoryRequest(lnx.OomScoreAdj); value != 0 { - qty := resapi.NewQuantity(value, resapi.DecimalSI) - resources.Requests[corev1.ResourceMemory] = *qty - } - - // calculate memory limit + // get memory limit if value := lnx.MemoryLimitInBytes; value > 0 { qty := resapi.NewQuantity(value, resapi.DecimalSI) resources.Limits[corev1.ResourceMemory] = *qty } + // set or calculate CPU limit, set memory request if known + if qos == corev1.PodQOSGuaranteed { + resources.Limits[corev1.ResourceCPU] = resources.Requests[corev1.ResourceCPU] + resources.Requests[corev1.ResourceMemory] = resources.Limits[corev1.ResourceMemory] + } else { + if value := QuotaToMilliCPU(lnx.CpuQuota, lnx.CpuPeriod); value > 0 { + qty := resapi.NewMilliQuantity(value, resapi.DecimalSI) + resources.Limits[corev1.ResourceCPU] = *qty + } + } + return resources } @@ -126,34 +127,6 @@ func MilliCPUToQuota(milliCPU int64) (int64, int64) { return quota, period } -// MemoryRequestToOomScoreAdj -- We don't do this direction (we leave the adjustment intact)... -func MemoryRequestToOomScoreAdj(namespace string, configSource string, qos corev1.PodQOSClass, - memRequest int64) int64 { - panic("this shouldn't be called... better leave the OOM score adjustment intact.") - // return 0 -} - -// OomScoreAdjToMemoryRequest tries to convert OOM score to original memory request. -func OomScoreAdjToMemoryRequest(value int64) int64 { - if value == 0 { - return 0 - } - - switch value { - case int64(1000 + guaranteedOOMScoreAdj): - // lossy case, let's use boundary OOMScoreAdj value - value = int64(1000 + guaranteedOOMScoreAdj - 1) - return (memoryCapacity * (int64(1000) - value)) / int64(1000) - - case int64(besteffortOOMScoreAdj - 1): - value = int64(besteffortOOMScoreAdj) - return (memoryCapacity * (int64(1000) - value)) / int64(1000) - - default: - return (memoryCapacity * (int64(1000) - value)) / int64(1000) - } -} - // getMemoryCapacity parses memory capacity from /proc/meminfo (mimicking cAdvisor). func getMemoryCapacity() int64 { var data []byte