diff --git a/pkg/kubelet/cm/topologymanager/policy.go b/pkg/kubelet/cm/topologymanager/policy.go index 7ffa0a79f10..05f46714eab 100644 --- a/pkg/kubelet/cm/topologymanager/policy.go +++ b/pkg/kubelet/cm/topologymanager/policy.go @@ -24,6 +24,7 @@ import ( type Policy interface { //Returns Policy Name Name() string - //Returns Pod Admit Handler Response based on hints and policy type - CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult + // Returns a merged TopologyHint based on input from hint providers + // and a Pod Admit Handler Response based on hints and policy type + Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) } diff --git a/pkg/kubelet/cm/topologymanager/policy_best_effort.go b/pkg/kubelet/cm/topologymanager/policy_best_effort.go index c0611d31a10..97c61f031da 100644 --- a/pkg/kubelet/cm/topologymanager/policy_best_effort.go +++ b/pkg/kubelet/cm/topologymanager/policy_best_effort.go @@ -39,8 +39,14 @@ func (p *bestEffortPolicy) Name() string { return PolicyBestEffort } -func (p *bestEffortPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +func (p *bestEffortPolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { return lifecycle.PodAdmitResult{ Admit: true, } } + +func (p *bestEffortPolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) { + hint := mergeProvidersHints(p, p.numaNodes, providersHints) + admit := p.canAdmitPodResult(&hint) + return hint, admit +} diff --git a/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go b/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go index 9505c72f89f..1459a823bc3 100644 --- a/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go @@ -41,7 +41,7 @@ func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) { for _, tc := range tcases { numaNodes := []int{0, 1} policy := NewBestEffortPolicy(numaNodes) - result := policy.CanAdmitPodResult(&tc.hint) + result := policy.(*bestEffortPolicy).canAdmitPodResult(&tc.hint) if result.Admit != tc.expected { t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit) diff --git a/pkg/kubelet/cm/topologymanager/policy_none.go b/pkg/kubelet/cm/topologymanager/policy_none.go index aacdf19aa10..7e5010a765d 100644 --- a/pkg/kubelet/cm/topologymanager/policy_none.go +++ b/pkg/kubelet/cm/topologymanager/policy_none.go @@ -36,8 +36,12 @@ func (p *nonePolicy) Name() string { return PolicyNone } -func (p *nonePolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +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) +} diff --git a/pkg/kubelet/cm/topologymanager/policy_none_test.go b/pkg/kubelet/cm/topologymanager/policy_none_test.go index 76f46e490b4..cafcaf044b6 100644 --- a/pkg/kubelet/cm/topologymanager/policy_none_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_none_test.go @@ -58,7 +58,7 @@ func TestPolicyNoneCanAdmitPodResult(t *testing.T) { for _, tc := range tcases { policy := NewNonePolicy() - result := policy.CanAdmitPodResult(&tc.hint) + 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) diff --git a/pkg/kubelet/cm/topologymanager/policy_restricted.go b/pkg/kubelet/cm/topologymanager/policy_restricted.go index 02e188a35b8..cc522980c0e 100644 --- a/pkg/kubelet/cm/topologymanager/policy_restricted.go +++ b/pkg/kubelet/cm/topologymanager/policy_restricted.go @@ -38,7 +38,7 @@ func (p *restrictedPolicy) Name() string { return PolicyRestricted } -func (p *restrictedPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +func (p *restrictedPolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { if !hint.Preferred { return lifecycle.PodAdmitResult{ Admit: false, @@ -50,3 +50,9 @@ func (p *restrictedPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAd Admit: true, } } + +func (p *restrictedPolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) { + hint := mergeProvidersHints(p, p.numaNodes, providersHints) + admit := p.canAdmitPodResult(&hint) + return hint, admit +} diff --git a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go b/pkg/kubelet/cm/topologymanager/policy_restricted_test.go index 2543d8af0c3..11119a0400c 100644 --- a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_restricted_test.go @@ -41,7 +41,7 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) { for _, tc := range tcases { numaNodes := []int{0, 1} policy := NewRestrictedPolicy(numaNodes) - result := policy.CanAdmitPodResult(&tc.hint) + result := policy.(*restrictedPolicy).canAdmitPodResult(&tc.hint) if result.Admit != tc.expected { t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit) diff --git a/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go b/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go index 7173ef94c5d..6f29e1c27b7 100644 --- a/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go +++ b/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go @@ -39,7 +39,7 @@ func (p *singleNumaNodePolicy) Name() string { return PolicySingleNumaNode } -func (p *singleNumaNodePolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +func (p *singleNumaNodePolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { if !hint.Preferred { return lifecycle.PodAdmitResult{ Admit: false, @@ -51,3 +51,9 @@ func (p *singleNumaNodePolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.P Admit: true, } } + +func (p *singleNumaNodePolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) { + hint := mergeProvidersHints(p, p.numaNodes, providersHints) + admit := p.canAdmitPodResult(&hint) + return hint, admit +} 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 594fef71c6b..cdc895295fc 100644 --- a/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go @@ -36,7 +36,7 @@ func TestPolicySingleNumaNodeCanAdmitPodResult(t *testing.T) { for _, tc := range tcases { numaNodes := []int{0, 1} policy := NewSingleNumaNodePolicy(numaNodes) - result := policy.CanAdmitPodResult(&tc.hint) + result := policy.(*singleNumaNodePolicy).canAdmitPodResult(&tc.hint) if result.Admit != tc.expected { t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit) diff --git a/pkg/kubelet/cm/topologymanager/topology_manager.go b/pkg/kubelet/cm/topologymanager/topology_manager.go index f8288d8cef8..3600d3700db 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager.go +++ b/pkg/kubelet/cm/topologymanager/topology_manager.go @@ -63,8 +63,6 @@ type manager struct { podMap map[string]string //Topology Manager Policy policy Policy - //List of NUMA Nodes available on the underlying machine - numaNodes []int } // HintProvider is an interface for components that want to collaborate to @@ -157,7 +155,6 @@ func NewManager(numaNodeInfo cputopology.NUMANodeInfo, topologyPolicyName string podTopologyHints: pth, podMap: pm, policy: policy, - numaNodes: numaNodes, } return manager, nil @@ -325,11 +322,11 @@ func mergeProvidersHints(policy Policy, numaNodes []int, providersHints []map[st } // Collect Hints from hint providers and pass to policy to retrieve the best one. -func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) TopologyHint { +func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) (TopologyHint, lifecycle.PodAdmitResult) { providersHints := m.accumulateProvidersHints(pod, container) - bestHint := mergeProvidersHints(m.policy, m.numaNodes, providersHints) + bestHint, admit := m.policy.Merge(providersHints) klog.Infof("[topologymanager] ContainerTopologyHint: %v", bestHint) - return bestHint + return bestHint, admit } func (m *manager) AddHintProvider(h HintProvider) { @@ -361,8 +358,7 @@ func (m *manager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitR c := make(map[string]TopologyHint) for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) { - result := m.calculateAffinity(*pod, container) - admitPod := m.policy.CanAdmitPodResult(&result) + result, admitPod := m.calculateAffinity(*pod, container) if !admitPod.Admit { return admitPod } diff --git a/pkg/kubelet/cm/topologymanager/topology_manager_test.go b/pkg/kubelet/cm/topologymanager/topology_manager_test.go index 0a79d10d38f..49c4e1cea88 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager_test.go +++ b/pkg/kubelet/cm/topologymanager/topology_manager_test.go @@ -114,15 +114,17 @@ func TestCalculateAffinity(t *testing.T) { policy Policy }{ { - name: "TopologyHint not set", - hp: []HintProvider{}, + name: "TopologyHint not set", + policy: NewBestEffortPolicy(numaNodes), + hp: []HintProvider{}, expected: TopologyHint{ NUMANodeAffinity: NewTestBitMask(numaNodes...), Preferred: true, }, }, { - name: "HintProvider returns empty non-nil map[string][]TopologyHint", + name: "HintProvider returns empty non-nil map[string][]TopologyHint", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{}, @@ -134,7 +136,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "HintProvider returns -nil map[string][]TopologyHint from provider", + name: "HintProvider returns -nil map[string][]TopologyHint from provider", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -148,7 +151,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "HintProvider returns empty non-nil map[string][]TopologyHint from provider", + name: "HintProvider returns empty non-nil map[string][]TopologyHint from provider", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -162,7 +166,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Single TopologyHint with Preferred as true and NUMANodeAffinity as nil", + name: "Single TopologyHint with Preferred as true and NUMANodeAffinity as nil", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -181,7 +186,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Single TopologyHint with Preferred as false and NUMANodeAffinity as nil", + name: "Single TopologyHint with Preferred as false and NUMANodeAffinity as nil", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -200,7 +206,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 hint each, same mask, both preferred 1/2", + name: "Two providers, 1 hint each, same mask, both preferred 1/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -229,7 +236,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 hint each, same mask, both preferred 2/2", + name: "Two providers, 1 hint each, same mask, both preferred 2/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -258,7 +266,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", + name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -287,7 +296,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", + name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -316,7 +326,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 hint each, no common mask", + name: "Two providers, 1 hint each, no common mask", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -345,7 +356,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2", + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -374,7 +386,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2", + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -403,7 +416,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 no hints, 1 single hint preferred 1/2", + name: "Two providers, 1 no hints, 1 single hint preferred 1/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{}, &mockHintProvider{ @@ -423,7 +437,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 no hints, 1 single hint preferred 2/2", + name: "Two providers, 1 no hints, 1 single hint preferred 2/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{}, &mockHintProvider{ @@ -443,7 +458,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 with 2 hints, 1 with single hint matching 1/2", + name: "Two providers, 1 with 2 hints, 1 with single hint matching 1/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -476,7 +492,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 with 2 hints, 1 with single hint matching 2/2", + name: "Two providers, 1 with 2 hints, 1 with single hint matching 2/2", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -509,7 +526,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching", + name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -542,7 +560,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Two providers, both with 2 hints, matching narrower preferred hint from both", + name: "Two providers, both with 2 hints, matching narrower preferred hint from both", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -579,7 +598,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Ensure less narrow preferred hints are chosen over narrower non-preferred hints", + name: "Ensure less narrow preferred hints are chosen over narrower non-preferred hints", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -620,7 +640,8 @@ func TestCalculateAffinity(t *testing.T) { }, }, { - name: "Multiple resources, same provider", + name: "Multiple resources, same provider", + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -727,9 +748,8 @@ func TestCalculateAffinity(t *testing.T) { mngr := manager{ policy: tc.policy, hintProviders: tc.hp, - numaNodes: numaNodes, } - actual := mngr.calculateAffinity(v1.Pod{}, v1.Container{}) + actual, _ := mngr.calculateAffinity(v1.Pod{}, v1.Container{}) if !actual.NUMANodeAffinity.IsEqual(tc.expected.NUMANodeAffinity) { t.Errorf("Expected NUMANodeAffinity in result to be %v, got %v", tc.expected.NUMANodeAffinity, actual.NUMANodeAffinity) } @@ -1107,7 +1127,6 @@ func TestAdmit(t *testing.T) { policy: tc.policy, podTopologyHints: make(map[string]map[string]TopologyHint), hintProviders: tc.hp, - numaNodes: numaNodes, } pod := &v1.Pod{