Add a design doc for multi-node-maintenance#3505
Conversation
Signed-off-by: Matthias Einwag <meinwag@nvidia.com>
Summary by CodeRabbit
WalkthroughAdds a design document for the Multi-Node Maintenance Executor, covering node-scoped maintenance APIs, safeguarding semantics, progress tracking, conflict-free execution, implementation options, supported activities, and explicit out-of-scope responsibilities. ChangesMulti-Node Maintenance Executor
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔍 Container Scan SummaryNo Grype artifacts were found to aggregate. |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3505.docs.buildwithfern.com/infra-controller |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
docs/design/multi-node-maintenance.md (1)
12-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the Markdown heading hierarchy.
# Overviewfollowed by### What are multi-node maintenance operations?skips a heading level, and markdownlint flags this at Line 24. MakeOverviewan##heading or lower the child heading consistently.As per path instructions, Markdown must be structurally correct and clear.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/multi-node-maintenance.md` around lines 12 - 24, Fix the heading hierarchy in the maintenance design document by changing the “Overview” heading to an appropriate level that makes “What are multi-node maintenance operations?” a valid nested heading. Preserve the existing section structure and content.Sources: Path instructions, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/multi-node-maintenance.md`:
- Around line 98-101: Update the MultiNodeMaintenanceResponse protobuf example
to assign a field number to maintenance_id, preserving its common.UUID type and
existing field name.
- Around line 66-72: Resolve the API ambiguity in the design document by
explicitly marking the rack-scoped API defined in the earlier API section as the
legacy contract, or remove it if it is no longer supported. Ensure the proposed
design consistently documents node-set-based requests without rack_id so
implementers use the correct endpoint and request shape.
- Around line 103-110: Clarify the semantics of an empty MaintenanceScope and
require validation that prevents it from being silently interpreted as a
fleet-wide operation. Explicitly define whether an empty scope is invalid or
represents no targets, document the chosen behavior, and ensure requests with
all of machine_ids, switch_ids, power_shelf_ids, and racks empty are rejected or
handled safely.
- Around line 18-20: Correct the spelling, grammar, and technical wording
throughout the referenced Markdown documentation, including “scale-up,” “any
kind,” “MNME-related,” “non-service-impacting,” “automatically,” “a set …
followed by,” “NICo-managed,” “MNME’s,” and “maintenance.” Remove the duplicated
“not” in “does not not have,” while preserving the documented technical meaning.
- Around line 157-173: Expand the MultiNodeMaintenance lifecycle to define
failure and timeout behavior for safeguarding, update application, and
guard-state release, including bounded retries and recorded phase/error state.
Specify how the periodic MNME task reconstructs and resumes in-progress
operations after process restart, and ensure recovery attempts release any nodes
left in guard state.
- Around line 121-130: Complete the MultiNodeMaintenanceStatus contract by
defining MultiNodeMaintenanceResult and documenting each result’s target node,
per-node phase transitions, and error details. Specify how partial failures and
mixed node states are represented while retaining the operation-level
LifecycleStatus, so consumers can distinguish in-progress, successful, and
failed node work.
- Around line 77-101: Define idempotency for ScheduleMultiNodeMaintenance by
adding an idempotency token to MultiNodeMaintenanceRequest and documenting that
retries with the same token and equivalent request return the existing
maintenance_id instead of enqueueing another operation. Specify the behavior for
token reuse with a different request.
- Around line 148-156: Update the maintenance-operation claiming flow to
atomically transition an operation from “Scheduled” to “In Progress” while
reserving its complete conflict set. Ensure the reservation includes directly
impacted nodes, indirectly impacted nodes, and all extra_safeguard_nodes,
preventing concurrent schedulers from claiming overlapping operations.
- Around line 91-95: Update the maintenance API design around
danger_skip_safeguarding and ignored_health_alert_classifications to require an
explicit admin-only permission, following the rack maintenance API model.
Specify durable audit logging for every request that sets either safety-bypass
field, including the requester and submitted bypass values.
---
Nitpick comments:
In `@docs/design/multi-node-maintenance.md`:
- Around line 12-24: Fix the heading hierarchy in the maintenance design
document by changing the “Overview” heading to an appropriate level that makes
“What are multi-node maintenance operations?” a valid nested heading. Preserve
the existing section structure and content.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2a852132-ed1f-4ea0-8a69-c2732b515e88
📒 Files selected for processing (1)
docs/design/multi-node-maintenance.md
| - MNME allows to express safe maintenance operations for systems where maintenance side-effects do not exactly match rack boundaries. It supports architectures where the rack is no longer equivalent to the scaleup domain. That includes systems where | ||
| - the maintenance domain (e.g. NVLink failure domain) is smaller than a rack - e.g. if a GB200 rack is subdivided into multiple "mini racks" | ||
| - the maintenance domain spans multiple racks due to a any kind of shared resources |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the documented spelling and grammar before merging.
Please fix the typos and wording, including scale-up, any kind, MNME-related, non-service-impacting, automatically, a set ... followed by, NICo-managed, MNME’s, maintenance, and the duplicated not in “does not not have.”
As per path instructions, Markdown documentation must meet spelling, grammar, clarity, and technical-writing standards.
Also applies to: 38-38, 67-70, 156-156, 168-168, 176-176, 182-188
🧰 Tools
🪛 LanguageTool
[grammar] ~18-~18: Ensure spelling is correct
Context: ...the rack is no longer equivalent to the scaleup domain. That includes systems where -...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 18 - 20, Correct the
spelling, grammar, and technical wording throughout the referenced Markdown
documentation, including “scale-up,” “any kind,” “MNME-related,”
“non-service-impacting,” “automatically,” “a set … followed by,” “NICo-managed,”
“MNME’s,” and “maintenance.” Remove the duplicated “not” in “does not not have,”
while preserving the documented technical meaning.
Sources: Path instructions, Linters/SAST tools
| There are however multiple key differences: | ||
| 1. The user no longer specifies a `rack_id`, but just the set of nodes to perform operations on. The system will automatically detect relationships between the affected nodes and any others managed by NICo and make sure that all nodes are in a "safe" state which allows the changes to be performed in a non-service impacting fashion. E.g. | ||
| 1. if a NVSwitch update is triggered, MNME will automatically detect all compute trays associated with the same NVLink domain as the switch, and make sure they are in safeguarded state (explanation below) before applying the update - the compute tray nodes do not have to get referenced. | ||
| 2. if an update is scheduled that touches a certain set of compute trays in the rack (e.g. MachineA, MachineB) while other update on other machines are in progress (MachineC), the update will get executed immediately. If the set of machines is however overlapping, the update is scheduled behind the previous. | ||
| 2. NICo site-admins can explicitly reference additional nodes that the maintenance operation is impacting - for use-cases where the system internally is not able to determine the full impact. Every referenced node will also be moved into a safeguarded state. | ||
| 3. NICo site-admins can explicitly choose to perform the update application without moving the nodes into a safeguarded state. This expert option can be used. | ||
| 4. Site admins will observe the progress of the update application no longer via status changes on the rack state machine, but by querying the maintenance status for the ID returned by the maintenance request. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve the API contradiction between the current and proposed designs.
Lines 40–64 define a rack-scoped API with rack_id, but this section says callers no longer specify rack_id. Explicitly label the first API as the legacy contract or remove it; otherwise implementers may expose the wrong endpoint and request shape.
As per path instructions, the design document must be technically correct and operator-usable.
🧰 Tools
🪛 LanguageTool
[grammar] ~67-~67: Use a hyphen to join words.
Context: ...changes to be performed in a non-service impacting fashion. E.g. 1. if a NVSwi...
(QB_NEW_EN_HYPHEN)
[style] ~70-~70: As a shorter alternative for ‘able to’, consider using “can not”.
Context: ...r use-cases where the system internally is not able to determine the full impact. Every refere...
(BE_ABLE_TO)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 66 - 72, Resolve the API
ambiguity in the design document by explicitly marking the rack-scoped API
defined in the earlier API section as the legacy contract, or remove it if it is
no longer supported. Ensure the proposed design consistently documents
node-set-based requests without rack_id so implementers use the correct endpoint
and request shape.
Source: Path instructions
| rpc ScheduleMultiNodeMaintenance(MultiNodeMaintenanceRequest) returns (MultiNodeMaintenanceResponse); | ||
| rpc FindMultiNodeMaintenanceOperationsByIds(MultiNodeMaintenanceIds) returns (FindMultiNodeMaintenanceOperationsByIdsResponse); | ||
|
|
||
| message MultiNodeMaintenanceRequest { | ||
| // The set of nodes that the operation will be carried out on directly. | ||
| // Additional nodes which will be affected by the change will be determined by the system automatically | ||
| MaintenanceScope scope = 1; | ||
|
|
||
| // Which maintenance activities to run. Empty means all activities. | ||
| repeated MaintenanceActivityConfig activities = 2; | ||
|
|
||
| // Additional nodes that need to be moved into a safeguarded state while the maintenance is performed | ||
| MaintenanceScope extra_safeguard_nodes = 3; | ||
|
|
||
| // Allows to skip any safeguarding operations. The maintenance operation will be performed immediately | ||
| bool danger_skip_safeguarding = 4; | ||
|
|
||
| // By default the system will only start updates on nodes if all of them are healthy. Adding health alert classifications to this list will allow to enforce update scheduling on nodes which are unhealthy for specific reasons | ||
| repeated string ignored_health_alert_classifications = 5; | ||
| } | ||
|
|
||
| message MultiNodeMaintenanceResponse { | ||
| // An ID that can be used to query the status of the maintenance operation | ||
| common.UUID maintenance_id; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n## target file lines 1-220\n'
cat -n docs/design/multi-node-maintenance.md | sed -n '1,220p'
printf '\n## search for maintenance_id and idempotency references\n'
rg -n "idempot|dedup|retry|maintenance_id|ScheduleMultiNodeMaintenance|FindMultiNodeMaintenanceOperationsByIds" docs . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: NVIDIA/infra-controller
Length of output: 50383
Define ScheduleMultiNodeMaintenance idempotency. A retry after a timeout can enqueue duplicate operations unless the API adds an idempotency token or explicitly states how equivalent requests map to the same maintenance_id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 77 - 101, Define
idempotency for ScheduleMultiNodeMaintenance by adding an idempotency token to
MultiNodeMaintenanceRequest and documenting that retries with the same token and
equivalent request return the existing maintenance_id instead of enqueueing
another operation. Specify the behavior for token reuse with a different
request.
Source: Path instructions
| // Allows to skip any safeguarding operations. The maintenance operation will be performed immediately | ||
| bool danger_skip_safeguarding = 4; | ||
|
|
||
| // By default the system will only start updates on nodes if all of them are healthy. Adding health alert classifications to this list will allow to enforce update scheduling on nodes which are unhealthy for specific reasons | ||
| repeated string ignored_health_alert_classifications = 5; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files docs/design/multi-node-maintenance.md
echo
echo "== relevant search hits =="
rg -n --hidden -S "danger_skip_safeguarding|ignored_health_alert_classifications|safeguard|maintenance" docs . \
-g '!*node_modules*' -g '!*.png' -g '!*.jpg' -g '!*.jpeg' -g '!*.gif' | sed -n '1,220p'
echo
echo "== doc excerpt around flagged lines =="
sed -n '70,120p' docs/design/multi-node-maintenance.mdRepository: NVIDIA/infra-controller
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== design doc excerpt ==\n'
sed -n '1,220p' docs/design/multi-node-maintenance.md | nl -ba | sed -n '80,120p'
printf '\n== exact symbol search ==\n'
rg -n -S "danger_skip_safeguarding|ignored_health_alert_classifications" .
printf '\n== readiness/handler search ==\n'
rg -n -S "readiness|ready.*maintenance|maintenance.*ready|safeguard|authorization|audit" docs . | sed -n '1,220p'Repository: NVIDIA/infra-controller
Length of output: 227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== targeted excerpt ==\n'
sed -n '36,110p' docs/design/multi-node-maintenance.md
printf '\n== auth/audit mentions in the design doc ==\n'
rg -n -S "authorization|authorize|auth|audit|allowlist|allow-list|role|permission|operator|logging|log" docs/design/multi-node-maintenance.md
printf '\n== related maintenance permission references ==\n'
rg -n -S "OnDemandRackMaintenance permission|NicoAdminCLI role|permission is granted|audit" docs/architecture/state_machines/rack.md docs/architecture/state_machines/rackstatemachine.mdRepository: NVIDIA/infra-controller
Length of output: 4809
Gate the bypass flags behind an explicit permission and audit trail.
danger_skip_safeguarding and ignored_health_alert_classifications are safety bypasses, but the design does not say which role/permission may set them or how such requests are recorded. Mirror the rack maintenance API’s admin-only model here and require durable audit logging for any request that uses these fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 91 - 95, Update the
maintenance API design around danger_skip_safeguarding and
ignored_health_alert_classifications to require an explicit admin-only
permission, following the rack maintenance API model. Specify durable audit
logging for every request that sets either safety-bypass field, including the
requester and submitted bypass values.
| message MultiNodeMaintenanceResponse { | ||
| // An ID that can be used to query the status of the maintenance operation | ||
| common.UUID maintenance_id; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Add the missing protobuf field number.
common.UUID maintenance_id; is invalid protobuf syntax. It must include a field number, for example common.UUID maintenance_id = 1;.
As per path instructions, documentation examples must be technically correct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 98 - 101, Update the
MultiNodeMaintenanceResponse protobuf example to assign a field number to
maintenance_id, preserving its common.UUID type and existing field name.
Source: Path instructions
| // Contains the set of nodes a maintenance operation is impacting | ||
| message MaintenanceScope { | ||
| repeated string machine_ids = 1; | ||
| repeated string switch_ids = 2; | ||
| repeated string power_shelf_ids = 3; | ||
| // Optionally specified if a one or multiple Racks are target of the request | ||
| repeated RackId racks = 4; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Define or reject an empty maintenance scope.
MaintenanceScope allows every node and rack list to be empty, but the document does not say whether that means “all managed nodes,” “no targets,” or an invalid request. Given the predecessor contract’s broad-scope behavior, silently interpreting this as a fleet-wide operation would be dangerous; require explicit semantics and validation.
As per path instructions, the design must be technically correct and safe for operators.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 103 - 110, Clarify the
semantics of an empty MaintenanceScope and require validation that prevents it
from being silently interpreted as a fleet-wide operation. Explicitly define
whether an empty scope is invalid or represents no targets, document the chosen
behavior, and ensure requests with all of machine_ids, switch_ids,
power_shelf_ids, and racks empty are rejected or handled safely.
Source: Path instructions
| message FindMultiNodeMaintenanceOperationsByIdsResponse { | ||
| repeated MultiNodeMaintenanceStatus status; | ||
| } | ||
|
|
||
| message MultiNodeMaintenanceStatus { | ||
| common.UUID id = 1; | ||
| // Current state of the operation | ||
| LifecycleStatus status = 2; | ||
| // Results. These will be available as soon as parts of the operations finish | ||
| repeated MultiNodeMaintenanceResult results = 3; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced document and search for the missing type definition.
FILE="docs/design/multi-node-maintenance.md"
echo "== File size =="
wc -l "$FILE"
echo
echo "== Outline / nearby headings =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== Relevant excerpt around lines 100-160 =="
sed -n '100,160p' "$FILE" | cat -n
echo
echo "== Search for MultiNodeMaintenanceResult and related status fields =="
rg -n "MultiNodeMaintenanceResult|MultiNodeMaintenanceStatus|LifecycleStatus|guarded|partial-failure|partial failure|target node|execution phase|results" docs/design/multi-node-maintenance.mdRepository: NVIDIA/infra-controller
Length of output: 5744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the document references the result type elsewhere, inspect the surrounding context.
rg -n -C 3 "MultiNodeMaintenanceResult" docs/design/multi-node-maintenance.md || true
# Search the repo for any actual schema/type definition with that name.
rg -n "message\s+MultiNodeMaintenanceResult|type\s+MultiNodeMaintenanceResult|enum\s+MultiNodeMaintenanceResult" .Repository: NVIDIA/infra-controller
Length of output: 431
Complete the status contract in docs/design/multi-node-maintenance.md:122-130. MultiNodeMaintenanceResult is still undefined, and the status model only exposes a coarse LifecycleStatus plus opaque results; it does not specify per-node phase, target node, error handling, or how partial failures are represented. Add the missing type and spell out the node-level state transitions before publishing this API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 121 - 130, Complete the
MultiNodeMaintenanceStatus contract by defining MultiNodeMaintenanceResult and
documenting each result’s target node, per-node phase transitions, and error
details. Specify how partial failures and mixed node states are represented
while retaining the operation-level LifecycleStatus, so consumers can
distinguish in-progress, successful, and failed node work.
Source: Path instructions
| In each iteration of the periodic task, MNME will perform the following steps: | ||
| 1. Identify executable maintenance operations and start them via the following steps: | ||
| 1. Query the list of requested maintenance operations. These will need to get stored in the NICo database after receiving a `ScheduleMultiNodeMaintenance` call. | ||
| 2. For each planned maintenance operation, determine the full set of impacted nodes. This contains the directly impacted nodes, as well as indirectly impacted nodes. Indirectly impacted nodes can be determined based on the activity type and various links between entities, e.g. | ||
| - Pure compute tray firmware updates do not directly have side effects on other nodes -> The list of indirectly impacted nodes is empty | ||
| - Switch operations will impact all nodes that share the same NVLink domain | ||
| 3. If the set of impacted nodes does not intersect with any update that is already in progress, advance the update task from "Scheduled" to "In Progress". The intersection could be determined in 2 ways: | ||
| 1. Directly load information about all in-progress updates and perform intersection operation. | ||
| 2. Check health-alerts on affected nodes (`MultiNodeUpdateInProgress`). This requires the start of the update to place the health alert atomatically on all affected nodes. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== file outline ==\n'
wc -l docs/design/multi-node-maintenance.md
sed -n '130,190p' docs/design/multi-node-maintenance.md
printf '\n== search terms ==\n'
rg -n "ScheduleMultiNodeMaintenance|MultiNodeUpdateInProgress|extra_safeguard_nodes|In Progress|Scheduled|claim|atomic|transaction" docs src . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || trueRepository: NVIDIA/infra-controller
Length of output: 50381
🏁 Script executed:
set -euo pipefail
echo '== multi-node-maintenance excerpt =='
nl -ba docs/design/multi-node-maintenance.md | sed -n '140,175p'
echo
echo '== mentions of safeguard/reservation semantics =='
rg -n "safeguard|reserve|reservation|claim|transaction|health-alert|MultiNodeUpdateInProgress|ScheduleMultiNodeMaintenance" docs/design/multi-node-maintenance.md docs -g '!**/node_modules/**' || trueRepository: NVIDIA/infra-controller
Length of output: 238
🏁 Script executed:
set -euo pipefail
echo '== design excerpt =='
sed -n '140,175p' docs/design/multi-node-maintenance.md
echo
echo '== targeted references =='
rg -n -C 2 "extra_safeguard_nodes|MultiNodeUpdateInProgress|ScheduleMultiNodeMaintenance|impacted nodes|Scheduled.*In Progress|In Progress.*Scheduled|health-alert" docs/design/multi-node-maintenance.md crates/api-core/src/tests/machine_update_manager.rs crates/api-core/src -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || trueRepository: NVIDIA/infra-controller
Length of output: 6940
🏁 Script executed:
set -euo pipefail
sed -n '80,95p' docs/design/multi-node-maintenance.mdRepository: NVIDIA/infra-controller
Length of output: 1115
Claim maintenance operations atomically and reserve every affected node. The Scheduled→In Progress transition needs to be part of the same claim to avoid overlapping schedulers accepting two requests at once. Include direct nodes, indirectly impacted nodes, and extra_safeguard_nodes in the conflict set.
🧰 Tools
🪛 LanguageTool
[grammar] ~156-~156: Ensure spelling is correct
Context: ...of the update to place the health alert atomatically on all affected nodes. 2. For all "In P...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 148 - 156, Update the
maintenance-operation claiming flow to atomically transition an operation from
“Scheduled” to “In Progress” while reserving its complete conflict set. Ensure
the reservation includes directly impacted nodes, indirectly impacted nodes, and
all extra_safeguard_nodes, preventing concurrent schedulers from claiming
overlapping operations.
| 2. For all "In Progress" updates, perform the following steps: | ||
| 1. Initiate triggers that let the nodes to start moving all impacted nodes (direct and indirect impact) into a guard state. E.g. for Compute Trays, set a flag that make the nodes boot scout on the next restart. | ||
| 2. Wait for these nodes to reach the guard state. | ||
| 3. Apply the actual update. This can e.g. happen via calling RMS APIs, and waiting for the results of the update. | ||
| 4. After update completion, set a flag that releases the nodes from the guard state (in their individual state machines) | ||
| 5. Wait for all nodes to finish exiting the guard state | ||
|
|
||
| The steps within 2) are equivalent to the steps executed by the Rack state machine for on-demand maintenance. | ||
|
|
||
| ### Implementation options | ||
|
|
||
| As described in the previous chapter, MNME contains 2 major components: A set of deployment-wide operations which identify newly executable maintenance operations, followed by a the execution of all in-progress options, which can get parallelized. | ||
|
|
||
| These 2 steps can be implemented in the following fashions: | ||
| - A single task performs both of step 1) and 2). Step 2) could be parallelized via some fork/join mechanism (using `tokio::spawn` or `tokio::task::JoinSet`). This model would be equivalent to components like NVLink Monitor. | ||
| - The periodic MNME main task only performs step 1). In order to schedule updates, it creates `MultiNodeMaintenance` objects, where each object is lifecycle is managed by the existing state controller framework. This model would allow to reuse code for concurrent state management on a set of maintenance objects. | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant excerpt ==\n'
sed -n '130,220p' docs/design/multi-node-maintenance.md | cat -n
printf '\n== Search for failure/retry/restart semantics ==\n'
rg -n -i 'fail|failure|timeout|retry|restart|recover|recovery|persist|state machine|guard state|stranded' docs/design/multi-node-maintenance.mdRepository: NVIDIA/infra-controller
Length of output: 7578
Define failure, timeout, and restart recovery semantics. The lifecycle only covers the happy path; it does not say what happens when safeguarding, the update, or guard-state release fails, how long each phase may block, or how a periodic run resumes after a process restart. Persist phase/error state, add retry/timeout behavior, and ensure nodes cannot remain stranded in guard state.
🧰 Tools
🪛 LanguageTool
[style] ~172-~172: Consider a more concise word here.
Context: ...c MNME main task only performs step 1). In order to schedule updates, it creates `MultiNode...
(IN_ORDER_TO_PREMIUM)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/multi-node-maintenance.md` around lines 157 - 173, Expand the
MultiNodeMaintenance lifecycle to define failure and timeout behavior for
safeguarding, update application, and guard-state release, including bounded
retries and recorded phase/error state. Specify how the periodic MNME task
reconstructs and resumes in-progress operations after process restart, and
ensure recovery attempts release any nodes left in guard state.
Source: Path instructions
|
I agree with the direction of this design, especially on
When it comes to the routine execution, queuing, and precondition wait-checking, I think it overlaps with what Flow already does today, which has a workflow engine with repeated state/health precondition checking before issuing actual operation calls into Core. There is also already a forced execution path there. How about the following:
With this comes the question of scheduled maintenance (from ingestion), which could in short term call the "tray set" component management Core API from within the Core, with a simple retry loop wrapping it (nothing should really be competing for a rack still in ingestion). Other cases of non-on-demand maintenance triggering logic probably should be moved to Flow and be implemented with the task automation capability there. |
2 participants
This design document document proposes a new component within NICo-core which allows execution of multi-node maintenance operations in a conflict-free and efficient manner.
Multi-Node maintenance executor (MNME) is a replacement for the current maintenance execution via the rack-state machine. It improves over it in the following areas:
Implementing MNME moves most logic out of the Rack State Machine into a different component, which reduces the need for having a type visible in NICo (
message Rack) which does not actually physically exist.