Skip to content

fix(machine-controller): gate Assigned/Ready on provisioning-completion on evidence for custom iPXE boots - #5087

Draft
hwadekar-nv wants to merge 1 commit into
NVIDIA:mainfrom
hwadekar-nv:fix/custom-ipxe-boot
Draft

fix(machine-controller): gate Assigned/Ready on provisioning-completion on evidence for custom iPXE boots#5087
hwadekar-nv wants to merge 1 commit into
NVIDIA:mainfrom
hwadekar-nv:fix/custom-ipxe-boot

Conversation

@hwadekar-nv

Copy link
Copy Markdown
Contributor

Description

NICo previously treated a one-time custom iPXE boot as complete the moment the tenant's script was served: WaitingForRebootToReady declared the instance Assigned/Ready right after issuing the reboot, and /api/v0/pxe/boot consumed the one-shot use_custom_pxe_on_boot request on the first serve. If the script then failed before installing an OS (e.g. a transient HTTP 403 on the kernel fetch), every later network boot received "exit into the OS" against an empty disk — an infinite boot loop reported as Ready, recoverable only by a manual rebootWithCustomIpxe.

This PR makes provisioning success an observed fact instead of an assumption. Provisioning boots (new instances, and reboots with rebootWithCustomIpxe/boot_with_custom_ipxe) now enter a new Assigned/WaitingForProvisioningComplete state instead of Ready, and the one-shot request stays armed until there is evidence the tenant's operating system actually installed.

Key behaviors:

  • While waiting, every PXE request re-serves the tenant's script instead of consuming the request on first serve — a failed install retries naturally, with each serve recorded in instances.custom_pxe_serve_count / custom_pxe_last_served_at.
  • Completion evidence: a fresh phone-home contact (cleared when the boot is armed) for phone-home-enrolled OSes, or the host going quiet for provisioning_quiet_window (default 15m) otherwise.
  • Bounded attempts: exceeding max_provisioning_serves or provisioning_deadline transitions the instance to Failed { ProvisioningFailed } — a clear terminal state for upstream orchestrators instead of a silent loop; tenants see Provisioning (not Ready) while the wait is in flight.
  • Recovery: rebootWithCustomIpxe and instance deletion are honored from both the waiting state and the ProvisioningFailed state, handing back to the Assigned/Ready flow without waiting for timeouts.
  • Plain reboots, always-PXE instances (run_provisioning_instructions_on_every_boot), DPU flows, and dev boot overrides are unchanged; the legacy consume-on-serve path in Assigned/Ready remains for machines already mid-flight.

Related issues

Type of Change

  • Fix - Bug fix or regression repair

Testing

  • Unit tests added/updated

Tests executed:
cargo test -p api-core
cargo test -p machine-controller
cargo test -p api-model

Covers: script re-served on repeated PXE requests without consuming the request; serve accounting; phone-home and quiet-window completion; serve-budget and deadline failure into ProvisioningFailed; recovery via rebootWithCustomIpxe from waiting and failed states; always-PXE and plain-reboot behavior unchanged.

Additional Notes

  • New instances columns via migration 20260817154233_custom_pxe_serve_tracking.sql (custom_pxe_serve_count, custom_pxe_last_served_at); both reset when a new provisioning boot is armed.
  • New controller config knobs with defaults (no deployment changes required): provisioning_quiet_window (15m), max_provisioning_serves, provisioning_deadline.
  • Completion/failure decision logic is isolated in machine-controller/src/handler/provisioning_completion.rs for testability.
  • WaitingForProvisioningComplete carries only started_at/deadline; serve bookkeeping lives on the instance row to avoid stale copies in persisted state.
  • State machine documentation updated in docs/architecture/state_machines/managedhost.md; tenant-facing behavior documented in docs/configuration/tenant_management.md.

@hwadekar-nv
hwadekar-nv requested review from a team and polarweasel as code owners August 18, 2026 00:39
@hwadekar-nv
hwadekar-nv marked this pull request as draft August 18, 2026 00:40
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a0a2654b-2af8-4798-8ea1-0a97f0b6693a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

@copy-pr-bot

copy-pr-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

@chet chet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't look at the change completely right now, but I just wanna make sure this isn't colliding with the work that Brian did to allow machine allocations to go into unmanaged networks.

I'll have to find his PR later, but the idea is that for one of our partners, as soon as the machine gets allocated and rebooted, we don't actually see the machine again until they release it.

Brian did the work to make this happen, it was either in June or the end of May.

@hwadekar-nv

Copy link
Copy Markdown
Contributor Author

Thank you, @chet, for your insight. When I reviewed the existing get notification (OS completion) feature, I discovered that it was only activated when the phone home feature was enabled. I will contact @bcavnvidia to confirm if there is a similar feature that could be referenced instead of the new one. There has been a recent bug reported that is attempting to fix these issues.

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.

2 participants