From b9196b0f36842128aa670c298df782b885f288f9 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 | 85 +++++++++++++++++++++++++++-------- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/pkg/k8shandler/common.go b/pkg/k8shandler/common.go index 423ca46f7..04b09ff80 100644 --- a/pkg/k8shandler/common.go +++ b/pkg/k8shandler/common.go @@ -463,7 +463,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 @@ -488,9 +487,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 174b6108e..1d4981326 100644 --- a/pkg/k8shandler/common_test.go +++ b/pkg/k8shandler/common_test.go @@ -3,6 +3,7 @@ package k8shandler import ( "reflect" "testing" + "fmt" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -47,7 +48,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 @@ -55,7 +56,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 @@ -97,7 +98,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)) } } @@ -123,7 +124,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)) } } @@ -134,8 +135,7 @@ func TestResourcesNoCommonNoNodeDefined(t *testing.T) { nodeRequirements := v1.ResourceRequirements{} - expected := buildResource( - defaultTestCpuLimit, + expected := buildNoCPULimitResource( defaultTestCpuRequest, defaultTestMemLimit, defaultTestMemRequest, @@ -144,7 +144,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)) } } @@ -173,7 +173,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)) } } @@ -202,7 +202,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)) } } @@ -216,8 +216,7 @@ func TestResourcesCommonRequestAndNoNodeDefined(t *testing.T) { nodeRequirements := v1.ResourceRequirements{} - expected := buildResource( - commonCpuValue, + expected := buildNoCPULimitResource( commonCpuValue, commonMemValue, commonMemValue, @@ -226,7 +225,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)) } } @@ -250,7 +249,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)) } } @@ -264,8 +263,7 @@ func TestResourcesNoCommonAndNodeRequestDefined(t *testing.T) { nodeMemValue, ) - expected := buildResource( - nodeCpuValue, + expected := buildNoCPULimitResource( nodeCpuValue, nodeMemValue, nodeMemValue, @@ -274,7 +272,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)) } } @@ -298,7 +296,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)) } } @@ -325,7 +323,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)) } } @@ -352,7 +350,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)) } } @@ -404,6 +402,18 @@ func buildResourceOnlyLimits(cpuLimit, memLimit resource.Quantity) v1.ResourceRe } } +func buildNoCPULimitResource(cpuRequest, memLimit, memRequest resource.Quantity) v1.ResourceRequirements { + return v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: memLimit, + }, + Requests: v1.ResourceList{ + v1.ResourceCPU: cpuRequest, + v1.ResourceMemory: memRequest, + }, + } +} + func areResourcesSame(lhs, rhs v1.ResourceRequirements) bool { if !areQuantitiesSame(lhs.Limits.Cpu(), rhs.Limits.Cpu()) { @@ -428,3 +438,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 +}