From db4e8a1469c16abcb0872510fa48c0147f52cb98 Mon Sep 17 00:00:00 2001
From: Juana De La Cuesta <juanita.delacuestamorales@hashicorp.com>
Date: Thu, 28 Mar 2024 20:11:31 +0100
Subject: [PATCH] [gh-19729] Fix logic for updating terminal allocs on clients
 with max client disconnect (#20181) (#20253)

Only ignore allocs on terminal states that are updated
---------

Co-authored-by: Tim Gross <tgross@hashicorp.com>
---
 scheduler/reconcile_util.go      |  4 +--
 scheduler/reconcile_util_test.go | 62 ++++++++++++++++----------------
 2 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/scheduler/reconcile_util.go b/scheduler/reconcile_util.go
index 7cf7c3fb14c..5287c66bcde 100644
--- a/scheduler/reconcile_util.go
+++ b/scheduler/reconcile_util.go
@@ -300,9 +300,9 @@ func (a allocSet) filterByTainted(taintedNodes map[string]*structs.Node, serverS
 		}
 
 		if alloc.TerminalStatus() && !reconnect {
-			// Terminal allocs, if supportsDisconnectedClient and not reconnect,
+			// Server-terminal allocs, if supportsDisconnectedClient and not reconnect,
 			// are probably stopped replacements and should be ignored
-			if supportsDisconnectedClients {
+			if supportsDisconnectedClients && alloc.ServerTerminalStatus() {
 				ignore[alloc.ID] = alloc
 				continue
 			}
diff --git a/scheduler/reconcile_util_test.go b/scheduler/reconcile_util_test.go
index 600b9b4918a..0e4f1d49f85 100644
--- a/scheduler/reconcile_util_test.go
+++ b/scheduler/reconcile_util_test.go
@@ -379,10 +379,10 @@ func TestAllocSet_filterByTainted(t *testing.T) {
 			taintedNodes:                nodes,
 			skipNilNodeTest:             false,
 			all: allocSet{
-				// Allocs on reconnected nodes that are complete are ignored
-				"ignored-reconnect-complete": {
-					ID:            "ignored-reconnect-complete",
-					Name:          "ignored-reconnect-complete",
+				// Allocs on reconnected nodes that are complete need to be updated to stop
+				"untainted-reconnect-complete": {
+					ID:            "untainted-reconnect-complete",
+					Name:          "untainted-reconnect-complete",
 					ClientStatus:  structs.AllocClientStatusComplete,
 					DesiredStatus: structs.AllocDesiredStatusRun,
 					Job:           testJob,
@@ -413,10 +413,10 @@ func TestAllocSet_filterByTainted(t *testing.T) {
 					TaskGroup:     "web",
 					AllocStates:   unknownAllocState,
 				},
-				// Replacement allocs that are complete are ignored
-				"ignored-reconnect-complete-replacement": {
-					ID:                 "ignored-reconnect-complete-replacement",
-					Name:               "ignored-reconnect-complete",
+				// Replacement allocs that are complete need to be updated
+				"untainted-reconnect-complete-replacement": {
+					ID:                 "untainted-reconnect-complete-replacement",
+					Name:               "untainted-reconnect-complete",
 					ClientStatus:       structs.AllocClientStatusComplete,
 					DesiredStatus:      structs.AllocDesiredStatusRun,
 					Job:                testJob,
@@ -449,7 +449,29 @@ func TestAllocSet_filterByTainted(t *testing.T) {
 					PreviousAllocation: "untainted-reconnect-lost",
 				},
 			},
-			untainted:     allocSet{},
+			untainted: allocSet{
+				"untainted-reconnect-complete": {
+					ID:            "untainted-reconnect-complete",
+					Name:          "untainted-reconnect-complete",
+					ClientStatus:  structs.AllocClientStatusComplete,
+					DesiredStatus: structs.AllocDesiredStatusRun,
+					Job:           testJob,
+					NodeID:        "normal",
+					TaskGroup:     "web",
+					AllocStates:   unknownAllocState,
+				},
+				"untainted-reconnect-complete-replacement": {
+					ID:                 "untainted-reconnect-complete-replacement",
+					Name:               "untainted-reconnect-complete",
+					ClientStatus:       structs.AllocClientStatusComplete,
+					DesiredStatus:      structs.AllocDesiredStatusRun,
+					Job:                testJob,
+					NodeID:             "normal",
+					TaskGroup:          "web",
+					AllocStates:        unknownAllocState,
+					PreviousAllocation: "untainted-reconnect-complete",
+				},
+			},
 			migrate:       allocSet{},
 			disconnecting: allocSet{},
 			reconnecting: allocSet{
@@ -465,17 +487,6 @@ func TestAllocSet_filterByTainted(t *testing.T) {
 				},
 			},
 			ignore: allocSet{
-
-				"ignored-reconnect-complete": {
-					ID:            "ignored-reconnect-complete",
-					Name:          "ignored-reconnect-complete",
-					ClientStatus:  structs.AllocClientStatusComplete,
-					DesiredStatus: structs.AllocDesiredStatusRun,
-					Job:           testJob,
-					NodeID:        "normal",
-					TaskGroup:     "web",
-					AllocStates:   unknownAllocState,
-				},
 				"ignored-reconnect-lost": {
 					ID:            "ignored-reconnect-lost",
 					Name:          "ignored-reconnect-lost",
@@ -486,17 +497,6 @@ func TestAllocSet_filterByTainted(t *testing.T) {
 					TaskGroup:     "web",
 					AllocStates:   unknownAllocState,
 				},
-				"ignored-reconnect-complete-replacement": {
-					ID:                 "ignored-reconnect-complete-replacement",
-					Name:               "ignored-reconnect-complete",
-					ClientStatus:       structs.AllocClientStatusComplete,
-					DesiredStatus:      structs.AllocDesiredStatusRun,
-					Job:                testJob,
-					NodeID:             "normal",
-					TaskGroup:          "web",
-					AllocStates:        unknownAllocState,
-					PreviousAllocation: "untainted-reconnect-complete",
-				},
 				"ignored-reconnect-failed-replacement": {
 					ID:                 "ignored-reconnect-failed-replacement",
 					Name:               "ignored-reconnect-failed",