runtime-rs: split amend_spec function

amend_spec do two works:

- modify the spec
- check if the pid namespace is enabled

This make it confusable. So split it into two functions.

Fixes: #5062

Signed-off-by: Bin Liu <bin@hyper.sh>

src/runtime-rs/crates/runtimes/virt_container/src/container_manager/container.rs

@@ -80,8 +80,9 @@ impl Container {
         let mut inner = self.inner.write().await;
         let toml_config = self.resource_manager.config().await;
         let config = &self.config;
-        let sandbox_pidns = amend_spec(&mut spec, toml_config.runtime.disable_guest_seccomp)
+        amend_spec(&mut spec, toml_config.runtime.disable_guest_seccomp).context("amend spec")?;
-            .context("load spec")?;
+        let sandbox_pidns = is_pid_namespace_enabled(&spec);
+
         // handler rootfs
         let rootfs = self
             .resource_manager
@@ -373,7 +374,7 @@ impl Container {
     }
 }
 
-fn amend_spec(spec: &mut oci::Spec, disable_guest_seccomp: bool) -> Result<bool> {
+fn amend_spec(spec: &mut oci::Spec, disable_guest_seccomp: bool) -> Result<()> {
     // hook should be done on host
     spec.hooks = None;
 
@@ -401,33 +402,29 @@ fn amend_spec(spec: &mut oci::Spec, disable_guest_seccomp: bool) -> Result<bool>
         }
 
         linux.namespaces = ns;
-
-        return Ok(handle_pid_namespace(&linux.namespaces));
     }
 
-    Ok(false)
+    Ok(())
 }
 
-// handle_pid_namespace checks if Pid namespace for a container needs to be shared with its sandbox
+// is_pid_namespace_enabled checks if Pid namespace for a container needs to be shared with its sandbox
 // pid namespace.
-fn handle_pid_namespace(namespaces: &[oci::LinuxNamespace]) -> bool {
+fn is_pid_namespace_enabled(spec: &oci::Spec) -> bool {
-    for n in namespaces.iter() {
+    if let Some(linux) = spec.linux.as_ref() {
-        match n.r#type.as_str() {
+        for n in linux.namespaces.iter() {
-            oci::PIDNAMESPACE => {
+            if n.r#type.as_str() == oci::PIDNAMESPACE {
-                if !n.path.is_empty() {
+                return !n.path.is_empty();
-                    return true;
-                }
             }
-            _ => continue,
         }
     }
+
     false
 }
 
 #[cfg(test)]
 mod tests {
     use super::amend_spec;
-    use crate::container_manager::container::handle_pid_namespace;
+    use super::is_pid_namespace_enabled;
     #[test]
     fn test_amend_spec_disable_guest_seccomp() {
         let mut spec = oci::Spec {
@@ -448,38 +445,69 @@ mod tests {
         amend_spec(&mut spec, true).unwrap();
         assert!(spec.linux.as_ref().unwrap().seccomp.is_none());
     }
+
     #[test]
-    fn test_handle_pid_namespace() {
+    fn test_is_pid_namespace_enabled() {
-        let namespaces = vec![
+        struct TestData<'a> {
-            oci::LinuxNamespace {
+            desc: &'a str,
-                r#type: "pid".to_string(),
+            namespaces: Vec<oci::LinuxNamespace>,
-                path: "".to_string(),
+            result: bool,
+        }
+
+        let tests = &[
+            TestData {
+                desc: "no pid namespace",
+                namespaces: vec![oci::LinuxNamespace {
+                    r#type: "network".to_string(),
+                    path: "".to_string(),
+                }],
+                result: false,
             },
-            oci::LinuxNamespace {
+            TestData {
-                r#type: "network".to_string(),
+                desc: "empty pid namespace path",
-                path: "".to_string(),
+                namespaces: vec![
+                    oci::LinuxNamespace {
+                        r#type: "pid".to_string(),
+                        path: "".to_string(),
+                    },
+                    oci::LinuxNamespace {
+                        r#type: "network".to_string(),
+                        path: "".to_string(),
+                    },
+                ],
+                result: false,
             },
-            oci::LinuxNamespace {
+            TestData {
-                r#type: "ipc".to_string(),
+                desc: "pid namespace is set",
-                path: "".to_string(),
+                namespaces: vec![
-            },
+                    oci::LinuxNamespace {
-            oci::LinuxNamespace {
+                        r#type: "pid".to_string(),
-                r#type: "uts".to_string(),
+                        path: "/some/path".to_string(),
-                path: "".to_string(),
+                    },
-            },
+                    oci::LinuxNamespace {
-            oci::LinuxNamespace {
+                        r#type: "network".to_string(),
-                r#type: "mount".to_string(),
+                        path: "".to_string(),
-                path: "".to_string(),
+                    },
-            },
+                ],
-            oci::LinuxNamespace {
+                result: true,
-                r#type: "user".to_string(),
-                path: "".to_string(),
-            },
-            oci::LinuxNamespace {
-                r#type: "cgroup".to_string(),
-                path: "".to_string(),
             },
         ];
-        assert!(!handle_pid_namespace(&namespaces));
+
+        let mut spec = oci::Spec::default();
+
+        for (i, d) in tests.iter().enumerate() {
+            spec.linux = Some(oci::Linux {
+                namespaces: d.namespaces.clone(),
+                ..Default::default()
+            });
+
+            assert_eq!(
+                d.result,
+                is_pid_namespace_enabled(&spec),
+                "test[{}]: {:?}",
+                i,
+                d.desc
+            );
+        }
     }
 }