Bug 1725148: Updating to not set CPU limit unless it is explicitly set

pkg/k8shandler/common.go

@@ -486,7 +486,6 @@ func newResourceRequirements(nodeResRequirements, commonResRequirements v1.Resou
 				if nodeLimitCPU.IsZero() {
 					// limit is zero use request for both
 					requestCPU = nodeRequestCPU
-					limitCPU = nodeRequestCPU
 				} else {
 					// both aren't zero
 					requestCPU = nodeRequestCPU
@@ -511,9 +510,7 @@ func newResourceRequirements(nodeResRequirements, commonResRequirements v1.Resou
 
 		if nodeLimitCPU.IsZero() {
 			// no node request mem, check that common has it
-			if commonLimitCPU.IsZero() {
-				limitCPU = commonRequestCPU
-			} else {
+			if !commonLimitCPU.IsZero() {
 				limitCPU = commonLimitCPU
 			}
 		} else {

pkg/k8shandler/common_test.go

@@ -3,6 +3,7 @@ package k8shandler
 import (
 	"reflect"
 	"testing"
+	"fmt"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -46,15 +47,15 @@ var (
 
   6. common has request only
      node has no settings
-     -> request and limit set to be same and use common settings
+     -> cpu/mem request and mem limit set to be same and use common settings
 
   7. common has limit only
      node has no settings
      -> request and limit set to be same and use common settings
 
   8. common has no settings
      node only has request
-     -> request and limit set to be same and use node settings
+     -> cpu/mem request and mem limit set to be same and use node settings
 
   9. common has no settings
      node only has limit
@@ -96,7 +97,7 @@ func TestResourcesCommonAndNodeDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -122,7 +123,7 @@ func TestResourcesNoNodeDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -133,7 +134,7 @@ func TestResourcesNoCommonNoNodeDefined(t *testing.T) {
 
 	nodeRequirements := v1.ResourceRequirements{}
 
-	expected := buildDefaultResource(
+	expected := buildNoCPULimitResource(
 		defaultTestCpuRequest,
 		defaultTestMemLimit,
 		defaultTestMemRequest,
@@ -142,7 +143,7 @@ func TestResourcesNoCommonNoNodeDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -171,7 +172,7 @@ func TestResourcesCommonAndNodeRequestDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -200,7 +201,7 @@ func TestResourcesCommonAndNodeLimitDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -214,8 +215,7 @@ func TestResourcesCommonRequestAndNoNodeDefined(t *testing.T) {
 
 	nodeRequirements := v1.ResourceRequirements{}
 
-	expected := buildResource(
-		commonCpuValue,
+	expected := buildNoCPULimitResource(
 		commonCpuValue,
 		commonMemValue,
 		commonMemValue,
@@ -224,7 +224,7 @@ func TestResourcesCommonRequestAndNoNodeDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -248,7 +248,7 @@ func TestResourcesCommonLimitAndNoNodeDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -262,8 +262,7 @@ func TestResourcesNoCommonAndNodeRequestDefined(t *testing.T) {
 		nodeMemValue,
 	)
 
-	expected := buildResource(
-		nodeCpuValue,
+	expected := buildNoCPULimitResource(
 		nodeCpuValue,
 		nodeMemValue,
 		nodeMemValue,
@@ -272,7 +271,7 @@ func TestResourcesNoCommonAndNodeRequestDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -296,7 +295,7 @@ func TestResourcesNoCommonAndNodeLimitDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -323,7 +322,7 @@ func TestResourcesCommonLimitAndNodeResourceDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -350,7 +349,7 @@ func TestResourcesCommonResourceAndNodeLimitDefined(t *testing.T) {
 	actual := newResourceRequirements(nodeRequirements, commonRequirements)
 
 	if !areResourcesSame(actual, expected) {
-		t.Errorf("Expected %v but got %v", expected, actual)
+		t.Errorf("Expected %v but got %v", printResource(expected), printResource(actual))
 	}
 }
 
@@ -505,7 +504,7 @@ func buildResourceOnlyLimits(cpuLimit, memLimit resource.Quantity) v1.ResourceRe
 	}
 }
 
-func buildDefaultResource(cpuRequest, memLimit, memRequest resource.Quantity) v1.ResourceRequirements {
+func buildNoCPULimitResource(cpuRequest, memLimit, memRequest resource.Quantity) v1.ResourceRequirements {
 	return v1.ResourceRequirements{
 		Limits: v1.ResourceList{
 			v1.ResourceMemory: memLimit,
@@ -541,3 +540,40 @@ func areResourcesSame(lhs, rhs v1.ResourceRequirements) bool {
 func areQuantitiesSame(lhs, rhs *resource.Quantity) bool {
 	return lhs.Cmp(*rhs) == 0
 }
+
+func printResource(resource v1.ResourceRequirements) string {
+	pretty := "\n{\n"
+
+	memLimit := resource.Limits.Memory()
+	memRequest := resource.Requests.Memory()
+	cpuLimit := resource.Limits.Cpu()
+	cpuRequest := resource.Requests.Cpu()
+
+	if !memRequest.IsZero() || !cpuRequest.IsZero() {
+		pretty = fmt.Sprintf("%s\tRequest:\n", pretty)
+
+		if !memRequest.IsZero() {
+			pretty = fmt.Sprintf("%s\t\tMemory: %s\n", pretty, memRequest)
+		}
+
+		if !cpuRequest.IsZero() {
+			pretty = fmt.Sprintf("%s\t\tCpu: %s\n", pretty, cpuRequest)
+		}
+	}
+
+	if !memLimit.IsZero() || !cpuLimit.IsZero() {
+		pretty = fmt.Sprintf("%s\tLimit:\n", pretty)
+
+		if !memLimit.IsZero() {
+			pretty = fmt.Sprintf("%s\t\tMemory: %s\n", pretty, memLimit)
+		}
+
+		if !cpuLimit.IsZero() {
+			pretty = fmt.Sprintf("%s\t\tCpu: %s\n", pretty, cpuLimit)
+		}
+	}
+
+	pretty = fmt.Sprintf("%s}\n", pretty)
+
+	return pretty
+}