Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/network container improvements #1423

Merged
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
d26eb22
Return a better error message when stopping container that doesn't exist
elizabethengelman Jun 28, 2024
321a48f
Fix doc strings for network container commands
elizabethengelman Jul 1, 2024
ae4ea7f
Pull shared container args to shared.rs
elizabethengelman Jul 1, 2024
5c67caf
Allow for overwriting the default container name with -c
elizabethengelman Jul 1, 2024
70b6c39
Check in generated doc updates
elizabethengelman Jul 1, 2024
89bceec
Refactor printlns
elizabethengelman Jul 1, 2024
d800be5
Clippy & cleanup errors
elizabethengelman Jul 3, 2024
196c5e5
Rename ContainerArgs -> Args
elizabethengelman Jul 15, 2024
07ccbda
Update cmd/soroban-cli/src/commands/network/container/start.rs
elizabethengelman Jul 15, 2024
523adb2
Use expect instead of unwrap in get_container_name
elizabethengelman Jul 15, 2024
c81f788
Add get_additional_flags fn on Args to reduce duplication
elizabethengelman Jul 15, 2024
ce13f84
Add get_container_name_arg fn on Args to reduce duplication
elizabethengelman Jul 15, 2024
2a16a42
Apply suggestions from code review
elizabethengelman Jul 15, 2024
8704b32
Add a comment explaining get_container_name_arg
elizabethengelman Jul 15, 2024
479fb13
Clean up doc comments
elizabethengelman Jul 16, 2024
305c281
Simplify stop and logs commmands
elizabethengelman Jul 16, 2024
2f17a58
Update the docs
elizabethengelman Jul 16, 2024
299e036
Revert "Simplify stop and logs commmands"
elizabethengelman Jul 16, 2024
c2670e2
Rename container_name arg to name
elizabethengelman Jul 16, 2024
a9fa5b0
Update help docs
elizabethengelman Jul 16, 2024
aac2440
Clippy
elizabethengelman Jul 16, 2024
3ed3586
Move connect_to_docker to Arg impl
elizabethengelman Jul 16, 2024
fcb28dc
Refactor start command: move helper fns to Cmd impl
elizabethengelman Jul 16, 2024
33b51bd
Merge branch 'main' into fix/network-container-improvements
elizabethengelman Jul 16, 2024
e997527
Default container name to network name
elizabethengelman Jul 16, 2024
aadfc63
Update stop and logs commands to accept the container name
elizabethengelman Jul 16, 2024
e96b8e1
Update docs
elizabethengelman Jul 16, 2024
a18ac18
Merge branch 'main' into fix/network-container-improvements
elizabethengelman Jul 17, 2024
f79b59d
Merge branch 'main' into fix/network-container-improvements
elizabethengelman Jul 17, 2024
0c95562
Create a Name type to handle internal vs external name conventions
elizabethengelman Jul 18, 2024
235f521
Prefix container names with 'stellar-'
elizabethengelman Jul 18, 2024
03d29ac
Add tests for Name struct
elizabethengelman Jul 18, 2024
392e1f6
Refactor Name fns
elizabethengelman Jul 18, 2024
42a9bfc
Merge branch 'main' into fix/network-container-improvements
elizabethengelman Jul 18, 2024
6fbb3f3
Merge branch 'main' into fix/network-container-improvements
elizabethengelman Jul 18, 2024
ec3d39f
Update Name::new to just take in one arg
elizabethengelman Jul 18, 2024
996dea4
Refactor Name
elizabethengelman Jul 18, 2024
c7c465c
Merge branch 'main' into fix/network-container-improvements
elizabethengelman Jul 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions FULL_HELP_DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ Start a container running a Stellar node, RPC, API, and friendbot (faucet).

By default, when starting a testnet container, without any optional arguments, it will run the equivalent of the following docker command:

`docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable-soroban-rpc`
`docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable rpc,horizon`

**Usage:** `stellar network start [OPTIONS] <NETWORK>`

Expand All @@ -1073,6 +1073,7 @@ By default, when starting a testnet container, without any optional arguments, i

###### **Options:**

* `-c`, `--container-name <CONTAINER_NAME>` — Optional argument to specify the container name
leighmcculloch marked this conversation as resolved.
Show resolved Hide resolved
* `-d`, `--docker-host <DOCKER_HOST>` — Optional argument to override the default docker host. This is useful when you are using a non-standard docker host path for your Docker-compatible container runtime, e.g. Docker Desktop defaults to $HOME/.docker/run/docker.sock instead of /var/run/docker.sock
* `-l`, `--limits <LIMITS>` — Optional argument to specify the limits for the local network only
* `-p`, `--ports-mapping <PORTS_MAPPING>` — Argument to specify the `HOST_PORT:CONTAINER_PORT` mapping
Expand All @@ -1089,17 +1090,18 @@ By default, when starting a testnet container, without any optional arguments, i

Stop a network started with `network start`. For example, if you ran `stellar network start local`, you can use `stellar network stop local` to stop it.

**Usage:** `stellar network stop [OPTIONS] <NETWORK>`
**Usage:** `stellar network stop [OPTIONS] [NETWORK]`

###### **Arguments:**

* `<NETWORK>` — Network to stop
* `<NETWORK>` — Network to stop (used in container name generation)

Possible values: `local`, `testnet`, `futurenet`, `pubnet`


###### **Options:**

* `-c`, `--container-name <CONTAINER_NAME>` — Optional argument to specify the container name
elizabethengelman marked this conversation as resolved.
Show resolved Hide resolved
* `-d`, `--docker-host <DOCKER_HOST>` — Optional argument to override the default docker host. This is useful when you are using a non-standard docker host path for your Docker-compatible container runtime, e.g. Docker Desktop defaults to $HOME/.docker/run/docker.sock instead of /var/run/docker.sock


Expand All @@ -1122,17 +1124,18 @@ Commands to start, stop and get logs for a quickstart container

Tail logs of a running network container

**Usage:** `stellar network container logs [OPTIONS] <NETWORK>`
**Usage:** `stellar network container logs [OPTIONS] [NETWORK]`

###### **Arguments:**

* `<NETWORK>` — Network to tail
* `<NETWORK>` — Network container to tail (used in container name generation)

Possible values: `local`, `testnet`, `futurenet`, `pubnet`


###### **Options:**

* `-c`, `--container-name <CONTAINER_NAME>` — Optional argument to specify the container name
* `-d`, `--docker-host <DOCKER_HOST>` — Optional argument to override the default docker host. This is useful when you are using a non-standard docker host path for your Docker-compatible container runtime, e.g. Docker Desktop defaults to $HOME/.docker/run/docker.sock instead of /var/run/docker.sock


Expand All @@ -1143,11 +1146,11 @@ Start network

Start a container running a Stellar node, RPC, API, and friendbot (faucet).

`stellar network start NETWORK [OPTIONS]`
`stellar network container start NETWORK [OPTIONS]`

By default, when starting a testnet container, without any optional arguments, it will run the equivalent of the following docker command:

`docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable-soroban-rpc`
`docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable rpc,horizon`

**Usage:** `stellar network container start [OPTIONS] <NETWORK>`

Expand All @@ -1160,6 +1163,7 @@ By default, when starting a testnet container, without any optional arguments, i

###### **Options:**

* `-c`, `--container-name <CONTAINER_NAME>` — Optional argument to specify the container name
* `-d`, `--docker-host <DOCKER_HOST>` — Optional argument to override the default docker host. This is useful when you are using a non-standard docker host path for your Docker-compatible container runtime, e.g. Docker Desktop defaults to $HOME/.docker/run/docker.sock instead of /var/run/docker.sock
* `-l`, `--limits <LIMITS>` — Optional argument to specify the limits for the local network only
* `-p`, `--ports-mapping <PORTS_MAPPING>` — Argument to specify the `HOST_PORT:CONTAINER_PORT` mapping
Expand All @@ -1174,17 +1178,18 @@ By default, when starting a testnet container, without any optional arguments, i

Stop a network started with `network container start`. For example, if you ran `network container start local`, you can use `network container stop local` to stop it

**Usage:** `stellar network container stop [OPTIONS] <NETWORK>`
**Usage:** `stellar network container stop [OPTIONS] [NETWORK]`

###### **Arguments:**

* `<NETWORK>` — Network to stop
* `<NETWORK>` — Network to stop (used in container name generation)

Possible values: `local`, `testnet`, `futurenet`, `pubnet`


###### **Options:**

* `-c`, `--container-name <CONTAINER_NAME>` — Optional argument to specify the container name
* `-d`, `--docker-host <DOCKER_HOST>` — Optional argument to override the default docker host. This is useful when you are using a non-standard docker host path for your Docker-compatible container runtime, e.g. Docker Desktop defaults to $HOME/.docker/run/docker.sock instead of /var/run/docker.sock


Expand Down
4 changes: 2 additions & 2 deletions cmd/soroban-cli/src/commands/network/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ pub enum Cmd {
///
/// Start a container running a Stellar node, RPC, API, and friendbot (faucet).
///
/// `stellar network start NETWORK [OPTIONS]`
/// `stellar network container start NETWORK [OPTIONS]`
///
/// By default, when starting a testnet container, without any optional arguments, it will run the equivalent of the following docker command:
///
/// `docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable-soroban-rpc`
/// `docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable rpc,horizon`
Start(start::Cmd),
/// Stop a network started with `network container start`. For example, if you ran `network container start local`, you can use `network container stop local` to stop it.
Stop(stop::Cmd),
Expand Down
17 changes: 10 additions & 7 deletions cmd/soroban-cli/src/commands/network/container/logs.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use futures_util::TryStreamExt;

use crate::commands::network::container::shared::{
connect_to_docker, Error as ConnectionError, Network, DOCKER_HOST_HELP,
connect_to_docker, Error as ConnectionError, Network,
};

use super::shared::Args;

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error(transparent)]
Expand All @@ -15,17 +17,18 @@ pub enum Error {

#[derive(Debug, clap::Parser, Clone)]
pub struct Cmd {
/// Network to tail
pub network: Network,
#[command(flatten)]
pub container_args: Args,

#[arg(short = 'd', long, help = DOCKER_HOST_HELP, env = "DOCKER_HOST")]
pub docker_host: Option<String>,
/// Network container to tail (used in container name generation)
#[arg(required_unless_present = "container_name")]
pub network: Option<Network>,
}

impl Cmd {
pub async fn run(&self) -> Result<(), Error> {
let container_name = format!("stellar-{}", self.network);
let docker = connect_to_docker(&self.docker_host).await?;
let container_name = self.container_args.get_container_name(self.network);
let docker = connect_to_docker(&self.container_args.docker_host).await?;
let logs_stream = &mut docker.logs(
&container_name,
Some(bollard::container::LogsOptions {
Expand Down
49 changes: 48 additions & 1 deletion cmd/soroban-cli/src/commands/network/container/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,54 @@ pub enum Error {
UnsupportedURISchemeError { uri: String },
}

#[derive(ValueEnum, Debug, Clone, PartialEq)]
#[derive(Debug, clap::Parser, Clone)]
pub struct Args {
/// Optional argument to specify the container name
#[arg(short = 'c', long, required_unless_present = "network")]
pub container_name: Option<String>,

elizabethengelman marked this conversation as resolved.
Show resolved Hide resolved
/// Optional argument to override the default docker host. This is useful when you are using a non-standard docker host path for your Docker-compatible container runtime, e.g. Docker Desktop defaults to $HOME/.docker/run/docker.sock instead of /var/run/docker.sock
#[arg(short = 'd', long, help = DOCKER_HOST_HELP, env = "DOCKER_HOST")]
pub docker_host: Option<String>,
}

impl Args {
pub(crate) fn get_additional_flags(&self) -> String {
self.docker_host
.as_ref()
.map(|docker_host| format!("--docker-host {docker_host}"))
.unwrap_or_default()
}

pub(crate) fn get_container_name(&self, network: Option<Network>) -> String {
self.container_name.as_ref().map_or_else(
|| {
format!(
"stellar-{}",
network.expect("Container name and/or network are required.")
)
},
|container_name| container_name.to_string(),
)
}

// This method is used in start.rs to create a message for the user to let them know how to stop the container they
// just started, and how to view its logs. For both `stop` and `logs` the user is able to pass in either the network
// (and we generate the container name) or the container name directly. Which is why we need to check if the
// container_name is present or not here.
pub(crate) fn get_container_name_arg(&self, network: Option<Network>) -> String {
self.container_name.as_ref().map_or_else(
|| {
network
.expect("Container name and/or network are required.")
.to_string()
},
|container_name| format!("--container-name {container_name}"),
)
}
}

#[derive(ValueEnum, Debug, Copy, Clone, PartialEq)]
pub enum Network {
Local,
Testnet,
Expand Down
45 changes: 28 additions & 17 deletions cmd/soroban-cli/src/commands/network/container/start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,31 @@ use bollard::{
use futures_util::TryStreamExt;

use crate::commands::network::container::shared::{
connect_to_docker, Error as ConnectionError, Network, DOCKER_HOST_HELP,
connect_to_docker, Error as ConnectionError, Network,
};

use super::shared::Args;

const DEFAULT_PORT_MAPPING: &str = "8000:8000";
const DOCKER_IMAGE: &str = "docker.io/stellar/quickstart";

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error("⛔ ️Failed to connect to docker: {0}")]
ConnectionError(#[from] ConnectionError),
DockerConnectionFailed(#[from] ConnectionError),

#[error("⛔ ️Failed to create container: {0}")]
BollardErr(#[from] bollard::errors::Error),
CreateContainerFailed(#[from] bollard::errors::Error),
}

#[derive(Debug, clap::Parser, Clone)]
pub struct Cmd {
#[command(flatten)]
pub container_args: Args,

/// Network to start
pub network: Network,

#[arg(short = 'd', long, help = DOCKER_HOST_HELP, env = "DOCKER_HOST")]
pub docker_host: Option<String>,

/// Optional argument to specify the limits for the local network only
#[arg(short = 'l', long)]
pub limits: Option<String>,
Expand All @@ -56,7 +58,7 @@ impl Cmd {
}

async fn run_docker_command(cmd: &Cmd) -> Result<(), Error> {
let docker = connect_to_docker(&cmd.docker_host).await?;
let docker = connect_to_docker(&cmd.container_args.docker_host).await?;
elizabethengelman marked this conversation as resolved.
Show resolved Hide resolved

let image = get_image_name(cmd);
docker
Expand Down Expand Up @@ -87,7 +89,7 @@ async fn run_docker_command(cmd: &Cmd) -> Result<(), Error> {
..Default::default()
};

let container_name = format!("stellar-{}", cmd.network);
let container_name = cmd.container_args.get_container_name(Some(cmd.network));
let create_container_response = docker
.create_container(
Some(CreateContainerOptions {
Expand All @@ -105,18 +107,27 @@ async fn run_docker_command(cmd: &Cmd) -> Result<(), Error> {
)
.await?;
println!("✅ Container started: {container_name}");
let stop_message = format!(
"ℹ️ To stop this container run: stellar network stop {network} {additional_flags}",
network = &cmd.network,
additional_flags = if cmd.docker_host.is_some() {
format!("--docker-host {}", cmd.docker_host.as_ref().unwrap())
} else {
String::new()
}
print_log_message(cmd);
print_stop_message(cmd);
Ok(())
}

fn print_log_message(cmd: &Cmd) {
let log_message = format!(
"ℹ️ To see the logs for this container run: stellar network container logs {arg} {additional_flags}",
arg = cmd.container_args.get_container_name_arg(Some(cmd.network)),
additional_flags = cmd.container_args.get_additional_flags(),
);
println!("{log_message}");
}

fn print_stop_message(cmd: &Cmd) {
let stop_message = format!(
"ℹ️ To stop this container run: stellar network container stop {arg} {additional_flags}",
arg = cmd.container_args.get_container_name_arg(Some(cmd.network)),
additional_flags = cmd.container_args.get_additional_flags(),
);
println!("{stop_message}");
Ok(())
}

fn get_container_args(cmd: &Cmd) -> Vec<String> {
Expand Down
48 changes: 37 additions & 11 deletions cmd/soroban-cli/src/commands/network/container/stop.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,54 @@
use crate::commands::network::container::shared::{
connect_to_docker, Error as ConnectionError, Network, DOCKER_HOST_HELP,
connect_to_docker, Error as BollardConnectionError, Network,
};

use super::shared::Args;

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error("Failed to stop container: {0}")]
StopContainerError(#[from] ConnectionError),
#[error("⛔ Failed to connect to docker: {0}")]
DockerConnectionFailed(#[from] BollardConnectionError),

#[error("⛔ Container {container_name} not found")]
ContainerNotFound {
container_name: String,
#[source]
source: bollard::errors::Error,
},

#[error("⛔ Failed to stop container: {0}")]
ContainerStopFailed(#[from] bollard::errors::Error),
}

#[derive(Debug, clap::Parser, Clone)]
pub struct Cmd {
/// Network to stop
pub network: Network,
#[command(flatten)]
pub container_args: Args,

#[arg(short = 'd', long, help = DOCKER_HOST_HELP, env = "DOCKER_HOST")]
pub docker_host: Option<String>,
/// Network to stop (used in container name generation)
#[arg(required_unless_present = "container_name")]
pub network: Option<Network>,
}

impl Cmd {
pub async fn run(&self) -> Result<(), Error> {
let container_name = format!("stellar-{}", self.network);
let docker = connect_to_docker(&self.docker_host).await?;
println!("ℹ️ Stopping container: {container_name}");
docker.stop_container(&container_name, None).await.unwrap();
let container_name = self.container_args.get_container_name(self.network);
let docker = connect_to_docker(&self.container_args.docker_host).await?;
println!("ℹ️ Stopping container: {container_name}");
docker
.stop_container(&container_name, None)
.await
.map_err(|e| {
let msg = e.to_string();
if msg.contains("No such container") {
Error::ContainerNotFound {
container_name: container_name.clone(),
source: e,
}
} else {
Error::ContainerStopFailed(e)
}
})?;
println!("✅ Container stopped: {container_name}");
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/soroban-cli/src/commands/network/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub enum Cmd {
///
/// By default, when starting a testnet container, without any optional arguments, it will run the equivalent of the following docker command:
///
/// `docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable-soroban-rpc`
/// `docker run --rm -p 8000:8000 --name stellar stellar/quickstart:testing --testnet --enable rpc,horizon`
Start(container::StartCmd),
/// ⚠️ Deprecated: use `stellar container stop` instead
///
Expand Down
Loading