fix: resolve inference.local DNS inside sandbox on macOS - #471
Closed
ross-shulyha wants to merge 3 commits into
Closed
fix: resolve inference.local DNS inside sandbox on macOS#471ross-shulyha wants to merge 3 commits into
ross-shulyha wants to merge 3 commits into
Conversation
On macOS (Docker Desktop / Colima), the OpenShell gateway does not inject `inference.local` into k3s CoreDNS, so sandbox pods cannot resolve the inference proxy endpoint at `https://inference.local/v1`. This breaks local Ollama and vLLM inference on Apple Silicon machines. Add `scripts/fix-inference-dns-macos.sh` which patches the CoreDNS Corefile configmap with an inline hosts entry pointing `inference.local` to the Traefik ingress ClusterIP. The script runs automatically during `nemoclaw onboard` (step 2 — gateway setup) on Darwin, after the existing Colima CoreDNS fix. Tested on macOS 15.4 (Apple M4 Max) with Docker Desktop and Ollama local inference — sandbox successfully resolves `inference.local` and completes inference requests through the local provider. Fixes NVIDIA#260
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a macOS-only DNS patch step to onboarding: when running on Darwin, Changes
Sequence DiagramsequenceDiagram
participant Onboard as Onboard (bin/lib/onboard.js)
participant Shell as Local Shell / Container
participant Kubectl as kubectl / K8s API
participant ConfigMap as CoreDNS ConfigMap
participant CoreDNS as CoreDNS Deployment
Onboard->>Onboard: detect platform === "darwin"
Onboard->>Shell: run scripts/fix-inference-dns-macos.sh
Shell->>Shell: find openshell-cluster container
Shell->>Kubectl: fetch traefik Service / node IPs
Kubectl-->>Shell: return target IP
Shell->>ConfigMap: generate JSON merge patch to add hosts mapping
Shell->>Kubectl: kubectl patch configmap (kube-system coredns)
Kubectl-->>CoreDNS: apply patch / restart rollout
CoreDNS-->>Kubectl: rollout status
Kubectl-->>Shell: patch and rollout acknowledged
Shell-->>Onboard: script completes (success or ignored errors)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/fix-inference-dns-macos.sh (2)
57-70: Consider validating TARGET_IP format.While
TARGET_IPcomes from trusted kubectl output, adding a basic IP format validation before using it in the Corefile would provide defense-in-depth against unexpected kubectl output or edge cases.🛡️ Optional: Add IP validation
if [ -z "$TARGET_IP" ]; then echo "WARN: Could not determine target IP for inference.local. DNS fix skipped." exit 0 fi + +# Basic IP format validation +if ! echo "$TARGET_IP" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$'; then + echo "WARN: Invalid IP format '$TARGET_IP'. DNS fix skipped." + exit 0 +fi echo "Patching CoreDNS: inference.local -> $TARGET_IP"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/fix-inference-dns-macos.sh` around lines 57 - 70, The script currently uses TARGET_IP (populated via docker exec ... kubectl) without validating it's a well-formed IP; add a simple validation after the assignments to ensure TARGET_IP matches IPv4 (or IPv6 if needed) syntax and exit with a warning if it doesn't. Locate the TARGET_IP variable and the block that echoes "Patching CoreDNS: inference.local -> $TARGET_IP" and insert a validation check for the format (e.g., regex test for IPv4 octets or use grep -E) that prints an error and exits when the value is empty or fails the pattern before exporting TARGET_IP and patching the Corefile. Ensure the validation covers both attempts to set TARGET_IP (the traefik svc and the node InternalIP) so bad kubectl output is rejected.
71-91: Consider adding Corefile format validation.The Python-based text manipulation is cleaner than sed/awk for this task. However, the marker string
'hosts /etc/coredns/NodeHosts {'on line 75 is brittle—spacing or format changes in k3s CoreDNS could cause silent misses, falling through to theforward .injection path.Consider logging which injection path was used (NodeHosts vs standalone hosts block) to aid debugging when the patched config doesn't work as expected.
💡 Optional: Add path visibility for debugging
if marker in corefile: corefile = corefile.replace( marker, marker + '\n ' + target_ip + ' inference.local' ) + print('DEBUG: Injected into existing NodeHosts block', file=sys.stderr) else: # No NodeHosts block found — inject a standalone hosts block before # the forward directive so inference.local still resolves. inject = ' hosts {\n ' + target_ip + ' inference.local\n fallthrough\n }\n' if 'forward .' in corefile: corefile = corefile.replace(' forward .', inject + ' forward .') + print('DEBUG: Injected standalone hosts block before forward', file=sys.stderr) else: # Last resort: append before closing brace corefile = corefile.rstrip().rstrip('}') + inject + '}\n' + print('DEBUG: Appended hosts block at end of Corefile', file=sys.stderr)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/fix-inference-dns-macos.sh` around lines 71 - 91, The current PATCHED_COREFILE python snippet uses a brittle exact marker string ('hosts /etc/coredns/NodeHosts {') to detect where to inject the host entry and falls back silently to other injection paths; update the logic in the PATCHED_COREFILE block to (1) validate the Corefile format before and after modifications (e.g., ensure balanced braces and presence of either a NodeHosts block or a forward directive), (2) match the NodeHosts block using a regex tolerant of whitespace/indentation (instead of the exact marker), (3) add explicit logging (to stdout/stderr) that reports which path was taken ("NodeHosts injected", "standalone hosts injected before forward", or "appended before closing brace") along with the TARGET_IP and COREFILE preview/summary, and (4) exit non‑zero if validation fails so callers can detect a bad patch; reference PATCHED_COREFILE, marker, TARGET_IP and COREFILE when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/fix-inference-dns-macos.sh`:
- Around line 57-70: The script currently uses TARGET_IP (populated via docker
exec ... kubectl) without validating it's a well-formed IP; add a simple
validation after the assignments to ensure TARGET_IP matches IPv4 (or IPv6 if
needed) syntax and exit with a warning if it doesn't. Locate the TARGET_IP
variable and the block that echoes "Patching CoreDNS: inference.local ->
$TARGET_IP" and insert a validation check for the format (e.g., regex test for
IPv4 octets or use grep -E) that prints an error and exits when the value is
empty or fails the pattern before exporting TARGET_IP and patching the Corefile.
Ensure the validation covers both attempts to set TARGET_IP (the traefik svc and
the node InternalIP) so bad kubectl output is rejected.
- Around line 71-91: The current PATCHED_COREFILE python snippet uses a brittle
exact marker string ('hosts /etc/coredns/NodeHosts {') to detect where to inject
the host entry and falls back silently to other injection paths; update the
logic in the PATCHED_COREFILE block to (1) validate the Corefile format before
and after modifications (e.g., ensure balanced braces and presence of either a
NodeHosts block or a forward directive), (2) match the NodeHosts block using a
regex tolerant of whitespace/indentation (instead of the exact marker), (3) add
explicit logging (to stdout/stderr) that reports which path was taken
("NodeHosts injected", "standalone hosts injected before forward", or "appended
before closing brace") along with the TARGET_IP and COREFILE preview/summary,
and (4) exit non‑zero if validation fails so callers can detect a bad patch;
reference PATCHED_COREFILE, marker, TARGET_IP and COREFILE when making these
changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 07b54565-e119-49f9-a22b-96553c7b7d7a
📒 Files selected for processing (2)
bin/lib/onboard.jsscripts/fix-inference-dns-macos.sh
Address CodeRabbit review feedback — validate that TARGET_IP is a well-formed IPv4 address before patching the CoreDNS Corefile. Rejects unexpected kubectl output with a warning instead of injecting a malformed hosts entry.
Contributor
There was a problem hiding this comment.
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 `@scripts/fix-inference-dns-macos.sh`:
- Around line 67-70: The current TARGET_IP validation only checks the
dotted-digit shape and allows out-of-range octets; replace the simple grep with
a stricter IPv4 check that enforces 0-255 per octet. Update the existing
conditional that uses echo "$TARGET_IP" | grep -qE
'^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$' to use a regex that validates each octet (for
example:
^((25[0-5]|2[0-4][0-9]|1?[0-9]{1,2})\.){3}(25[0-5]|2[0-4][0-9]|1?[0-9]{1,2})$)
or call a small utility (eg. inet_pton via python -c) to reject invalid numeric
ranges before proceeding with the CoreDNS patch; keep the same warning message
and early exit behavior when validation fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de3b64d2-e9d3-4278-8338-71cb36b7de31
📒 Files selected for processing (1)
scripts/fix-inference-dns-macos.sh
Use strict per-octet 0-255 regex instead of simple digit-dot pattern, as suggested by CodeRabbit review.
Contributor
|
Thanks for suggesting a fix for the inference.local DNS issue on macOS, this should resolve problems with local inference on Apple Silicon machines. |
Contributor
|
Thanks for the macOS inference.local DNS fix. This PR has conflicts with the current codebase — the macOS inference path has changed since March. Could you rebase against main and resolve them? Happy to review once it's updated. |
This was referenced Apr 21, 2026
Contributor
|
Thanks for this — #275 and #464 were tackling the same macOS inference routing issue at the same time. The script-based CoreDNS fix approach here is superseded: Closing as superseded. Feel free to reopen if you find gaps in the current macOS local inference path. |
wscurran
added
area: local-models
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On macOS (Docker Desktop and Colima),
inference.localis never injected into the k3s CoreDNS configuration by the OpenShell gateway. This means sandbox pods cannot resolve the inference proxy endpoint athttps://inference.local/v1, which breaks all local inference (Ollama, vLLM) on Apple Silicon machines.This PR adds a lightweight DNS fix that runs automatically during
nemoclaw onboardon macOS, making local Ollama inference work out of the box — no manual workarounds needed.Related Issue
Fixes #260 — macOS/Apple Silicon Support Tracking — Known Gaps & Fixes
Also addresses the DNS resolution aspect of #314.
Changes
scripts/fix-inference-dns-macos.sh— Patches the CoreDNS Corefile configmap to add an inline hosts entry mappinginference.local→ Traefik ingress ClusterIP (with node IP fallback). The script is idempotent and skips patching ifinference.localis already present.bin/lib/onboard.js— Calls the DNS fix script during gateway setup (step 2) on Darwin, after the existing Colima CoreDNS fix and before sandbox creation.Type of Change
Testing
npm testpasses (167/167 tests, 0 failures).nemoclaw onboardwithNEMOCLAW_PROVIDER=ollamacompletes successfullyinference.local → 172.18.0.2(Traefik ClusterIP)inference.localand processes inference requests via local Ollamauname -s != Darwin→ exits cleanly).Reproduction steps (before this fix)
After this fix
Checklist
General
Code Changes
Summary by CodeRabbit