nrt: filter: introduce and use filterInfo

add a helper struct to group together all the context
we need to carry in the internal implementation functions.
The goals are to streamline the existing code, to
reduce redundant call and to make room for more
arguments to be passed through, like the topology manager
configuration.

Signed-off-by: Francesco Romani <[email protected]>

Author: ffromani

pkg/noderesourcetopology/filter.go

@@ -41,16 +41,7 @@ const highestNUMAID = 8
 
 type PolicyHandler func(pod *v1.Pod, zoneMap topologyv1alpha2.ZoneList) *framework.Status
 
-func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, zones topologyv1alpha2.ZoneList, nodeInfo *framework.NodeInfo) *framework.Status {
-	lh.V(5).Info("container level single NUMA node handler")
-
-	// prepare NUMANodes list from zoneMap
-	nodes := createNUMANodeList(lh, zones)
-	qos := v1qos.GetPodQOS(pod)
-
-	// Node() != nil already verified in Filter(), which is the only public entry point
-	logNumaNodes(lh, "container handler NUMA resources", nodeInfo.Node().Name, nodes)
-
+func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, info *filterInfo) *framework.Status {
 	// the init containers are running SERIALLY and BEFORE the normal containers.
 	// https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#understanding-init-containers
 	// therefore, we don't need to accumulate their resources together
@@ -59,7 +50,7 @@ func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, zones topology
 		clh := lh.WithValues(logging.KeyContainer, initContainer.Name, logging.KeyContainerKind, cntKind)
 		clh.V(6).Info("desired resources", stringify.ResourceListToLoggable(initContainer.Resources.Requests)...)
 
-		_, match := resourcesAvailableInAnyNUMANodes(clh, nodes, initContainer.Resources.Requests, qos, nodeInfo)
+		_, match := resourcesAvailableInAnyNUMANodes(clh, info, initContainer.Resources.Requests)
 		if !match {
 			msg := "cannot align " + cntKind + " container"
 			// we can't align init container, so definitely we can't align a pod
@@ -72,7 +63,7 @@ func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, zones topology
 		clh := lh.WithValues(logging.KeyContainer, container.Name, logging.KeyContainerKind, logging.KindContainerApp)
 		clh.V(6).Info("container requests", stringify.ResourceListToLoggable(container.Resources.Requests)...)
 
-		numaID, match := resourcesAvailableInAnyNUMANodes(clh, nodes, container.Resources.Requests, qos, nodeInfo)
+		numaID, match := resourcesAvailableInAnyNUMANodes(clh, info, container.Resources.Requests)
 		if !match {
 			// we can't align container, so definitely we can't align a pod
 			clh.V(2).Info("cannot align container")
@@ -81,7 +72,7 @@ func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, zones topology
 
 		// subtract the resources requested by the container from the given NUMA.
 		// this is necessary, so we won't allocate the same resources for the upcoming containers
-		err := subtractResourcesFromNUMANodeList(clh, nodes, numaID, qos, container.Resources.Requests)
+		err := subtractResourcesFromNUMANodeList(clh, info.numaNodes, numaID, info.qos, container.Resources.Requests)
 		if err != nil {
 			// this is an internal error which should never happen
 			return framework.NewStatus(framework.Error, "inconsistent resource accounting", err.Error())
@@ -93,14 +84,14 @@ func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, zones topology
 
 // resourcesAvailableInAnyNUMANodes checks for sufficient resource and return the NUMAID that would be selected by Kubelet.
 // this function requires NUMANodeList with properly populated NUMANode, NUMAID should be in range 0-63
-func resourcesAvailableInAnyNUMANodes(lh logr.Logger, numaNodes NUMANodeList, resources v1.ResourceList, qos v1.PodQOSClass, nodeInfo *framework.NodeInfo) (int, bool) {
+func resourcesAvailableInAnyNUMANodes(lh logr.Logger, info *filterInfo, resources v1.ResourceList) (int, bool) {
 	numaID := highestNUMAID
 	bitmask := bm.NewEmptyBitMask()
 	// set all bits, each bit is a NUMA node, if resources couldn't be aligned
 	// on the NUMA node, bit should be unset
 	bitmask.Fill()
 
-	nodeResources := util.ResourceList(nodeInfo.Allocatable)
+	nodeResources := util.ResourceList(info.node.Allocatable)
 
 	for resource, quantity := range resources {
 		if quantity.IsZero() {
@@ -121,14 +112,14 @@ func resourcesAvailableInAnyNUMANodes(lh logr.Logger, numaNodes NUMANodeList, re
 		// obvious, bits which are not in the NUMA id's range would be unset
 		hasNUMAAffinity := false
 		resourceBitmask := bm.NewEmptyBitMask()
-		for _, numaNode := range numaNodes {
+		for _, numaNode := range info.numaNodes {
 			numaQuantity, ok := numaNode.Resources[resource]
 			if !ok {
 				continue
 			}
 
 			hasNUMAAffinity = true
-			if !isResourceSetSuitable(qos, resource, quantity, numaQuantity) {
+			if !isResourceSetSuitable(info.qos, resource, quantity, numaQuantity) {
 				continue
 			}
 
@@ -161,18 +152,11 @@ func resourcesAvailableInAnyNUMANodes(lh logr.Logger, numaNodes NUMANodeList, re
 	return numaID, ret
 }
 
-func singleNUMAPodLevelHandler(lh logr.Logger, pod *v1.Pod, zones topologyv1alpha2.ZoneList, nodeInfo *framework.NodeInfo) *framework.Status {
-	lh.V(5).Info("pod level single NUMA node handler")
-
+func singleNUMAPodLevelHandler(lh logr.Logger, pod *v1.Pod, info *filterInfo) *framework.Status {
 	resources := util.GetPodEffectiveRequest(pod)
-
-	nodes := createNUMANodeList(lh, zones)
-
-	// Node() != nil already verified in Filter(), which is the only public entry point
-	logNumaNodes(lh, "pod handler NUMA resources", nodeInfo.Node().Name, nodes)
 	lh.V(6).Info("pod desired resources", stringify.ResourceListToLoggable(resources)...)
 
-	numaID, match := resourcesAvailableInAnyNUMANodes(lh, createNUMANodeList(lh, zones), resources, v1qos.GetPodQOS(pod), nodeInfo)
+	numaID, match := resourcesAvailableInAnyNUMANodes(lh, info, resources)
 	if !match {
 		lh.V(2).Info("cannot align pod", "name", pod.Name)
 		return framework.NewStatus(framework.Unschedulable, "cannot align pod")
@@ -186,7 +170,8 @@ func (tm *TopologyMatch) Filter(ctx context.Context, cycleState *framework.Cycle
 	if nodeInfo.Node() == nil {
 		return framework.NewStatus(framework.Error, "node not found")
 	}
-	if v1qos.GetPodQOS(pod) == v1.PodQOSBestEffort && !resourcerequests.IncludeNonNative(pod) {
+	qos := v1qos.GetPodQOS(pod)
+	if qos == v1.PodQOSBestEffort && !resourcerequests.IncludeNonNative(pod) {
 		return nil
 	}
 
@@ -212,26 +197,36 @@ func (tm *TopologyMatch) Filter(ctx context.Context, cycleState *framework.Cycle
 
 	lh.V(4).Info("found nrt data", "object", stringify.NodeResourceTopologyResources(nodeTopology), "conf", conf.String())
 
-	handler := filterHandlerFromTopologyManager(conf)
+	handler, boundary := filterHandlerFromTopologyManager(conf)
 	if handler == nil {
 		return nil
 	}
-	status := handler(lh, pod, nodeTopology.Zones, nodeInfo)
+
+	numaNodes := createNUMANodeList(lh, nodeTopology.Zones)
+	lh.V(4).Info("aligning resources", "boundary", boundary, "numaCells", len(numaNodes))
+	fi := filterInfo{
+		nodeName:        nodeName,
+		node:            nodeInfo,
+		topologyManager: conf,
+		qos:             qos,
+		numaNodes:       numaNodes,
+	}
+	status := handler(lh, pod, &fi)
 	if status != nil {
 		tm.nrtCache.NodeMaybeOverReserved(nodeName, pod)
 	}
 	return status
 }
 
-func filterHandlerFromTopologyManager(conf nodeconfig.TopologyManager) filterFn {
+func filterHandlerFromTopologyManager(conf nodeconfig.TopologyManager) (filterFn, string) {
 	if conf.Policy != kubeletconfig.SingleNumaNodeTopologyManagerPolicy {
-		return nil
+		return nil, ""
 	}
 	if conf.Scope == kubeletconfig.PodTopologyManagerScope {
-		return singleNUMAPodLevelHandler
+		return singleNUMAPodLevelHandler, "pod"
 	}
 	if conf.Scope == kubeletconfig.ContainerTopologyManagerScope {
-		return singleNUMAContainerLevelHandler
+		return singleNUMAContainerLevelHandler, "container"
 	}
-	return nil // cannot happen
+	return nil, "" // cannot happen
 }

pkg/noderesourcetopology/plugin.go

@@ -30,6 +30,7 @@ import (
 	apiconfig "sigs.k8s.io/scheduler-plugins/apis/config"
 	"sigs.k8s.io/scheduler-plugins/apis/config/validation"
 	nrtcache "sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/cache"
+	"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/nodeconfig"
 
 	"github.com/go-logr/logr"
 	topologyapi "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology"
@@ -48,7 +49,15 @@ func init() {
 	utilruntime.Must(topologyv1alpha2.AddToScheme(scheme))
 }
 
-type filterFn func(lh logr.Logger, pod *v1.Pod, zones topologyv1alpha2.ZoneList, nodeInfo *framework.NodeInfo) *framework.Status
+type filterInfo struct {
+	nodeName        string // shortcut, used very often
+	node            *framework.NodeInfo
+	topologyManager nodeconfig.TopologyManager
+	qos             v1.PodQOSClass
+	numaNodes       NUMANodeList
+}
+
+type filterFn func(logr.Logger, *v1.Pod, *filterInfo) *framework.Status
 type scoringFn func(logr.Logger, *v1.Pod, topologyv1alpha2.ZoneList) (int64, *framework.Status)
 
 // TopologyMatch plugin which run simplified version of TopologyManager's admit handler

pkg/noderesourcetopology/pluginhelpers.go

@@ -180,10 +180,3 @@ func getForeignPodsDetectMode(lh logr.Logger, cfg *apiconfig.NodeResourceTopolog
 	}
 	return foreignPodsDetect
 }
-
-func logNumaNodes(lh logr.Logger, desc, nodeName string, nodes NUMANodeList) {
-	for _, numaNode := range nodes {
-		numaItems := []interface{}{"numaCell", numaNode.NUMAID}
-		lh.V(6).Info(desc, stringify.ResourceListToLoggableWithValues(numaItems, numaNode.Resources)...)
-	}
-}