Skip to content

feat: add max remediation attempts per equivalence group - #1639

Closed
iacker wants to merge 3 commits into
NVIDIA:mainfrom
iacker:feat/max-remediation-attempts-1543
Closed

feat: add max remediation attempts per equivalence group#1639
iacker wants to merge 3 commits into
NVIDIA:mainfrom
iacker:feat/max-remediation-attempts-1543

Conversation

@iacker

@iacker iacker commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

This PR implements a configurable maximum number of remediation attempts per equivalence group to prevent infinite remediation loops.

Changes

  • Added MaxRetryAttempts field to TomlConfig (default 0 = unlimited, preserving current behavior)
  • Added RetryCount field to EquivalenceGroupState in the node annotation
  • The reconciler now increments the retry count on each remediation attempt
  • Remediation is skipped when the retry limit is exceeded, marking the event as failed
  • Added unit tests verifying retry count increments, independence per group, and persistence across pod restarts

Implementation

The retry counter persists across pod restarts because it's stored in the node's annotation (nvidia.com/fault-remediation-state) rather than in-memory state. Each time a remediation CR is created for an equivalence group, the counter increments and is written back to the annotation.

When MaxRetryAttempts is configured and the limit is reached, the remediation is skipped with the same behavior as unsupported events (marked as failed, event processed, but no CR created).

Testing

cd fault-remediation
go test ./pkg/annotation/... -run TestRetry -v
go test ./pkg/annotation/...
go test ./pkg/config/...

All tests pass.

Fixes #1543

Summary by CodeRabbit

  • New Features
    • Added configurable maximum remediation retry attempts, with unlimited retries retained as the default.
    • Remediation retry counts now persist across restarts and are tracked independently for each equivalence group.
  • Bug Fixes
    • Remediation events exceeding the configured retry limit are now safely marked as failed and skipped.
    • Improved GPU recovery matching to recognize replacement hardware and clear stale fault conditions.
    • Preserved existing behavior for non-GPU events and GPUs without stable identity information.

iacker added 3 commits August 14, 2026 01:26
Signed-off-by: Billard <82095453+iacker@users.noreply.github.com>
Signed-off-by: Billard <82095453+iacker@users.noreply.github.com>
This addresses issue NVIDIA#1543 by implementing a configurable limit
on the number of remediation attempts per equivalence group.

Changes:
- Add MaxRetryAttempts config field to TomlConfig
- Add RetryCount field to EquivalenceGroupState (persists in node annotation)
- Increment retry count on each remediation attempt
- Skip remediation when retry limit is exceeded
- Add tests for retry count tracking and persistence

The retry counter survives pod restarts because it's stored in the
node's annotation rather than in-memory state.

Fixes NVIDIA#1543

Signed-off-by: Billard <82095453+iacker@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: de47718f-3dd1-4852-8769-50bc09c0e202

📥 Commits

Reviewing files that changed from the base of the PR and between 42672ae and be8d771.

📒 Files selected for processing (7)
  • fault-remediation/pkg/annotation/annotation.go
  • fault-remediation/pkg/annotation/annotation_interface.go
  • fault-remediation/pkg/annotation/retry_count_test.go
  • fault-remediation/pkg/config/config.go
  • fault-remediation/pkg/reconciler/reconciler.go
  • platform-connectors/pkg/connectors/kubernetes/k8s_platform_connector_test.go
  • platform-connectors/pkg/connectors/kubernetes/process_node_events.go

📝 Walkthrough

Walkthrough

The change persists remediation retry counts per equivalence group, enforces configurable retry limits, and normalizes GPU recovery entities to clear stale conditions after hardware replacement.

Changes

Remediation retry limits

Layer / File(s) Summary
Persist remediation retry state
fault-remediation/pkg/annotation/annotation_interface.go, fault-remediation/pkg/annotation/annotation.go, fault-remediation/pkg/annotation/retry_count_test.go
EquivalenceGroupState stores RetryCount. Existing groups increment the count during updates. Tests cover increments, group isolation, and persistence across manager instances.
Enforce configured retry limits
fault-remediation/pkg/config/config.go, fault-remediation/pkg/reconciler/reconciler.go
MaxRetryAttempts configures the limit. The reconciler checks retry state before remediation and records events as failed and skipped when the limit is reached.

GPU recovery identity handling

Layer / File(s) Summary
Normalize GPU recovery entities
platform-connectors/pkg/connectors/kubernetes/process_node_events.go, platform-connectors/pkg/connectors/kubernetes/k8s_platform_connector_test.go
GPU recovery uses stable GPU or PCI identities when available. UUID fallback remains for events without stable identities. Tests verify stale UUID condition cleanup and unchanged non-GPU behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Reconciler
  participant NodeAnnotationManager
  participant NodeAnnotations
  participant RemediationStatus

  Reconciler->>NodeAnnotationManager: check equivalence group retryCount
  NodeAnnotationManager->>NodeAnnotations: read persisted group state
  NodeAnnotationManager-->>Reconciler: return retryCount
  Reconciler->>RemediationStatus: mark failed and skipped at limit
Loading

Suggested reviewers: xrfxlp

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@iacker

iacker commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Closing this in favor of #1640 which contains only the intended commit for #1543. This PR accidentally included two commits from the old #1587 work.

@iacker iacker closed this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Enforce a maximum number of remediation attempts per node

1 participant