libcnb-test: Migrate from Bollard to Docker CLI (#621)

Previously libcnb-test ran its various Docker related actions/commands
(excluding those handled by Pack CLI) using the bollard crate.

Whilst at first glance using a Rust client for Docker seems preferable,
it turns out that using Bollard/the Docker daemon API actually has a
number of disadvantages, which are explained in more detail in #620.

Now:
- The Docker CLI is used instead of Bollard+Tokio+the Docker daemon API,
  which is simpler to understand/maintain and also avoids ~55 additional
  transitive dependencies.
- The `run_shell_command` and `shell_exec` commands are now run as
  attached `docker run` invocations, making it trivial to check their
  exit code, fixing #446.
- Whenever errors occur, the error output is now easier for end users to
  understand, since it's presented as Docker CLI output with which they
  will be more familiar (vs Docker daemon/Bollard API errors).

As part of this change, `ContainerConfig::entrypoint` can no longer
accept a vector of strings, since the Docker CLI's `--entrypoint` arg
only accepts a single value, unlike the Docker daemon API. However,
since the purpose of `libcnb-test` is to replicate typical end-user
usage of the buildpacks and resultant images, this actually improves
alignment of the framework with the use-cases we want to test.

Fixes #446.
Closes #620.
GUS-W-11382688.
GUS-W-13853580.

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<SocketAddr>`. ([#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
 

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"]

libcnb-test/src/container_config.rs

@@ -27,7 +27,7 @@ use std::collections::{HashMap, HashSet};
 /// ```
 #[derive(Default)]
 pub struct ContainerConfig {
- pub(crate) entrypoint: Option<Vec<String>>,
+ pub(crate) entrypoint: Option<String>,
  pub(crate) command: Option<Vec<String>>,
  pub(crate) env: HashMap<String, String>,
  pub(crate) exposed_ports: HashSet<u16>,
@@ -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<I: IntoIterator<Item = S>, S: Into<String>>(
- &mut self,
- entrypoint: I,
- ) -> &mut Self {
- self.entrypoint = Some(entrypoint.into_iter().map(S::into).collect());
+ pub fn entrypoint(&mut self, entrypoint: impl Into<String>) -> &mut Self {
+ self.entrypoint = Some(entrypoint.into());
  self
  }
 

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<T: Into<String> + Serialize>(
- &self,
- logs_options: bollard::container::LogsOptions<T>,
- ) -> 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:
- /// <https://github.com/heroku/libcnb.rs/issues/446>
+ /// 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<str>) -> 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}"),
  );
  }
 }