Merge pull request #1067 from saschagrunert/pause-paths

Mimic pinns namespace path layout

.golangci.yml

@@ -97,14 +97,16 @@ linters:
     # - wsl
 linters-settings:
   funlen:
-    lines: 150
+    lines: 155
     statements: 50
   varnamelen:
     min-name-length: 1
   cyclop:
-    max-complexity: 30
+    max-complexity: 35
   gocognit:
-    min-complexity: 45
+    min-complexity: 50
+  gocyclo:
+    min-complexity: 50
   nestif:
     min-complexity: 15
   errcheck:

conmon-rs/common/proto/conmon.capnp

@@ -128,6 +128,8 @@ interface Conmon {
         namespaces @1 :List(Namespace); # The list of namespaces to unshare.
         uidMappings @2 :List(Text); # User ID mappings when unsharing the user namespace.
         gidMappings @3 :List(Text); # Group ID mappings when unsharing the user namespace.
+        basePath @4 :Text; # The root path for storing the namespaces.
+        podId @5 :Text; # The pod identifier.
     }
 
     enum Namespace {

conmon-rs/server/src/config.rs

@@ -136,16 +136,35 @@ pub struct Config {
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, Subcommand)]
 /// Possible subcommands.
 pub enum Commands {
-    /// Run pause instead of the server.
+    /// Run pause, which bind mounts selected namespaces to the local file system.
+    ///
+    /// If a namespace is not selected by one of the flags, then it will fallback to the host
+    /// namespace and still create the bind mount to it. All namespaces are mounted to
+    /// /var/run/[ipc,pid,net,user,uts]ns/$POD_ID, whereas the POD_ID is being passed from the
+    /// client.
+    ///
+    /// Tracking of the pause PID will be done by using a file in /var/run/conmonrs/$POD_ID.pid,
+    /// which gets removed together with the mounted namespaces if `conmonrs pause` terminates.
+    ///
+    /// UID and GID mappings are required if unsharing of the user namespace (via `--user`) is
+    /// selected.
     Pause {
         #[clap(
-            env(concat!(prefix!(), "PAUSE_PATH")),
-            long("path"),
+            default_value("/var/run"),
+            env(concat!(prefix!(), "PAUSE_BASE_PATH")),
+            long("base-path"),
             short('p'),
             value_name("PATH")
         )]
         /// The base path for pinning the namespaces.
-        path: PathBuf,
+        base_path: PathBuf,
+
+        #[clap(
+            env(concat!(prefix!(), "PAUSE_POD_ID")),
+            long("pod-id"),
+        )]
+        /// The unique pod identifier for referring to the namespaces.
+        pod_id: String,
 
         #[clap(long("ipc"))]
         /// Unshare the IPC namespace.

conmon-rs/server/src/pause.rs

@@ -20,14 +20,16 @@ use std::{
     process::{exit, Command},
 };
 use strum::{AsRefStr, Display, EnumIter, EnumString, IntoEnumIterator, IntoStaticStr};
-use tracing::{debug, error, info};
-use uuid::Uuid;
+use tracing::{debug, info, trace, warn};
 
 /// The main structure for this module.
 #[derive(Debug, CopyGetters, Getters)]
 pub struct Pause {
     #[get = "pub"]
-    path: PathBuf,
+    base_path: PathBuf,
+
+    #[get = "pub"]
+    pod_id: String,
 
     #[get = "pub"]
     namespaces: Vec<Namespace>,
@@ -39,21 +41,18 @@ pub struct Pause {
 /// The global shared multiple pause instance.
 static PAUSE: OnceCell<Pause> = OnceCell::new();
 
-/// The global path for storing bin mounted namespaces.
-const PAUSE_PATH: &str = "/var/run/conmonrs";
-
-/// The file path for storing the pause PID.
-const PAUSE_PID_FILE: &str = ".pause_pid";
-
 impl Pause {
     /// Retrieve the global instance of pause
     pub fn init_shared(
+        base_path: &str,
+        pod_id: &str,
         namespaces: Reader<conmon::Namespace>,
         uid_mappings: Vec<String>,
         gid_mappings: Vec<String>,
     ) -> Result<&'static Pause> {
         PAUSE.get_or_try_init(|| {
-            Self::init(namespaces, uid_mappings, gid_mappings).context("init pause")
+            Self::init(base_path, pod_id, namespaces, uid_mappings, gid_mappings)
+                .context("init pause")
         })
     }
 
@@ -66,34 +65,39 @@ impl Pause {
     pub fn stop(&self) {
         info!("Stopping pause");
         for namespace in self.namespaces() {
-            if let Err(e) = namespace.umount(self.path()) {
+            if let Err(e) = namespace.umount(self.base_path(), self.pod_id()) {
                 debug!("Unable to umount namespace {namespace}: {:#}", e);
             }
         }
-        if let Err(e) = fs::remove_dir_all(self.path()) {
-            error!(
-                "Unable to remove pause path {}: {:#}",
-                self.path().display(),
-                e
-            );
-        }
 
         info!("Killing pause PID: {}", self.pid());
         if let Err(e) = kill(self.pid(), Signal::SIGTERM) {
-            error!("Unable to kill pause PID {}: {:#}", self.pid(), e);
+            warn!("Unable to kill pause PID {}: {:#}", self.pid(), e);
+        }
+
+        let pause_pid_path = Self::pause_pid_path(self.base_path(), self.pod_id());
+        if let Err(e) = fs::remove_file(&pause_pid_path) {
+            debug!(
+                "Unable to remove pause PID path {}: {:#}",
+                pause_pid_path.display(),
+                e
+            );
         }
     }
 
     /// Initialize a new pause instance.
     fn init(
+        base_path: &str,
+        pod_id: &str,
         init_namespaces: Reader<conmon::Namespace>,
         uid_mappings: Vec<String>,
         gid_mappings: Vec<String>,
     ) -> Result<Self> {
         debug!("Initializing pause");
 
-        let mut args: Vec<String> = vec![];
+        let mut args: Vec<String> = vec![format!("--pod-id={pod_id}")];
         let mut namespaces = vec![];
+
         for namespace in init_namespaces.iter() {
             match namespace? {
                 conmon::Namespace::Ipc => {
@@ -138,15 +142,15 @@ impl Pause {
         debug!("Pause namespaces: {:?}", namespaces);
         debug!("Pause args: {:?}", args);
 
-        let path = PathBuf::from(PAUSE_PATH).join(Uuid::new_v4().to_string());
-        fs::create_dir_all(&path).context("create base path")?;
-        debug!("Pause base path: {}", path.display());
+        let base_path = PathBuf::from(base_path);
+        fs::create_dir_all(&base_path).context("create base path")?;
+        debug!("Pause base path: {}", base_path.display());
 
         let program = env::args().next().context("no args set")?;
         let mut child = Command::new(program)
             .arg("pause")
-            .arg("--path")
-            .arg(&path)
+            .arg("--base-path")
+            .arg(&base_path)
             .args(args)
             .spawn()
             .context("run pause")?;
@@ -156,24 +160,34 @@ impl Pause {
             bail!("exit status not ok: {status}")
         }
 
-        let pid = fs::read_to_string(path.join(PAUSE_PID_FILE))
+        let pid = fs::read_to_string(Self::pause_pid_path(&base_path, pod_id))
             .context("read pause PID path")?
             .trim()
             .parse::<u32>()
             .context("parse pause PID")?;
         info!("Pause PID is: {pid}");
 
         Ok(Self {
-            path,
+            base_path,
+            pod_id: pod_id.to_owned(),
             namespaces: Namespace::iter().collect(),
             pid: Pid::from_raw(pid as pid_t),
         })
     }
 
+    /// Retrieve the pause PID path for a base and pod ID.
+    fn pause_pid_path<T: AsRef<Path>>(base_path: T, pod_id: &str) -> PathBuf {
+        base_path
+            .as_ref()
+            .join("conmonrs")
+            .join(format!("{pod_id}.pid"))
+    }
+
     #[allow(clippy::too_many_arguments)]
     /// Run a new pause instance.
     pub fn run<T: AsRef<Path> + Copy>(
-        path: T,
+        base_path: T,
+        pod_id: &str,
         ipc: bool,
         pid: bool,
         net: bool,
@@ -209,16 +223,22 @@ impl Pause {
         let (mut sock_parent, mut sock_child) =
             UnixStream::pair().context("create unix socket pair")?;
         const MSG: &[u8] = &[1];
+        let mut res = [0];
 
         match unsafe { fork().context("forking process")? } {
             ForkResult::Parent { child } => {
-                let mut file = File::create(path.as_ref().join(PAUSE_PID_FILE))
-                    .context("create pause PID file")?;
+                let pause_pid_path = Self::pause_pid_path(base_path, pod_id);
+                fs::create_dir_all(
+                    pause_pid_path
+                        .parent()
+                        .context("no parent for pause PID path")?,
+                )
+                .context("create pause PID parent path")?;
+                let mut file = File::create(pause_pid_path).context("create pause PID file")?;
                 write!(file, "{child}").context("write child to pause file")?;
 
                 if user {
                     // Wait for user namespace creation
-                    let mut res = [0];
                     sock_parent.read_exact(&mut res)?;
 
                     // Write mappings
@@ -229,6 +249,9 @@ impl Pause {
                     sock_parent.write_all(MSG)?;
                 }
 
+                // Wait for mounts to be created
+                sock_parent.read_exact(&mut res)?;
+
                 exit(0);
             }
 
@@ -239,7 +262,6 @@ impl Pause {
                 sock_child.write_all(MSG)?;
 
                 // Wait for the mappings to be written
-                let mut res = [0];
                 sock_child.read_exact(&mut res)?;
 
                 // Set the UID and GID
@@ -258,12 +280,15 @@ impl Pause {
 
         // We bind all namespaces, if not unshared then we use the host namespace.
         for namespace in Namespace::iter() {
-            namespace.bind(path.as_ref()).context(format!(
+            namespace.bind(base_path, pod_id).context(format!(
                 "bind namespace to path: {}",
-                namespace.path(path).display(),
+                namespace.path(base_path, pod_id).display(),
             ))?;
         }
 
+        // Notify that all mounts are created
+        sock_child.write_all(MSG)?;
+
         let mut signals = Signals::new(TERM_SIGNALS).context("register signals")?;
         signals.forever().next().context("no signal number")?;
         Ok(())
@@ -323,9 +348,15 @@ pub enum Namespace {
 }
 
 impl Namespace {
-    /// Bind the namespace to the provided base path.
-    pub fn bind<T: AsRef<Path>>(&self, path: T) -> Result<()> {
-        let bind_path = self.path(path);
+    /// Bind the namespace to the provided base path and pod ID.
+    pub fn bind<T: AsRef<Path>>(&self, path: T, pod_id: &str) -> Result<()> {
+        let bind_path = self.path(path, pod_id);
+        fs::create_dir_all(
+            bind_path
+                .parent()
+                .context("no parent namespace bind path")?,
+        )
+        .context("create namespace parent path")?;
         File::create(&bind_path).context("create namespace bind path")?;
         let source_path = PathBuf::from("/proc/self/ns").join(self.as_ref());
 
@@ -342,14 +373,17 @@ impl Namespace {
     }
 
     /// Umount the namespace.
-    pub fn umount<T: AsRef<Path>>(&self, path: T) -> Result<()> {
-        let bind_path = self.path(path);
-        umount(&bind_path).context("umount namespace")
+    pub fn umount<T: AsRef<Path>>(&self, path: T, pod_id: &str) -> Result<()> {
+        let bind_path = self.path(path, pod_id);
+        if let Err(e) = umount(&bind_path) {
+            trace!("Unable to umount namespace {self}: {:#}", e);
+        }
+        fs::remove_file(&bind_path).context("remove namespace bind path")
     }
 
-    /// Retrieve the bind path of the namespace for the provided base path.
-    pub fn path<T: AsRef<Path>>(&self, path: T) -> PathBuf {
-        path.as_ref().join(self.as_ref())
+    /// Retrieve the bind path of the namespace for the provided base path and pod ID.
+    pub fn path<T: AsRef<Path>>(&self, path: T, pod_id: &str) -> PathBuf {
+        path.as_ref().join(format!("{self}ns")).join(pod_id)
     }
 
     pub fn to_capnp_namespace(self) -> conmon::Namespace {

conmon-rs/server/src/rpc.rs

@@ -360,15 +360,19 @@ impl conmon::Server for Server {
     ) -> Promise<(), capnp::Error> {
         debug!("Got a create namespaces request");
         let req = pry!(pry!(params.get()).get_request());
+        let pod_id = pry_err!(req.get_pod_id());
 
-        let span = debug_span!(
-            "create_namespaces",
-            uuid = Uuid::new_v4().to_string().as_str()
-        );
+        if pod_id.is_empty() {
+            return Promise::err(Error::failed("no pod ID provided".into()));
+        }
+
+        let span = new_root_span!("create_namespaces", pod_id);
         let _enter = span.enter();
         pry_err!(Telemetry::set_parent_context(pry!(req.get_metadata())));
 
         let pause = pry_err!(Pause::init_shared(
+            pry!(req.get_base_path()),
+            pod_id,
             pry!(req.get_namespaces()),
             capnp_vec_str!(req.get_uid_mappings()),
             capnp_vec_str!(req.get_gid_mappings()),
@@ -380,7 +384,12 @@ impl conmon::Server for Server {
 
         for (idx, namespace) in pause.namespaces().iter().enumerate() {
             let mut ns = namespaces.reborrow().get(pry_err!(idx.try_into()));
-            ns.set_path(&namespace.path(pause.path()).display().to_string());
+            ns.set_path(
+                &namespace
+                    .path(pause.base_path(), pod_id)
+                    .display()
+                    .to_string(),
+            );
             ns.set_type(namespace.to_capnp_namespace());
         }
 

conmon-rs/server/src/server.rs

@@ -64,7 +64,8 @@ impl Server {
         }
 
         if let Some(Commands::Pause {
-            path,
+            base_path,
+            pod_id,
             ipc,
             pid,
             net,
@@ -75,7 +76,8 @@ impl Server {
         }) = server.config().command()
         {
             Pause::run(
-                path,
+                base_path,
+                pod_id,
                 *ipc,
                 *pid,
                 *net,