From 767301c11a1450da9af845553df6ecf886cf2cdf Mon Sep 17 00:00:00 2001 From: Eric Wolinetz Date: Fri, 28 Jun 2019 09:54:31 -0500 Subject: [PATCH] Updating to not set CPU limit unless it is explicitly set --- pkg/k8shandler/common.go | 5 +-- pkg/k8shandler/common_test.go | 74 ++++++++++++++++++++++++++--------- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/pkg/k8shandler/common.go b/pkg/k8shandler/common.go index 62273d194..cb06ec497 100644 --- a/pkg/k8shandler/common.go +++ b/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 { diff --git a/pkg/k8shandler/common_test.go b/pkg/k8shandler/common_test.go index 75fcafd39..bf232341a 100644 --- a/pkg/k8shandler/common_test.go +++ b/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,7 +47,7 @@ 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 @@ -54,7 +55,7 @@ var ( 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 +}