Skip to content

Commit 3bae1a8

Browse files
fix: retry events received while an equivalent maintenance CR is in progress (#1550)
1 parent 3628b8f commit 3bae1a8

7 files changed

Lines changed: 950 additions & 102 deletions

File tree

commons/pkg/statemanager/statemanager.go

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@
7070
// - [NO LABEL]: No nvsentinel-state label present (healthy node)
7171
// - [TERMINAL]: Terminal states with no forward transitions
7272
// - remediation-succeeded and remediation-failed are terminal for a single failure, but
73-
// fault-remediation may recompute between them on a partial recovery (see below)
73+
// fault-remediation may recompute between them on a partial recovery, and either may
74+
// return to remediating when a new remediation cycle starts (see below)
7475
// - All state names match the dgxc.nvidia.com/nvsentinel-state label values
7576
// - Label removal (removeStateLabel=true) bypasses all validation
7677
//
@@ -98,6 +99,12 @@
9899
// remediation-succeeded → remediation-failed (fault-remediation: a remaining active failure is
99100
// unsupported or failed remediation)
100101
//
102+
// Re-remediation (a new remediation cycle starts while the node stays quarantined):
103+
// remediation-succeeded → remediating (fault-remediation: a new remediation-ready event
104+
// arrived after the previous maintenance CR completed,
105+
// e.g. a post-reboot fault)
106+
// remediation-failed → remediating (fault-remediation: a failed CR is retried with a new CR)
107+
//
101108
// Label Removal (from ANY state):
102109
// * → (no label) (removeStateLabel=true - supports canceled drains)
103110
//
@@ -115,8 +122,10 @@
115122
// Invalid Transitions:
116123
// drain-succeeded → drain-failed (cannot reverse drain result)
117124
// drain-failed → remediating (terminal state - no remediation)
118-
// remediation-succeeded → * (except remediation-failed via partial-recovery recompute)
119-
// remediation-failed → * (except remediation-succeeded via partial-recovery recompute)
125+
// remediation-succeeded → * (except remediation-failed via partial-recovery recompute
126+
// and remediating via re-remediation)
127+
// remediation-failed → * (except remediation-succeeded via partial-recovery recompute
128+
// and remediating via re-remediation)
120129
//
121130
// # Example Sequences
122131
//
@@ -150,11 +159,13 @@
150159
// drain-failed has no valid forward transitions (only label removal):
151160
// - drain-failed: Remediation doesn't process failed drains
152161
//
153-
// remediation-succeeded and remediation-failed are terminal for a single failure. The only
154-
// forward transition allowed is between the two of them, when fault-remediation recomputes the
155-
// node label from the remaining active failures during a partial recovery:
156-
// - remediation-succeeded: Success state (may be recomputed to remediation-failed)
157-
// - remediation-failed: Failure state (may be recomputed to remediation-succeeded)
162+
// remediation-succeeded and remediation-failed are terminal for a single failure. Two forward
163+
// transitions are allowed: between the two of them, when fault-remediation recomputes the node
164+
// label from the remaining active failures during a partial recovery, and back to remediating,
165+
// when fault-remediation starts a new remediation cycle (a remediation-ready event arrived after
166+
// the previous maintenance CR completed, or a failed CR is retried with a new CR):
167+
// - remediation-succeeded: Success state (may be recomputed or re-enter remediating)
168+
// - remediation-failed: Failure state (may be recomputed or re-enter remediating)
158169
package statemanager
159170

160171
import (
@@ -403,9 +414,13 @@ func validateStateTransition(nodeName, currentValue string, exists bool, targetS
403414
// remediation-succeeded and remediation-failed are terminal for a single failure, but a
404415
// partial recovery (a tracked failure clears while the node stays quarantined) lets
405416
// fault-remediation recompute the node label from the remaining active failures, which can
406-
// move between the two terminal remediation outcomes.
407-
RemediationSucceededLabelValue: {RemediationFailedLabelValue},
408-
RemediationFailedLabelValue: {RemediationSucceededLabelValue},
417+
// move between the two terminal remediation outcomes. Both states can also return to
418+
// remediating: a new remediation-ready event can arrive after an equivalent maintenance CR
419+
// completed (for example a post-reboot fault while the node is still quarantined), and a
420+
// failed CR is retried with a new CR, so fault-remediation legitimately starts another
421+
// remediation cycle within the same quarantine session.
422+
RemediationSucceededLabelValue: {RemediationFailedLabelValue, RemediatingLabelValue},
423+
RemediationFailedLabelValue: {RemediationSucceededLabelValue, RemediatingLabelValue},
409424
}
410425

411426
currentState := NVSentinelStateLabelValue(currentValue)

commons/pkg/statemanager/statemanager_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,11 @@ func TestStateTransitionValidProgression(t *testing.T) {
307307
// Partial-recovery recompute between terminal remediation outcomes (fault-remediation)
308308
{"RemediationFailed to RemediationSucceeded", string(RemediationFailedLabelValue), RemediationSucceededLabelValue, true, false},
309309
{"RemediationSucceeded to RemediationFailed", string(RemediationSucceededLabelValue), RemediationFailedLabelValue, true, false},
310+
// A new remediation cycle can start after a terminal remediation outcome while the node
311+
// stays quarantined: a post-session event creates a new CR after the previous CR
312+
// completed (issue #1536), and a failed CR is retried with a new CR (fault-remediation).
313+
{"RemediationSucceeded to Remediating", string(RemediationSucceededLabelValue), RemediatingLabelValue, true, false},
314+
{"RemediationFailed to Remediating", string(RemediationFailedLabelValue), RemediatingLabelValue, true, false},
310315

311316
// Unexpected progressions (return error but label is still updated)
312317
// This allows callers to emit error metrics while labels reflect reality

fault-remediation/pkg/metrics/metrics.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
const (
2525
CRStatusCreated = "created"
2626
CRStatusSkipped = "skipped"
27+
CRStatusWaiting = "waiting"
2728
)
2829

2930
var (

0 commit comments

Comments
 (0)