Merge pull request intel#252 from klihub/fixes/cache/never-calculate-…

…memory-request fixes/cache: stop trying to calculate memory requests
askervin · May 5, 2020 · 6b8c62f · 6b8c62f
2 parents 06cc399 + 01eba84
commit 6b8c62f
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 61 deletions.
diff --git 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
@@ -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