NVIDIA / NVIDIA/NemoClaw

Internal DNS commands use a second Docker authority selector that never probes

Open
#10,632 0 comments 0 reactions 0 assignees View on GitHub
area: cli area: networking
Dominant language
TypeScript
Stars
22.5k
Forks
3.1k
Avg merge
1d 1h
Merged PRs (30d)
715

Description

## Description

`src/lib/actions/dns/index.ts` carries its own `detectDockerHost` (lines 111-145), separate from the one in `src/lib/platform.ts`. The DNS copy selects a container socket by **file existence alone**: it never checks whether the Docker CLI default already works, and it never probes the socket it picks.

```ts
// src/lib/actions/dns/index.ts:111-145 (abridged)
if (env.DOCKER_HOST) return { dockerHost: env.DOCKER_HOST, ... };

const colimaSocket = findFirstSocket([...colima paths...]);
if (colimaSocket) return { dockerHost: `unix://${colimaSocket}`, runtime: "colima" };

const podmanSocket = findFirstSocket([...podman paths...]);
if (podmanSocket) return { dockerHost: `unix://${podmanSocket}`, runtime: "podman" };
```

That is the defect #8816 reported, still live in this second path: on a host with a healthy Docker default and an installed-but-stale Podman socket, the DNS commands send Docker operations to Podman. A stale socket **file** is enough — the socket does not have to be serving.

`src/lib/platform.ts` no longer behaves this way. It probes the CLI default first and keeps it when it answers, requires a candidate socket to answer and identify itself before adopting it, and refuses to choose when two engines answer.

## Why the runner does not cover it

`runFixCoreDns` (`:207`) and `runSetupDnsProxy` (`:330`) call the local selector directly, and the internal commands invoke those functions without runner initialisation:

- `src/commands/internal/dns/fix-coredns.ts:23`
- `src/commands/internal/dns/setup-proxy.ts:24`

So the `DOCKER_HOST` that `src/lib/runner.ts` sets at import does not govern these paths.

## Expected behavior

One Docker authority selector for the CLI. The DNS commands should reach the same decision as every other Docker operation: keep a working default, adopt a discovered socket only after it answers and identifies itself, and never silently prefer a socket that merely exists.

## Suggested shape

Have the DNS commands consume `detectDockerHost` from `src/lib/platform.ts` instead of the local copy, and delete the local one. The platform helper already accepts injected `env`, `existsSync`, and `probeDockerHost`, which is what the DNS tests need.

Two details for whoever takes it:

1. The DNS copy honours `XDG_RUNTIME_DIR` for the Podman path; the platform copy derives the runtime directory from `uid`. Check that the merged behaviour keeps the hosts that rely on the former working.
2. The DNS copy returns a `runtime` label (`colima`, `podman`, `custom`, `unknown`) that its callers use. The platform helper returns an engine identity from a real probe, so the mapping needs to be deliberate rather than assumed.

## Notes

- Surfaced by the PR Review Advisor on #10379, which fixes the equivalent defect in the platform selector. It is out of scope there: different commands, different call path, and its own test surface.
- Related: #8816, #10367, and #10622.

Contributor guide

Open the contributing guide

Research direction

Compare src/lib/actions/dns/index.ts with src/lib/platform.ts, then trace runFixCoreDns and runSetupDnsProxy from the internal command files. Run the DNS tests and verify the shared selector keeps a working Docker default, probes discovered sockets, preserves XDG_RUNTIME_DIR behavior, and maps the runtime label deliberately without choosing a stale socket.

Written by the indexing model from the issue text.

Assessment

Tech stack
docker, typescript
Domain
infrastructure, networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.