Skip to content

Shell out to docker instead of using bollard#2629

Merged
fnando merged 3 commits into
mainfrom
drop-bollard
Jul 13, 2026
Merged

Shell out to docker instead of using bollard#2629
fnando merged 3 commits into
mainfrom
drop-bollard

Conversation

@fnando

@fnando fnando commented Jun 26, 2026

Copy link
Copy Markdown
Member

What

Replaces the bollard Docker API client with direct docker CLI invocations in the stellar container start|stop|logs commands. The --docker-host flag (and the DOCKER_HOST env var) is now applied by setting DOCKER_HOST on the spawned docker process; host resolution is otherwise left to docker itself. The unused bollard dependency in stellar-ledger is also removed.

Why

The bollard connection code copied unreleased upstream source, pinned a hardcoded API version, and reimplemented host/socket resolution plus a Docker Desktop fallback that the docker CLI already handles natively. It also rejected any host scheme it didn't recognize — e.g. ssh://, which docker supports — with URI scheme is not supported. Shelling out to docker removes that hand-rolled logic and lets docker resolve the connection, including remote ssh:// hosts.

$ stellar container start --docker-host ssh://fnando@fnando-server.local
ℹ️ Starting local network
ℹ️ testing: Pulling from stellar/quickstart
ℹ️ Digest: sha256:3b98cc624a235215bbb2dedd18bd3ec3a498053b6b6533533b6a0d94354a7885
ℹ️ Status: Image is up to date for stellar/quickstart:testing
✅ Started container
🔎 Watch logs with `stellar container logs local --docker-host ssh://fnando@fnando-server.local`
ℹ️ Stop the container with `stellar container stop local --docker-host ssh://fnando@fnando-server.local`

$ stellar container start --docker-host ssh://fnando@fnando-server.local
ℹ️ Starting local network
ℹ️ testing: Pulling from stellar/quickstart
ℹ️ Digest: sha256:3b98cc624a235215bbb2dedd18bd3ec3a498053b6b6533533b6a0d94354a7885
ℹ️ Status: Image is up to date for stellar/quickstart:testing
❌ error: a container named "stellar-local" already running

$ stellar container logs --docker-host ssh://fnando@fnando-server.local
Starting Stellar Quickstart
versions:
  quickstart: 16a57418b555ab859e133550c449ff15c2f5c06e
  xdr:
    stellar-xdr 27.0.0 (5262803470be965e42f80023d12fba12808c774a)
    xdr: 68fa1ac55692f68ad2a2ca549d0a283273554439
    features: <none>
  ...

$ stellar container stop --docker-host ssh://fnando@fnando-server.local
ℹ️ Stopping local container
✅ Container stopped

$ stellar container stop --docker-host ssh://fnando@fnando-server.local
ℹ️ Stopping local container
❌ error: container local not found

Known limitations

The docker binary must now be installed and on the user's PATH; if it isn't, commands fail with a "is docker installed and on your PATH?" hint.

Copilot AI review requested due to automatic review settings June 26, 2026 19:44
@github-project-automation github-project-automation Bot moved this to Backlog (Not Ready) in DevX Jun 26, 2026
@fnando fnando moved this from Backlog (Not Ready) to Needs Review in DevX Jun 26, 2026
@fnando fnando self-assigned this Jun 26, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e557b3582

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread cmd/soroban-cli/src/commands/container/shared.rs Outdated

Copilot AI 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.

Pull request overview

This pull request replaces the bollard-based Docker integration in stellar container start|stop|logs with direct docker CLI invocations, delegating host/socket resolution (including ssh://) to Docker itself and removing the unused bollard dependency from the workspace.

Changes:

  • Reimplemented container start, stop, and logs to shell out to docker, applying --docker-host/DOCKER_HOST by setting DOCKER_HOST on the spawned process.
  • Simplified container error handling by parsing docker stderr for “already in use” / “No such container” cases.
  • Removed the bollard dependency from the workspace and affected crates.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cmd/soroban-cli/src/commands/container/start.rs Switches container startup/pull logic to docker pull + docker run, adds stderr-based error mapping.
cmd/soroban-cli/src/commands/container/stop.rs Switches container stop to docker stop and maps “No such container” via stderr.
cmd/soroban-cli/src/commands/container/logs.rs Switches log tailing to docker logs -f --tail all with inherited stdio.
cmd/soroban-cli/src/commands/container/shared.rs Replaces bollard connection logic with a docker_command() builder that sets DOCKER_HOST when provided.
cmd/soroban-cli/Cargo.toml Removes bollard dependency from the soroban-cli crate.
cmd/crates/stellar-ledger/Cargo.toml Removes bollard dependency from stellar-ledger crate.
Cargo.toml Removes bollard from workspace dependencies.
Cargo.lock Removes bollard from the resolved dependency graph.

Comment thread cmd/soroban-cli/src/commands/container/start.rs
Comment thread cmd/soroban-cli/src/commands/container/start.rs
@fnando
fnando requested a review from mootz12 June 26, 2026 20:27
@fnando
fnando enabled auto-merge (squash) June 26, 2026 21:17
@fnando
fnando requested a review from sagpatil June 30, 2026 18:53

@mootz12 mootz12 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.

A few notes in-line, looks good tho

Comment thread cmd/soroban-cli/src/commands/container/logs.rs
Comment thread cmd/soroban-cli/src/commands/container/start.rs
@fnando
fnando merged commit f35667d into main Jul 13, 2026
227 checks passed
@fnando
fnando deleted the drop-bollard branch July 13, 2026 14:03
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in DevX Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants