scheduler: correct NodeNUMAResource handling with NodeFullPCPUsOnly a…

…nd preferredCPUBindPolicy (koordinator-sh#1891) Signed-off-by: Joseph <joseph.t.lee@outlook.com>
saintube · Feb 22, 2024 · 2e13ffa · 2e13ffa
1 parent a16e547
commit 2e13ffa
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 9 deletions.
diff --git a/pkg/scheduler/plugins/nodenumaresource/plugin.go b/pkg/scheduler/plugins/nodenumaresource/plugin.go
@@ -302,15 +302,18 @@ func (p *Plugin) Filter(ctx context.Context, cycleState *framework.CycleState, p
 		if !topologyOptions.CPUTopology.IsValid() {
 			return framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrInvalidCPUTopology)
 		}
-		nodeRequiredFullPCPUsOnly := extension.GetNodeCPUBindPolicy(node.Labels, topologyOptions.Policy) == extension.NodeCPUBindPolicyFullPCPUsOnly
-		if nodeRequiredFullPCPUsOnly || state.requiredCPUBindPolicy == schedulingconfig.CPUBindPolicyFullPCPUs {
+		nodeCPUBindPolicy := extension.GetNodeCPUBindPolicy(node.Labels, topologyOptions.Policy)
+		if nodeCPUBindPolicy == extension.NodeCPUBindPolicyFullPCPUsOnly ||
+			state.requiredCPUBindPolicy == schedulingconfig.CPUBindPolicyFullPCPUs {
 			if state.numCPUsNeeded%topologyOptions.CPUTopology.CPUsPerCore() != 0 {
 				return framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrSMTAlignmentError)
 			}
 
-			if nodeRequiredFullPCPUsOnly &&
-				(state.requiredCPUBindPolicy != schedulingconfig.CPUBindPolicyFullPCPUs || state.preferredCPUBindPolicy != schedulingconfig.CPUBindPolicyFullPCPUs) {
-				return framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrRequiredFullPCPUsPolicy)
+			if nodeCPUBindPolicy == extension.NodeCPUBindPolicyFullPCPUsOnly {
+				if (state.requiredCPUBindPolicy != "" && state.requiredCPUBindPolicy != schedulingconfig.CPUBindPolicyFullPCPUs) ||
+					(state.preferredCPUBindPolicy != "" && state.preferredCPUBindPolicy != schedulingconfig.CPUBindPolicyFullPCPUs) {
+					return framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrRequiredFullPCPUsPolicy)
+				}
 			}
 		}
 

diff --git a/pkg/scheduler/plugins/nodenumaresource/plugin_test.go b/pkg/scheduler/plugins/nodenumaresource/plugin_test.go
@@ -593,7 +593,7 @@ func TestPlugin_Filter(t *testing.T) {
 			want: nil,
 		},
 		{
-			name: "verify FullPCPUsOnly with SMTAlignmentError",
+			name: "failed to verify Node FullPCPUsOnly with SMTAlignmentError",
 			nodeLabels: map[string]string{
 				extension.LabelNodeCPUBindPolicy: string(extension.NodeCPUBindPolicyFullPCPUsOnly),
 			},
@@ -607,17 +607,41 @@ func TestPlugin_Filter(t *testing.T) {
 			want:            framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrSMTAlignmentError),
 		},
 		{
-			name: "verify required FullPCPUs SMTAlignmentError",
+			name: "verify Node FullPCPUsOnly",
+			nodeLabels: map[string]string{
+				extension.LabelNodeCPUBindPolicy: string(extension.NodeCPUBindPolicyFullPCPUsOnly),
+			},
 			state: &preFilterState{
 				requestCPUBind:         true,
-				requiredCPUBindPolicy:  schedulingconfig.CPUBindPolicyFullPCPUs,
 				preferredCPUBindPolicy: schedulingconfig.CPUBindPolicyFullPCPUs,
-				numCPUsNeeded:          5,
+				numCPUsNeeded:          4,
+			},
+			cpuTopology:     buildCPUTopologyForTest(2, 1, 4, 2),
+			allocationState: NewNodeAllocation("test-node-1"),
+			want:            nil,
+		},
+		{
+			name: "failed to verify required FullPCPUs SMTAlignmentError",
+			state: &preFilterState{
+				requestCPUBind:        true,
+				requiredCPUBindPolicy: schedulingconfig.CPUBindPolicyFullPCPUs,
+				numCPUsNeeded:         5,
 			},
 			cpuTopology:     buildCPUTopologyForTest(2, 1, 4, 2),
 			allocationState: NewNodeAllocation("test-node-1"),
 			want:            framework.NewStatus(framework.UnschedulableAndUnresolvable, ErrSMTAlignmentError),
 		},
+		{
+			name: "verify required FullPCPUs",
+			state: &preFilterState{
+				requestCPUBind:        true,
+				requiredCPUBindPolicy: schedulingconfig.CPUBindPolicyFullPCPUs,
+				numCPUsNeeded:         4,
+			},
+			cpuTopology:     buildCPUTopologyForTest(2, 1, 4, 2),
+			allocationState: NewNodeAllocation("test-node-1"),
+			want:            nil,
+		},
 		{
 			name: "verify FullPCPUsOnly with preferred SpreadByPCPUs",
 			nodeLabels: map[string]string{