From dc36924c373f9ca54ebee6dbc4a264058646c6ea Mon Sep 17 00:00:00 2001
From: Adrian Chiris <adrianc@mellanox.com>
Date: Thu, 5 Dec 2019 06:15:20 +0000
Subject: [PATCH] Update policy_none removing canAdmitPodResult Update unit
 tests for none_policy Add Name test for policy_restricted

---
 pkg/kubelet/cm/topologymanager/policy_none.go |  8 +---
 .../cm/topologymanager/policy_none_test.go    | 47 +++++++++++++------
 .../topologymanager/policy_restricted_test.go | 20 +++++++-
 3 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/pkg/kubelet/cm/topologymanager/policy_none.go b/pkg/kubelet/cm/topologymanager/policy_none.go
index 7e5010a765d..2b79fc0c4ef 100644
--- a/pkg/kubelet/cm/topologymanager/policy_none.go
+++ b/pkg/kubelet/cm/topologymanager/policy_none.go
@@ -36,12 +36,6 @@ func (p *nonePolicy) Name() string {
 	return PolicyNone
 }
 
-func (p *nonePolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult {
-	return lifecycle.PodAdmitResult{
-		Admit: true,
-	}
-}
-
 func (p *nonePolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) {
-	return TopologyHint{}, p.canAdmitPodResult(nil)
+	return TopologyHint{}, lifecycle.PodAdmitResult{Admit: true}
 }
diff --git a/pkg/kubelet/cm/topologymanager/policy_none_test.go b/pkg/kubelet/cm/topologymanager/policy_none_test.go
index cafcaf044b6..e65d9317467 100644
--- a/pkg/kubelet/cm/topologymanager/policy_none_test.go
+++ b/pkg/kubelet/cm/topologymanager/policy_none_test.go
@@ -17,10 +17,11 @@ limitations under the License.
 package topologymanager
 
 import (
+	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
 	"testing"
 )
 
-func TestName(t *testing.T) {
+func TestPolicyNoneName(t *testing.T) {
 	tcases := []struct {
 		name     string
 		expected string
@@ -38,30 +39,46 @@ func TestName(t *testing.T) {
 	}
 }
 
-func TestPolicyNoneCanAdmitPodResult(t *testing.T) {
+func TestPolicyNoneMerge(t *testing.T) {
 	tcases := []struct {
-		name     string
-		hint     TopologyHint
-		expected bool
+		name           string
+		providersHints []map[string][]TopologyHint
+		expectedHint   TopologyHint
+		expectedAdmit  lifecycle.PodAdmitResult
 	}{
 		{
-			name:     "Preferred is set to false in topology hints",
-			hint:     TopologyHint{nil, false},
-			expected: true,
+			name:           "merged empty providers hints",
+			providersHints: []map[string][]TopologyHint{},
+			expectedHint:   TopologyHint{},
+			expectedAdmit:  lifecycle.PodAdmitResult{Admit: true},
 		},
 		{
-			name:     "Preferred is set to true in topology hints",
-			hint:     TopologyHint{nil, true},
-			expected: true,
+			name: "merge with a single provider with a single preferred resource",
+			providersHints: []map[string][]TopologyHint{
+				{
+					"resource": {{NUMANodeAffinity: NewTestBitMask(0, 1), Preferred: true}},
+				},
+			},
+			expectedHint:  TopologyHint{},
+			expectedAdmit: lifecycle.PodAdmitResult{Admit: true},
+		},
+		{
+			name: "merge with a single provider with a single non-preferred resource",
+			providersHints: []map[string][]TopologyHint{
+				{
+					"resource": {{NUMANodeAffinity: NewTestBitMask(0, 1), Preferred: false}},
+				},
+			},
+			expectedHint:  TopologyHint{},
+			expectedAdmit: lifecycle.PodAdmitResult{Admit: true},
 		},
 	}
 
 	for _, tc := range tcases {
 		policy := NewNonePolicy()
-		result := policy.(*nonePolicy).canAdmitPodResult(&tc.hint)
-
-		if result.Admit != tc.expected {
-			t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit)
+		result, admit := policy.Merge(tc.providersHints)
+		if !result.IsEqual(tc.expectedHint) || admit.Admit != tc.expectedAdmit.Admit {
+			t.Errorf("Test Case: %s: Expected merge hint to be %v, got %v", tc.name, tc.expectedHint, result)
 		}
 	}
 }
diff --git a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go b/pkg/kubelet/cm/topologymanager/policy_restricted_test.go
index f9cf840708c..d9463ea08a3 100644
--- a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go
+++ b/pkg/kubelet/cm/topologymanager/policy_restricted_test.go
@@ -20,6 +20,24 @@ import (
 	"testing"
 )
 
+func TestPolicyRestrictedName(t *testing.T) {
+	tcases := []struct {
+		name     string
+		expected string
+	}{
+		{
+			name:     "New Restricted Policy",
+			expected: "restricted",
+		},
+	}
+	for _, tc := range tcases {
+		policy := NewRestrictedPolicy([]int{0, 1})
+		if policy.Name() != tc.expected {
+			t.Errorf("Expected Policy Name to be %s, got %s", tc.expected, policy.Name())
+		}
+	}
+}
+
 func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
 	tcases := []struct {
 		name     string
@@ -58,7 +76,7 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
 	}
 }
 
-func TestRestrictedPolicyMerge(t *testing.T) {
+func TestPolicyRestrictedMerge(t *testing.T) {
 	numaNodes := []int{0, 1}
 	policy := NewRestrictedPolicy(numaNodes)