diff --git a/CHANGELOG.md b/CHANGELOG.md index e79b586c..a551483f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,15 +14,20 @@ separate changelogs for each crate were used. If you need to refer to these old - `libcnb-test`: - `ContainerContext::address_for_port` now returns `SocketAddr` directly instead of `Option`. ([#605](https://github.com/heroku/libcnb.rs/pull/605)) + - Docker commands are now run using the Docker CLI instead of Bollard and the Docker daemon API. ([#620](https://github.com/heroku/libcnb.rs/pull/620)) + - `ContainerConfig::entrypoint` now accepts a String rather than a vector of strings. Any arguments to the entrypoint should be moved to `ContainerConfig::command`. ([#620](https://github.com/heroku/libcnb.rs/pull/620)) + - `TestRunner::new` has been removed, since its only purpose was for advanced configuration that's no longer applicable. Use `TestRunner::default` instead. ([#620](https://github.com/heroku/libcnb.rs/pull/620)) - `LogOutput` no longer exposes `stdout_raw` and `stderr_raw`. ([#607](https://github.com/heroku/libcnb.rs/pull/607)) - - Improved wording of panic error messages. ([#619](https://github.com/heroku/libcnb.rs/pull/619)) + - Improved wording of panic error messages. ([#619](https://github.com/heroku/libcnb.rs/pull/619) and [#620](https://github.com/heroku/libcnb.rs/pull/620)) - `libcnb-package`: buildpack target directory now contains the target triple. Users that implicitly rely on the output directory need to adapt. The output of `cargo libcnb package` will refer to the new locations. ([#580](https://github.com/heroku/libcnb.rs/pull/580)) - `libherokubuildpack`: Switch the `flate2` decompression backend from `miniz_oxide` to `zlib`. ([#593](https://github.com/heroku/libcnb.rs/pull/593)) - Bump minimum external dependency versions. ([#587](https://github.com/heroku/libcnb.rs/pull/587)) ### Fixed -- `libcnb-test`: `ContainerContext::expose_port` now only exposes the port to localhost. ([#610](https://github.com/heroku/libcnb.rs/pull/610)) +- `libcnb-test`: + - `TestContext::run_shell_command` and `ContainerContext::shell_exec` now validate the exit code of the spawned commands and panic if they are non-zero. ([#620](https://github.com/heroku/libcnb.rs/pull/620)) + - `ContainerContext::expose_port` now only exposes the port to localhost. ([#610](https://github.com/heroku/libcnb.rs/pull/610)) ## [0.13.0] 2023-06-21 diff --git a/libcnb-test/Cargo.toml b/libcnb-test/Cargo.toml index e04c7f1d..58e6208c 100644 --- a/libcnb-test/Cargo.toml +++ b/libcnb-test/Cargo.toml @@ -12,23 +12,13 @@ readme = "README.md" include = ["src/**/*", "LICENSE", "README.md"] [dependencies] -bollard = "0.14.0" cargo_metadata = "0.17.0" fastrand = "2.0.0" fs_extra = "1.3.0" libcnb-data.workspace = true libcnb-package.workspace = true -serde = "1.0.166" tempfile = "3.6.0" -tokio = "1.29.1" -tokio-stream = "0.1.14" [dev-dependencies] indoc = "2.0.2" ureq = { version = "2.7.1", default-features = false } - -[features] -# Enables experimental support for connecting to a remote Docker daemon. -# Disabled by default since support is only partly implemented, and also -# results in many more dependencies due to requiring TLS. -remote-docker = ["bollard/ssl"] diff --git a/libcnb-test/src/container_config.rs b/libcnb-test/src/container_config.rs index 9696c34f..c3828db4 100644 --- a/libcnb-test/src/container_config.rs +++ b/libcnb-test/src/container_config.rs @@ -27,7 +27,7 @@ use std::collections::{HashMap, HashSet}; /// ``` #[derive(Default)] pub struct ContainerConfig { - pub(crate) entrypoint: Option>, + pub(crate) entrypoint: Option, pub(crate) command: Option>, pub(crate) env: HashMap, pub(crate) exposed_ports: HashSet, @@ -76,17 +76,14 @@ impl ContainerConfig { /// BuildConfig::new("heroku/builder:22", "test-fixtures/app"), /// |context| { /// // ... - /// context.start_container(ContainerConfig::new().entrypoint(["worker"]), |container| { + /// context.start_container(ContainerConfig::new().entrypoint("worker"), |container| { /// // ... /// }); /// }, /// ); /// ``` - pub fn entrypoint, S: Into>( - &mut self, - entrypoint: I, - ) -> &mut Self { - self.entrypoint = Some(entrypoint.into_iter().map(S::into).collect()); + pub fn entrypoint(&mut self, entrypoint: impl Into) -> &mut Self { + self.entrypoint = Some(entrypoint.into()); self } diff --git a/libcnb-test/src/container_context.rs b/libcnb-test/src/container_context.rs index 9b2b53d7..882968a1 100644 --- a/libcnb-test/src/container_context.rs +++ b/libcnb-test/src/container_context.rs @@ -1,19 +1,17 @@ +use crate::docker::{ + DockerExecCommand, DockerLogsCommand, DockerPortCommand, DockerRemoveContainerCommand, +}; use crate::log::LogOutput; -use crate::{container_port_mapping, util}; -use crate::{log, TestContext}; -use bollard::container::RemoveContainerOptions; -use bollard::exec::{CreateExecOptions, StartExecResults}; -use serde::Serialize; +use crate::util; use std::net::SocketAddr; /// Context of a launched container. -pub struct ContainerContext<'a> { +pub struct ContainerContext { /// The randomly generated name of this container. pub container_name: String, - pub(crate) test_context: &'a TestContext<'a>, } -impl<'a> ContainerContext<'a> { +impl ContainerContext { /// Gets the container's log output until the current point in time. /// /// Note: This method will only return logs until the current point in time. It will not @@ -44,12 +42,8 @@ impl<'a> ContainerContext<'a> { /// Panics if there was an error retrieving the logs from the container. #[must_use] pub fn logs_now(&self) -> LogOutput { - self.logs_internal(bollard::container::LogsOptions { - stdout: true, - stderr: true, - tail: "all", - ..bollard::container::LogsOptions::default() - }) + util::run_command(DockerLogsCommand::new(&self.container_name)) + .unwrap_or_else(|command_err| panic!("Error fetching container logs:\n\n{command_err}")) } /// Gets the container's log output until the container stops. @@ -82,30 +76,10 @@ impl<'a> ContainerContext<'a> { /// Panics if there was an error retrieving the logs from the container. #[must_use] pub fn logs_wait(&self) -> LogOutput { - self.logs_internal(bollard::container::LogsOptions { - follow: true, - stdout: true, - stderr: true, - tail: "all", - ..bollard::container::LogsOptions::default() - }) - } - - #[must_use] - fn logs_internal + Serialize>( - &self, - logs_options: bollard::container::LogsOptions, - ) -> LogOutput { - self.test_context - .runner - .tokio_runtime - .block_on(log::consume_container_log_output( - self.test_context - .runner - .docker - .logs(&self.container_name, Some(logs_options)), - )) - .expect("Could not consume container log output") + let mut docker_logs_command = DockerLogsCommand::new(&self.container_name); + docker_logs_command.follow(true); + util::run_command(docker_logs_command) + .unwrap_or_else(|command_err| panic!("Error fetching container logs:\n\n{command_err}")) } /// Returns the local address of an exposed container port. @@ -137,23 +111,15 @@ impl<'a> ContainerContext<'a> { /// was not exposed using [`ContainerConfig::expose_port`](crate::ContainerConfig::expose_port). #[must_use] pub fn address_for_port(&self, port: u16) -> SocketAddr { - self.test_context.runner.tokio_runtime.block_on(async { - self.test_context - .runner - .docker - .inspect_container(&self.container_name, None) - .await - .expect("Could not inspect container") - .network_settings - .and_then(|network_settings| network_settings.ports) - .and_then(|ports| { - container_port_mapping::parse_port_map(&ports) - .expect("Could not parse container port mapping") - .get(&port) - .copied() - }) - .expect("Could not find specified port in container port mapping") - }) + let docker_port_command = DockerPortCommand::new(&self.container_name, port); + util::run_command(docker_port_command) + .unwrap_or_else(|command_err| { + panic!("Error obtaining container port mapping:\n\n{command_err}") + }) + .stdout + .trim() + .parse() + .expect("Couldn't parse `docker port` output") } /// Executes a shell command inside an already running container. @@ -176,60 +142,22 @@ impl<'a> ContainerContext<'a> { /// /// # Panics /// - /// Panics if it was not possible to exec into the container, or if there was an error - /// retrieving the logs from the exec command. - /// - /// Note: Does not currently panic if the command exits with a non-zero status code due to: - /// + /// Panics if it was not possible to exec into the container, or if the command + /// exited with a non-zero exit code. pub fn shell_exec(&self, command: impl AsRef) -> LogOutput { - self.test_context.runner.tokio_runtime.block_on(async { - let create_exec_result = self - .test_context - .runner - .docker - .create_exec( - &self.container_name, - CreateExecOptions { - cmd: Some(vec![util::CNB_LAUNCHER_BINARY, command.as_ref()]), - attach_stdout: Some(true), - ..CreateExecOptions::default() - }, - ) - .await - .expect("Could not create container exec instance"); - - let start_exec_result = self - .test_context - .runner - .docker - .start_exec(&create_exec_result.id, None) - .await - .expect("Could not start container exec instance"); - - match start_exec_result { - StartExecResults::Attached { output, .. } => { - log::consume_container_log_output(output) - .await - .expect("Could not consume container log output") - } - StartExecResults::Detached => LogOutput::default(), - } - }) + let docker_exec_command = DockerExecCommand::new( + &self.container_name, + [util::CNB_LAUNCHER_BINARY, command.as_ref()], + ); + util::run_command(docker_exec_command) + .unwrap_or_else(|command_err| panic!("Error performing docker exec:\n\n{command_err}")) } } -impl<'a> Drop for ContainerContext<'a> { +impl Drop for ContainerContext { fn drop(&mut self) { - // We do not care if container removal succeeded or not. Panicking here would result in - // SIGILL since this function might be called in a Tokio runtime. - let _remove_container_result = self.test_context.runner.tokio_runtime.block_on( - self.test_context.runner.docker.remove_container( - &self.container_name, - Some(RemoveContainerOptions { - force: true, - ..RemoveContainerOptions::default() - }), - ), + util::run_command(DockerRemoveContainerCommand::new(&self.container_name)).unwrap_or_else( + |command_err| panic!("Error removing Docker container:\n\n{command_err}"), ); } } diff --git a/libcnb-test/src/container_port_mapping.rs b/libcnb-test/src/container_port_mapping.rs deleted file mode 100644 index 26d72307..00000000 --- a/libcnb-test/src/container_port_mapping.rs +++ /dev/null @@ -1,229 +0,0 @@ -use bollard::container::Config; -use bollard::models::{HostConfig, PortBinding, PortMap}; -use std::collections::{HashMap, HashSet}; -use std::net::{AddrParseError, SocketAddr}; -use std::num::ParseIntError; - -/// Parse a Bollard [`PortMap`](bollard::models::PortMap) into a simpler, better typed, form. -pub(crate) fn parse_port_map( - port_map: &bollard::models::PortMap, -) -> Result, PortMapParseError> { - port_map.iter().map(parse_port_map_entry).collect() -} - -#[derive(Debug, Eq, PartialEq)] -pub(crate) enum PortMapParseError { - UnexpectedOrMissingContainerPortSuffix, - ContainerPortParseError(ParseIntError), - NoBindingForPort(u16), - MissingHostIpAddress, - MissingHostPort, - HostIpAddressParseError(AddrParseError), - HostPortParseError(ParseIntError), -} - -fn parse_port_map_entry( - port_mapping: (&String, &Option>), -) -> Result<(u16, SocketAddr), PortMapParseError> { - let (container_port, mappings) = port_mapping; - - let container_port = container_port - .strip_suffix("/tcp") - .ok_or(PortMapParseError::UnexpectedOrMissingContainerPortSuffix) - .and_then(|port_string| { - port_string - .parse() - .map_err(PortMapParseError::ContainerPortParseError) - })?; - - let port_binding = mappings - .clone() - .unwrap_or_default() - .first() - .cloned() - .ok_or(PortMapParseError::NoBindingForPort(container_port))?; - - let host_address = port_binding - .host_ip - .ok_or(PortMapParseError::MissingHostIpAddress) - .and_then(|host_ip| { - host_ip - .parse() - .map_err(PortMapParseError::HostIpAddressParseError) - })?; - - let host_port = port_binding - .host_port - .ok_or(PortMapParseError::MissingHostPort) - .and_then(|host_port| { - host_port - .parse() - .map_err(PortMapParseError::HostPortParseError) - })?; - - Ok((container_port, SocketAddr::new(host_address, host_port))) -} - -/// Create a new Bollard container config with the given exposed ports. -/// -/// The exposed ports will be forwarded to random ports on the host. -pub(crate) fn port_mapped_container_config( - ports: &HashSet, -) -> bollard::container::Config { - Config { - host_config: Some(HostConfig { - port_bindings: Some( - ports - .iter() - .map(|port| { - ( - format!("{port}/tcp"), - Some(vec![PortBinding { - host_ip: Some(String::from("127.0.0.1")), - host_port: None, - }]), - ) - }) - .collect::(), - ), - ..HostConfig::default() - }), - exposed_ports: Some( - ports - .iter() - .map(|port| { - ( - format!("{port}/tcp"), - #[allow(clippy::zero_sized_map_values)] - HashMap::new(), // Bollard requires this zero sized value map, - ) - }) - .collect(), - ), - ..Config::default() - } -} - -#[cfg(test)] -mod tests { - use super::*; - use bollard::models::PortMap; - - #[test] - fn parse_port_map_simple() { - let port_map: PortMap = HashMap::from([ - ( - String::from("12345/tcp"), - Some(vec![PortBinding { - host_ip: Some(String::from("0.0.0.0")), - host_port: Some(String::from("57233")), - }]), - ), - ( - String::from("80/tcp"), - Some(vec![PortBinding { - host_ip: Some(String::from("127.0.0.1")), - host_port: Some(String::from("51039")), - }]), - ), - ]); - - let expected_result: HashMap = HashMap::from([ - (12345, "0.0.0.0:57233".parse().unwrap()), - (80, "127.0.0.1:51039".parse().unwrap()), - ]); - - assert_eq!(parse_port_map(&port_map).unwrap(), expected_result); - } - - #[test] - fn parse_port_map_multiple_bindings() { - let port_map: PortMap = HashMap::from([( - String::from("12345/tcp"), - Some(vec![ - PortBinding { - host_ip: Some(String::from("0.0.0.0")), - host_port: Some(String::from("57233")), - }, - PortBinding { - host_ip: Some(String::from("0.0.0.0")), - host_port: Some(String::from("57234")), - }, - ]), - )]); - - let expected_result: HashMap = - HashMap::from([(12345, "0.0.0.0:57233".parse().unwrap())]); - - assert_eq!(parse_port_map(&port_map).unwrap(), expected_result); - } - - #[test] - fn parse_port_map_non_tcp_port_binding() { - let port_map: PortMap = HashMap::from([( - String::from("12345/udp"), - Some(vec![PortBinding { - host_ip: Some(String::from("0.0.0.0")), - host_port: Some(String::from("57233")), - }]), - )]); - - assert_eq!( - parse_port_map(&port_map), - Err(PortMapParseError::UnexpectedOrMissingContainerPortSuffix) - ); - } - - #[test] - fn port_mapped_container_config_simple() { - let config = port_mapped_container_config(&HashSet::from([80, 443, 22])); - - assert_eq!( - config.exposed_ports, - Some(HashMap::from([ - ( - String::from("80/tcp"), - #[allow(clippy::zero_sized_map_values)] - HashMap::new() - ), - ( - String::from("443/tcp"), - #[allow(clippy::zero_sized_map_values)] - HashMap::new() - ), - ( - String::from("22/tcp"), - #[allow(clippy::zero_sized_map_values)] - HashMap::new() - ) - ])) - ); - - assert_eq!( - config.host_config.unwrap().port_bindings, - Some(HashMap::from([ - ( - String::from("80/tcp"), - Some(vec![PortBinding { - host_ip: Some(String::from("127.0.0.1")), - host_port: None, - }]), - ), - ( - String::from("443/tcp"), - Some(vec![PortBinding { - host_ip: Some(String::from("127.0.0.1")), - host_port: None, - }]), - ), - ( - String::from("22/tcp"), - Some(vec![PortBinding { - host_ip: Some(String::from("127.0.0.1")), - host_port: None, - }]), - ) - ])) - ); - } -} diff --git a/libcnb-test/src/docker.rs b/libcnb-test/src/docker.rs new file mode 100644 index 00000000..4a300842 --- /dev/null +++ b/libcnb-test/src/docker.rs @@ -0,0 +1,378 @@ +use std::collections::{BTreeMap, BTreeSet}; +use std::process::Command; + +/// Represents a `docker run` command. +#[derive(Clone, Debug)] +pub(crate) struct DockerRunCommand { + command: Option>, + container_name: String, + detach: bool, + entrypoint: Option, + env: BTreeMap, + exposed_ports: BTreeSet, + image_name: String, + platform: Option, + remove: bool, +} + +impl DockerRunCommand { + pub fn new(image_name: impl Into, container_name: impl Into) -> Self { + Self { + command: None, + container_name: container_name.into(), + detach: false, + entrypoint: None, + env: BTreeMap::new(), + exposed_ports: BTreeSet::new(), + image_name: image_name.into(), + platform: None, + remove: false, + } + } + + pub fn command, S: Into>(&mut self, command: I) -> &mut Self { + self.command = Some(command.into_iter().map(S::into).collect()); + self + } + + pub fn detach(&mut self, detach: bool) -> &mut Self { + self.detach = detach; + self + } + + pub fn entrypoint(&mut self, entrypoint: impl Into) -> &mut Self { + self.entrypoint = Some(entrypoint.into()); + self + } + + pub fn env(&mut self, k: impl Into, v: impl Into) -> &mut Self { + self.env.insert(k.into(), v.into()); + self + } + + pub fn expose_port(&mut self, port: u16) -> &mut Self { + self.exposed_ports.insert(port); + self + } + + pub fn platform(&mut self, platform: impl Into) -> &mut Self { + self.platform = Some(platform.into()); + self + } + + pub fn remove(&mut self, remove: bool) -> &mut Self { + self.remove = remove; + self + } +} + +impl From for Command { + fn from(docker_run_command: DockerRunCommand) -> Self { + let mut command = Command::new("docker"); + command.args(["run", "--name", &docker_run_command.container_name]); + + if docker_run_command.detach { + command.arg("--detach"); + } + + if docker_run_command.remove { + command.arg("--rm"); + } + + if let Some(platform) = docker_run_command.platform { + command.args(["--platform", &platform]); + } + + if let Some(entrypoint) = docker_run_command.entrypoint { + command.args(["--entrypoint", &entrypoint]); + } + + for (env_key, env_value) in &docker_run_command.env { + command.args(["--env", &format!("{env_key}={env_value}")]); + } + + for port in &docker_run_command.exposed_ports { + command.args(["--publish", &format!("127.0.0.1::{port}")]); + } + + command.arg(docker_run_command.image_name); + + if let Some(container_command) = docker_run_command.command { + command.args(container_command); + } + + command + } +} + +/// Represents a `docker exec` command. +#[derive(Clone, Debug)] +pub(crate) struct DockerExecCommand { + command: Vec, + container_name: String, +} + +impl DockerExecCommand { + pub fn new, S: Into>( + container_name: impl Into, + command: I, + ) -> Self { + Self { + command: command.into_iter().map(S::into).collect(), + container_name: container_name.into(), + } + } +} + +impl From for Command { + fn from(docker_exec_command: DockerExecCommand) -> Self { + let mut command = Command::new("docker"); + command + .args(["exec", &docker_exec_command.container_name]) + .args(docker_exec_command.command); + command + } +} + +/// Represents a `docker logs` command. +#[derive(Clone, Debug)] +pub(crate) struct DockerLogsCommand { + container_name: String, + follow: bool, +} + +impl DockerLogsCommand { + pub fn new(container_name: impl Into) -> Self { + Self { + container_name: container_name.into(), + follow: false, + } + } + + pub fn follow(&mut self, follow: bool) -> &mut Self { + self.follow = follow; + self + } +} + +impl From for Command { + fn from(docker_logs_command: DockerLogsCommand) -> Self { + let mut command = Command::new("docker"); + command.args(["logs", &docker_logs_command.container_name]); + + if docker_logs_command.follow { + command.arg(String::from("--follow")); + } + + command + } +} + +/// Represents a `docker port` command. +#[derive(Clone, Debug)] +pub(crate) struct DockerPortCommand { + container_name: String, + port: u16, +} + +impl DockerPortCommand { + pub fn new(container_name: impl Into, port: u16) -> Self { + Self { + container_name: container_name.into(), + port, + } + } +} + +impl From for Command { + fn from(docker_port_command: DockerPortCommand) -> Self { + let mut command = Command::new("docker"); + command.args([ + "port", + &docker_port_command.container_name, + &docker_port_command.port.to_string(), + ]); + command + } +} + +/// Represents a `docker rm` command. +#[derive(Clone, Debug)] +pub(crate) struct DockerRemoveContainerCommand { + container_name: String, + force: bool, +} + +impl DockerRemoveContainerCommand { + pub fn new(container_name: impl Into) -> Self { + Self { + container_name: container_name.into(), + force: true, + } + } +} + +impl From for Command { + fn from(docker_remove_container_command: DockerRemoveContainerCommand) -> Self { + let mut command = Command::new("docker"); + command.args(["rm", &docker_remove_container_command.container_name]); + + if docker_remove_container_command.force { + command.arg("--force"); + } + + command + } +} + +/// Represents a `docker rmi` command. +#[derive(Clone, Debug)] +pub(crate) struct DockerRemoveImageCommand { + force: bool, + image_name: String, +} + +impl DockerRemoveImageCommand { + pub fn new(container_name: impl Into) -> Self { + Self { + force: true, + image_name: container_name.into(), + } + } +} + +impl From for Command { + fn from(docker_remove_image_command: DockerRemoveImageCommand) -> Self { + let mut command = Command::new("docker"); + command.args(["rmi", &docker_remove_image_command.image_name]); + + if docker_remove_image_command.force { + command.arg("--force"); + } + + command + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::ffi::OsStr; + + #[test] + fn from_docker_run_command_to_command() { + let mut docker_run_command = DockerRunCommand::new("my-image", "my-container"); + + // Default usage + let command: Command = docker_run_command.clone().into(); + assert_eq!(command.get_program(), "docker"); + assert_eq!( + command.get_args().collect::>(), + vec!["run", "--name", "my-container", "my-image"] + ); + + // With optional flag/arguments set + docker_run_command.command(["echo", "hello"]); + docker_run_command.detach(true); + docker_run_command.entrypoint("/usr/bin/bash"); + docker_run_command.env("BAR", "2"); + docker_run_command.env("FOO", "1"); + docker_run_command.expose_port(12345); + docker_run_command.expose_port(55555); + docker_run_command.platform("linux/amd64"); + docker_run_command.remove(true); + + let command: Command = docker_run_command.clone().into(); + assert_eq!( + command.get_args().collect::>(), + vec![ + "run", + "--name", + "my-container", + "--detach", + "--rm", + "--platform", + "linux/amd64", + "--entrypoint", + "/usr/bin/bash", + "--env", + "BAR=2", + "--env", + "FOO=1", + "--publish", + "127.0.0.1::12345", + "--publish", + "127.0.0.1::55555", + "my-image", + "echo", + "hello", + ] + ); + } + + #[test] + fn from_docker_exec_command_to_command() { + let docker_exec_command = DockerExecCommand::new("my-container", ["ps"]); + let command: Command = docker_exec_command.into(); + assert_eq!(command.get_program(), "docker"); + assert_eq!( + command.get_args().collect::>(), + vec!["exec", "my-container", "ps"] + ); + } + + #[test] + fn from_docker_logs_command_to_command() { + let mut docker_logs_command = DockerLogsCommand::new("my-container"); + + // Default usage + let command: Command = docker_logs_command.clone().into(); + assert_eq!(command.get_program(), "docker"); + assert_eq!( + command.get_args().collect::>(), + vec!["logs", "my-container"] + ); + + // With optional flag/arguments set + docker_logs_command.follow(true); + + let command: Command = docker_logs_command.clone().into(); + assert_eq!( + command.get_args().collect::>(), + vec!["logs", "my-container", "--follow"] + ); + } + + #[test] + fn from_docker_port_command_to_command() { + let docker_port_command = DockerPortCommand::new("my-container", 12345); + let command: Command = docker_port_command.into(); + assert_eq!(command.get_program(), "docker"); + assert_eq!( + command.get_args().collect::>(), + vec!["port", "my-container", "12345"] + ); + } + + #[test] + fn from_docker_remove_container_command_to_command() { + let docker_remove_container_command = DockerRemoveContainerCommand::new("my-container"); + let command: Command = docker_remove_container_command.into(); + assert_eq!(command.get_program(), "docker"); + assert_eq!( + command.get_args().collect::>(), + vec!["rm", "my-container", "--force"] + ); + } + + #[test] + fn from_docker_remove_image_command_to_command() { + let docker_remove_image_command = DockerRemoveImageCommand::new("my-image"); + let command: Command = docker_remove_image_command.into(); + assert_eq!(command.get_program(), "docker"); + assert_eq!( + command.get_args().collect::>(), + vec!["rmi", "my-image", "--force"] + ); + } +} diff --git a/libcnb-test/src/lib.rs b/libcnb-test/src/lib.rs index c0aa8358..dcf7a9f8 100644 --- a/libcnb-test/src/lib.rs +++ b/libcnb-test/src/lib.rs @@ -10,7 +10,7 @@ mod build; mod build_config; mod container_config; mod container_context; -mod container_port_mapping; +mod docker; mod log; mod macros; mod pack; diff --git a/libcnb-test/src/log.rs b/libcnb-test/src/log.rs index e8ccb52b..519b6717 100644 --- a/libcnb-test/src/log.rs +++ b/libcnb-test/src/log.rs @@ -1,42 +1,6 @@ -use tokio_stream::{Stream, StreamExt}; - /// Log output from a command. #[derive(Debug, Default)] pub struct LogOutput { pub stdout: String, pub stderr: String, } - -pub(crate) async fn consume_container_log_output< - E, - S: Stream> + Unpin, ->( - stream: S, -) -> Result { - stream - .collect::, E>>() - .await - .map(|log_output_chunks| { - let mut stdout_raw = Vec::new(); - let mut stderr_raw = Vec::new(); - - for log_output_chunk in log_output_chunks { - match log_output_chunk { - bollard::container::LogOutput::StdOut { message } => { - stdout_raw.append(&mut message.to_vec()); - } - bollard::container::LogOutput::StdErr { message } => { - stderr_raw.append(&mut message.to_vec()); - } - unimplemented_message => { - unimplemented!("message unimplemented: {unimplemented_message}") - } - } - } - - LogOutput { - stdout: String::from_utf8_lossy(&stdout_raw).to_string(), - stderr: String::from_utf8_lossy(&stderr_raw).to_string(), - } - }) -} diff --git a/libcnb-test/src/test_context.rs b/libcnb-test/src/test_context.rs index 93e230b1..e838516d 100644 --- a/libcnb-test/src/test_context.rs +++ b/libcnb-test/src/test_context.rs @@ -1,10 +1,6 @@ +use crate::docker::{DockerRemoveImageCommand, DockerRunCommand}; use crate::pack::PackSbomDownloadCommand; -use crate::{ - container_port_mapping, util, BuildConfig, ContainerConfig, ContainerContext, LogOutput, - TestRunner, -}; -use bollard::container::{Config, CreateContainerOptions, StartContainerOptions}; -use bollard::image::RemoveImageOptions; +use crate::{util, BuildConfig, ContainerConfig, ContainerContext, LogOutput, TestRunner}; use libcnb_data::buildpack::BuildpackId; use libcnb_data::layer::LayerName; use libcnb_data::sbom::SbomFormat; @@ -46,7 +42,7 @@ impl<'a> TestContext<'a> { /// /// // Start the container using the specified process-type: /// // https://buildpacks.io/docs/app-developer-guide/run-an-app/#non-default-process-type - /// context.start_container(ContainerConfig::new().entrypoint(["worker"]), |container| { + /// context.start_container(ContainerConfig::new().entrypoint("worker"), |container| { /// // ... /// }); /// @@ -54,7 +50,7 @@ impl<'a> TestContext<'a> { /// // https://buildpacks.io/docs/app-developer-guide/run-an-app/#non-default-process-type-with-additional-arguments /// context.start_container( /// ContainerConfig::new() - /// .entrypoint(["another-process"]) + /// .entrypoint("another-process") /// .command(["--additional-arg"]), /// |container| { /// // ... @@ -67,7 +63,7 @@ impl<'a> TestContext<'a> { /// // otherwise use the convenience function `TestContext::run_shell_command` instead. /// context.start_container( /// ContainerConfig::new() - /// .entrypoint(["launcher"]) + /// .entrypoint("launcher") /// .command(["for i in {1..3}; do echo \"${i}\"; done"]), /// |container| { /// // ... @@ -90,38 +86,30 @@ impl<'a> TestContext<'a> { let config = config.borrow(); let container_name = util::random_docker_identifier(); - self.runner.tokio_runtime.block_on(async { - self.runner - .docker - .create_container( - Some(CreateContainerOptions { - name: container_name.clone(), - ..CreateContainerOptions::default() - }), - Config { - image: Some(self.image_name.clone()), - env: Some(config.env.iter().map(|(k, v)| format!("{k}={v}")).collect()), - entrypoint: config.entrypoint.clone(), - cmd: config.command.clone(), - ..container_port_mapping::port_mapped_container_config( - &config.exposed_ports, - ) - }, - ) - .await - .expect("Could not create container"); + let mut docker_run_command = DockerRunCommand::new(&self.image_name, &container_name); + docker_run_command.detach(true); + docker_run_command.platform(self.determine_container_platform()); - self.runner - .docker - .start_container(&container_name, None::>) - .await - .expect("Could not start container"); + if let Some(entrypoint) = &config.entrypoint { + docker_run_command.entrypoint(entrypoint); + } + + if let Some(command) = &config.command { + docker_run_command.command(command); + } + + config.env.iter().for_each(|(key, value)| { + docker_run_command.env(key, value); }); - f(ContainerContext { - container_name, - test_context: self, + config.exposed_ports.iter().for_each(|port| { + docker_run_command.expose_port(*port); }); + + util::run_command(docker_run_command) + .unwrap_or_else(|command_err| panic!("Error starting container:\n\n{command_err}")); + + f(ContainerContext { container_name }); } /// Run the provided shell command. @@ -145,7 +133,7 @@ impl<'a> TestContext<'a> { /// ); /// ``` /// - /// This is a convenience function for running shell commands inside the image, and is equivalent to: + /// This is a convenience function for running shell commands inside the image, that is roughly equivalent to: /// ```no_run /// use libcnb_test::{BuildConfig, ContainerConfig, TestRunner}; /// @@ -155,7 +143,7 @@ impl<'a> TestContext<'a> { /// // ... /// context.start_container( /// ContainerConfig::new() - /// .entrypoint(["launcher"]) + /// .entrypoint("launcher") /// .command(["for i in {1..3}; do echo \"${i}\"; done"]), /// |container| { /// let log_output = container.logs_wait(); @@ -166,23 +154,38 @@ impl<'a> TestContext<'a> { /// ); /// ``` /// - /// # Panics + /// However, in addition to requiring less boilerplate, `run_shell_command` is also able + /// to validate the exit status of the container, so should be used instead of `start_container` + /// where possible. /// - /// Panics if there was an error starting the container. + /// # Panics /// - /// Note: Does not currently panic if the command exits with a non-zero status code due to: - /// + /// Panics if there was an error starting the container, or the command exited with a non-zero + /// exit code. pub fn run_shell_command(&self, command: impl Into) -> LogOutput { - let mut log_output = LogOutput::default(); - self.start_container( - ContainerConfig::new() - .entrypoint(vec![util::CNB_LAUNCHER_BINARY]) - .command(&[command.into()]), - |context| { - log_output = context.logs_wait(); - }, - ); - log_output + let mut docker_run_command = + DockerRunCommand::new(&self.image_name, util::random_docker_identifier()); + docker_run_command + .remove(true) + .platform(self.determine_container_platform()) + .entrypoint(util::CNB_LAUNCHER_BINARY) + .command([command.into()]); + + util::run_command(docker_run_command) + .unwrap_or_else(|command_err| panic!("Error running container:\n\n{command_err}")) + } + + // We set an explicit platform when starting containers to prevent the Docker CLI's + // "no specific platform was requested" warning from cluttering the captured logs. + fn determine_container_platform(&self) -> &str { + match self.config.target_triple.as_str() { + "aarch64-unknown-linux-musl" => "linux/arm64", + "x86_64-unknown-linux-musl" => "linux/amd64", + _ => unimplemented!( + "Unable to determine container platform from target triple '{}'. Please file a GitHub issue.", + self.config.target_triple + ), + } } /// Downloads SBOM files from the built image into a temporary directory. @@ -267,19 +270,8 @@ impl<'a> TestContext<'a> { impl<'a> Drop for TestContext<'a> { fn drop(&mut self) { - // We do not care if image removal succeeded or not. Panicking here would result in - // SIGILL since this function might be called in a Tokio runtime. - let _image_delete_result = - self.runner - .tokio_runtime - .block_on(self.runner.docker.remove_image( - &self.image_name, - Some(RemoveImageOptions { - force: true, - ..RemoveImageOptions::default() - }), - None, - )); + util::run_command(DockerRemoveImageCommand::new(&self.image_name)) + .unwrap_or_else(|command_err| panic!("Error removing Docker image:\n\n{command_err}")); } } diff --git a/libcnb-test/src/test_runner.rs b/libcnb-test/src/test_runner.rs index 5200088f..cb481762 100644 --- a/libcnb-test/src/test_runner.rs +++ b/libcnb-test/src/test_runner.rs @@ -3,10 +3,8 @@ use crate::util::CommandError; use crate::{ app, build, util, BuildConfig, BuildpackReference, LogOutput, PackResult, TestContext, }; -use bollard::Docker; use std::borrow::Borrow; use std::env; -use std::env::VarError; use std::path::PathBuf; /// Runner for libcnb integration tests. @@ -23,53 +21,10 @@ use std::path::PathBuf; /// }, /// ) /// ``` -pub struct TestRunner { - pub(crate) docker: Docker, - pub(crate) tokio_runtime: tokio::runtime::Runtime, -} - -impl Default for TestRunner { - fn default() -> Self { - let tokio_runtime = - tokio::runtime::Runtime::new().expect("Could not create internal Tokio runtime"); - - let docker = match env::var("DOCKER_HOST") { - #[cfg(target_family = "unix")] - Ok(docker_host) if docker_host.starts_with("unix://") => { - Docker::connect_with_unix_defaults() - } - Ok(docker_host) - if docker_host.starts_with("tcp://") || docker_host.starts_with("https://") => - { - #[cfg(not(feature = "remote-docker"))] - panic!("Cannot connect to DOCKER_HOST '{docker_host}' since it requires TLS. Please use a local Docker daemon instead (recommended), or else enable the experimental `remote-docker` feature."); - #[cfg(feature = "remote-docker")] - Docker::connect_with_ssl_defaults() - } - Ok(docker_host) => panic!("Cannot connect to unsupported DOCKER_HOST '{docker_host}'"), - Err(VarError::NotPresent) => Docker::connect_with_local_defaults(), - Err(VarError::NotUnicode(_)) => { - panic!("DOCKER_HOST environment variable is not unicode encoded!") - } - } - .expect("Could not connect to local Docker daemon"); - - Self::new(tokio_runtime, docker) - } -} +#[derive(Default)] +pub struct TestRunner {} impl TestRunner { - /// Creates a new runner that uses the given Tokio runtime and Docker connection. - /// - /// This function is meant for advanced use-cases where fine control over the Tokio runtime - /// and/or Docker connection is required. For the common use-cases, use `Runner::default`. - pub fn new(tokio_runtime: tokio::runtime::Runtime, docker: Docker) -> Self { - Self { - docker, - tokio_runtime, - } - } - /// Starts a new integration test build. /// /// This function copies the application to a temporary directory (if necessary), cross-compiles the current diff --git a/libcnb-test/tests/integration_test.rs b/libcnb-test/tests/integration_test.rs index d28bf97f..cabf21b0 100644 --- a/libcnb-test/tests/integration_test.rs +++ b/libcnb-test/tests/integration_test.rs @@ -310,7 +310,7 @@ fn starting_containers() { ); // Overriding the default entrypoint, but using the default command. - context.start_container(ContainerConfig::new().entrypoint(["worker"]), |container| { + context.start_container(ContainerConfig::new().entrypoint("worker"), |container| { let all_log_output = container.logs_wait(); assert_empty!(all_log_output.stderr); assert_eq!(all_log_output.stdout, "this is the worker process!\n"); @@ -319,7 +319,7 @@ fn starting_containers() { // Overriding both the entrypoint and command. context.start_container( ContainerConfig::new() - .entrypoint(["echo-args"]) + .entrypoint("echo-args") .command(["$GREETING", "$DESIGNATION"]) .envs([("GREETING", "Hello"), ("DESIGNATION", "World")]), |container| { @@ -339,16 +339,20 @@ fn starting_containers() { #[test] #[ignore = "integration test"] -#[should_panic( - expected = "unable to start container process: exec: \\\"nonexistent-command\\\": executable file not found in $PATH" -)] +#[should_panic(expected = "Error starting container: + +docker command failed with exit code 127! + +## stderr: + +docker: Error response from daemon:")] fn start_container_spawn_failure() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") .buildpacks([BuildpackReference::Other(String::from(PROCFILE_URL))]), |context| { context.start_container( - ContainerConfig::new().entrypoint(["nonexistent-command"]), + ContainerConfig::new().entrypoint("nonexistent-command"), |_| { unreachable!("The test should fail before the ContainerContext is invoked."); }, @@ -359,7 +363,13 @@ fn start_container_spawn_failure() { #[test] #[ignore = "integration test"] -#[should_panic(expected = "is not running")] +#[should_panic(expected = "Error performing docker exec: + +docker command failed with exit code 1! + +## stderr: + +Error response from daemon:")] fn shell_exec_when_container_has_crashed() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") @@ -367,7 +377,7 @@ fn shell_exec_when_container_has_crashed() { |context| { context.start_container( ContainerConfig::new() - .entrypoint(["launcher"]) + .entrypoint("launcher") .command(["exit 1"]), |container| { thread::sleep(Duration::from_secs(1)); @@ -380,8 +390,18 @@ fn shell_exec_when_container_has_crashed() { #[test] #[ignore = "integration test"] -// TODO: This test should panic: https://github.com/heroku/libcnb.rs/issues/446 -// #[should_panic(expected = "TODO")] +#[should_panic(expected = "Error performing docker exec: + +docker command failed with exit code 1! + +## stderr: + +some stderr + +## stdout: + +some stdout +")] fn shell_exec_nonzero_exit_status() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") @@ -397,8 +417,18 @@ fn shell_exec_nonzero_exit_status() { #[test] #[ignore = "integration test"] -// TODO: This test should panic: https://github.com/heroku/libcnb.rs/issues/446 -// #[should_panic(expected = "TODO")] +#[should_panic(expected = "Error running container: + +docker command failed with exit code 1! + +## stderr: + +some stderr + +## stdout: + +some stdout +")] fn run_shell_command_nonzero_exit_status() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") @@ -418,7 +448,7 @@ fn logs_work_after_container_crashed() { |context| { context.start_container( ContainerConfig::new() - .entrypoint(["launcher"]) + .entrypoint("launcher") .command(["echo 'some stdout'; echo 'some stderr' >&2; exit 1"]), |container| { thread::sleep(Duration::from_secs(1)); @@ -448,7 +478,13 @@ fn expose_port_invalid_port() { #[test] #[ignore = "integration test"] -#[should_panic(expected = "Could not find specified port in container port mapping")] +#[should_panic(expected = "Error obtaining container port mapping: + +docker command failed with exit code 1! + +## stderr: + +Error: No public port '12345")] fn address_for_port_when_port_not_exposed() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") @@ -464,7 +500,13 @@ fn address_for_port_when_port_not_exposed() { #[test] #[ignore = "integration test"] // TODO: Improve the UX here: https://github.com/heroku/libcnb.rs/issues/482 -#[should_panic(expected = "Could not find specified port in container port mapping")] +#[should_panic(expected = "Error obtaining container port mapping: + +docker command failed with exit code 1! + +## stderr: + +Error: No public port '12345")] fn address_for_port_when_container_crashed() { TestRunner::default().build( BuildConfig::new("heroku/builder:22", "test-fixtures/procfile") @@ -472,7 +514,7 @@ fn address_for_port_when_container_crashed() { |context| { context.start_container( ContainerConfig::new() - .entrypoint(["launcher"]) + .entrypoint("launcher") .command(["exit 1"]) .expose_port(TEST_PORT), |container| {