Topology Manager Implementation based on Interfaces

Co-authored-by: Kevin Klues <kklues@nvidia.com>
Co-authored-by: Conor Nolan <conor.nolan@intel.com>
Co-authored-by: Sreemanti Ghosh <sreemanti.ghosh@intel.com>

pkg/kubelet/cm/topologymanager/BUILD

@@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 go_library(
     name = "go_default_library",
     srcs = [
+        "fake_topology_manager.go",
         "policy.go",
         "policy_none.go",
         "policy_preferred.go",
@@ -15,6 +16,7 @@ go_library(
         "//pkg/kubelet/cm/topologymanager/socketmask:go_default_library",
         "//pkg/kubelet/lifecycle:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
     ],
 )
 
@@ -38,9 +40,17 @@ filegroup(
 go_test(
     name = "go_default_test",
     srcs = [
+        "fake_topology_manager_test.go",
         "policy_none_test.go",
         "policy_preferred_test.go",
         "policy_strict_test.go",
+        "topology_manager_test.go",
     ],
     embed = [":go_default_library"],
+    deps = [
+        "//pkg/kubelet/cm/topologymanager/socketmask:go_default_library",
+        "//pkg/kubelet/lifecycle:go_default_library",
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
+    ],
 )

pkg/kubelet/cm/topologymanager/fake_topology_manager.go

@@ -0,0 +1,57 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topologymanager
+
+import (
+	"k8s.io/api/core/v1"
+	"k8s.io/klog"
+	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
+)
+
+type fakeManager struct{}
+
+//NewFakeManager returns an instance of FakeManager
+func NewFakeManager() Manager {
+	klog.Infof("[fake topologymanager] NewFakeManager")
+	return &fakeManager{}
+}
+
+func (m *fakeManager) GetAffinity(podUID string, containerName string) TopologyHint {
+	klog.Infof("[fake topologymanager] GetAffinity podUID: %v container name:  %v", podUID, containerName)
+	return TopologyHint{}
+}
+
+func (m *fakeManager) AddHintProvider(h HintProvider) {
+	klog.Infof("[fake topologymanager] AddHintProvider HintProvider:  %v", h)
+}
+
+func (m *fakeManager) AddContainer(pod *v1.Pod, containerID string) error {
+	klog.Infof("[fake topologymanager] AddContainer  pod: %v container id:  %v", pod, containerID)
+	return nil
+}
+
+func (m *fakeManager) RemoveContainer(containerID string) error {
+	klog.Infof("[fake topologymanager] RemoveContainer container id:  %v", containerID)
+	return nil
+}
+
+func (m *fakeManager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult {
+	klog.Infof("[fake topologymanager] Topology Admit Handler")
+	return lifecycle.PodAdmitResult{
+		Admit: true,
+	}
+}

pkg/kubelet/cm/topologymanager/fake_topology_manager_test.go

@@ -0,0 +1,161 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topologymanager
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
+)
+
+func TestNewFakeManager(t *testing.T) {
+	fm := NewFakeManager()
+
+	if _, ok := fm.(Manager); !ok {
+		t.Errorf("Result is not Manager type")
+
+	}
+}
+
+func TestFakeGetAffinity(t *testing.T) {
+	tcases := []struct {
+		name          string
+		containerName string
+		podUID        string
+		expected      TopologyHint
+	}{
+		{
+			name:          "Case1",
+			containerName: "nginx",
+			podUID:        "0aafa4c4-38e8-11e9-bcb1-a4bf01040474",
+			expected:      TopologyHint{},
+		},
+	}
+	for _, tc := range tcases {
+		fm := fakeManager{}
+		actual := fm.GetAffinity(tc.podUID, tc.containerName)
+		if !reflect.DeepEqual(actual, tc.expected) {
+			t.Errorf("Expected Affinity in result to be %v, got %v", tc.expected, actual)
+		}
+	}
+}
+
+func TestFakeAddContainer(t *testing.T) {
+	testCases := []struct {
+		name        string
+		containerID string
+		podUID      types.UID
+	}{
+		{
+			name:        "Case1",
+			containerID: "nginx",
+			podUID:      "0aafa4c4-38e8-11e9-bcb1-a4bf01040474",
+		},
+		{
+			name:        "Case2",
+			containerID: "Busy_Box",
+			podUID:      "b3ee37fc-39a5-11e9-bcb1-a4bf01040474",
+		},
+	}
+	fm := fakeManager{}
+	mngr := manager{}
+	mngr.podMap = make(map[string]string)
+	for _, tc := range testCases {
+		pod := v1.Pod{}
+		pod.UID = tc.podUID
+		err := fm.AddContainer(&pod, tc.containerID)
+		if err != nil {
+			t.Errorf("Expected error to be nil but got: %v", err)
+
+		}
+
+	}
+}
+
+func TestFakeRemoveContainer(t *testing.T) {
+	testCases := []struct {
+		name        string
+		containerID string
+		podUID      string
+	}{
+		{
+			name:        "Case1",
+			containerID: "nginx",
+			podUID:      "0aafa4c4-38e8-11e9-bcb1-a4bf01040474",
+		},
+		{
+			name:        "Case2",
+			containerID: "Busy_Box",
+			podUID:      "b3ee37fc-39a5-11e9-bcb1-a4bf01040474",
+		},
+	}
+	fm := fakeManager{}
+	mngr := manager{}
+	mngr.podMap = make(map[string]string)
+	for _, tc := range testCases {
+		err := fm.RemoveContainer(tc.containerID)
+		if err != nil {
+			t.Errorf("Expected error to be nil but got: %v", err)
+		}
+
+	}
+
+}
+
+func TestFakeAdmit(t *testing.T) {
+	tcases := []struct {
+		name     string
+		result   lifecycle.PodAdmitResult
+		qosClass v1.PodQOSClass
+		expected bool
+	}{
+		{
+			name:     "QOSClass set as Guaranteed",
+			result:   lifecycle.PodAdmitResult{},
+			qosClass: v1.PodQOSGuaranteed,
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Burstable",
+			result:   lifecycle.PodAdmitResult{},
+			qosClass: v1.PodQOSBurstable,
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as BestEffort",
+			result:   lifecycle.PodAdmitResult{},
+			qosClass: v1.PodQOSBestEffort,
+			expected: true,
+		},
+	}
+	fm := fakeManager{}
+	for _, tc := range tcases {
+		mngr := manager{}
+		mngr.podTopologyHints = make(map[string]map[string]TopologyHint)
+		podAttr := lifecycle.PodAdmitAttributes{}
+		pod := v1.Pod{}
+		pod.Status.QOSClass = tc.qosClass
+		podAttr.Pod = &pod
+		actual := fm.Admit(&podAttr)
+		if reflect.DeepEqual(actual, tc.result) {
+			t.Errorf("Error occurred, expected Admit in result to be %v got %v", tc.result, actual.Admit)
+		}
+	}
+}

pkg/kubelet/cm/topologymanager/topology_manager.go

@@ -18,6 +18,7 @@ package topologymanager
 
 import (
 	"k8s.io/api/core/v1"
+	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask"
 	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
 )
@@ -37,6 +38,18 @@ type Manager interface {
 	Store
 }
 
+type manager struct {
+	//The list of components registered with the Manager
+	hintProviders []HintProvider
+	//Mapping of a Pods mapping of Containers and their TopologyHints
+	//Indexed by PodUID to ContainerName
+	podTopologyHints map[string]map[string]TopologyHint
+	//Mapping of PodUID to ContainerID for Adding/Removing Pods from PodTopologyHints mapping
+	podMap map[string]string
+	//Topology Manager Policy
+	policy Policy
+}
+
 //HintProvider interface is to be implemented by Hint Providers
 type HintProvider interface {
 	GetTopologyHints(pod v1.Pod, container v1.Container) []TopologyHint
@@ -54,3 +67,226 @@ type TopologyHint struct {
 	// allocation for the Container. It is set to false otherwise.
 	Preferred bool
 }
+
+var _ Manager = &manager{}
+
+//NewManager creates a new TopologyManager based on provided policy
+func NewManager(topologyPolicyName string) Manager {
+	klog.Infof("[topologymanager] Creating topology manager with %s policy", topologyPolicyName)
+	var policy Policy
+
+	switch topologyPolicyName {
+
+	case PolicyNone:
+		policy = NewNonePolicy()
+
+	case PolicyPreferred:
+		policy = NewPreferredPolicy()
+
+	case PolicyStrict:
+		policy = NewStrictPolicy()
+
+	default:
+		klog.Errorf("[topologymanager] Unknown policy %s, using default policy %s", topologyPolicyName, PolicyNone)
+		policy = NewNonePolicy()
+	}
+
+	var hp []HintProvider
+	pth := make(map[string]map[string]TopologyHint)
+	pm := make(map[string]string)
+	manager := &manager{
+		hintProviders:    hp,
+		podTopologyHints: pth,
+		podMap:           pm,
+		policy:           policy,
+	}
+
+	return manager
+}
+
+func (m *manager) GetAffinity(podUID string, containerName string) TopologyHint {
+	return m.podTopologyHints[podUID][containerName]
+}
+
+// Iterate over all permutations of hints in 'allProviderHints [][]TopologyHint'.
+//
+// This procedure is implemented as a recursive function over the set of hints
+// in 'allproviderHints[i]'. It applies the function 'callback' to each
+// permutation as it is found. It is the equivalent of:
+//
+// for i := 0; i < len(providerHints[0]); i++
+//     for j := 0; j < len(providerHints[1]); j++
+//         for k := 0; k < len(providerHints[2]); k++
+//             ...
+//             for z := 0; z < len(providerHints[-1]); z++
+//                 permutation := []TopologyHint{
+//                     providerHints[0][i],
+//                     providerHints[1][j],
+//                     providerHints[2][k],
+//                     ...
+//                     provideryHints[-1][z]
+//                 }
+//                 callback(permutation)
+func (m *manager) iterateAllProviderTopologyHints(allProviderHints [][]TopologyHint, callback func([]TopologyHint)) {
+	// Internal helper function to accumulate the permutation before calling the callback.
+	var iterate func(i int, accum []TopologyHint)
+	iterate = func(i int, accum []TopologyHint) {
+		// Base case: we have looped through all providers and have a full permutation.
+		if i == len(allProviderHints) {
+			callback(accum)
+			return
+		}
+
+		// Loop through all hints for provider 'i', and recurse to build the
+		// the permutation of this hint with all hints from providers 'i++'.
+		for j := range allProviderHints[i] {
+			iterate(i+1, append(accum, allProviderHints[i][j]))
+		}
+	}
+	iterate(0, []TopologyHint{})
+}
+
+// Merge the hints from all hint providers to find the best one.
+func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) TopologyHint {
+	// Set the default hint to return from this function as an any-socket
+	// affinity with an unpreferred allocation. This will only be returned if
+	// no better hint can be found when merging hints from each hint provider.
+	defaultAffinity, _ := socketmask.NewSocketMask()
+	defaultAffinity.Fill()
+	defaultHint := TopologyHint{defaultAffinity, false}
+
+	// Loop through all hint providers and save an accumulated list of the
+	// hints returned by each hint provider. If no hints are provided, assume
+	// that provider has no preference for topology-aware allocation.
+	var allProviderHints [][]TopologyHint
+	for _, provider := range m.hintProviders {
+		// Get the TopologyHints from a provider.
+		hints := provider.GetTopologyHints(pod, container)
+
+		// If hints is nil, overwrite 'hints' with a preferred any-socket affinity.
+		if hints == nil || len(hints) == 0 {
+			klog.Infof("[topologymanager] Hint Provider has no preference for socket affinity")
+			affinity, _ := socketmask.NewSocketMask()
+			affinity.Fill()
+			hints = []TopologyHint{{affinity, true}}
+		}
+
+		// Accumulate the sorted hints into a [][]TopologyHint slice
+		allProviderHints = append(allProviderHints, hints)
+	}
+
+	// Iterate over all permutations of hints in 'allProviderHints'. Merge the
+	// hints in each permutation by taking the bitwise-and of their affinity masks.
+	// Return the hint with the narrowest SocketAffinity of all merged
+	// permutations that have at least one socket set. If no merged mask can be
+	// found that has at least one socket set, return the 'defaultHint'.
+	bestHint := defaultHint
+	m.iterateAllProviderTopologyHints(allProviderHints, func(permutation []TopologyHint) {
+		// Get the SocketAffinity from each hint in the permutation and see if any
+		// of them encode unpreferred allocations.
+		preferred := true
+		var socketAffinities []socketmask.SocketMask
+		for _, hint := range permutation {
+			// Only consider hints that have an actual SocketAffinity set.
+			if hint.SocketAffinity != nil {
+				if !hint.Preferred {
+					preferred = false
+				}
+				socketAffinities = append(socketAffinities, hint.SocketAffinity)
+			}
+		}
+
+		// Merge the affinities using a bitwise-and operation.
+		mergedAffinity, _ := socketmask.NewSocketMask()
+		mergedAffinity.Fill()
+		mergedAffinity.And(socketAffinities...)
+
+		// Build a mergedHintfrom the merged affinity mask, indicating if an
+		// preferred allocation was used to generate the affinity mask or not.
+		mergedHint := TopologyHint{mergedAffinity, preferred}
+
+		// Only consider mergedHints that result in a SocketAffinity > 0 to
+		// replace the current bestHint.
+		if mergedHint.SocketAffinity.Count() == 0 {
+			return
+		}
+
+		// If the current bestHint is non-preferred and the new mergedHint is
+		// preferred, always choose the preferred hint over the non-preferred one.
+		if mergedHint.Preferred && !bestHint.Preferred {
+			bestHint = mergedHint
+			return
+		}
+
+		// If the current bestHint is preferred and the new mergedHint is
+		// non-preferred, never update bestHint, regardless of mergedHint's
+		// narowness.
+		if !mergedHint.Preferred && bestHint.Preferred {
+			return
+		}
+
+		// If mergedHint and bestHint has the same preference, only consider
+		// mergedHints that have a narrower SocketAffinity than the
+		// SocketAffinity in the current bestHint.
+		if !mergedHint.SocketAffinity.IsNarrowerThan(bestHint.SocketAffinity) {
+			return
+		}
+
+		// In all other cases, update bestHint to the current mergedHint
+		bestHint = mergedHint
+	})
+
+	klog.Infof("[topologymanager] ContainerTopologyHint: %v", bestHint)
+
+	return bestHint
+}
+
+func (m *manager) AddHintProvider(h HintProvider) {
+	m.hintProviders = append(m.hintProviders, h)
+}
+
+func (m *manager) AddContainer(pod *v1.Pod, containerID string) error {
+	m.podMap[containerID] = string(pod.UID)
+	return nil
+}
+
+func (m *manager) RemoveContainer(containerID string) error {
+	podUIDString := m.podMap[containerID]
+	delete(m.podTopologyHints, podUIDString)
+	delete(m.podMap, containerID)
+	klog.Infof("[topologymanager] RemoveContainer - Container ID: %v podTopologyHints: %v", containerID, m.podTopologyHints)
+	return nil
+}
+
+func (m *manager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult {
+	klog.Infof("[topologymanager] Topology Admit Handler")
+	if m.policy.Name() == "none" {
+		klog.Infof("[topologymanager] Skipping calculate topology affinity as policy: none")
+		return lifecycle.PodAdmitResult{
+			Admit: true,
+		}
+	}
+	pod := attrs.Pod
+	c := make(map[string]TopologyHint)
+	klog.Infof("[topologymanager] Pod QoS Level: %v", pod.Status.QOSClass)
+
+	if pod.Status.QOSClass == v1.PodQOSGuaranteed {
+		for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) {
+			result := m.calculateAffinity(*pod, container)
+			admitPod := m.policy.CanAdmitPodResult(result.Preferred)
+			if admitPod.Admit == false {
+				return admitPod
+			}
+			c[container.Name] = result
+		}
+		m.podTopologyHints[string(pod.UID)] = c
+		klog.Infof("[topologymanager] Topology Affinity for Pod: %v are %v", pod.UID, m.podTopologyHints[string(pod.UID)])
+
+	} else {
+		klog.Infof("[topologymanager] Topology Manager only affinitises Guaranteed pods.")
+	}
+
+	return lifecycle.PodAdmitResult{
+		Admit: true,
+	}
+}

pkg/kubelet/cm/topologymanager/topology_manager_test.go

@@ -0,0 +1,804 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package topologymanager
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask"
+	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
+)
+
+func NewTestSocketMask(sockets ...int) socketmask.SocketMask {
+	s, _ := socketmask.NewSocketMask(sockets...)
+	return s
+}
+
+func NewTestSocketMaskFull() socketmask.SocketMask {
+	s, _ := socketmask.NewSocketMask()
+	s.Fill()
+	return s
+}
+
+func TestNewManager(t *testing.T) {
+	tcases := []struct {
+		name       string
+		policyType string
+	}{
+		{
+			name:       "Policy is set preferred",
+			policyType: "preferred",
+		},
+		{
+			name:       "Policy is set to strict",
+			policyType: "strict",
+		},
+		{
+			name:       "Policy is set to unknown",
+			policyType: "unknown",
+		},
+	}
+
+	for _, tc := range tcases {
+		mngr := NewManager(tc.policyType)
+
+		if _, ok := mngr.(Manager); !ok {
+			t.Errorf("result is not Manager type")
+		}
+	}
+}
+
+type mockHintProvider struct {
+	th []TopologyHint
+}
+
+func (m *mockHintProvider) GetTopologyHints(pod v1.Pod, container v1.Container) []TopologyHint {
+	return m.th
+}
+
+func TestGetAffinity(t *testing.T) {
+	tcases := []struct {
+		name          string
+		containerName string
+		podUID        string
+		expected      TopologyHint
+	}{
+		{
+			name:          "case1",
+			containerName: "nginx",
+			podUID:        "0aafa4c4-38e8-11e9-bcb1-a4bf01040474",
+			expected:      TopologyHint{},
+		},
+	}
+	for _, tc := range tcases {
+		mngr := manager{}
+		actual := mngr.GetAffinity(tc.podUID, tc.containerName)
+		if !reflect.DeepEqual(actual, tc.expected) {
+			t.Errorf("Expected Affinity in result to be %v, got %v", tc.expected, actual)
+		}
+	}
+}
+
+func TestCalculateAffinity(t *testing.T) {
+	tcases := []struct {
+		name     string
+		hp       []HintProvider
+		expected TopologyHint
+	}{
+		{
+			name: "TopologyHint not set",
+			hp:   []HintProvider{},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMaskFull(),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "HintProvider returns empty non-nil []TopologyHint",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMaskFull(),
+				Preferred:      true,
+			},
+		},
+
+		{
+			name: "Single TopologyHint with Preferred as true and SocketAffinity as nil",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: nil,
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMaskFull(),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Single TopologyHint with Preferred as false and SocketAffinity as nil",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: nil,
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMaskFull(),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 hint each, same mask, both preferred 1/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(0),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 hint each, same mask, both preferred 2/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(1),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(0),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(1),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 hint each, no common mask",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMaskFull(),
+				Preferred:      false,
+			},
+		},
+		{
+			name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(0),
+				Preferred:      false,
+			},
+		},
+		{
+			name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(1),
+				Preferred:      false,
+			},
+		},
+		{
+			name: "Two providers, 1 no hints, 1 single hint preferred 1/2",
+			hp: []HintProvider{
+				&mockHintProvider{},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(0),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 no hints, 1 single hint preferred 2/2",
+			hp: []HintProvider{
+				&mockHintProvider{},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(1),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 with 2 hints, 1 with single hint matching 1/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(0),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 with 2 hints, 1 with single hint matching 2/2",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(1),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(0),
+				Preferred:      false,
+			},
+		},
+		{
+			name: "Two providers, both with 2 hints, matching narrower preferred hint from both",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(0),
+				Preferred:      true,
+			},
+		},
+		{
+			name: "Ensure less narrow preferred hints are chosen over narrower non-preferred hints",
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				SocketAffinity: NewTestSocketMask(1),
+				Preferred:      true,
+			},
+		},
+	}
+
+	for _, tc := range tcases {
+		mngr := manager{}
+		mngr.hintProviders = tc.hp
+		actual := mngr.calculateAffinity(v1.Pod{}, v1.Container{})
+		if !actual.SocketAffinity.IsEqual(tc.expected.SocketAffinity) {
+			t.Errorf("Expected SocketAffinity in result to be %v, got %v", tc.expected.SocketAffinity, actual.SocketAffinity)
+		}
+		if actual.Preferred != tc.expected.Preferred {
+			t.Errorf("Expected Affinity preference in result to be %v, got %v", tc.expected.Preferred, actual.Preferred)
+		}
+	}
+}
+
+func TestAddContainer(t *testing.T) {
+	testCases := []struct {
+		name        string
+		containerID string
+		podUID      types.UID
+	}{
+		{
+			name:        "Case1",
+			containerID: "nginx",
+			podUID:      "0aafa4c4-38e8-11e9-bcb1-a4bf01040474",
+		},
+		{
+			name:        "Case2",
+			containerID: "Busy_Box",
+			podUID:      "b3ee37fc-39a5-11e9-bcb1-a4bf01040474",
+		},
+	}
+	mngr := manager{}
+	mngr.podMap = make(map[string]string)
+	for _, tc := range testCases {
+		pod := v1.Pod{}
+		pod.UID = tc.podUID
+		err := mngr.AddContainer(&pod, tc.containerID)
+		if err != nil {
+			t.Errorf("Expected error to be nil but got: %v", err)
+		}
+		if val, ok := mngr.podMap[tc.containerID]; ok {
+			if reflect.DeepEqual(val, pod.UID) {
+				t.Errorf("Error occurred")
+			}
+		} else {
+			t.Errorf("Error occurred, Pod not added to podMap")
+		}
+	}
+}
+
+func TestRemoveContainer(t *testing.T) {
+	testCases := []struct {
+		name        string
+		containerID string
+		podUID      types.UID
+	}{
+		{
+			name:        "Case1",
+			containerID: "nginx",
+			podUID:      "0aafa4c4-38e8-11e9-bcb1-a4bf01040474",
+		},
+		{
+			name:        "Case2",
+			containerID: "Busy_Box",
+			podUID:      "b3ee37fc-39a5-11e9-bcb1-a4bf01040474",
+		},
+	}
+	var len1, len2 int
+	mngr := manager{}
+	mngr.podMap = make(map[string]string)
+	for _, tc := range testCases {
+		mngr.podMap[tc.containerID] = string(tc.podUID)
+		len1 = len(mngr.podMap)
+		err := mngr.RemoveContainer(tc.containerID)
+		len2 = len(mngr.podMap)
+		if err != nil {
+			t.Errorf("Expected error to be nil but got: %v", err)
+		}
+		if len1-len2 != 1 {
+			t.Errorf("Remove Pod resulted in error")
+		}
+	}
+
+}
+func TestAddHintProvider(t *testing.T) {
+	var len1 int
+	tcases := []struct {
+		name string
+		hp   []HintProvider
+	}{
+		{
+			name: "Add HintProvider",
+			hp: []HintProvider{
+				&mockHintProvider{},
+			},
+		},
+	}
+	mngr := manager{}
+	for _, tc := range tcases {
+		mngr.hintProviders = []HintProvider{}
+		len1 = len(mngr.hintProviders)
+		mngr.AddHintProvider(tc.hp[0])
+	}
+	len2 := len(mngr.hintProviders)
+	if len2-len1 != 1 {
+		t.Errorf("error")
+	}
+}
+
+func TestAdmit(t *testing.T) {
+	tcases := []struct {
+		name     string
+		result   lifecycle.PodAdmitResult
+		qosClass v1.PodQOSClass
+		policy   Policy
+		hp       []HintProvider
+		expected bool
+	}{
+		{
+			name:     "QOSClass set as BestEffort. None Policy. No Hints.",
+			qosClass: v1.PodQOSBestEffort,
+			policy:   NewNonePolicy(),
+			hp:       []HintProvider{},
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Guaranteed. None Policy. No Hints.",
+			qosClass: v1.PodQOSGuaranteed,
+			policy:   NewNonePolicy(),
+			hp:       []HintProvider{},
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Guaranteed. Preferred Policy. Preferred Affinity.",
+			qosClass: v1.PodQOSGuaranteed,
+			policy:   NewPreferredPolicy(),
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Guaranteed. Preferred Policy. More than one Preferred Affinity.",
+			qosClass: v1.PodQOSGuaranteed,
+			policy:   NewPreferredPolicy(),
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Guaranteed. Preferred Policy. No Preferred Affinity.",
+			qosClass: v1.PodQOSGuaranteed,
+			policy:   NewPreferredPolicy(),
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Guaranteed. Strict Policy. Preferred Affinity.",
+			qosClass: v1.PodQOSGuaranteed,
+			policy:   NewStrictPolicy(),
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Guaranteed. Strict Policy. More than one Preferred affinity.",
+			qosClass: v1.PodQOSGuaranteed,
+			policy:   NewStrictPolicy(),
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(1),
+							Preferred:      true,
+						},
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name:     "QOSClass set as Guaranteed. Strict Policy. No Preferred affinity.",
+			qosClass: v1.PodQOSGuaranteed,
+			policy:   NewStrictPolicy(),
+			hp: []HintProvider{
+				&mockHintProvider{
+					[]TopologyHint{
+						{
+							SocketAffinity: NewTestSocketMask(0, 1),
+							Preferred:      false,
+						},
+					},
+				},
+			},
+			expected: false,
+		},
+	}
+	for _, tc := range tcases {
+		man := manager{}
+		man.policy = tc.policy
+		man.podTopologyHints = make(map[string]map[string]TopologyHint)
+		man.hintProviders = tc.hp
+		pod := &v1.Pod{
+			Spec: v1.PodSpec{
+				Containers: []v1.Container{
+					{
+						Resources: v1.ResourceRequirements{},
+					},
+				},
+			},
+		}
+		podAttr := lifecycle.PodAdmitAttributes{}
+		pod.Status.QOSClass = tc.qosClass
+		podAttr.Pod = pod
+		actual := man.Admit(&podAttr)
+		if actual.Admit != tc.expected {
+			t.Errorf("Error occurred, expected Admit in result to be %v got %v", tc.expected, actual.Admit)
+		}
+	}
+}