refactor Container methods

src/authoritative_name_server.rs

@@ -13,9 +13,9 @@ impl AuthoritativeNameServer {
     pub fn start(domain: Domain) -> Result<Self> {
         let container = Container::run()?;
 
-        container.exec(&["mkdir", "-p", "/etc/nsd/zones"])?;
+        container.status_ok(&["mkdir", "-p", "/etc/nsd/zones"])?;
-        let zone_path = "/etc/nsd/zones/main.zone";
 
+        let zone_path = "/etc/nsd/zones/main.zone";
         container.cp(
             "/etc/nsd/nsd.conf",
             &nsd_conf(domain.fqdn()),
@@ -87,12 +87,11 @@ mod tests {
         let ip_addr = tld_ns.ipv4_addr();
 
         let client = Container::run()?;
-        let output = client.exec(&["dig", &format!("@{ip_addr}"), "SOA", "com."])?;
+        let output = client.output(&["dig", &format!("@{ip_addr}"), "SOA", "com."])?;
 
         assert!(output.status.success());
-        let stdout = core::str::from_utf8(&output.stdout)?;
+        eprintln!("{}", output.stdout);
-        println!("{stdout}");
+        assert!(output.stdout.contains("status: NOERROR"));
-        assert!(stdout.contains("status: NOERROR"));
 
         Ok(())
     }
@@ -103,12 +102,11 @@ mod tests {
         let ip_addr = root_ns.ipv4_addr();
 
         let client = Container::run()?;
-        let output = client.exec(&["dig", &format!("@{ip_addr}"), "SOA", "."])?;
+        let output = client.output(&["dig", &format!("@{ip_addr}"), "SOA", "."])?;
 
         assert!(output.status.success());
-        let stdout = core::str::from_utf8(&output.stdout)?;
+        eprintln!("{}", output.stdout);
-        println!("{stdout}");
+        assert!(output.stdout.contains("status: NOERROR"));
-        assert!(stdout.contains("status: NOERROR"));
 
         Ok(())
     }

src/container.rs

@@ -2,17 +2,17 @@ use core::str;
 use std::fs;
 use std::net::Ipv4Addr;
 use std::path::Path;
-use std::process::{self, Child, Output};
+use std::process::{self, Child, ExitStatus};
 use std::process::{Command, Stdio};
 use std::sync::atomic::AtomicUsize;
 use std::sync::{atomic, Once};
 
 use tempfile::NamedTempFile;
 
-use crate::Result;
+use crate::{Error, Result};
 
 pub struct Container {
-    _name: String,
+    name: String,
     id: String,
     // TODO probably also want the IPv6 address
     ipv4_addr: Ipv4Addr,
@@ -33,7 +33,6 @@ impl Container {
             .join("docker")
             .join(format!("{binary}.Dockerfile"));
         let docker_dir_path = manifest_dir.join("docker");
-        dbg!(&image_tag);
 
         let mut command = Command::new("docker");
         command
@@ -50,29 +49,23 @@ impl Container {
 
         let mut command = Command::new("docker");
         let pid = process::id();
-        let container_name = format!(
+        let count = COUNT.fetch_add(1, atomic::Ordering::Relaxed);
-            "{binary}-{pid}-{}",
+        let name = format!("{binary}-{pid}-{count}");
-            COUNT.fetch_add(1, atomic::Ordering::Relaxed)
+        command
-        );
+            .args(["run", "--rm", "--detach", "--name", &name])
-        command.args(["run", "--rm", "--detach", "--name", &container_name]);
-        let output = command
             .arg("-it")
             .arg(image_tag)
-            .args(["sleep", "infinity"])
+            .args(["sleep", "infinity"]);
-            .output()?;
 
-        if !output.status.success() {
+        let output: Output = checked_output(&mut command)?.try_into()?;
-            return Err(format!("`{command:?}` failed").into());
+        let id = output.stdout;
-        }
-
-        let id = str::from_utf8(&output.stdout)?.trim().to_string();
         dbg!(&id);
 
         let ipv4_addr = get_ipv4_addr(&id)?;
 
         Ok(Self {
             id,
-            _name: container_name,
+            name,
             ipv4_addr,
         })
     }
@@ -86,37 +79,49 @@ impl Container {
 
         let mut command = Command::new("docker");
         command.args(["cp", &src_path, &dest_path]);
+        checked_output(&mut command)?;
 
-        let status = command.status()?;
+        self.status_ok(&["chmod", chmod, path_in_container])?;
-        if !status.success() {
-            return Err(format!("`{command:?}` failed").into());
-        }
-
-        let command = &["chmod", chmod, path_in_container];
-        let output = self.exec(command)?;
-        if !output.status.success() {
-            return Err(format!("`{command:?}` failed").into());
-        }
 
         Ok(())
     }
 
-    pub fn exec(&self, cmd: &[&str]) -> Result<Output> {
+    /// Similar to `std::process::Command::output` but runs `command_and_args` in the container
+    pub fn output(&self, command_and_args: &[&str]) -> Result<Output> {
         let mut command = Command::new("docker");
-        command.args(["exec", "-t", &self.id]).args(cmd);
+        command
+            .args(["exec", "-t", &self.id])
+            .args(command_and_args);
 
-        let output = command.output()?;
+        command.output()?.try_into()
+    }
 
-        Ok(output)
+    /// Similar to `std::process::Command::status` but runs `command_and_args` in the container
+    pub fn status(&self, command_and_args: &[&str]) -> Result<ExitStatus> {
+        let mut command = Command::new("docker");
+        command
+            .args(["exec", "-t", &self.id])
+            .args(command_and_args);
+
+        Ok(command.status()?)
+    }
+
+    /// Like `Self::status` but checks that `command_and_args` executed successfully
+    pub fn status_ok(&self, command_and_args: &[&str]) -> Result<()> {
+        let status = self.status(command_and_args)?;
+
+        if status.success() {
+            Ok(())
+        } else {
+            Err(format!("[{}] `{command_and_args:?}` failed", self.name).into())
+        }
     }
 
     pub fn spawn(&self, cmd: &[&str]) -> Result<Child> {
         let mut command = Command::new("docker");
         command.args(["exec", "-t", &self.id]).args(cmd);
 
-        let child = command.spawn()?;
+        Ok(command.spawn()?)
-
-        Ok(child)
     }
 
     pub fn ipv4_addr(&self) -> Ipv4Addr {
@@ -124,7 +129,44 @@ impl Container {
     }
 }
 
-// TODO cache this to avoid calling `docker inspect` every time
+#[derive(Debug)]
+pub struct Output {
+    pub status: ExitStatus,
+    pub stderr: String,
+    pub stdout: String,
+}
+
+impl TryFrom<process::Output> for Output {
+    type Error = Error;
+
+    fn try_from(output: process::Output) -> Result<Self> {
+        let mut stderr = String::from_utf8(output.stderr)?;
+        while stderr.ends_with('\n') {
+            stderr.pop();
+        }
+
+        let mut stdout = String::from_utf8(output.stdout)?;
+        while stdout.ends_with('\n') {
+            stdout.pop();
+        }
+
+        Ok(Self {
+            status: output.status,
+            stderr,
+            stdout,
+        })
+    }
+}
+
+fn checked_output(command: &mut Command) -> Result<process::Output> {
+    let output = command.output()?;
+    if output.status.success() {
+        Ok(output)
+    } else {
+        Err(format!("`{command:?}` failed").into())
+    }
+}
+
 fn get_ipv4_addr(container_id: &str) -> Result<Ipv4Addr> {
     let mut command = Command::new("docker");
     command
@@ -169,7 +211,7 @@ mod tests {
     fn run_works() -> Result<()> {
         let container = Container::run()?;
 
-        let output = container.exec(&["true"])?;
+        let output = container.output(&["true"])?;
         assert!(output.status.success());
 
         Ok(())
@@ -180,7 +222,7 @@ mod tests {
         let container = Container::run()?;
         let ipv4_addr = container.ipv4_addr();
 
-        let output = container.exec(&["ping", "-c1", &format!("{ipv4_addr}")])?;
+        let output = container.output(&["ping", "-c1", &format!("{ipv4_addr}")])?;
         assert!(output.status.success());
 
         Ok(())
@@ -194,12 +236,11 @@ mod tests {
         let contents = "hello";
         container.cp(path, contents, CHMOD_RW_EVERYONE)?;
 
-        let output = container.exec(&["cat", path])?;
+        let output = container.output(&["cat", path])?;
         dbg!(&output);
 
         assert!(output.status.success());
-
+        assert_eq!(contents, output.stdout);
-        assert_eq!(contents, core::str::from_utf8(&output.stdout)?);
 
         Ok(())
     }

src/lib.rs

@@ -7,7 +7,7 @@ pub type Result<T> = core::result::Result<T, Error>;
 const CHMOD_RW_EVERYONE: &str = "666";
 
 mod authoritative_name_server;
-mod container;
+pub mod container;
 mod recursive_resolver;
 
 pub enum Domain<'a> {

src/recursive_resolver.rs

@@ -68,10 +68,11 @@ mod tests {
         let resolver_ip_addr = resolver.ipv4_addr();
 
         let container = Container::run()?;
-        let output = container.exec(&["dig", &format!("@{}", resolver_ip_addr), "example.com"])?;
+        let output =
+            container.output(&["dig", &format!("@{}", resolver_ip_addr), "example.com"])?;
 
-        let stdout = core::str::from_utf8(&output.stdout)?;
+        assert!(output.status.success());
-        assert!(stdout.contains("status: NOERROR"));
+        assert!(output.stdout.contains("status: NOERROR"));
 
         Ok(())
     }