Merge pull request #909 from askervin/5LT_empty_balloon_is_new

balloons: prefer an empty existing balloon over creating new

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()) {

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

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