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.

Sign up for GitHub

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

[DRAFT] add extension traits to run container without explicitly creating a c… #503

Closed
wants to merge 3 commits into from
Closed

[DRAFT] add extension traits to run container without explicitly creating a c… #503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6d456f
add extension traits to run container without explicitly creating a c…
Jul 1, 2023
cd95e61
update tests to use singleton docker client
Jul 2, 2023
ccc896b
add ability start container directly from Image without needing to wr…
Jul 2, 2023
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
2 changes: 1 addition & 1 deletion testcontainers/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition = "2021"
keywords = [ "docker", "testcontainers" ]
license = "MIT OR Apache-2.0"
repository = "https://github.com/testcontainers/testcontainers-rs"
rust-version = "1.60.0"
rust-version = "1.70.0"
DDtKey marked this conversation as resolved.
Show resolved Hide resolved
description = "A library for integration-testing against docker containers from within Rust."

[dependencies]
Expand Down
5 changes: 3 additions & 2 deletions testcontainers/src/clients.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ mod cli;
#[cfg(feature = "experimental")]
mod http;

pub use self::cli::Cli;
pub(crate) use self::cli::docker_client as cli_docker_client;
pub use self::cli::RunViaCli;

#[cfg(feature = "experimental")]
pub use self::http::Http;
pub use self::http::RunViaHttp;
137 changes: 64 additions & 73 deletions testcontainers/src/clients/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,39 @@ use std::{
collections::HashMap,
ffi::{OsStr, OsString},
process::{Child, Command, Stdio},
sync::{Arc, RwLock},
sync::{OnceLock, RwLock},
thread::sleep,
time::{Duration, Instant},
};

const ONE_SECOND: Duration = Duration::from_secs(1);
const ZERO: Duration = Duration::from_secs(0);

/// Implementation of the Docker client API using the docker cli.
///
/// This (fairly naive) implementation of the Docker client API simply creates `Command`s to the `docker` CLI. It thereby assumes that the `docker` CLI is installed and that it is in the PATH of the current execution environment.
#[derive(Debug)]
pub struct Cli {
inner: Arc<Client>,
static CLI_DOCKER: OnceLock<Client> = OnceLock::new();
pub(crate) fn docker_client() -> &'static Client {
CLI_DOCKER.get_or_init(|| Client::new::<env::Os>())
}
Copy link
Author

Choose a reason for hiding this comment

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

Here we have global client which is used implicitly by Docker implementation.


impl Cli {
pub fn run<I: Image>(&self, image: impl Into<RunnableImage<I>>) -> Container<'_, I> {
let image = image.into();
pub trait RunViaCli<I: Image> {
fn start(self) -> Container<I>;
}

if let Some(network) = image.network() {
if self.inner.create_network_if_not_exists(network) {
let mut guard = self
.inner
impl<I> RunViaCli<I> for I
where
I: Image,
I::Args: Default,
{
fn start(self) -> Container<I> {
DDtKey marked this conversation as resolved.
Show resolved Hide resolved
RunnableImage::from(self).start()
}
}

impl<I: Image> RunViaCli<I> for RunnableImage<I> {
fn start(self) -> Container<I> {
let docker = docker_client();
if let Some(network) = self.network() {
if docker.create_network_if_not_exists(network) {
let mut guard = docker
.created_networks
.write()
.expect("failed to lock RwLock");
Expand All @@ -39,7 +48,7 @@ impl Cli {
}
}

let mut command = Client::build_run_command(&image, self.inner.command());
let mut command = Client::build_run_command(&self, docker.command());

log::debug!("Executing command: {:?}", command);

Expand All @@ -52,19 +61,16 @@ impl Cli {
.to_string();

#[cfg(feature = "watchdog")]
if self.inner.command == env::Command::Remove {
if docker.command == env::Command::Remove {
crate::watchdog::register(container_id.clone());
}

self.inner.register_container_started(container_id.clone());

self.block_until_ready(&container_id, image.ready_conditions());
docker.register_container_started(container_id.clone());

let client = Cli {
inner: self.inner.clone(),
};
docker.block_until_ready(&container_id, self.ready_conditions());

let container = Container::new(container_id, client, image, self.inner.command);
let container: Container<I> =
Container::new(container_id, docker.clone(), self, docker.command);

for cmd in container
.image()
Expand All @@ -78,7 +84,7 @@ impl Cli {
}

#[derive(Debug)]
struct Client {
pub(crate) struct Client {
/// The docker CLI has an issue that if you request logs for a container
/// too quickly after it was started up, the resulting stream will never
/// emit any data, even if the container is already emitting logs.
Expand Down Expand Up @@ -241,35 +247,25 @@ impl Client {
}
}

impl Default for Cli {
fn default() -> Self {
Self::new::<env::Os>()
}
}

impl Cli {
impl Client {
pub fn new<E>() -> Self
where
E: GetEnvValue,
{
Self {
inner: Arc::new(Client {
container_startup_timestamps: Default::default(),
created_networks: Default::default(),
binary: "docker".into(),
command: env::command::<E>().unwrap_or_default(),
}),
container_startup_timestamps: Default::default(),
created_networks: Default::default(),
binary: "docker".into(),
command: env::command::<E>().unwrap_or_default(),
}
}
}

impl Docker for Cli {
impl Docker for &Client {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need the Docker trait now? By virtue of the RunViaCli trait, the implementation is no longer pluggable so I believe we could just remove it now.

fn stdout_logs(&self, id: &str) -> LogStream {
self.inner
.wait_at_least_one_second_after_container_was_started(id);
self.wait_at_least_one_second_after_container_was_started(id);

let child = self
.inner
.command()
.arg("logs")
.arg("-f")
Expand All @@ -283,11 +279,9 @@ impl Docker for Cli {
}

fn stderr_logs(&self, id: &str) -> LogStream {
self.inner
.wait_at_least_one_second_after_container_was_started(id);
self.wait_at_least_one_second_after_container_was_started(id);

let child = self
.inner
.command()
.arg("logs")
.arg("-f")
Expand All @@ -311,7 +305,6 @@ impl Docker for Cli {

fn inspect(&self, id: &str) -> ContainerInspectResponse {
let output = self
.inner
.command()
.arg("inspect")
.arg(id)
Expand All @@ -335,7 +328,6 @@ impl Docker for Cli {

fn rm(&self, id: &str) {
let output = self
.inner
.command()
.arg("rm")
.arg("-f")
Expand All @@ -356,8 +348,7 @@ impl Docker for Cli {
}

fn stop(&self, id: &str) {
self.inner
.command()
self.command()
.arg("stop")
.arg(id)
.stdout(Stdio::piped())
Expand All @@ -368,8 +359,7 @@ impl Docker for Cli {
}

fn start(&self, id: &str) {
self.inner
.command()
self.command()
.arg("start")
.arg(id)
.stdout(Stdio::piped())
Expand All @@ -381,7 +371,6 @@ impl Docker for Cli {

fn exec(&self, id: &str, cmd: String) -> std::process::Output {
let exec_output = self
.inner
.command()
.arg("exec")
.arg(id)
Expand Down Expand Up @@ -410,7 +399,7 @@ impl Docker for Cli {
self.stderr_logs(id).wait_for_message(&message).unwrap()
}
WaitFor::Duration { length } => {
std::thread::sleep(length);
sleep(length);
}
WaitFor::Healthcheck => loop {
use HealthStatusEnum::*;
Expand Down Expand Up @@ -555,8 +544,7 @@ mod tests {
#[test]
#[should_panic(expected = "Failed to remove docker container")]
fn cli_rm_command_should_panic_on_invalid_container() {
let docker = Cli::default();
docker.rm("!INVALID_NAME_DUE_TO_SYMBOLS!");
docker_client().rm("!INVALID_NAME_DUE_TO_SYMBOLS!");
unreachable!()
}

Expand Down Expand Up @@ -626,24 +614,26 @@ mod tests {
#[test]
fn should_create_network_if_image_needs_it_and_drop_it_in_the_end() {
{
let docker = Cli::default();
let docker = docker_client();

assert!(!docker.inner.network_exists("awesome-net"));
assert!(!docker.network_exists("awesome-net"));

// creating the first container creates the network
let _container1 =
docker.run(RunnableImage::from(HelloWorld::default()).with_network("awesome-net"));
let _container1: Container<HelloWorld> = RunnableImage::from(HelloWorld::default())
.with_network("awesome-net")
.start();
// creating a 2nd container doesn't fail because check if the network exists already
let _container2 =
docker.run(RunnableImage::from(HelloWorld::default()).with_network("awesome-net"));
let _container2 = RunnableImage::from(HelloWorld::default())
.with_network("awesome-net")
.start();

assert!(docker.inner.network_exists("awesome-net"));
assert!(docker.network_exists("awesome-net"));
}

{
let docker = Cli::default();
let docker = docker_client();
// original client has been dropped, should clean up networks
assert!(!docker.inner.network_exists("awesome-net"))
assert!(!docker.network_exists("awesome-net"))
}
}

Expand All @@ -660,37 +650,38 @@ mod tests {
let network_name = "foobar-net";

{
let docker = Cli::new::<FakeEnvAlwaysKeep>();
let docker = docker_client();

assert!(!docker.inner.network_exists(network_name));
assert!(!docker.network_exists(network_name));

// creating the first container creates the network
let container1 =
docker.run(RunnableImage::from(HelloWorld::default()).with_network(network_name));
let container1 = RunnableImage::from(HelloWorld::default())
.with_network(network_name)
.start();

assert!(docker.inner.network_exists(network_name));
assert!(docker.network_exists(network_name));

// remove container, so network can get cleaned up after the test
docker.rm(container1.id());
}

let docker = Cli::default();
let docker = docker_client();

assert!(
docker.inner.network_exists(network_name),
docker.network_exists(network_name),
"network should still exist after client is dropped"
);

docker.inner.delete_networks(vec![network_name]);
docker.delete_networks(vec![network_name]);
}

#[test]
fn should_wait_for_at_least_one_second_before_fetching_logs() {
let _ = pretty_env_logger::try_init();
let docker = Cli::default();
let docker = docker_client();

let before_run = Instant::now();
let container = docker.run(HelloWorld::default());
let container = HelloWorld::default().start();
let after_run = Instant::now();

let before_logs = Instant::now();
Expand Down
Loading