From b5ca4989e3a691e438696e7e63652817b4791867 Mon Sep 17 00:00:00 2001 From: nolancon Date: Mon, 9 Dec 2019 08:27:38 +0000 Subject: [PATCH] Update unit tests: - Update filterHints test to reflect changes in previous commit. - Some common test cases achieve differing expected results based on policy due to independent merge strategies. These cases are moved into individual policy based test functions. --- .../policy_single_numa_node_test.go | 76 ++++- pkg/kubelet/cm/topologymanager/policy_test.go | 275 ++++++++++++------ 2 files changed, 254 insertions(+), 97 deletions(-) diff --git a/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go b/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go index 7847716f8fa..b0173e424f1 100644 --- a/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go @@ -59,18 +59,45 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) { name string allResources [][]TopologyHint expectedResources [][]TopologyHint + expectedExists bool }{ { name: "filter empty resources", allResources: [][]TopologyHint{}, expectedResources: [][]TopologyHint(nil), + expectedExists: false, }, { - name: "filter hints with nil socket mask", + name: "filter hints with nil socket mask, preferred true", + allResources: [][]TopologyHint{ + { + {NUMANodeAffinity: nil, Preferred: true}, + }, + }, + expectedResources: [][]TopologyHint{ + []TopologyHint(nil), + }, + expectedExists: true, + }, + + { + name: "filter hints with nil socket mask, preferred false", allResources: [][]TopologyHint{ { {NUMANodeAffinity: nil, Preferred: false}, }, + }, + expectedResources: [][]TopologyHint{ + []TopologyHint(nil), + }, + expectedExists: false, + }, + { + name: "filter hints with nil socket mask, preferred both true", + allResources: [][]TopologyHint{ + { + {NUMANodeAffinity: nil, Preferred: true}, + }, { {NUMANodeAffinity: nil, Preferred: true}, }, @@ -79,6 +106,40 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) { []TopologyHint(nil), []TopologyHint(nil), }, + expectedExists: true, + }, + { + name: "filter hints with nil socket mask, preferred both false", + allResources: [][]TopologyHint{ + { + {NUMANodeAffinity: nil, Preferred: false}, + }, + { + {NUMANodeAffinity: nil, Preferred: false}, + }, + }, + expectedResources: [][]TopologyHint{ + []TopologyHint(nil), + []TopologyHint(nil), + }, + expectedExists: false, + }, + + { + name: "filter hints with nil socket mask, preferred true and false", + allResources: [][]TopologyHint{ + { + {NUMANodeAffinity: nil, Preferred: true}, + }, + { + {NUMANodeAffinity: nil, Preferred: false}, + }, + }, + expectedResources: [][]TopologyHint{ + []TopologyHint(nil), + []TopologyHint(nil), + }, + expectedExists: false, }, { name: "filter hints with nil socket mask", @@ -100,6 +161,7 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) { {NUMANodeAffinity: NewTestBitMask(1), Preferred: true}, }, }, + expectedExists: false, }, { name: "filter hints with empty resource socket mask", @@ -118,6 +180,7 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) { }, []TopologyHint(nil), }, + expectedExists: false, }, { name: "filter hints with wide sockemask", @@ -146,22 +209,25 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) { {NUMANodeAffinity: NewTestBitMask(0), Preferred: true}, {NUMANodeAffinity: NewTestBitMask(1), Preferred: true}, }, - { - {NUMANodeAffinity: NewTestBitMask(3), Preferred: false}, - }, + []TopologyHint(nil), []TopologyHint(nil), }, + expectedExists: false, }, } numaNodes := []int{0, 1, 2, 3} for _, tc := range tcases { policy := NewSingleNumaNodePolicy(numaNodes) - actual := policy.(*singleNumaNodePolicy).filterHints(tc.allResources) + actual, exists := policy.(*singleNumaNodePolicy).filterHints(tc.allResources) if !reflect.DeepEqual(tc.expectedResources, actual) { t.Errorf("Test Case: %s", tc.name) t.Errorf("Expected result to be %v, got %v", tc.expectedResources, actual) } + if !reflect.DeepEqual(tc.expectedResources, actual) { + t.Errorf("Test Case: %s", tc.name) + t.Errorf("Expected result to be %v, got %v", tc.expectedExists, exists) + } } } diff --git a/pkg/kubelet/cm/topologymanager/policy_test.go b/pkg/kubelet/cm/topologymanager/policy_test.go index 5e8a61b6394..9e32c6989b2 100644 --- a/pkg/kubelet/cm/topologymanager/policy_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_test.go @@ -203,64 +203,6 @@ func commonPolicyMergeTestCases(numaNodes []int) []policyMergeTestCase { Preferred: false, }, }, - { - name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: false, - }, - }, - { - name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: false, - }, - }, { name: "Two providers, 1 no hints, 1 single hint preferred 1/2", hp: []HintProvider{ @@ -367,39 +309,6 @@ func commonPolicyMergeTestCases(numaNodes []int) []policyMergeTestCase { Preferred: true, }, }, - { - name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: false, - }, - }, { name: "Two providers, both with 2 hints, matching narrower preferred hint from both", hp: []HintProvider{ @@ -578,6 +487,97 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase Preferred: true, }, }, + { + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, + { + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: false, + }, + }, + { + name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, } } @@ -612,7 +612,7 @@ func (p *singleNumaNodePolicy) mergeTestCases(numaNodes []int) []policyMergeTest }, }, expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), + NUMANodeAffinity: NewTestBitMask(numaNodes...), Preferred: false, }, }, @@ -646,6 +646,97 @@ func (p *singleNumaNodePolicy) mergeTestCases(numaNodes []int) []policyMergeTest Preferred: true, }, }, + { + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: false, + }, + }, + { + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: false, + }, + }, + { + name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: false, + }, + }, } }