feat(OnlineRepair): Add RequestOnlineRepair CLI template; Block instance release when aggregate health includes PreventDeletion by sunilkumar-nvidia · Pull Request #1184 · NVIDIA/infra-controller-core

sunilkumar-nvidia · 2026-04-28T02:22:57Z

Description

This work adds online repair support for instances from the core: operators can mark a machine for handoff to online repair, block normal instance release while that state is present, then clear it when repair is done.

Typical flow

Use admin-cli to insert a health report from the request-online-repair template (merge source request-online-repair).
Run manual online repair on the instance (outside these changes).
When the machine is healthy again, remove that health report entry (same source) so release and health state can proceed.

Code changes

Introduce HealthAlertClassification::prevent_deletion(). ReleaseInstance is rejected when aggregate host health includes that classification, with InstanceReleaseBlockedByPreventDeletion. Admin force-delete is not subject to this check.
Add the request-online-repair template in admin-cli (classifications include prevent allocations, suppressed external alerting, and prevent deletion), with unit tests.
Add an API integration test that merges a health report carrying PreventDeletion, expects release to fail, then expects release to succeed after the entry is removed.

Type of Change

Add - New feature or capability
Change - Changes in existing functionality
Fix - Bug fixes
Remove - Removed features or deprecated functionality
Internal - Internal changes (refactoring, tests, docs, etc.)

Related Issues (Optional)

Breaking Changes

This PR contains breaking changes

Testing

Unit tests added/updated
Integration tests added/updated
Manual testing performed
No testing required (docs, internal refactor, etc.)

Additional Notes

Summary by CodeRabbit

New Features
- Added "request-online-repair" health report template option to the admin CLI.
- Added new "PreventDeletion" alert classification that prevents instance release operations when active.
Tests
- Added integration test verifying instance release is blocked when prevent_deletion health alerts are present.

Introducing new HealthOverride Classification - PreventDeletion

ajf · 2026-04-28T19:35:56Z

@coderabbitai review

coderabbitai · 2026-04-28T19:36:02Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

ajf · 2026-04-28T19:37:00Z

I'm testing coderabbit out on this PR @sunilkumar-nvidia :)

coderabbitai · 2026-04-28T19:41:37Z

Walkthrough

A new "request-online-repair" health report template and "PreventDeletion" alert classification are introduced, with release-handler logic that blocks instance release operations when the prevent-deletion classification appears in aggregate health.

Changes

Cohort / File(s)	Summary
Health Report Template Addition `crates/admin-cli/src/machine/health_report/args.rs`, `crates/admin-cli/src/machine/health_report/cmd.rs`, `crates/admin-cli/src/machine/tests.rs`	New `RequestOnlineRepair` template variant added to CLI enum, implemented in `get_health_report` with dedicated alert source/ID/target and `prevent_deletion` classification, verified via parsing test.
Alert Classification `crates/health-report/src/lib.rs`	New `prevent_deletion()` constructor added to `HealthAlertClassification` producing the `"PreventDeletion"` classification string value, with unit test.
Instance Release Blocking `crates/api-model/src/lib.rs`, `crates/api/src/handlers/instance.rs`	New `ConfigValidationError::InstanceReleaseBlockedByPreventDeletion` variant introduced; release handler enhanced with guard checking aggregate health for `prevent_deletion` classification, rejecting release if found.
Release Blocking Integration Test `crates/api/src/tests/instance.rs`	New SQLx-backed integration test verifying instance release is rejected when health override contains `prevent_deletion`, and succeeds after override removal.

Sequence Diagram

sequenceDiagram
    participant Client
    participant APIHandler as Release Handler
    participant DB as Database/Health
    
    Client->>APIHandler: release_instance()
    APIHandler->>APIHandler: Check if marked for deletion
    alt Already deleted
        APIHandler->>Client: Return success
    else Not deleted
        APIHandler->>DB: Load managed-host snapshot<br/>with aggregate_health
        DB->>APIHandler: Return health status
        APIHandler->>APIHandler: Check for prevent_deletion<br/>in classifications
        alt prevent_deletion present
            APIHandler->>Client: Return ConfigValidationError<br/>(InstanceReleaseBlockedByPreventDeletion)
        else prevent_deletion absent
            APIHandler->>APIHandler: Proceed with release
            APIHandler->>Client: Return success
        end
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A request for repair, a gentle care,
Prevention of deletion, beyond compare,
No hasty release when health must mend,
Online we heal, our systems defend! 🌿✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the two main changes: adding a RequestOnlineRepair CLI template and blocking instance release when PreventDeletion alert is present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/api/src/tests/instance.rs`:
- Around line 4895-4923: The calls to
common::api_fixtures::send_health_report_override and
common::api_fixtures::remove_health_report_override reference a non-existent
module path (causing E0425); fix by either importing/using the correct fixtures
module that actually provides these helpers (e.g., change
common::api_fixtures::... to the external fixtures crate path that exports
send_health_report_override/remove_health_report_override or add pub functions
with those names to crate::tests::common), and update the call sites to use the
correct module (or add a use statement) so send_health_report_override and
remove_health_report_override resolve.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 940bea96-3d86-444a-a05e-55d4b33a0b57

📥 Commits

Reviewing files that changed from the base of the PR and between 696f99d and e4215bb.

📒 Files selected for processing (7)

crates/admin-cli/src/machine/health_report/args.rs
crates/admin-cli/src/machine/health_report/cmd.rs
crates/admin-cli/src/machine/tests.rs
crates/api-model/src/lib.rs
crates/api/src/handlers/instance.rs
crates/api/src/tests/instance.rs
crates/health-report/src/lib.rs

github-actions · 2026-05-11T07:11:21Z

🌿 Preview your docs: https://nvidia-preview-pull-request-1184.docs.buildwithfern.com/infra-controller

williampnvidia

Approved with feedback.

wminckler · 2026-05-11T11:27:29Z

            ];
        }
+        // Same shape as TenantReportedIssue; distinct merge source and probe id for online repair.
+        // Includes `PreventDeletion` so `ReleaseInstance` is blocked while this merge is present (not admin force-delete).


If this is really stopping instance release maybe "prevent deletion" isn't the best. If "deletion" is needed, maybe qualify it with "instance". I.e. PreventInstanceDeletion

I'd prefer something like "PreventInstanceRelease"

To align with the existing PreventAllocation classification—which prevents a machine from being allocated to any tenant as an instance—I’m proposing PreventDeletion . This would similarly prevent an instance or machine from being removed or deleted.
Let me know if I’m off track here.

My comment is only about the name PreventDeletion. In core, we don't "delete" instances, we "release" them. To add to the confusion, you can "delete" a machine, which is unrelated to this change

hwadekar-nv · 2026-05-11T22:28:08Z

+
+    if snapshot
+        .aggregate_health
+        .has_classification(&HealthAlertClassification::prevent_deletion())


When tenant marked OnlineRepair=Failed in Instance label, we should allow Instance to be deleted according to design, are we still considering that path?

yes, we should allow instance to be deleted when marked with Label OnLineRepair=Failed. However, this happens in two steps:

Call API - ExitOnlineRepair - This API removes the HealthOverride(RequestOnlineRepair) applied on the instance.

Call API - ReleaseInstance - To release/delete the instance from the tenant for repair (offline).

…nstances

copy-pr-bot · 2026-05-12T10:16:30Z

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.

…ntDeletion Signed-off-by: Sunil Kumar <sunilkumar@nvidia.com>

sunilkumar-nvidia added 3 commits April 23, 2026 12:19

Introducing new HealthOverride - RequestOnlineRepair

f520235

Introducing new HealthOverride Classification - PreventDeletion

Updating the admin-cli to apply the RequestOnlineRepair HealthOverride

37fa745

Merge remote-tracking branch 'upstream/main' into onlinerepair

e4215bb

sunilkumar-nvidia requested a review from a team as a code owner April 28, 2026 02:22

sunilkumar-nvidia self-assigned this Apr 28, 2026

coderabbitai Bot reviewed Apr 28, 2026

View reviewed changes

Comment thread crates/api/src/tests/instance.rs Outdated

Merge upstream/main into onlinerepair

ddf439c

sunilkumar-nvidia added 4 commits May 11, 2026 10:52

Resolving non-existent module path

91d99be

Merge remote-tracking branch 'upstream/main' into onlinerepair

3aa7063

fix fmt checks

c4ea248

fix clippy errors

9cc1feb

williampnvidia approved these changes May 11, 2026

View reviewed changes

Comment thread crates/health-report/src/lib.rs

Comment thread crates/api/src/handlers/instance.rs Outdated

wminckler reviewed May 11, 2026

View reviewed changes

hwadekar-nv reviewed May 11, 2026

View reviewed changes

fix(api): persist repair-tenant release cleanup for already-deleted i…

f3d531f

…nstances

sunilkumar-nvidia added 2 commits May 12, 2026 10:27

test(health-report): assert RequestOnlineRepair report includes Preve…

33bf335

…ntDeletion Signed-off-by: Sunil Kumar <sunilkumar@nvidia.com>

Merge upstream/main into onlinerepair

269bae5

sunilkumar-nvidia force-pushed the onlinerepair branch from 92b9535 to 269bae5 Compare May 12, 2026 10:29

Conversation

sunilkumar-nvidia commented Apr 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Related Issues (Optional)

Breaking Changes

Testing

Additional Notes

Summary by CodeRabbit

Uh oh!

ajf commented Apr 28, 2026

Uh oh!

coderabbitai Bot commented Apr 28, 2026

Uh oh!

ajf commented Apr 28, 2026

Uh oh!

coderabbitai Bot commented Apr 28, 2026

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

williampnvidia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wminckler May 11, 2026

Choose a reason for hiding this comment

Uh oh!

sunilkumar-nvidia May 12, 2026

Choose a reason for hiding this comment

Uh oh!

wminckler May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hwadekar-nv May 11, 2026

Choose a reason for hiding this comment

Uh oh!

sunilkumar-nvidia May 12, 2026

Choose a reason for hiding this comment

Uh oh!

copy-pr-bot Bot commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sunilkumar-nvidia commented Apr 28, 2026 •

edited by coderabbitai Bot

Loading

wminckler May 13, 2026 •

edited

Loading