From 4324a3a3b358939931de6abcadd804785a823bf2 Mon Sep 17 00:00:00 2001 From: Antti Kervinen Date: Mon, 24 Oct 2022 15:06:03 +0300 Subject: [PATCH] balloons: prefer an empty existing balloon over creating new When a balloon type has PreferNewBalloon set, prefer assigning a container to an empty existing balloons over to a new ones. Without this change the best balloon instances (created due to MinBalloons > 0) are left empty until new balloon instances cannot be created anymore. --- .../builtin/balloons/balloons-policy.go | 19 ++-- .../test09-isolated/balloons-isolated.cfg | 27 ++++++ .../n4c16/test09-isolated/code.var.sh | 90 +++++++++++++++++++ 3 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/balloons-isolated.cfg create mode 100644 test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/code.var.sh diff --git a/pkg/cri/resource-manager/policy/builtin/balloons/balloons-policy.go b/pkg/cri/resource-manager/policy/builtin/balloons/balloons-policy.go index 763b9ac4a..8ebab2f20 100644 --- a/pkg/cri/resource-manager/policy/builtin/balloons/balloons-policy.go +++ b/pkg/cri/resource-manager/policy/builtin/balloons/balloons-policy.go @@ -572,19 +572,22 @@ func (p *balloons) chooseBalloonInstance(blnDef *BalloonDef, fm FillMethod, c ca return p.balloons[0], nil case FillDefaultBalloon: return p.balloons[1], nil - case FillNewBalloon: - if newBln, err := p.newBalloon(blnDef, true); err == nil { - p.balloons = append(p.balloons, newBln) - return newBln, nil - } else { - return nil, nil + case FillNewBalloon, FillNewBalloonMust: + // Choosing an existing balloon without containers is + // preferred over instantiating a new balloon. + for _, bln := range p.balloonsByDef(blnDef) { + if len(bln.PodIDs) == 0 { + return bln, nil + } } - case FillNewBalloonMust: if newBln, err := p.newBalloon(blnDef, true); err == nil { p.balloons = append(p.balloons, newBln) return newBln, nil } else { - return nil, err + if fm == FillNewBalloonMust { + return nil, err + } + return nil, nil } case FillSameNamespace: for _, bln := range p.balloonsByNamespace(c.GetNamespace()) { diff --git a/test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/balloons-isolated.cfg b/test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/balloons-isolated.cfg new file mode 100644 index 000000000..577ce34b8 --- /dev/null +++ b/test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/balloons-isolated.cfg @@ -0,0 +1,27 @@ +policy: + Active: balloons + ReservedResources: + CPU: cpuset:0 + balloons: + BalloonTypes: + - Name: isolated-pods + MinCPUs: 0 + MaxCPUs: 2 + CPUClass: turbo + MinBalloons: 2 + PreferNewBalloons: true + PreferSpreadingPods: false + - Name: isolated-ctrs + MinCPUs: 1 + MaxCPUs: 1 + CPUClass: turbo + MinBalloons: 2 + PreferNewBalloons: true + PreferSpreadingPods: true +instrumentation: + HTTPEndpoint: :8891 + PrometheusExport: true +logger: + Debug: policy + Klog: + skip_headers: true diff --git a/test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/code.var.sh b/test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/code.var.sh new file mode 100644 index 000000000..d4ce671f5 --- /dev/null +++ b/test/e2e/policies.test-suite/balloons/n4c16/test09-isolated/code.var.sh @@ -0,0 +1,90 @@ +terminate cri-resmgr +cri_resmgr_cfg=${TEST_DIR}/balloons-isolated.cfg cri_resmgr_extra_args="-metrics-interval 4s" launch cri-resmgr + +verify-metrics-has-line 'balloon="isolated-pods\[0\]"' +verify-metrics-has-line 'balloon="isolated-pods\[1\]"' +verify-metrics-has-no-line 'balloon="isolated-pods\[2\]"' + +# pod0: besteffort +CPUREQ="" CPULIM="" MEMREQ="" MEMLIM="" +POD_ANNOTATION="balloon.balloons.cri-resource-manager.intel.com: isolated-pods" +CONTCOUNT=2 create balloons-busybox +report allowed +verify 'len(cpus["pod0c0"]) == 1' \ + 'len(cpus["pod0c1"]) == 1' \ + 'cpus["pod0c0"] == cpus["pod0c1"]' +# Even if the isolated balloon type has PreferNewBalloons=1, adding +# this pod0 or pod1 must not create a new balloon because existing +# empty balloons should be filled first. +verify-metrics-has-line 'balloon="isolated-pods\[0\]"' +verify-metrics-has-line 'balloon="isolated-pods\[1\]"' +verify-metrics-has-no-line 'balloon="isolated-pods\[2\]"' + +# pod1: guaranteed +CPUREQ="600m" CPULIM="600m" MEMREQ="100M" MEMLIM="100M" +POD_ANNOTATION="balloon.balloons.cri-resource-manager.intel.com: isolated-pods" +CONTCOUNT=2 create balloons-busybox +report allowed +verify 'len(cpus["pod0c0"]) == 1' \ + 'len(cpus["pod0c1"]) == 1' \ + 'len(cpus["pod1c0"]) == 2' \ + 'len(cpus["pod1c1"]) == 2' \ + 'cpus["pod0c0"] == cpus["pod0c1"]' \ + 'cpus["pod1c0"] == cpus["pod1c1"]' \ + 'disjoint_sets(cpus["pod0c0"], cpus["pod1c0"])' +verify-metrics-has-line 'balloon="isolated-pods\[0\]"' +verify-metrics-has-line 'balloon="isolated-pods\[1\]"' +verify-metrics-has-no-line 'balloon="isolated-pods\[2\]"' + +# pod2: burstable +CPUREQ="100m" CPULIM="200m" +POD_ANNOTATION="balloon.balloons.cri-resource-manager.intel.com: isolated-pods" +CONTCOUNT=2 create balloons-busybox +report allowed +verify 'len(cpus["pod0c0"]) == 1' \ + 'len(cpus["pod0c1"]) == 1' \ + 'len(cpus["pod1c0"]) == 2' \ + 'len(cpus["pod1c1"]) == 2' \ + 'len(cpus["pod2c0"]) == 1' \ + 'len(cpus["pod2c1"]) == 1' \ + 'cpus["pod0c0"] == cpus["pod0c1"]' \ + 'cpus["pod1c0"] == cpus["pod1c1"]' \ + 'cpus["pod2c0"] == cpus["pod2c1"]' \ + 'disjoint_sets(cpus["pod0c0"], cpus["pod1c0"], cpus["pod2c0"])' +verify-metrics-has-line 'balloon="isolated-pods\[0\]"' +verify-metrics-has-line 'balloon="isolated-pods\[1\]"' +verify-metrics-has-line 'balloon="isolated-pods\[2\]"' +verify-metrics-has-no-line 'balloon="isolated-pods\[3\]"' + +# pod3: isolated containers +CPUREQ="" CPULIM="" MEMREQ="" MEMLIM="" +POD_ANNOTATION="balloon.balloons.cri-resource-manager.intel.com: isolated-ctrs" +CONTCOUNT=4 create balloons-busybox +report allowed +verify 'len(cpus["pod0c0"]) == 1' \ + 'len(cpus["pod0c1"]) == 1' \ + 'len(cpus["pod1c0"]) == 2' \ + 'len(cpus["pod1c1"]) == 2' \ + 'len(cpus["pod2c0"]) == 1' \ + 'len(cpus["pod2c1"]) == 1' \ + 'len(cpus["pod3c0"]) == 1' \ + 'len(cpus["pod3c1"]) == 1' \ + 'len(cpus["pod3c2"]) == 1' \ + 'len(cpus["pod3c3"]) == 1' \ + 'cpus["pod0c0"] == cpus["pod0c1"]' \ + 'cpus["pod1c0"] == cpus["pod1c1"]' \ + 'cpus["pod2c0"] == cpus["pod2c1"]' \ + 'disjoint_sets(cpus["pod0c0"], cpus["pod1c0"], cpus["pod2c0"])' \ + 'disjoint_sets(cpus["pod3c0"], cpus["pod3c1"], cpus["pod3c2"], cpus["pod3c3"])' \ + 'disjoint_sets(cpus["pod0c0"], cpus["pod1c0"], cpus["pod2c0"], cpus["pod3c0"], cpus["pod3c1"], cpus["pod3c2"], cpus["pod3c3"])' +verify-metrics-has-line 'balloon="isolated-pods\[0\]"' +verify-metrics-has-line 'balloon="isolated-pods\[1\]"' +verify-metrics-has-line 'balloon="isolated-pods\[2\]"' +verify-metrics-has-no-line 'balloon="isolated-pods\[3\]"' +verify-metrics-has-line 'balloon="isolated-ctrs\[0\]"' +verify-metrics-has-line 'balloon="isolated-ctrs\[1\]"' +verify-metrics-has-line 'balloon="isolated-ctrs\[2\]"' +verify-metrics-has-line 'balloon="isolated-ctrs\[3\]"' + +terminate cri-resmgr +launch cri-resmgr