From 82b5a9cee75bde68ca5f2e24da67aaebb6bdf509 Mon Sep 17 00:00:00 2001
From: Lorenzo Torracchi <lorenzo.torracchi@travelappeal.com>
Date: Tue, 29 Oct 2019 11:32:55 +0100
Subject: [PATCH 1/2] Cpu% and Memory% based on requests rather than limits

---
 internal/resource/pod.go      |  2 --
 internal/resource/pod_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/internal/resource/pod.go b/internal/resource/pod.go
index af2061d4..31a58666 100644
--- a/internal/resource/pod.go
+++ b/internal/resource/pod.go
@@ -324,8 +324,6 @@ func (r *Pod) gatherPodMX(po *v1.Pod) (c, p metric) {
 func containerResources(co v1.Container) (cpu, mem *resource.Quantity) {
 	req, limit := co.Resources.Requests, co.Resources.Limits
 	switch {
-	case len(req) != 0 && len(limit) != 0:
-		cpu, mem = limit.Cpu(), limit.Memory()
 	case len(req) != 0:
 		cpu, mem = req.Cpu(), req.Memory()
 	case len(limit) != 0:
diff --git a/internal/resource/pod_test.go b/internal/resource/pod_test.go
index a47ccdd7..fc9fc3c6 100644
--- a/internal/resource/pod_test.go
+++ b/internal/resource/pod_test.go
@@ -44,6 +44,28 @@ func TestPodFields(t *testing.T) {
 	assert.Equal(t, "fred", r[0])
 }
 
+func TestPodFieldsPercentageCpuAndMemRelatedToContainerRequestSpec(t *testing.T) {
+	metrics := makeMxPod("fred", "250m", "256Mi")
+
+	r := NewPodWithMetrics(metrics, v1.ResourceRequirements{
+		Requests: makeRes("500m", "512Mi"),
+	}).Fields("blee")
+
+	assert.Equal(t, "150", r[6])
+	assert.Equal(t, "150", r[7])
+}
+
+func TestPodFieldsPercentageCpuAndMemFallbackToContainerLimitSpecWhenRequestNotDefined(t *testing.T) {
+	metrics := makeMxPod("fred", "250m", "256Mi")
+
+	r := NewPodWithMetrics(metrics, v1.ResourceRequirements{
+		Limits:   makeRes("1000m", "1024Mi"),
+	}).Fields("blee")
+
+	assert.Equal(t, "75", r[6])
+	assert.Equal(t, "75", r[7])
+}
+
 func TestPodMarshal(t *testing.T) {
 	mc := NewMockConnection()
 	mr := NewMockCruder()
@@ -100,6 +122,11 @@ func BenchmarkPodFields(b *testing.B) {
 
 // ----------------------------------------------------------------------------
 // Helpers...
+func makePodWithContainerSpec(resources v1.ResourceRequirements) *v1.Pod {
+	pod := makePod()
+	pod.Spec.Containers[0].Resources = resources
+	return pod
+}
 
 func makePod() *v1.Pod {
 	var i int32 = 1
@@ -159,6 +186,13 @@ func newPod() resource.Columnar {
 	return resource.NewPod(mc).New(makePod())
 }
 
+func NewPodWithMetrics(metrics mv1beta1.PodMetrics, resources v1.ResourceRequirements) resource.Columnar {
+	mc := NewMockConnection()
+	r := resource.NewPod(mc).New(makePodWithContainerSpec(resources))
+	r.SetPodMetrics(&metrics)
+	return r
+}
+
 func poYaml() string {
 	return `apiVersion: v1
 kind: Pod

From b84dfff4587fb6ad68240a1c760631e242c744ea Mon Sep 17 00:00:00 2001
From: Lorenzo Torracchi <lorenzo.torracchi@travelappeal.com>
Date: Wed, 30 Oct 2019 10:59:31 +0100
Subject: [PATCH 2/2] Test refactored using table testing

---
 internal/resource/pod_test.go | 47 +++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/internal/resource/pod_test.go b/internal/resource/pod_test.go
index fc9fc3c6..6a6eb915 100644
--- a/internal/resource/pod_test.go
+++ b/internal/resource/pod_test.go
@@ -44,26 +44,37 @@ func TestPodFields(t *testing.T) {
 	assert.Equal(t, "fred", r[0])
 }
 
-func TestPodFieldsPercentageCpuAndMemRelatedToContainerRequestSpec(t *testing.T) {
-	metrics := makeMxPod("fred", "250m", "256Mi")
+func TestPodFieldsPercentageCpuAndMemRelatedToContainerRequestSpecAndFallbackToLimit(t *testing.T) {
+	percentageTests := []struct {
+		resources             v1.ResourceRequirements
+		metrics               mv1beta1.PodMetrics
+		expectedCpuPercentage string
+		expectedMemPercentage string
+	}{
+		{
+			v1.ResourceRequirements{
+				Requests: makeRes("500m", "512Mi"),
+			},
+			makeMxPod("fred", "250m", "256Mi"),
+			"150",
+			"150",
+		},
+		{
+			v1.ResourceRequirements{
+				Limits: makeRes("1000m", "1024Mi"),
+			},
+			makeMxPod("fred", "250m", "256Mi"),
+			"75",
+			"75",
+		},
+	}
 
-	r := NewPodWithMetrics(metrics, v1.ResourceRequirements{
-		Requests: makeRes("500m", "512Mi"),
-	}).Fields("blee")
+	for _, percentageTest := range percentageTests {
+		r := NewPodWithMetrics(percentageTest.metrics, percentageTest.resources).Fields("blee")
 
-	assert.Equal(t, "150", r[6])
-	assert.Equal(t, "150", r[7])
-}
-
-func TestPodFieldsPercentageCpuAndMemFallbackToContainerLimitSpecWhenRequestNotDefined(t *testing.T) {
-	metrics := makeMxPod("fred", "250m", "256Mi")
-
-	r := NewPodWithMetrics(metrics, v1.ResourceRequirements{
-		Limits:   makeRes("1000m", "1024Mi"),
-	}).Fields("blee")
-
-	assert.Equal(t, "75", r[6])
-	assert.Equal(t, "75", r[7])
+		assert.Equal(t, percentageTest.expectedCpuPercentage, r[6])
+		assert.Equal(t, percentageTest.expectedMemPercentage, r[7])
+	}
 }
 
 func TestPodMarshal(t *testing.T) {