Merge pull request kubernetes#81722 from klueska/upstream-add-socket-awreness-to-topologymanager

Add NUMA Node awareness to the TopologyManager

Author: k8s-ci-robot

pkg/kubelet/cm/container_manager_linux.go

@@ -289,6 +289,7 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I
 
 	if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.TopologyManager) {
 		cm.topologyManager, err = topologymanager.NewManager(
+			numaNodeInfo,
 			nodeConfig.ExperimentalTopologyManagerPolicy,
 		)
 

pkg/kubelet/cm/topologymanager/BUILD

@@ -13,6 +13,7 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/kubelet/cm/cpumanager/topology:go_default_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",

pkg/kubelet/cm/topologymanager/topology_manager.go

@@ -21,10 +21,23 @@ import (
 
 	"k8s.io/api/core/v1"
 	"k8s.io/klog"
+	cputopology "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/topology"
 	"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask"
 	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
 )
 
+const (
+	// maxAllowableNUMANodes specifies the maximum number of NUMA Nodes that
+	// the TopologyManager supports on the underlying machine.
+	//
+	// At present, having more than this number of NUMA Nodes will result in a
+	// state explosion when trying to enumerate possible NUMAAffinity masks and
+	// generate hints for them. As such, if more NUMA Nodes than this are
+	// present on a machine and the TopologyManager is enabled, an error will
+	// be returned and the TopologyManager will not be loaded.
+	maxAllowableNUMANodes = 8
+)
+
 //Manager interface provides methods for Kubelet to manage pod topology hints
 type Manager interface {
 	//Manager implements pod admit handler interface
@@ -50,6 +63,8 @@ type manager struct {
 	podMap map[string]string
 	//Topology Manager Policy
 	policy Policy
+	//List of NUMA Nodes available on the underlying machine
+	numaNodes []int
 }
 
 //HintProvider interface is to be implemented by Hint Providers
@@ -73,7 +88,7 @@ type TopologyHint struct {
 var _ Manager = &manager{}
 
 //NewManager creates a new TopologyManager based on provided policy
-func NewManager(topologyPolicyName string) (Manager, error) {
+func NewManager(numaNodeInfo cputopology.NUMANodeInfo, topologyPolicyName string) (Manager, error) {
 	klog.Infof("[topologymanager] Creating topology manager with %s policy", topologyPolicyName)
 	var policy Policy
 
@@ -92,6 +107,15 @@ func NewManager(topologyPolicyName string) (Manager, error) {
 		return nil, fmt.Errorf("unknown policy: \"%s\"", topologyPolicyName)
 	}
 
+	var numaNodes []int
+	for node := range numaNodeInfo {
+		numaNodes = append(numaNodes, node)
+	}
+
+	if len(numaNodes) > maxAllowableNUMANodes {
+		return nil, fmt.Errorf("unsupported on machines with more than %v NUMA Nodes", maxAllowableNUMANodes)
+	}
+
 	var hp []HintProvider
 	pth := make(map[string]map[string]TopologyHint)
 	pm := make(map[string]string)
@@ -100,6 +124,7 @@ func NewManager(topologyPolicyName string) (Manager, error) {
 		podTopologyHints: pth,
 		podMap:           pm,
 		policy:           policy,
+		numaNodes:        numaNodes,
 	}
 
 	return manager, nil
@@ -149,12 +174,9 @@ func (m *manager) iterateAllProviderTopologyHints(allProviderHints [][]TopologyH
 
 // 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-numa
-	// 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}
+	// Set the default affinity as an any-numa affinity containing the list
+	// of NUMA Nodes available on this machine.
+	defaultAffinity, _ := socketmask.NewSocketMask(m.numaNodes...)
 
 	// Loop through all hint providers and save an accumulated list of the
 	// hints returned by each hint provider. If no hints are provided, assume
@@ -167,27 +189,21 @@ func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) Topology
 		// If hints is nil, insert a single, preferred any-numa hint into allProviderHints.
 		if len(hints) == 0 {
 			klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with any resource")
-			affinity, _ := socketmask.NewSocketMask()
-			affinity.Fill()
-			allProviderHints = append(allProviderHints, []TopologyHint{{affinity, true}})
+			allProviderHints = append(allProviderHints, []TopologyHint{{defaultAffinity, true}})
 			continue
 		}
 
 		// Otherwise, accumulate the hints for each resource type into allProviderHints.
 		for resource := range hints {
 			if hints[resource] == nil {
 				klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with resource '%s'", resource)
-				affinity, _ := socketmask.NewSocketMask()
-				affinity.Fill()
-				allProviderHints = append(allProviderHints, []TopologyHint{{affinity, true}})
+				allProviderHints = append(allProviderHints, []TopologyHint{{defaultAffinity, true}})
 				continue
 			}
 
 			if len(hints[resource]) == 0 {
 				klog.Infof("[topologymanager] Hint Provider has no possible NUMA affinities for resource '%s'", resource)
-				affinity, _ := socketmask.NewSocketMask()
-				affinity.Fill()
-				allProviderHints = append(allProviderHints, []TopologyHint{{affinity, false}})
+				allProviderHints = append(allProviderHints, []TopologyHint{{defaultAffinity, false}})
 				continue
 			}
 
@@ -199,8 +215,8 @@ func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) Topology
 	// hints in each permutation by taking the bitwise-and of their affinity masks.
 	// Return the hint with the narrowest NUMANodeAffinity of all merged
 	// permutations that have at least one NUMA ID set. If no merged mask can be
-	// found that has at least one NUMA ID set, return the 'defaultHint'.
-	bestHint := defaultHint
+	// found that has at least one NUMA ID set, return the 'defaultAffinity'.
+	bestHint := TopologyHint{defaultAffinity, false}
 	m.iterateAllProviderTopologyHints(allProviderHints, func(permutation []TopologyHint) {
 		// Get the NUMANodeAffinity from each hint in the permutation and see if any
 		// of them encode unpreferred allocations.
@@ -217,8 +233,7 @@ func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) Topology
 		}
 
 		// Merge the affinities using a bitwise-and operation.
-		mergedAffinity, _ := socketmask.NewSocketMask()
-		mergedAffinity.Fill()
+		mergedAffinity, _ := socketmask.NewSocketMask(m.numaNodes...)
 		mergedAffinity.And(numaAffinities...)
 
 		// Build a mergedHintfrom the merged affinity mask, indicating if an

pkg/kubelet/cm/topologymanager/topology_manager_test.go

@@ -33,12 +33,6 @@ func NewTestSocketMask(sockets ...int) socketmask.SocketMask {
 	return s
 }
 
-func NewTestSocketMaskFull() socketmask.SocketMask {
-	s, _ := socketmask.NewSocketMask()
-	s.Fill()
-	return s
-}
-
 func TestNewManager(t *testing.T) {
 	tcases := []struct {
 		description    string
@@ -64,7 +58,7 @@ func TestNewManager(t *testing.T) {
 	}
 
 	for _, tc := range tcases {
-		mngr, err := NewManager(tc.policyName)
+		mngr, err := NewManager(nil, tc.policyName)
 
 		if tc.expectedError != nil {
 			if !strings.Contains(err.Error(), tc.expectedError.Error()) {
@@ -111,6 +105,8 @@ func TestGetAffinity(t *testing.T) {
 }
 
 func TestCalculateAffinity(t *testing.T) {
+	numaNodes := []int{0, 1}
+
 	tcases := []struct {
 		name     string
 		hp       []HintProvider
@@ -120,7 +116,7 @@ func TestCalculateAffinity(t *testing.T) {
 			name: "TopologyHint not set",
 			hp:   []HintProvider{},
 			expected: TopologyHint{
-				NUMANodeAffinity: NewTestSocketMaskFull(),
+				NUMANodeAffinity: NewTestSocketMask(numaNodes...),
 				Preferred:        true,
 			},
 		},
@@ -132,7 +128,7 @@ func TestCalculateAffinity(t *testing.T) {
 				},
 			},
 			expected: TopologyHint{
-				NUMANodeAffinity: NewTestSocketMaskFull(),
+				NUMANodeAffinity: NewTestSocketMask(numaNodes...),
 				Preferred:        true,
 			},
 		},
@@ -146,7 +142,7 @@ func TestCalculateAffinity(t *testing.T) {
 				},
 			},
 			expected: TopologyHint{
-				NUMANodeAffinity: NewTestSocketMaskFull(),
+				NUMANodeAffinity: NewTestSocketMask(numaNodes...),
 				Preferred:        true,
 			},
 		},
@@ -160,7 +156,7 @@ func TestCalculateAffinity(t *testing.T) {
 				},
 			},
 			expected: TopologyHint{
-				NUMANodeAffinity: NewTestSocketMaskFull(),
+				NUMANodeAffinity: NewTestSocketMask(numaNodes...),
 				Preferred:        false,
 			},
 		},
@@ -179,7 +175,7 @@ func TestCalculateAffinity(t *testing.T) {
 				},
 			},
 			expected: TopologyHint{
-				NUMANodeAffinity: NewTestSocketMaskFull(),
+				NUMANodeAffinity: NewTestSocketMask(numaNodes...),
 				Preferred:        true,
 			},
 		},
@@ -198,7 +194,7 @@ func TestCalculateAffinity(t *testing.T) {
 				},
 			},
 			expected: TopologyHint{
-				NUMANodeAffinity: NewTestSocketMaskFull(),
+				NUMANodeAffinity: NewTestSocketMask(numaNodes...),
 				Preferred:        true,
 			},
 		},
@@ -343,7 +339,7 @@ func TestCalculateAffinity(t *testing.T) {
 				},
 			},
 			expected: TopologyHint{
-				NUMANodeAffinity: NewTestSocketMaskFull(),
+				NUMANodeAffinity: NewTestSocketMask(numaNodes...),
 				Preferred:        false,
 			},
 		},
@@ -662,8 +658,10 @@ func TestCalculateAffinity(t *testing.T) {
 	}
 
 	for _, tc := range tcases {
-		mngr := manager{}
-		mngr.hintProviders = tc.hp
+		mngr := manager{
+			hintProviders: tc.hp,
+			numaNodes:     numaNodes,
+		}
 		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)
@@ -926,10 +924,13 @@ func TestAdmit(t *testing.T) {
 		},
 	}
 	for _, tc := range tcases {
-		man := manager{}
-		man.policy = tc.policy
-		man.podTopologyHints = make(map[string]map[string]TopologyHint)
-		man.hintProviders = tc.hp
+		man := manager{
+			policy:           tc.policy,
+			podTopologyHints: make(map[string]map[string]TopologyHint),
+			hintProviders:    tc.hp,
+			numaNodes:        []int{0, 1},
+		}
+
 		pod := &v1.Pod{
 			Spec: v1.PodSpec{
 				Containers: []v1.Container{
@@ -938,10 +939,15 @@ func TestAdmit(t *testing.T) {
 					},
 				},
 			},
+			Status: v1.PodStatus{
+				QOSClass: tc.qosClass,
+			},
 		}
-		podAttr := lifecycle.PodAdmitAttributes{}
-		pod.Status.QOSClass = tc.qosClass
-		podAttr.Pod = pod
+
+		podAttr := lifecycle.PodAdmitAttributes{
+			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)