Skip to content

fix(e2e): stabilize double onboard route probe - #3620

Merged
cv merged 4 commits into
NVIDIA:mainfrom
jyaunches:fix/double-onboard-route-flake
May 15, 2026
Merged

fix(e2e): stabilize double onboard route probe#3620
cv merged 4 commits into
NVIDIA:mainfrom
jyaunches:fix/double-onboard-route-flake

Conversation

@jyaunches

@jyaunches jyaunches commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • retry post-repair inference.local probes after DNS/route repair to absorb propagation delay
  • retry the double-onboard probe-only recovery step before failing
  • expand failure diagnostics with forward list, sandbox resolv.conf, inference route probe, and DNS/gateway container logs

Nightly failure

Addresses flaky double-onboard-e2e failure from run 25931606052:

FAIL: Probe-only connect exited 1 after stopping e2e-double-b dashboard forward
Warning: failed to repair sandbox DNS proxy.
Error: inference.local is still unavailable inside e2e-double-b after DNS and route repair.

Validation

  • npm run build:cli
  • bash -n test/e2e/test-double-onboard.sh

Summary by CodeRabbit

  • Bug Fixes

    • Added configurable retry/delay for sandbox inference probes to improve reliability.
    • Improved probe classification to distinguish healthy vs. broken responses and return the first healthy result.
    • Enhanced failure handling to capture and show richer probe output when recovery fails.
  • Tests

    • Made sandbox connectivity recovery probe retry-capable with configurable attempts, delays, and timeouts.
    • Expanded test diagnostics to include per-sandbox DNS, routing, and probe response details.
  • Documentation

    • Updated generated test parity inventory to match the revised test script layout.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds bounded retry and delay options to sandbox inference-route probing, applies the mechanism to multiple post-repair/reset verification points, and enhances E2E diagnostics and probe-only recovery with configurable retries.

Changes

Sandbox Probe Retry and Resilience

Layer / File(s) Summary
Probe retry mechanism definition and implementation
src/lib/actions/sandbox/connect.ts
InferenceRouteProbeOptions type and post-repair probe constants; synchronous sleepSync helper; probeSandboxInferenceRoute rewritten to perform bounded retries with configurable delays and return early on a healthy probe or after final attempt.
Integration across repair and reset verification call sites
src/lib/actions/sandbox/connect.ts
Four verification points (post-VM DNS monkeypatch, post-DNS-proxy repair, and both managed route reset paths) now call probeSandboxInferenceRoute with retry/delay parameters instead of a single immediate probe.
E2E test diagnostics and probe-only recovery resilience
test/e2e/test-double-onboard.sh, test/e2e/docs/parity-inventory.generated.json
dump_diagnostics() now prints openshell forward list and per-sandbox /etc/resolv.conf plus an inference.local/v1/models curl probe; Phase 4 probe-only recovery uses a configurable retry loop and records diagnostics on failure; generated parity inventory line numbers updated.

Sequence Diagram

sequenceDiagram
  participant RepairFlow as Repair/Reset Flow
  participant Probe as probeSandboxInferenceRoute
  participant Inference as inference.local
  RepairFlow->>Probe: call with InferenceRouteProbeOptions (attempts, delayMs)
  loop up to N attempts
    Probe->>Inference: HTTP probe /v1/models
    alt Healthy (2xx/3xx)
      Inference-->>Probe: OK <status>
      Probe-->>RepairFlow: return OK result
    else Broken (other)
      Inference-->>Probe: BROKEN <status>
      Probe->>Probe: sleep delayMs
    end
  end
  Probe-->>RepairFlow: return final probe result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#3444: The main PR's connect.ts change to probe inference.local with new bounded retry/delay options after the same repair/reset steps directly overlaps the inference-route probing and recovery logic.

Suggested labels

fix, E2E, CI/CD, Sandbox

Suggested reviewers

  • ericksoa

Poem

🐰 I hopped the patch and timed each try,
Waiting a few seconds when probes went awry.
Multiple attempts, patient and spry,
Diagnostics whispered the DNS reply.
The sandbox now answers — the rabbit says "Yay!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix(e2e): stabilize double onboard route probe' directly and clearly summarizes the main change—adding retry mechanisms to stabilize inference route probes in e2e tests.
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 coderabbitai Bot 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.

Actionable comments posted: 1

🤖 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 `@test/e2e/test-double-onboard.sh`:
- Around line 658-665: The probe-only connect loop currently calls run_nemoclaw
without a per-attempt timeout, so if run_nemoclaw hangs the loop stalls; modify
the loop around run_nemoclaw (the call in the for loop that writes to
"$PROBE_LOG" and sets probe_exit/probe_output) to invoke the command with a
per-attempt timeout (e.g., using the timeout utility or a background+wait+kill
pattern) so each attempt is bounded, capture the timeout exit code into
probe_exit as before, and preserve writing stdout/stderr to "$PROBE_LOG" and the
existing break-on-success logic.
🪄 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: 8ed00d9b-909f-4eb5-8f3e-516959ab9fef

📥 Commits

Reviewing files that changed from the base of the PR and between 0964a7e and 9c98fec.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/connect.ts
  • test/e2e/test-double-onboard.sh

Comment thread test/e2e/test-double-onboard.sh Outdated
@jyaunches
jyaunches force-pushed the fix/double-onboard-route-flake branch 5 times, most recently from d31372d to c1abdb4 Compare May 15, 2026 21:52
@jyaunches
jyaunches force-pushed the fix/double-onboard-route-flake branch from c1abdb4 to c8666c3 Compare May 15, 2026 21:55
@cv
cv enabled auto-merge (squash) May 15, 2026 21:56
@cv
cv merged commit fdae301 into NVIDIA:main May 15, 2026
18 checks passed
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
@jyaunches
jyaunches deleted the fix/double-onboard-route-flake branch June 22, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants