[gh-19729] Fix logic for updating terminal allocs on clients with max…

… client disconnect (#20181) Only ignore allocs on terminal states that are updated --------- Co-authored-by: Tim Gross <[email protected]>
hashicorp · Mar 28, 2024 · 99d508c · 99d508c
1 parent e171e27
commit 99d508c
Show file tree

Hide file tree

Showing 3 changed files with 1,282 additions and 1,265 deletions.
diff --git a/scheduler/reconcile_test.go b/scheduler/reconcile_test.go
@@ -940,7 +940,7 @@ func TestReconciler_LostNode_PreventRescheduleOnLost(t *testing.T) {
 			maxClientDisconnect:     pointer.Of(10 * time.Second),
 			PreventRescheduleOnLost: false,
 			reschedulePolicy:        disabledReschedulePolicy,
-			expectPlace:             2,
+			expectPlace:             1,
 			expectStop:              1,
 			expectIgnore:            4,
 			expectDisconnect:        1,
@@ -951,13 +951,12 @@ func TestReconciler_LostNode_PreventRescheduleOnLost(t *testing.T) {
 			maxClientDisconnect:     pointer.Of(10 * time.Second),
 			PreventRescheduleOnLost: true,
 			reschedulePolicy:        disabledReschedulePolicy,
-			expectPlace:             1, // This behaviour needs to be verified
+			expectPlace:             0,
 			expectStop:              0,
 			expectIgnore:            5,
 			expectDisconnect:        2,
 			allocStatus:             structs.AllocClientStatusUnknown,
 		},
-
 		{
 			name:                    "PreventRescheduleOnLost off, MaxClientDisconnect off, Reschedule on",
 			maxClientDisconnect:     nil,
@@ -991,8 +990,8 @@ func TestReconciler_LostNode_PreventRescheduleOnLost(t *testing.T) {
 				Attempts: 1,
 			},
 			expectPlace:      3,
-			expectStop:       1,
-			expectIgnore:     3,
+			expectStop:       2,
+			expectIgnore:     2,
 			expectDisconnect: 1,
 			allocStatus:      structs.AllocClientStatusLost,
 		},
@@ -5447,7 +5446,7 @@ func TestReconciler_Disconnected_Client(t *testing.T) {
 			},
 		},
 		{
-			name:                    "ignore-reconnect-completed",
+			name:                    "update-reconnect-completed",
 			allocCount:              2,
 			replace:                 false,
 			disconnectedAllocCount:  2,
@@ -5456,11 +5455,11 @@ func TestReconciler_Disconnected_Client(t *testing.T) {
 			disconnectedAllocStates: disconnectAllocState,
 			isBatch:                 true,
 			expected: &resultExpectation{
-				place: 2,
+				place: 0,
 				desiredTGUpdates: map[string]*structs.DesiredUpdates{
 					"web": {
 						Ignore: 2,
-						Place:  2,
+						Place:  0,
 					},
 				},
 			},
@@ -5617,13 +5616,13 @@ func TestReconciler_Disconnected_Client(t *testing.T) {
 			disconnectedAllocStates:      disconnectAllocState,
 			shouldStopOnDisconnectedNode: true,
 			expected: &resultExpectation{
-				stop:  2,
+				stop:  4,
 				place: 2,
 				desiredTGUpdates: map[string]*structs.DesiredUpdates{
 					"web": {
-						Stop:   2,
+						Stop:   4,
 						Place:  2,
-						Ignore: 5,
+						Ignore: 3,
 					},
 				},
 			},

diff --git a/scheduler/reconcile_util.go b/scheduler/reconcile_util.go
@@ -293,9 +293,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
 			}