From 51d8b30e8fdb079d178c8e7d640d923bb0700b5f Mon Sep 17 00:00:00 2001
From: Ingo Oppermann <ingo@datarhei.com>
Date: Wed, 12 Jul 2023 11:53:39 +0200
Subject: [PATCH] Fix MaxCPU and MaxMemory semantics

If a limit of 0 (or negative) is given for both cpu and memory, then
no limiting will be triggered. If any value between 1 and 100 (inclusive)
is given, then limiting will be triggered when that limit is reached.

I.e. giving a limit of 100 doesn't not mean unlimited.
---
 resources/resources.go      | 57 +++++++++++++++++++++----------------
 resources/resources_test.go | 10 +++++++
 2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/resources/resources.go b/resources/resources.go
index aa910153..e6a371cc 100644
--- a/resources/resources.go
+++ b/resources/resources.go
@@ -54,6 +54,12 @@ type Config struct {
 }
 
 func New(config Config) (Resources, error) {
+	isUnlimited := false
+
+	if config.MaxCPU <= 0 && config.MaxMemory <= 0 {
+		isUnlimited = true
+	}
+
 	if config.MaxCPU <= 0 {
 		config.MaxCPU = 100
 	}
@@ -67,13 +73,10 @@ func New(config Config) (Resources, error) {
 	}
 
 	r := &resources{
-		maxCPU: config.MaxCPU,
-		psutil: config.PSUtil,
-		logger: config.Logger,
-	}
-
-	if config.MaxCPU == 100 && config.MaxMemory == 100 {
-		r.isUnlimited = true
+		maxCPU:      config.MaxCPU,
+		psutil:      config.PSUtil,
+		isUnlimited: isUnlimited,
+		logger:      config.Logger,
 	}
 
 	if r.logger == nil {
@@ -167,31 +170,35 @@ func (r *resources) observe(ctx context.Context, interval time.Duration) {
 
 			doCPULimit := false
 
-			if !r.isCPULimiting {
-				if cpuload > r.maxCPU {
-					r.logger.Debug().WithField("cpu", cpuload).Log("CPU limit reached")
+			if !r.isUnlimited {
+				if !r.isCPULimiting {
+					if cpuload >= r.maxCPU {
+						r.logger.Debug().WithField("cpu", cpuload).Log("CPU limit reached")
+						doCPULimit = true
+					}
+				} else {
 					doCPULimit = true
-				}
-			} else {
-				doCPULimit = true
-				if cpuload <= r.maxCPU {
-					r.logger.Debug().WithField("cpu", cpuload).Log("CPU limit released")
-					doCPULimit = false
+					if cpuload < r.maxCPU {
+						r.logger.Debug().WithField("cpu", cpuload).Log("CPU limit released")
+						doCPULimit = false
+					}
 				}
 			}
 
 			doMemoryLimit := false
 
-			if !r.isMemoryLimiting {
-				if vmstat.Used > r.maxMemory {
-					r.logger.Debug().WithField("memory", vmstat.Used).Log("Memory limit reached")
+			if !r.isUnlimited {
+				if !r.isMemoryLimiting {
+					if vmstat.Used >= r.maxMemory {
+						r.logger.Debug().WithField("memory", vmstat.Used).Log("Memory limit reached")
+						doMemoryLimit = true
+					}
+				} else {
 					doMemoryLimit = true
-				}
-			} else {
-				doMemoryLimit = true
-				if vmstat.Used <= r.maxMemory {
-					r.logger.Debug().WithField("memory", vmstat.Used).Log("Memory limit released")
-					doMemoryLimit = false
+					if vmstat.Used < r.maxMemory {
+						r.logger.Debug().WithField("memory", vmstat.Used).Log("Memory limit released")
+						doMemoryLimit = false
+					}
 				}
 			}
 
diff --git a/resources/resources_test.go b/resources/resources_test.go
index ec502265..0158c7f7 100644
--- a/resources/resources_test.go
+++ b/resources/resources_test.go
@@ -177,6 +177,16 @@ func TestHasLimits(t *testing.T) {
 
 	require.True(t, r.HasLimits())
 
+	r, err = New(Config{
+		MaxCPU:    100,
+		MaxMemory: 100,
+		PSUtil:    &util{},
+		Logger:    nil,
+	})
+	require.NoError(t, err)
+
+	require.True(t, r.HasLimits())
+
 	r, err = New(Config{
 		MaxCPU:    0,
 		MaxMemory: 0,