Skip to content

Commit

Permalink
cache: never try to calculate memory requests.
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@klihub
klihub committed Apr 29, 2020
1 parent b1e042a commit 01eba84
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 61 deletions.
34 changes: 19 additions & 15 deletions pkg/cri/resource-manager/cache/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()))
Expand All @@ -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)
}

Expand All @@ -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
}

Expand Down
65 changes: 19 additions & 46 deletions pkg/cri/resource-manager/cache/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand All @@ -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
}

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 01eba84

Please sign in to comment.