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: collect bridge IP address correctly #626

Merged
merged 4 commits into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ on:
pull_request:
merge_group:
push:
branches: [ main ]
branches: [main]

concurrency:
group: ${{ github.ref }}
Expand Down
16 changes: 14 additions & 2 deletions testcontainers/src/core/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -232,6 +234,16 @@ impl Client {
network.id
}

/// Inspects a network
pub(crate) async fn inspect_network(
&self,
name: &str,
) -> Result<Network, bollard::errors::Error> {
self.bollard
.inspect_network(name, Some(InspectNetworkOptions::<String>::default()))
.await
}

pub(crate) async fn create_container(
&self,
options: Option<CreateContainerOptions<String>>,
Expand Down
36 changes: 24 additions & 12 deletions testcontainers/src/core/containers/async_container.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use core::panic;
use std::{fmt, net::IpAddr, str::FromStr, sync::Arc};

use tokio::runtime::RuntimeFlavor;
Expand Down Expand Up @@ -139,25 +140,36 @@ 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));

let network_settings = self
.docker_client
.inspect_network(&network_mode)
.await
.unwrap_or_else(|_| panic!("container {} network mode does not exist", self.id));

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 = 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(settings.bridge)
.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));

Expand Down
34 changes: 34 additions & 0 deletions testcontainers/src/runners/async_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,40 @@ mod tests {
assert!(!client.network_exists("awesome-net-2").await)
}

#[tokio::test]
async fn async_should_rely_on_network_mode_when_network_is_provided_and_settings_bridge_empty()
Comment on lines +387 to +388
Copy link
Collaborator

@DDtKey DDtKey May 10, 2024

Choose a reason for hiding this comment

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

Hi @dlen!
I was about to merge the PR, but wanted to double-check the test coverage and current behavior in main.
And understood that these tests are passing against it.

I'm just wondering if this could be covered better, because if we change this back - we don't see any failures.
Sorry for the late notice

{
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())
}
DDtKey marked this conversation as resolved.
Show resolved Hide resolved

#[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;
Expand Down
26 changes: 26 additions & 0 deletions testcontainers/src/runners/sync_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,32 @@ mod tests {
);
}

#[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");
Expand Down