From 8304f363b4dd7599c932c47187290b3c9063451d Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Fri, 4 Feb 2022 09:46:23 -0500
Subject: [PATCH] scheduler: seed random shuffle nodes with eval ID

Processing an evaluation is nearly a pure function over the state
snapshot, but we randomly shuffle the nodes. This means that
developers can't take a given state snapshot and pass an evaluation
through it and be guaranteed the same plan results.

But the evaluation ID is already random, so if we use this as the seed
for shuffling the nodes we can greatly reduce the sources of
non-determinism. Unfortunately golang map iteration uses a global
source of randomness and not a goroutine-local one, but arguably
if the scheduler behavior is impacted by this, that's a bug in the
iteration.
---
 scheduler/feasible.go  |  2 +-
 scheduler/stack.go     |  2 +-
 scheduler/util.go      | 14 +++++++++++---
 scheduler/util_test.go |  2 +-
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/scheduler/feasible.go b/scheduler/feasible.go
index 3b10331c5c88..f78ee6180add 100644
--- a/scheduler/feasible.go
+++ b/scheduler/feasible.go
@@ -122,7 +122,7 @@ func (iter *StaticIterator) SetNodes(nodes []*structs.Node) {
 // is applied in-place
 func NewRandomIterator(ctx Context, nodes []*structs.Node) *StaticIterator {
 	// shuffle with the Fisher-Yates algorithm
-	shuffleNodes(nodes)
+	shuffleNodes(ctx.Plan().EvalID, nodes)
 
 	// Create a static iterator
 	return NewStaticIterator(ctx, nodes)
diff --git a/scheduler/stack.go b/scheduler/stack.go
index d2b546107f73..d5221c7c65f1 100644
--- a/scheduler/stack.go
+++ b/scheduler/stack.go
@@ -70,7 +70,7 @@ type GenericStack struct {
 
 func (s *GenericStack) SetNodes(baseNodes []*structs.Node) {
 	// Shuffle base nodes
-	shuffleNodes(baseNodes)
+	shuffleNodes(s.ctx.Plan().EvalID, baseNodes)
 
 	// Update the set of base nodes
 	s.source.SetNodes(baseNodes)
diff --git a/scheduler/util.go b/scheduler/util.go
index 869442b78f97..2ad49a2a8b13 100644
--- a/scheduler/util.go
+++ b/scheduler/util.go
@@ -1,6 +1,7 @@
 package scheduler
 
 import (
+	"encoding/binary"
 	"fmt"
 	"math/rand"
 	"reflect"
@@ -376,11 +377,18 @@ func taintedNodes(state State, allocs []*structs.Allocation) (map[string]*struct
 	return out, nil
 }
 
-// shuffleNodes randomizes the slice order with the Fisher-Yates algorithm
-func shuffleNodes(nodes []*structs.Node) {
+// shuffleNodes randomizes the slice order with the Fisher-Yates
+// algorithm. We seed the random source with the eval ID (which is
+// random) to aid in postmortem debuggging of specific evaluations and
+// state snapshots.
+func shuffleNodes(evalID string, nodes []*structs.Node) {
+
+	seed, _ := binary.Varint([]byte(evalID))
+	r := rand.New(rand.NewSource(seed))
+
 	n := len(nodes)
 	for i := n - 1; i > 0; i-- {
-		j := rand.Intn(i + 1)
+		j := r.Intn(i + 1)
 		nodes[i], nodes[j] = nodes[j], nodes[i]
 	}
 }
diff --git a/scheduler/util_test.go b/scheduler/util_test.go
index 1b5ec8d9a30a..9a31d6afe9aa 100644
--- a/scheduler/util_test.go
+++ b/scheduler/util_test.go
@@ -507,7 +507,7 @@ func TestShuffleNodes(t *testing.T) {
 	}
 	orig := make([]*structs.Node, len(nodes))
 	copy(orig, nodes)
-	shuffleNodes(nodes)
+	shuffleNodes(uuid.Generate(), nodes)
 	require.False(t, reflect.DeepEqual(nodes, orig))
 }