style: rename filterBy Tainted

Author: Juanadelacuesta

scheduler/reconciler/filters.go

@@ -264,16 +264,17 @@ var classificationRules = []classificationRule{
 	},
 }
 
-// filterByTainted takes a set of tainted nodes and filters the allocation set
+// classifyAllocs takes a set of tainted nodes and filters the allocation set
 // into the following groups:
-// 1. Those that exist on untainted nodes
-// 2. Those exist on nodes that are draining
-// 3. Those that exist on lost nodes or have expired
-// 4. Those that are on nodes that are disconnected, but have not had their ClientState set to unknown
-// 5. Those that are on a node that has reconnected.
-// 6. Those that are in a state that results in a noop.
-// 7. Those that are disconnected and need to be marked lost (and possibly replaced)
-func (set allocSet) filterByTainted(state ClusterState) (untainted, migrate, lost, disconnecting, reconnecting, ignore, expiring allocSet) {
+// 1. untainted: allocs on working nodes, which are in the correct state and don't need to be changed or updated
+// 2. migrate: allocs on nodes that are draining or need to be moved
+// 3. lost: allocs on lost nodes or have expired and need to be replaced
+// 4. disconnect: allocs on nodes that are disconnected, and need to be updated and maybe rescheduled
+// 5. reconnecting: allocs on a node that has reconnected and need to go through the reconnect flow
+// 6. ignore: allocs in a state that results in a noop that only need to be counted
+// 7. expiring: allocs on disconnected nodes and need to be marked lost (and possibly replaced)
+
+func (set allocSet) classifyAllocs(state ClusterState) (untainted, migrate, lost, disconnecting, reconnecting, ignore, expiring allocSet) {
 	untainted = make(allocSet)
 	migrate = make(allocSet)
 	lost = make(allocSet)
@@ -295,7 +296,7 @@ func (set allocSet) filterByTainted(state ClusterState) (untainted, migrate, los
 
 	for _, alloc := range set {
 		// Build context for classification rules.
-		ctx := allocContext{
+		allocRuntime := allocContext{
 			alloc: alloc,
 			now:   state.Now,
 		}
@@ -304,17 +305,17 @@ func (set allocSet) filterByTainted(state ClusterState) (untainted, migrate, los
 		if alloc.ClientStatus == structs.AllocClientStatusUnknown ||
 			alloc.ClientStatus == structs.AllocClientStatusRunning ||
 			alloc.ClientStatus == structs.AllocClientStatusFailed {
-			ctx.shouldReconnect = alloc.NeedsToReconnect()
+			allocRuntime.shouldReconnect = alloc.NeedsToReconnect()
 		}
 
 		// Get node taint information, preserving whether key existed so we can
 		// distinguish missing-node from GC'd-node semantics.
-		ctx.taintedNode, ctx.nodeIsTainted = state.TaintedNodes[alloc.NodeID]
+		allocRuntime.taintedNode, allocRuntime.nodeIsTainted = state.TaintedNodes[alloc.NodeID]
 
 		// Apply classification rules in order (first match wins).
 		classified := false
 		for _, rule := range classificationRules {
-			if rule.condition(ctx) {
+			if rule.condition(allocRuntime) {
 				targetSet := categoryMap[rule.category]
 				(*targetSet)[alloc.ID] = alloc
 				classified = true

scheduler/reconciler/filters_test.go

@@ -7,7 +7,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/shoenig/test/must"
@@ -75,7 +74,7 @@ func TestReconciler_filterServerTerminalAllocs(t *testing.T) {
 	})
 }
 
-func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
+func TestAllocSet_classifyAllocs_ClassificationRules(t *testing.T) {
 	now := time.Now()
 
 	nodes := map[string]*structs.Node{
@@ -136,30 +135,26 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 				a.AllocStates = unknownState
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "reconnecting",
 		},
 		{
 			name:     "terminal server status ignored",
 			alloc:    makeAlloc("c2", "ready", structs.AllocClientStatusRunning, structs.AllocDesiredStatusStop),
-			nodeMap:  nodes,
 			expected: "ignore",
 		},
 		{
 			name: "terminal canary migrate",
 			alloc: func() *structs.Allocation {
 				a := makeAlloc("c3", "ready", structs.AllocClientStatusComplete, structs.AllocDesiredStatusRun)
 				a.DeploymentStatus = &structs.AllocDeploymentStatus{Canary: true}
-				a.DesiredTransition = structs.DesiredTransition{Migrate: new(true)}
+				a.DesiredTransition = structs.DesiredTransition{Migrate: (true)}
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "migrate",
 		},
 		{
 			name:     "terminal untainted",
 			alloc:    makeAlloc("c4", "ready", structs.AllocClientStatusComplete, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "untainted",
 		},
 		{
@@ -170,7 +165,6 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 				a.AllocStates = unknownState
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "expiring",
 		},
 		{
@@ -180,19 +174,16 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 				a.AllocStates = unknownState
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "ignore",
 		},
 		{
 			name:     "disconnected unknown becomes untainted",
 			alloc:    makeAlloc("c7", "disconnected", structs.AllocClientStatusUnknown, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "untainted",
 		},
 		{
 			name:     "disconnected pending lost",
 			alloc:    makeAlloc("c8", "disconnected", structs.AllocClientStatusPending, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "lost",
 		},
 		{
@@ -202,23 +193,20 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 				a.Job = nil
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "lost",
 		},
 		{
 			name:     "disconnected grace period",
 			alloc:    makeAlloc("c10", "disconnected", structs.AllocClientStatusRunning, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "disconnecting",
 		},
 		{
 			name: "migrate flag",
 			alloc: func() *structs.Allocation {
 				a := makeAlloc("c11", "ready", structs.AllocClientStatusPending, structs.AllocDesiredStatusRun)
-				a.DesiredTransition = structs.DesiredTransition{Migrate: new(true)}
+				a.DesiredTransition = structs.DesiredTransition{Migrate: (true)}
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "migrate",
 		},
 		{
@@ -228,19 +216,16 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 				a.AllocStates = unknownState
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "reconnecting",
 		},
 		{
 			name:     "untainted on non tainted node",
 			alloc:    makeAlloc("c13", "missing", structs.AllocClientStatusRunning, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "untainted",
 		},
 		{
 			name:     "gc node lost",
 			alloc:    makeAlloc("c14", "gc", structs.AllocClientStatusRunning, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "lost",
 		},
 		{
@@ -250,7 +235,6 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 				setDisconnect(a, 5*time.Minute, false)
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "untainted",
 		},
 		{
@@ -260,19 +244,16 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 				setDisconnect(a, 5*time.Minute, false)
 				return a
 			}(),
-			nodeMap:  nodes,
 			expected: "disconnecting",
 		},
 		{
 			name:     "terminal node default lost",
 			alloc:    makeAlloc("c17", "down", structs.AllocClientStatusPending, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "lost",
 		},
 		{
 			name:     "other tainted node defaults to untainted",
 			alloc:    makeAlloc("c18", "initializing", structs.AllocClientStatusPending, structs.AllocDesiredStatusRun),
-			nodeMap:  nodes,
 			expected: "untainted",
 		},
 	}
@@ -282,7 +263,7 @@ func TestAllocSet_filterByTainted_ClassificationRules(t *testing.T) {
 			all := allocSet{tc.alloc.ID: tc.alloc}
 			state := ClusterState{Now: now, TaintedNodes: nodes}
 
-			untainted, migrate, lost, disconnecting, reconnecting, ignore, expiring := all.filterByTainted(state)
+			untainted, migrate, lost, disconnecting, reconnecting, ignore, expiring := all.classifyAllocs(state)
 
 			buckets := map[string]allocSet{
 				"untainted":     untainted,

scheduler/reconciler/reconcile_cluster.go

@@ -410,7 +410,7 @@ func (a *AllocReconciler) computeGroup(group string, all allocSet) (*ReconcileRe
 	canaries := a.cancelUnneededCanaries(&all, group, result)
 
 	// Determine what set of allocations are on tainted nodes
-	untainted, migrate, lost, disconnecting, reconnecting, ignore, expiring := all.filterByTainted(a.clusterState)
+	untainted, migrate, lost, disconnecting, reconnecting, ignore, expiring := all.classifyAllocs(a.clusterState)
 	result.DesiredTGUpdates[group].Ignore += uint64(len(ignore))
 
 	// Determine what set of terminal allocations need to be rescheduled

scheduler/reconciler/reconcile_cluster_prop_test.go

@@ -257,7 +257,7 @@ func TestAllocReconciler_cancelUnneededCanaries(t *testing.T) {
 			}
 		}
 		canarySet := all.fromKeys(expectedCanaries)
-		canariesOnUntaintedNodes, migrate, lost, _, _, _, _ := canarySet.filterByTainted(clusterState)
+		canariesOnUntaintedNodes, migrate, lost, _, _, _, _ := canarySet.classifyAllocs(clusterState)
 
 		stopSet = stopSet.union(migrate, lost)