CR comments 1

Author: davidLif

pkg/scheduler/plugins/topology/topology_plugin.go

@@ -21,7 +21,7 @@ type topologyPlugin struct {
 	enabled            bool
 	taskOrderFunc      common_info.LessFn
 	sessionStateGetter k8s_internal.SessionStateProvider
-	nodesInfoMap       map[string]*node_info.NodeInfo
+	nodesInfos         map[string]*node_info.NodeInfo
 	TopologyTrees      map[string]*TopologyInfo
 }
 
@@ -40,7 +40,7 @@ func (t *topologyPlugin) OnSessionOpen(ssn *framework.Session) {
 	topologies := ssn.Topologies
 	t.taskOrderFunc = ssn.TaskOrderFn
 	t.sessionStateGetter = ssn
-	t.nodesInfoMap = ssn.Nodes
+	t.nodesInfos = ssn.Nodes
 	t.initializeTopologyTree(topologies, ssn)
 
 	ssn.AddEventHandler(&framework.EventHandler{

pkg/scheduler/plugins/topology/topology_plugin_job_filtering.go

@@ -107,25 +107,26 @@ func (t *topologyPlugin) calcTreeAlocationData(job *podgroup_info.PodGroupInfo,
 	currentlyUpdatedDomains := map[TopologyDomainID]*TopologyDomainInfo{}
 	for len(latestUpdatedDomains) > 0 {
 		for _, domain := range latestUpdatedDomains {
-			if domain.Parent != nil {
-				// Known issue: shold be replaced with the real distance logic
-				// TODO: subsetsum on allocateablePods, from them find min distance
-				if domain.Parent.AllocatablePods < taskToAllocateCount {
-					// If the parent domain has less allocateable pods than the tasks to allocate,
-					// we need to update the parent domain with the current domain's allocateable pods
-					if domain.AllocatablePods < taskToAllocateCount {
-						domain.Parent.Distance += domain.Distance
-						domain.Parent.AllocatablePods += domain.AllocatablePods
-					} else {
-						domain.Parent.Distance = domain.Distance
-						domain.Parent.AllocatablePods = domain.AllocatablePods
-					}
-				} else if domain.Parent.Distance > domain.Distance {
+			if domain.Parent == nil {
+				continue
+			}
+			// Known issue: should be replaced with the real distance logic
+			// TODO: subsetsum on allocateablePods, from them find min distance
+			if domain.Parent.AllocatablePods < taskToAllocateCount {
+				// If the parent domain has less allocateable pods than the tasks to allocate,
+				// we need to update the parent domain with the current domain's allocateable pods
+				if domain.AllocatablePods < taskToAllocateCount {
+					domain.Parent.Distance += domain.Distance
+					domain.Parent.AllocatablePods += domain.AllocatablePods
+				} else {
 					domain.Parent.Distance = domain.Distance
 					domain.Parent.AllocatablePods = domain.AllocatablePods
 				}
-				currentlyUpdatedDomains[domain.Parent.ID] = domain.Parent
+			} else if domain.Parent.Distance > domain.Distance {
+				domain.Parent.Distance = domain.Distance
+				domain.Parent.AllocatablePods = domain.AllocatablePods
 			}
+			currentlyUpdatedDomains[domain.Parent.ID] = domain.Parent
 		}
 		latestUpdatedDomains = currentlyUpdatedDomains
 		currentlyUpdatedDomains = map[TopologyDomainID]*TopologyDomainInfo{}
@@ -180,7 +181,7 @@ func (t *topologyPlugin) calcAllocationsForLeafDomains(job *podgroup_info.PodGro
 		{Name: "1-pods-resources", ResReq: maxPodResources},
 	}
 	leafDomains := map[TopologyDomainID]*TopologyDomainInfo{}
-	for _, node := range t.nodesInfoMap {
+	for _, node := range t.nodesInfos {
 		allocateablePodsCount := calcNodeAccomedation(maxPodResources, allocationTestPods, node, len(tasksToAllocate))
 
 		leafDomainId := calcLeafDomainId(topologyTree.TopologyResource, node.Node.Labels)

pkg/scheduler/plugins/topology/topology_plugin_job_filtering_test.go

@@ -328,7 +328,7 @@ func TestTopologyPlugin_calcAllocationsForLeafDomains(t *testing.T) {
 			}
 			plugin := &topologyPlugin{
 				sessionStateGetter: session,
-				nodesInfoMap:       nodesInfoMap,
+				nodesInfos:         nodesInfoMap,
 			}
 
 			err, domains := plugin.calcAllocationsForLeafDomains(job, topologyTree)
@@ -1160,7 +1160,7 @@ func TestTopologyPlugin_calcTreeAlocationData(t *testing.T) {
 			}
 			plugin := &topologyPlugin{
 				sessionStateGetter: session,
-				nodesInfoMap:       nodesInfoMap,
+				nodesInfos:         nodesInfoMap,
 			}
 
 			// Call the function under test