Merge pull request #197 from utam0k/refactoring/namespaces-lifetime

reduce the number of clones by introducing lifetime to namespaces.

oci_spec/src/linux.rs

@@ -532,11 +532,10 @@ pub struct LinuxNamespace {
     #[serde(rename = "type")]
     /// Type is the type of namespace.
     pub typ: LinuxNamespaceType,
-
     #[serde(default, skip_serializing_if = "Option::is_none")]
     /// Path is a path to an existing namespace persisted on disk that can be joined and is of the
     /// same type
-    pub path: Option<String>,
+    pub path: Option<PathBuf>,
 }
 
 // Utility function to get default namespaces

src/container/builder_impl.rs

@@ -1,4 +1,5 @@
 use anyhow::{Context, Result};
+use nix::sched::CloneFlags;
 use oci_spec::Spec;
 use std::{fs, os::unix::prelude::RawFd, path::PathBuf};
 
@@ -54,12 +55,6 @@ impl<'a> ContainerBuilderImpl<'a> {
         let linux = self.spec.linux.as_ref().context("no linux in spec")?;
         let cgroups_path = utils::get_cgroup_path(&linux.cgroups_path, &self.container_id);
         let cmanager = cgroups::common::create_cgroup_manager(&cgroups_path, self.use_systemd)?;
-        let namespaces: Namespaces = linux
-            .namespaces
-            .as_ref()
-            .context("no namespaces in linux spec")?
-            .clone()
-            .into();
 
         // create the parent and child process structure so the parent and child process can sync with each other
         let (mut parent, parent_channel) = parent::ParentProcess::new(&self.rootless)?;
@@ -91,7 +86,12 @@ impl<'a> ContainerBuilderImpl<'a> {
             0
         });
 
-        let init_pid = fork::clone(cb, namespaces.clone_flags)?;
+        let clone_flags = linux
+            .namespaces
+            .as_ref()
+            .map(|ns| Namespaces::from(ns).clone_flags)
+            .unwrap_or_else(CloneFlags::empty);
+        let init_pid = fork::clone(cb, clone_flags)?;
         log::debug!("init pid is {:?}", init_pid);
 
         parent.wait_for_child_ready(init_pid)?;

src/container/tenant_builder.rs

@@ -303,7 +303,7 @@ impl TenantContainerBuilder {
                 let tenant_ns = LinuxNamespaceType::try_from(ns_type)?;
                 tenant_namespaces.push(LinuxNamespace {
                     typ: tenant_ns,
-                    path: Some(init_ns.path.to_string_lossy().to_string()),
+                    path: Some(init_ns.path.clone()),
                 })
             }
         }

src/namespaces.rs

@@ -18,14 +18,14 @@ use nix::{
 use oci_spec::LinuxNamespace;
 
 /// Holds information about namespaces
-pub struct Namespaces {
-    spaces: Vec<LinuxNamespace>,
+pub struct Namespaces<'a> {
+    spaces: &'a Vec<LinuxNamespace>,
     command: Box<dyn Syscall>,
     pub clone_flags: CloneFlags,
 }
 
-impl From<Vec<LinuxNamespace>> for Namespaces {
-    fn from(namespaces: Vec<LinuxNamespace>) -> Self {
+impl<'a> From<&'a Vec<LinuxNamespace>> for Namespaces<'a> {
+    fn from(namespaces: &'a Vec<LinuxNamespace>) -> Self {
         let clone_flags = namespaces.iter().filter(|ns| ns.path.is_none()).fold(
             CloneFlags::empty(),
             |mut cf, ns| {
@@ -43,8 +43,7 @@ impl From<Vec<LinuxNamespace>> for Namespaces {
     }
 }
 
-impl Namespaces {
-    /// sets namespaces as defined in structure to calling process
+impl<'a> Namespaces<'a> {
     pub fn apply_setns(&self) -> Result<()> {
         let to_enter: Vec<(CloneFlags, i32)> = self
             .spaces
@@ -53,7 +52,7 @@ impl Namespaces {
             .map(|ns| {
                 let space = CloneFlags::from_bits_truncate(ns.typ as i32);
                 let fd = fcntl::open(
-                    &*ns.path.as_ref().unwrap().clone(),
+                    &*ns.path.as_ref().unwrap(),
                     fcntl::OFlag::empty(),
                     stat::Mode::empty(),
                 )
@@ -94,11 +93,11 @@ mod tests {
         vec![
             LinuxNamespace {
                 typ: LinuxNamespaceType::Mount,
-                path: Some("/dev/null".to_string()),
+                path: Some("/dev/null".into()),
             },
             LinuxNamespace {
                 typ: LinuxNamespaceType::Network,
-                path: Some("/dev/null".to_string()),
+                path: Some("/dev/null".into()),
             },
             LinuxNamespace {
                 typ: LinuxNamespaceType::Pid,
@@ -118,7 +117,7 @@ mod tests {
     #[test]
     fn test_namespaces_set_ns() {
         let sample_linux_namespaces = gen_sample_linux_namespaces();
-        let namespaces: Namespaces = sample_linux_namespaces.into();
+        let namespaces = Namespaces::from(&sample_linux_namespaces);
         let test_command: &TestHelperSyscall = namespaces.command.as_any().downcast_ref().unwrap();
         assert!(namespaces.apply_setns().is_ok());
 
@@ -136,7 +135,7 @@ mod tests {
     #[test]
     fn test_namespaces_unshare() {
         let sample_linux_namespaces = gen_sample_linux_namespaces();
-        let namespaces: Namespaces = sample_linux_namespaces.into();
+        let namespaces = Namespaces::from(&sample_linux_namespaces);
         assert!(namespaces.apply_unshare(CloneFlags::CLONE_NEWIPC).is_ok());
 
         let test_command: &TestHelperSyscall = namespaces.command.as_any().downcast_ref().unwrap();

src/process/init.rs

@@ -111,17 +111,12 @@ pub struct ContainerInitArgs {
 pub fn container_init(args: ContainerInitArgs) -> Result<()> {
     let command = &args.syscall;
     let spec = &args.spec;
-    let linux = &spec.linux.as_ref().context("no linux in spec")?;
-    let namespaces: Namespaces = linux
-        .namespaces
-        .as_ref()
-        .context("no namepsaces in linux spec")?
-        .clone()
-        .into();
+    let linux = spec.linux.as_ref().context("no linux in spec")?;
+
     // need to create the notify socket before we pivot root, since the unix
     // domain socket used here is outside of the rootfs of container
     let mut notify_socket: NotifyListener = NotifyListener::new(&args.notify_path)?;
-    let proc = &spec.process.as_ref().context("no process in spec")?;
+    let proc = spec.process.as_ref().context("no process in spec")?;
     let mut envs: Vec<String> = proc.env.as_ref().unwrap_or(&vec![]).clone();
     let rootfs = &args.rootfs;
     let mut child = args.child;
@@ -167,7 +162,15 @@ pub fn container_init(args: ContainerInitArgs) -> Result<()> {
     }
 
     // join existing namespaces
-    namespaces.apply_setns()?;
+    let bind_service = if let Some(ns) = linux.namespaces.as_ref() {
+        let namespaces = Namespaces::from(ns);
+        namespaces.apply_setns()?;
+        namespaces
+            .clone_flags
+            .contains(sched::CloneFlags::CLONE_NEWUSER)
+    } else {
+        false
+    };
 
     if let Some(hostname) = spec.hostname.as_ref() {
         command.set_hostname(hostname)?;
@@ -178,14 +181,8 @@ pub fn container_init(args: ContainerInitArgs) -> Result<()> {
     }
 
     if args.init {
-        rootfs::prepare_rootfs(
-            spec,
-            rootfs,
-            namespaces
-                .clone_flags
-                .contains(sched::CloneFlags::CLONE_NEWUSER),
-        )
-        .with_context(|| "Failed to prepare rootfs")?;
+        rootfs::prepare_rootfs(spec, rootfs, bind_service)
+            .with_context(|| "Failed to prepare rootfs")?;
 
         // change the root of filesystem of the process to the rootfs
         command

src/rootless.rs

@@ -84,12 +84,13 @@ pub fn validate(spec: &Spec) -> Result<()> {
         bail!("rootless containers require at least one gid mapping")
     }
 
-    let namespaces: Namespaces = linux
-        .namespaces
-        .as_ref()
-        .context("rootless containers require namespaces in spec")?
-        .clone()
-        .into();
+    let namespaces = Namespaces::from(
+        linux
+            .namespaces
+            .as_ref()
+            .context("rootless containers require the namespaces.")?,
+    );
+
     if !namespaces.clone_flags.contains(CloneFlags::CLONE_NEWUSER) {
         bail!("rootless containers require the specification of a user namespace");
     }