Skip to content

fix(connect): recover docker-driver inference route without the cluster DNS repair (#3403) - #4551

Merged
cv merged 2 commits into
NVIDIA:mainfrom
latenighthackathon:fix/3403-docker-driver-dns-repair
May 29, 2026
Merged

fix(connect): recover docker-driver inference route without the cluster DNS repair (#3403)#4551
cv merged 2 commits into
NVIDIA:mainfrom
latenighthackathon:fix/3403-docker-driver-dns-repair

Conversation

@latenighthackathon

@latenighthackathon latenighthackathon commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

The docker-driver gateway now recovers a broken inference.local route through openshell inference set on connect, instead of running the k3s-only CoreDNS cluster repair that can never find its container under the docker driver. Reported on a Docker-driver host in #3403.

Related Issue

Closes #3403

Problem

shouldUseLegacyDnsProxyRepair in src/lib/actions/sandbox/connect.ts returned true for every driver except "vm", so a "docker" sandbox took the legacy repair path. That path (runSetupDnsProxy) patches CoreDNS inside an openshell-cluster-<name> container, which only the k3s/kubernetes gateway runs. The docker driver runs the gateway as nemoclaw-openshell-gateway with host networking and has no such container, so runSetupDnsProxy aborted with WARNING: Could not find gateway container for '<name>'. DNS proxy not installed. and inference.local stayed unreachable. After nemoclaw <name> connect, openclaw tui then failed with LLM request failed: network connection error. The contract elsewhere already excludes docker from this step: usesGatewayMetadataProbe (snapshot.ts) treats "docker" and "vm" as cluster-less drivers, and the snapshot DNS-proxy step is guarded by openshellDriver !== "docker".

Changes

  • Excluded "docker" from shouldUseLegacyDnsProxyRepair so the docker driver takes the non-legacy branch, which recovers the route via openshell inference set (the same reapplyVmInferenceRoute step the vm driver uses) and reports an accurate inference.local is unavailable ... Reapplying OpenShell inference route message instead of the misleading cluster-container warning.
  • Left the kubernetes driver on the legacy CoreDNS repair path, where the openshell-cluster-<name> container exists.
  • Added a docker-driver case in test/sandbox-connect-inference.test.ts: a broken inference.local probe now triggers the inference set reapply (not the legacy cluster repair), asserts no get service kube-dns call, and checks for the Reapplying OpenShell inference route / inference.local route repaired output.
  • Repointed the two existing tests that exercise the CoreDNS cluster repair and the managed-route reset from openshellDriver: "docker" to "kubernetes", since the openshell-cluster-<name> container only exists for the k3s driver.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Ran: full test/sandbox-connect-inference.test.ts suite passes (18/18); npm run typecheck:cli and npm run build:cli clean.


Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes

    • Refined DNS proxy repair logic to ensure docker sandboxes use the correct route recovery mechanism instead of legacy cluster DNS repair.
  • Tests

    • Updated DNS proxy repair tests to reflect Kubernetes-specific behavior.
    • Added regression test for docker sandbox route recovery verification.

Review Change Stack

…er DNS repair (NVIDIA#3403)

shouldUseLegacyDnsProxyRepair returned true for every driver except vm, so
a docker-driver sandbox ran the legacy repair that patches CoreDNS inside
an openshell-cluster-<name> container. The docker driver runs the gateway
as nemoclaw-openshell-gateway with host networking and has no such
container, so runSetupDnsProxy aborted with "Could not find gateway
container for <name>. DNS proxy not installed." and inference.local stayed
unreachable; nemoclaw <name> connect then left openclaw tui failing with
"LLM request failed: network connection error".

Exclude docker from the legacy predicate so the docker driver takes the
non-legacy path and recovers the route via openshell inference set,
matching usesGatewayMetadataProbe (snapshot.ts) and the !== docker guard
already on the snapshot DNS-proxy step. Repoint the two existing
connect-inference tests that exercised the cluster CoreDNS repair to the
kubernetes driver (where the cluster container exists) and add a
docker-driver regression test.

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR refines DNS proxy repair logic by excluding docker sandboxes from the legacy CoreDNS-in-container repair path, restricting it to kubernetes and other drivers. The implementation change is accompanied by updated and new tests that verify driver-specific repair behavior.

Changes

DNS Proxy Repair Driver Behavior

Layer / File(s) Summary
Legacy CoreDNS repair driver check
src/lib/actions/sandbox/connect.ts
shouldUseLegacyDnsProxyRepair now returns false for both vm and docker drivers, preserving the legacy CoreDNS repair path only for kubernetes and undefined drivers. Expanded inline documentation clarifies which gateway drivers have the legacy cluster CoreDNS patch and which rely on OpenShell route repair.
Test updates for driver-specific DNS repair
test/sandbox-connect-inference.test.ts
The 503-related DNS proxy repair test is updated to explicitly target the kubernetes driver. A new regression test verifies that docker sandboxes recover routes via inference set instead of the legacy kube-dns cluster repair. A managed route health check test fixture is also switched to kubernetes driver.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4221: Both PRs adjust how "docker" vs "vm/kubernetes" sandbox drivers are identified and handled so DNS-related repair logic and tests follow the correct non-legacy path for docker-driver sandboxes.

Suggested labels

bug, Docker, fix, Sandbox

Suggested reviewers

  • cv
  • ericksoa

Poem

🐰 A docker driver danced at the gate,
No CoreDNS patch to sedate,
Now inference set finds the way,
While Kubernetes repairs the day,
Legacy paths for special gates! 🏰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: modifying docker driver recovery to use route reapplication instead of cluster DNS repair.
Linked Issues check ✅ Passed The PR directly addresses #3403 by fixing DNS/network behavior that was causing inference.local unreachability on certain hosts, restoring connectivity after sandbox connect.
Out of Scope Changes check ✅ Passed All changes focus on the linked issue: modifications to shouldUseLegacyDnsProxyRepair logic and corresponding test updates are directly aligned with fixing docker driver DNS repair.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

@copy-pr-bot

copy-pr-bot Bot commented May 29, 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.

@cv cv added the v0.0.56 label May 29, 2026
@cv
cv enabled auto-merge (squash) May 29, 2026 23:52
@cv
cv merged commit a5e7e63 into NVIDIA:main May 29, 2026
18 checks passed
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
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.

NemoClaw installation not working

3 participants