Incorporate feedback

* add new type `Network` that holds `Arc`
* adjust network name to use `CARGO_PKG_NAME` env var, process id and
  counter
* remove function to remove network
* clone Network in container
* refactor Network tests

packages/dns-test/src/container.rs

@@ -67,6 +67,7 @@ impl Container {
             id,
             name,
             ipv4_addr,
+            _network: network.clone(),
         };
         Ok(Self {
             inner: Arc::new(inner),
@@ -172,6 +173,7 @@ struct Inner {
     id: String,
     // TODO probably also want the IPv6 address
     ipv4_addr: Ipv4Addr,
+    _network: Network,
 }
 
 /// NOTE unlike `std::process::Child`, the drop implementation of this type will `kill` the

packages/dns-test/src/container/network.rs

@@ -1,25 +1,57 @@
 use std::{
-    process::{Command, ExitStatus, Stdio},
-    sync::atomic::{self, AtomicUsize},
+    process::{self, Command, Stdio},
+    sync::{
+        atomic::{self, AtomicUsize},
+        Arc,
+    },
 };
 
 use crate::Result;
 
-const NETWORK_NAME: &str = "dnssec-network";
-
 /// Represents a network in which to put containers into.
-pub struct Network {
+#[derive(Clone)]
+pub struct Network(Arc<NetworkInner>);
+
+impl Network {
+    /// Returns the name of the network.
+    pub fn name(&self) -> &str {
+        self.0.name.as_str()
+    }
+
+    /// Returns the subnet mask
+    pub fn netmask(&self) -> &str {
+        &self.0.config.subnet
+    }
+}
+
+struct NetworkInner {
     name: String,
     config: NetworkConfig,
 }
 
 impl Network {
     pub fn new() -> Result<Self> {
-        let id = network_count();
-        let network_name = format!("{NETWORK_NAME}-{id}");
+        let pid = process::id();
+        let network_name = env!("CARGO_PKG_NAME");
+        Ok(Self(Arc::new(NetworkInner::new(pid, network_name)?)))
+    }
+}
 
-        // A network can exist, for example when a test panics
-        let _ = remove_network(network_name.as_str())?;
+/// This ensure the Docker network is deleted after the test runner process ends.
+impl Drop for NetworkInner {
+    fn drop(&mut self) {
+        let _ = Command::new("docker")
+            .args(["network", "rm", "--force", self.name.as_str()])
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .status();
+    }
+}
+
+impl NetworkInner {
+    pub fn new(pid: u32, network_name: &str) -> Result<Self> {
+        let count = network_count();
+        let network_name = format!("{network_name}-{pid}-{count}");
 
         let mut command = Command::new("docker");
         command
@@ -28,13 +60,13 @@ impl Network {
             .arg(&network_name);
 
         // create network
-        let output = command.output().unwrap();
+        let output = command.output()?;
         let stdout = String::from_utf8_lossy(&output.stdout);
         let stderr = String::from_utf8_lossy(&output.stderr);
-        assert!(
-            output.status.success(),
-            "--- STDOUT ---\n{stdout}\n--- STDERR ---\n{stderr}"
-        );
+
+        if !output.status.success() {
+            return Err(format!("--- STDOUT ---\n{stdout}\n--- STDERR ---\n{stderr}").into());
+        }
 
         // inspect & parse network details
         let config = get_network_config(&network_name)?;
@@ -44,16 +76,6 @@ impl Network {
             config,
         })
     }
-
-    /// Returns the name of the network.
-    pub fn name(&self) -> &str {
-        self.name.as_str()
-    }
-
-    /// Returns the subnet mask
-    pub fn netmask(&self) -> &str {
-        &self.config.subnet
-    }
 }
 
 /// Collects all important configs.
@@ -83,64 +105,6 @@ fn get_network_config(network_name: &str) -> Result<NetworkConfig> {
     Ok(NetworkConfig { subnet })
 }
 
-/// This ensure the Docker network is deleted after the test runner process ends.
-impl Drop for Network {
-    fn drop(&mut self) {
-        let _ = remove_network(&self.name);
-    }
-}
-
-/// Removes the given network.
-fn remove_network(network_name: &str) -> Result<ExitStatus> {
-    // Disconnects all attached containers
-    for container_id in get_attached_containers(network_name)? {
-        let mut command = Command::new("docker");
-        let _ = command
-            .args(["network", "disconnect", "--force"])
-            .args([network_name, container_id.as_str()])
-            .stdout(Stdio::null())
-            .stderr(Stdio::null())
-            .status()?;
-    }
-
-    // Remove the network
-    let mut command = Command::new("docker");
-    command
-        .args(["network", "rm", "--force", network_name])
-        .stdout(Stdio::null())
-        .stderr(Stdio::null());
-    Ok(command.status()?)
-}
-
-/// Finds the list of connected containers
-fn get_attached_containers(network_name: &str) -> Result<Vec<String>> {
-    let mut command = Command::new("docker");
-    command.args([
-        "network",
-        "inspect",
-        network_name,
-        "-f",
-        r#"{{ range $k, $v := .Containers }}{{ printf "%s\n" $k }}{{ end }}"#,
-    ]);
-
-    let output = command.output()?;
-    let container_ids = match output.status.success() {
-        true => {
-            let container_ids = std::str::from_utf8(&output.stdout)?
-                .trim()
-                .to_string()
-                .lines()
-                .filter(|line| !line.trim().is_empty())
-                .map(|line| line.to_string())
-                .collect::<Vec<_>>();
-            container_ids
-        }
-        false => vec![],
-    };
-
-    Ok(container_ids)
-}
-
 fn network_count() -> usize {
     static COUNT: AtomicUsize = AtomicUsize::new(1);
 
@@ -153,6 +117,35 @@ mod tests {
 
     use super::*;
 
+    /// Finds the list of connected containers
+    fn get_attached_containers(network_name: &str) -> Result<Vec<String>> {
+        let mut command = Command::new("docker");
+        command.args([
+            "network",
+            "inspect",
+            network_name,
+            "-f",
+            r#"{{ range $k, $v := .Containers }}{{ printf "%s\n" $k }}{{ end }}"#,
+        ]);
+
+        let output = command.output()?;
+        let container_ids = match output.status.success() {
+            true => {
+                let container_ids = std::str::from_utf8(&output.stdout)?
+                    .trim()
+                    .to_string()
+                    .lines()
+                    .filter(|line| !line.trim().is_empty())
+                    .map(|line| line.to_string())
+                    .collect::<Vec<_>>();
+                container_ids
+            }
+            false => vec![],
+        };
+
+        Ok(container_ids)
+    }
+
     #[test]
     fn create_works() -> Result<()> {
         assert!(Network::new().is_ok());
@@ -170,17 +163,18 @@ mod tests {
     #[test]
     fn remove_network_works() -> Result<()> {
         let network = Network::new().expect("Failed to create network");
-        let network_name = network.name().to_string();
         let nameserver = NameServer::new(FQDN::ROOT, &network)?;
 
         let container_ids = get_attached_containers(network.name())?;
         assert_eq!(1, container_ids.len());
         assert_eq!(&[nameserver.container_id().to_string()], &container_ids[..]);
 
-        drop(network);
+        drop(nameserver);
 
-        let container_ids = get_attached_containers(&network_name)?;
-        assert!(container_ids.is_empty());
+        let container_ids = get_attached_containers(network.name())?;
+        assert_eq!(0, container_ids.len());
+
+        drop(network);
 
         Ok(())
     }