From ce8f22f7a6aae2804d6b0b86b7b3f8872ede44b3 Mon Sep 17 00:00:00 2001 From: Diego Lendoiro Date: Thu, 9 May 2024 15:53:29 +0200 Subject: [PATCH 1/4] Collect correctly bridge ip when no --bridge is passed --- .../src/core/containers/async_container.rs | 36 +++++++++++++++---- testcontainers/src/runners/async_runner.rs | 16 +++++++++ testcontainers/src/runners/sync_runner.rs | 12 +++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/testcontainers/src/core/containers/async_container.rs b/testcontainers/src/core/containers/async_container.rs index ffb25c25..93edabca 100644 --- a/testcontainers/src/core/containers/async_container.rs +++ b/testcontainers/src/core/containers/async_container.rs @@ -1,3 +1,4 @@ +use core::panic; use std::{fmt, net::IpAddr, str::FromStr, sync::Arc}; use tokio::runtime::RuntimeFlavor; @@ -139,13 +140,25 @@ where /// Returns the bridge ip address of docker container as specified in NetworkSettings.Networks.IPAddress pub async fn get_bridge_ip_address(&self) -> IpAddr { - let result = self.docker_client.inspect(&self.id).await; + let container_settings = self.docker_client.inspect(&self.id).await; - let settings = result + let host_config = container_settings + .host_config + .unwrap_or_else(|| panic!("container {} has no host config settings", self.id)); + + let network_mode = host_config + .network_mode + .unwrap_or_else(|| panic!("container {} has no network mode", self.id)); + + if network_mode != "bridge" { + panic!("container {} is not in bridge mode", self.id); + } + + let network_settings = container_settings .network_settings .unwrap_or_else(|| panic!("container {} has no network settings", self.id)); - let mut networks = settings + let mut networks = network_settings .networks .unwrap_or_else(|| panic!("container {} has no any networks", self.id)); @@ -153,16 +166,27 @@ where .image .network() .clone() - .or(settings.bridge) + .or_else(|| { + network_settings + .bridge + .and_then(|b| if !b.is_empty() { Some(b) } else { None }) + }) + .or(Some("bridge".to_owned())) .unwrap_or_else(|| panic!("container {} has missing bridge name", self.id)); let ip = networks .remove(&bridge_name) .and_then(|network| network.ip_address) - .unwrap_or_else(|| panic!("container {} has missing bridge IP", self.id)); + .unwrap_or_else(|| { + panic!( + "container {} has missing bridge IP {:?}", + self.id, + self.image.network() + ) + }); IpAddr::from_str(&ip) - .unwrap_or_else(|_| panic!("container {} has invalid bridge IP", self.id)) + .unwrap_or_else(|_| panic!("container {} has invalid bridge IP", self.id,)) } /// Returns the host ip address of docker container diff --git a/testcontainers/src/runners/async_runner.rs b/testcontainers/src/runners/async_runner.rs index bd36676d..86fea655 100644 --- a/testcontainers/src/runners/async_runner.rs +++ b/testcontainers/src/runners/async_runner.rs @@ -384,6 +384,22 @@ mod tests { assert!(!client.network_exists("awesome-net-2").await) } + #[tokio::test] + async fn async_should_rely_on_default_bridge_network_when_no_network_provided_and_settings_bridge_empty( + ) { + let hello_world = GenericImage::new("simple_web_server", "latest") + .with_wait_for(WaitFor::message_on_stdout("server is ready")) + .with_wait_for(WaitFor::seconds(1)); + + let container = RunnableImage::from(hello_world.clone()).start().await; + + assert!(!container + .get_bridge_ip_address() + .await + .to_string() + .is_empty()) + } + #[tokio::test] async fn async_run_command_should_set_shared_memory_size() { let client = Client::lazy_client().await; diff --git a/testcontainers/src/runners/sync_runner.rs b/testcontainers/src/runners/sync_runner.rs index 905bacbd..2c1e37b6 100644 --- a/testcontainers/src/runners/sync_runner.rs +++ b/testcontainers/src/runners/sync_runner.rs @@ -188,6 +188,18 @@ mod tests { ); } + #[test] + fn async_should_rely_on_default_bridge_network_when_no_network_provided_and_settings_bridge_empty( + ) { + let hello_world = GenericImage::new("simple_web_server", "latest") + .with_wait_for(WaitFor::message_on_stdout("server is ready")) + .with_wait_for(WaitFor::seconds(1)); + + let container = RunnableImage::from(hello_world.clone()).start(); + + assert!(!container.get_bridge_ip_address().to_string().is_empty()) + } + #[test] fn sync_run_command_should_include_name() { let image = GenericImage::new("hello-world", "latest"); From c78c9497b67ad51f14dfc289ecb2a31e2c3949cc Mon Sep 17 00:00:00 2001 From: Diego Lendoiro Date: Thu, 9 May 2024 16:19:07 +0200 Subject: [PATCH 2/4] Added review recommendations --- testcontainers/src/core/containers/async_container.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/testcontainers/src/core/containers/async_container.rs b/testcontainers/src/core/containers/async_container.rs index 93edabca..2af2df35 100644 --- a/testcontainers/src/core/containers/async_container.rs +++ b/testcontainers/src/core/containers/async_container.rs @@ -166,12 +166,8 @@ where .image .network() .clone() - .or_else(|| { - network_settings - .bridge - .and_then(|b| if !b.is_empty() { Some(b) } else { None }) - }) - .or(Some("bridge".to_owned())) + .or_else(|| network_settings.bridge.filter(|b| !b.is_empty())) + .or_else(|| Some("bridge".to_owned())) .unwrap_or_else(|| panic!("container {} has missing bridge name", self.id)); let ip = networks From e1663ebf22dfdc5ef1677a1b3775d5430e3fa187 Mon Sep 17 00:00:00 2001 From: Diego Lendoiro Date: Thu, 9 May 2024 21:09:45 +0200 Subject: [PATCH 3/4] Use network_mode to determine the network and its driver --- .github/workflows/ci.yml | 2 +- testcontainers/src/core/client.rs | 16 ++++++- .../src/core/containers/async_container.rs | 38 +++++++---------- testcontainers/src/runners/async_runner.rs | 42 +++++++++++++++++-- testcontainers/src/runners/sync_runner.rs | 35 ++++++++++++++-- 5 files changed, 99 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c57e7a3b..a01adeb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,7 @@ on: pull_request: merge_group: push: - branches: [ main ] + branches: [main] concurrency: group: ${{ github.ref }} diff --git a/testcontainers/src/core/client.rs b/testcontainers/src/core/client.rs index 530d70b9..0d26ceff 100644 --- a/testcontainers/src/core/client.rs +++ b/testcontainers/src/core/client.rs @@ -5,10 +5,12 @@ use bollard::{ container::{Config, CreateContainerOptions, LogsOptions, RemoveContainerOptions}, exec::{CreateExecOptions, StartExecOptions, StartExecResults}, image::CreateImageOptions, - network::CreateNetworkOptions, + network::{CreateNetworkOptions, InspectNetworkOptions}, Docker, }; -use bollard_stubs::models::{ContainerCreateResponse, ContainerInspectResponse, HealthStatusEnum}; +use bollard_stubs::models::{ + ContainerCreateResponse, ContainerInspectResponse, HealthStatusEnum, Network, +}; use futures::{StreamExt, TryStreamExt}; use tokio::sync::OnceCell; @@ -232,6 +234,16 @@ impl Client { network.id } + /// Inspects a network + pub(crate) async fn inspect_network( + &self, + name: &str, + ) -> Result { + self.bollard + .inspect_network(name, Some(InspectNetworkOptions::::default())) + .await + } + pub(crate) async fn create_container( &self, options: Option>, diff --git a/testcontainers/src/core/containers/async_container.rs b/testcontainers/src/core/containers/async_container.rs index 2af2df35..dab29586 100644 --- a/testcontainers/src/core/containers/async_container.rs +++ b/testcontainers/src/core/containers/async_container.rs @@ -150,39 +150,31 @@ where .network_mode .unwrap_or_else(|| panic!("container {} has no network mode", self.id)); - if network_mode != "bridge" { - panic!("container {} is not in bridge mode", self.id); - } + let network_settings = self + .docker_client + .inspect_network(&network_mode) + .await + .unwrap_or_else(|_| panic!("container {} network mode does not exist", self.id)); - let network_settings = container_settings + network_settings + .driver + .unwrap_or_else(|| panic!("network {} is not in bridge mode", network_mode)); + + let container_network_settings = container_settings .network_settings .unwrap_or_else(|| panic!("container {} has no network settings", self.id)); - let mut networks = network_settings + let mut networks = container_network_settings .networks - .unwrap_or_else(|| panic!("container {} has no any networks", self.id)); - - let bridge_name = self - .image - .network() - .clone() - .or_else(|| network_settings.bridge.filter(|b| !b.is_empty())) - .or_else(|| Some("bridge".to_owned())) - .unwrap_or_else(|| panic!("container {} has missing bridge name", self.id)); + .unwrap_or_else(|| panic!("container {} has no networks", self.id)); let ip = networks - .remove(&bridge_name) + .remove(&network_mode) .and_then(|network| network.ip_address) - .unwrap_or_else(|| { - panic!( - "container {} has missing bridge IP {:?}", - self.id, - self.image.network() - ) - }); + .unwrap_or_else(|| panic!("container {} has missing bridge IP", self.id)); IpAddr::from_str(&ip) - .unwrap_or_else(|_| panic!("container {} has invalid bridge IP", self.id,)) + .unwrap_or_else(|_| panic!("container {} has invalid bridge IP", self.id)) } /// Returns the host ip address of docker container diff --git a/testcontainers/src/runners/async_runner.rs b/testcontainers/src/runners/async_runner.rs index 86fea655..975fe753 100644 --- a/testcontainers/src/runners/async_runner.rs +++ b/testcontainers/src/runners/async_runner.rs @@ -385,13 +385,13 @@ mod tests { } #[tokio::test] - async fn async_should_rely_on_default_bridge_network_when_no_network_provided_and_settings_bridge_empty( - ) { - let hello_world = GenericImage::new("simple_web_server", "latest") + async fn async_should_rely_on_network_mode_when_no_network_provided_and_settings_bridge_empty() + { + let web_server = GenericImage::new("simple_web_server", "latest") .with_wait_for(WaitFor::message_on_stdout("server is ready")) .with_wait_for(WaitFor::seconds(1)); - let container = RunnableImage::from(hello_world.clone()).start().await; + let container = RunnableImage::from(web_server.clone()).start().await; assert!(!container .get_bridge_ip_address() @@ -400,6 +400,40 @@ mod tests { .is_empty()) } + #[tokio::test] + async fn async_should_rely_on_network_mode_when_network_is_provided_and_settings_bridge_empty() + { + let web_server = GenericImage::new("simple_web_server", "latest") + .with_wait_for(WaitFor::message_on_stdout("server is ready")) + .with_wait_for(WaitFor::seconds(1)); + + let container = RunnableImage::from(web_server.clone()) + .with_network("bridge") + .start() + .await; + + assert!(!container + .get_bridge_ip_address() + .await + .to_string() + .is_empty()) + } + + #[tokio::test] + #[should_panic] + async fn async_should_panic_when_non_bridged_network_selected() { + let web_server = GenericImage::new("simple_web_server", "latest") + .with_wait_for(WaitFor::message_on_stdout("server is ready")) + .with_wait_for(WaitFor::seconds(1)); + + let container = RunnableImage::from(web_server.clone()) + .with_network("host") + .start() + .await; + + container.get_bridge_ip_address().await; + } + #[tokio::test] async fn async_run_command_should_set_shared_memory_size() { let client = Client::lazy_client().await; diff --git a/testcontainers/src/runners/sync_runner.rs b/testcontainers/src/runners/sync_runner.rs index 2c1e37b6..2f5d775c 100644 --- a/testcontainers/src/runners/sync_runner.rs +++ b/testcontainers/src/runners/sync_runner.rs @@ -189,17 +189,44 @@ mod tests { } #[test] - fn async_should_rely_on_default_bridge_network_when_no_network_provided_and_settings_bridge_empty( - ) { - let hello_world = GenericImage::new("simple_web_server", "latest") + fn sync_should_rely_on_network_mode_when_no_network_provided_and_settings_bridge_empty() { + let web_server = GenericImage::new("simple_web_server", "latest") .with_wait_for(WaitFor::message_on_stdout("server is ready")) .with_wait_for(WaitFor::seconds(1)); - let container = RunnableImage::from(hello_world.clone()).start(); + let container = RunnableImage::from(web_server.clone()) + .with_network("test") + .start(); assert!(!container.get_bridge_ip_address().to_string().is_empty()) } + #[test] + fn sync_should_rely_on_network_mode_when_network_is_provided_and_settings_bridge_empty() { + let web_server = GenericImage::new("simple_web_server", "latest") + .with_wait_for(WaitFor::message_on_stdout("server is ready")) + .with_wait_for(WaitFor::seconds(1)); + + let container = RunnableImage::from(web_server.clone()) + .with_network("bridge") + .start(); + + assert!(!container.get_bridge_ip_address().to_string().is_empty()) + } + + #[test] + #[should_panic] + fn sync_should_panic_when_non_bridged_network_selected() { + let web_server = GenericImage::new("simple_web_server", "latest") + .with_wait_for(WaitFor::message_on_stdout("server is ready")) + .with_wait_for(WaitFor::seconds(1)); + + let container = RunnableImage::from(web_server.clone()) + .with_network("host") + .start(); + + container.get_bridge_ip_address(); + } #[test] fn sync_run_command_should_include_name() { let image = GenericImage::new("hello-world", "latest"); From 0db75e4d4f27540db2239e39f3f657a2f7e6300e Mon Sep 17 00:00:00 2001 From: Diego Lendoiro Date: Thu, 9 May 2024 21:19:09 +0200 Subject: [PATCH 4/4] removed tests --- testcontainers/src/runners/async_runner.rs | 16 ---------------- testcontainers/src/runners/sync_runner.rs | 13 ------------- 2 files changed, 29 deletions(-) diff --git a/testcontainers/src/runners/async_runner.rs b/testcontainers/src/runners/async_runner.rs index 975fe753..f8f13441 100644 --- a/testcontainers/src/runners/async_runner.rs +++ b/testcontainers/src/runners/async_runner.rs @@ -384,22 +384,6 @@ mod tests { assert!(!client.network_exists("awesome-net-2").await) } - #[tokio::test] - async fn async_should_rely_on_network_mode_when_no_network_provided_and_settings_bridge_empty() - { - let web_server = GenericImage::new("simple_web_server", "latest") - .with_wait_for(WaitFor::message_on_stdout("server is ready")) - .with_wait_for(WaitFor::seconds(1)); - - let container = RunnableImage::from(web_server.clone()).start().await; - - assert!(!container - .get_bridge_ip_address() - .await - .to_string() - .is_empty()) - } - #[tokio::test] async fn async_should_rely_on_network_mode_when_network_is_provided_and_settings_bridge_empty() { diff --git a/testcontainers/src/runners/sync_runner.rs b/testcontainers/src/runners/sync_runner.rs index 2f5d775c..f0c3bbae 100644 --- a/testcontainers/src/runners/sync_runner.rs +++ b/testcontainers/src/runners/sync_runner.rs @@ -188,19 +188,6 @@ mod tests { ); } - #[test] - fn sync_should_rely_on_network_mode_when_no_network_provided_and_settings_bridge_empty() { - let web_server = GenericImage::new("simple_web_server", "latest") - .with_wait_for(WaitFor::message_on_stdout("server is ready")) - .with_wait_for(WaitFor::seconds(1)); - - let container = RunnableImage::from(web_server.clone()) - .with_network("test") - .start(); - - assert!(!container.get_bridge_ip_address().to_string().is_empty()) - } - #[test] fn sync_should_rely_on_network_mode_when_network_is_provided_and_settings_bridge_empty() { let web_server = GenericImage::new("simple_web_server", "latest")