Skip to content

fix(vm-driver): find Docker Desktop's socket when looking up local images - #4183

Open
shiju-nv wants to merge 2 commits into
NVIDIA:mainfrom
shiju-nv:fix/vm-driver-docker-desktop-socket-4155
Open

shiju-nv wants to merge 2 commits into
NVIDIA:mainfrom
shiju-nv:fix/vm-driver-docker-desktop-socket-4155

Conversation

@shiju-nv

@shiju-nv shiju-nv commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The MicroVM driver looks for a local container engine through DOCKER_HOST and /var/run/docker.sock only, then Podman. Docker Desktop for macOS can expose just ~/.docker/run/docker.sock, so the driver reports "no local container engine available" and silently pulls the image from a registry. This adds a second lookup step that uses the same socket candidates as the Docker driver, which already probes Docker Desktop's per-user socket.

Related Issue

Refs #4155. This PR fixes the lookup, which is the first complaint in the issue. The misleading Not authorized outcome and the fallback message are unchanged, so it does not close the issue.

Changes

  • connect_local_container_engine now delegates to connect_container_engine, which tries three steps in order: connect_with_local_defaults (kept first because it honours tcp:// and ssh:// DOCKER_HOST values that socket discovery does not), then the socket found by Docker socket discovery, then the Podman socket as before. A failed DOCKER_HOST does not stop the search, in the same way it already did not stop the Podman fallback.
  • The candidate list (DOCKER_HOST, /var/run/docker.sock, $HOME/.docker/run/docker.sock, $XDG_RUNTIME_DIR/docker.sock) moves from openshell-driver-docker into openshell_core::local_api_socket::detect_docker_socket, next to the socket probe helper it already used. openshell_driver_docker::detect_socket now calls it, so the Docker driver's behavior is unchanged and both drivers share one list.
  • The VM driver takes no new crate dependency. Cargo.toml and Cargo.lock are unchanged.
  • The two socket detectors are parameters of connect_container_engine so tests can control them without depending on the engines a host runs. Production callers pass the real detectors.
  • Five unit tests cover the new step, the ordering and the production wiring, using a fake Docker /_ping server on a temporary Unix socket.

Testing

  • Checks appropriate to the affected code and behavior pass (hosted CI runs on this PR)
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (if applicable): none added

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable): not applicable

Comment thread crates/openshell-driver-vm/Cargo.toml Resolved
@shiju-nv
shiju-nv requested a review from drew October 13, 2026 07:41
…ages

The VM driver looked for a local container engine through DOCKER_HOST and /var/run/docker.sock only, then Podman. Docker Desktop for macOS can expose just ~/.docker/run/docker.sock, so the driver found no engine and silently fell back to pulling the image from a registry.

Try the Docker driver's existing socket discovery between the default connection and the Podman fallback. connect_with_local_defaults stays first because it honours tcp:// and ssh:// DOCKER_HOST values that discovery does not. The detectors are parameters so tests control the ordering without depending on the engines a host runs.

This fixes the lookup only. The registry fallback and its error are unchanged.

Refs NVIDIA#4155

Signed-off-by: Shiju <shiju@nvidia.com>
Move the existing Docker socket detector beside the core socket probe and
keep the Docker driver's public wrapper. Let the VM driver call core and
remove its dependency on the Docker driver.

Preserve the socket candidates, response filters, and local-defaults then
Docker then Podman connection order.

Refs NVIDIA#4155

Signed-off-by: Shiju <shiju@nvidia.com>
@shiju-nv
shiju-nv force-pushed the fix/vm-driver-docker-desktop-socket-4155 branch from 8fc1396 to 3080769 Compare October 22, 2026 18:59
@shiju-nv shiju-nv added the test:e2e Requires end-to-end coverage label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown

Label test:e2e applied for 3080769. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants