scheduler: account for affinity value of zero in score normalization

If there are no affinities on a job, we don't want to count an affinity score of
zero in the number of scores we divide the normalized score by. This is how we
handle other scoring components like node reschedule penalties on nodes that
weren't running the previous allocation.

But we also exclude counting the affinity in the case where we have affinity but
the value is zero. In pathological cases, this can result in a node with a low
affinity being picked over a node with no affinity, because the denominator is 1
larger. Include zero-value affinities in the count of scores if the job has
affinities but the value just happens to be zero.

Fixes: #25621

Author: tgross

scheduler/rank.go

@@ -972,10 +972,8 @@ func (iter *NodeAffinityIterator) Next() *RankedNode {
 		}
 	}
 	normScore := totalAffinityScore / sumWeight
-	if totalAffinityScore != 0.0 {
-		option.Scores = append(option.Scores, normScore)
-		iter.ctx.Metrics().ScoreNode(option.Node, "node-affinity", normScore)
-	}
+	option.Scores = append(option.Scores, normScore)
+	iter.ctx.Metrics().ScoreNode(option.Node, "node-affinity", normScore)
 	return option
 }
 

scheduler/rank_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/shoenig/test"
 	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
 )
@@ -2267,13 +2268,19 @@ func TestNodeAffinityIterator(t *testing.T) {
 		{Node: mock.Node()},
 		{Node: mock.Node()},
 		{Node: mock.Node()},
+		{Node: mock.Node()},
 	}
 
 	nodes[0].Node.Attributes["kernel.version"] = "4.9"
 	nodes[1].Node.Datacenter = "dc2"
 	nodes[2].Node.Datacenter = "dc2"
 	nodes[2].Node.NodeClass = "large"
 
+	// this node should have zero affinity
+	nodes[4].Node.Attributes["kernel.version"] = "2.6"
+	nodes[4].Node.Datacenter = "dc3"
+	nodes[4].Node.NodeClass = "weird"
+
 	affinities := []*structs.Affinity{
 		{
 			Operand: "=",
@@ -2314,23 +2321,21 @@ func TestNodeAffinityIterator(t *testing.T) {
 	scoreNorm := NewScoreNormalizationIterator(ctx, nodeAffinity)
 
 	out := collectRanked(scoreNorm)
-	expectedScores := make(map[string]float64)
+
 	// Total weight = 300
 	// Node 0 matches two affinities(dc and kernel version), total weight = 150
-	expectedScores[nodes[0].Node.ID] = 0.5
+	test.Eq(t, 0.5, out[0].FinalScore)
 
 	// Node 1 matches an anti affinity, weight = -100
-	expectedScores[nodes[1].Node.ID] = -(1.0 / 3.0)
+	test.Eq(t, -(1.0 / 3.0), out[1].FinalScore)
 
 	// Node 2 matches one affinity(node class) with weight 50
-	expectedScores[nodes[2].Node.ID] = -(1.0 / 6.0)
+	test.Eq(t, -(1.0 / 6.0), out[2].FinalScore)
 
 	// Node 3 matches one affinity (dc) with weight = 100
-	expectedScores[nodes[3].Node.ID] = 1.0 / 3.0
-
-	require := require.New(t)
-	for _, n := range out {
-		require.Equal(expectedScores[n.Node.ID], n.FinalScore)
-	}
+	test.Eq(t, 1.0/3.0, out[3].FinalScore)
 
+	// Node 4 matches no affinities but should still generate a score
+	test.Eq(t, 0, out[4].FinalScore)
+	test.NotNil(t, out[4].Scores)
 }