make managing namespaces more resilient against inconsitent states

src/namespace/mod.rs

@@ -1,12 +1,12 @@
 mod raw;
 
 use crate::config::NamespaceName;
-use crate::link::{link_up_ns, LinkError};
-use crate::namespace::raw::{create_network_namespace, NamespaceSetupError};
+use crate::link::{LinkError, link_up_ns};
+use crate::namespace::raw::{NamespaceSetupError, create_network_namespace};
 use either::Either;
 use nix::errno::Errno;
-use nix::mount::{mount, umount2, MntFlags, MsFlags};
-use std::fs::{create_dir, read_dir, remove_file, File};
+use nix::mount::{MntFlags, MsFlags, mount, umount2};
+use std::fs::{File, create_dir, read_dir, remove_file};
 use std::io::{Error as IoError, ErrorKind};
 use std::iter::empty;
 use std::os::unix::fs::symlink;
@@ -21,7 +21,7 @@ pub struct NetNs {
 }
 
 impl NetNs {
-    pub fn existing() -> Result<impl Iterator<Item = NamespaceName>, NamespaceError> {
+    pub fn existing(include_broken: bool) -> Result<impl Iterator<Item = NamespaceName>, NamespaceError> {
         let dir = match read_dir("/var/run/netnsd") {
             Ok(dir) => Ok(dir),
             Err(error) if error.kind() == ErrorKind::NotFound => {
@@ -32,12 +32,14 @@ impl NetNs {
                 error,
             }),
         }?;
-        Ok(Either::Right(dir.flatten().flat_map(|entry| {
-            NamespaceName::try_from(entry.file_name()).ok()
-        })))
+        Ok(Either::Right(
+            dir.flatten()
+                .filter(move |entry| include_broken || entry.path().is_symlink())
+                .flat_map(|entry| NamespaceName::try_from(entry.file_name()).ok()),
+        ))
     }
 
-    /// Create a new named network namespace that will be removed when dropped
+    /// Create a new named network namespace
     pub fn new(name: NamespaceName) -> Result<Self, NamespaceError> {
         let parent = Path::new("/var/run/netns");
         let nsd_parent = Path::new("/var/run/netnsd");
@@ -47,12 +49,22 @@ impl NetNs {
         let path = parent.join(&name);
         let nsd_path = nsd_parent.join(&name);
 
+        remove_non_mount(&path).map_err(|error| NamespaceError::Delete {
+            error,
+            path: nsd_path.clone(),
+        })?;
+
         match File::create_new(&path) {
             Ok(_) => {}
             Err(e) if e.kind() == ErrorKind::AlreadyExists => {
                 info!(%name, "using existing network namespace");
 
                 if !nsd_path.is_symlink() {
+                    remove_file_if_exists(&nsd_path).map_err(|error| NamespaceError::Delete {
+                        error,
+                        path: nsd_path.clone(),
+                    })?;
+
                     symlink(&path, &nsd_path).map_err(|error| NamespaceError::Symlink {
                         error,
                         path: nsd_path.clone(),
@@ -140,12 +152,16 @@ impl NetNs {
     pub fn delete(self) -> Result<(), NamespaceError> {
         let name = self.path.file_name().unwrap().to_str().unwrap();
         info!(name, "deleting network namespace");
-        umount2(&self.path, MntFlags::MNT_DETACH).map_err(NamespaceError::UnMount)?;
-        remove_file(&self.path).map_err(|error| NamespaceError::Delete {
+        match umount2(&self.path, MntFlags::MNT_DETACH) {
+            Err(Errno::EINVAL) => Ok(()), // not a mountpoint, namespace doesn't exist
+            rest => rest,
+        }
+        .map_err(NamespaceError::UnMount)?;
+        remove_file_if_exists(&self.path).map_err(|error| NamespaceError::Delete {
             error,
             path: self.path,
         })?;
-        remove_file(&self.nsd_path).map_err(|error| NamespaceError::Delete {
+        remove_file_if_exists(&self.nsd_path).map_err(|error| NamespaceError::Delete {
             error,
             path: self.nsd_path,
         })?;
@@ -182,3 +198,21 @@ impl NamespaceError {
         NamespaceError::Create { path, error }
     }
 }
+
+/// `remove_file`, but ignore "file not found" errors
+fn remove_file_if_exists<P: AsRef<Path>>(path: P) -> std::io::Result<()> {
+    match remove_file(path) {
+        Err(err) if err.kind() == ErrorKind::NotFound => Ok(()),
+        rest => rest,
+    }
+}
+
+
+/// `remove_file`, but ignore errors if the file doesn't exist or is a mount point
+fn remove_non_mount<P: AsRef<Path>>(path: P) -> std::io::Result<()> {
+    match remove_file(path) {
+        Err(err) if err.kind() == ErrorKind::NotFound => Ok(()),
+        Err(err) if err.kind() == ErrorKind::ResourceBusy => Ok(()),
+        rest => rest,
+    }
+}