runk: Re-implement start operation using the agent codes

This commit re-implements `start` operation by leveraging the agent codes.
Currently, `runk` has own `start` mechanism even if the agent already
has the feature to handle starting a container. This worsen the maintainability
and `runk` cannot keep up with the changes on the agent side easily.
Hence, `runk` replaces own implementations with agent's ones.

Fixes: #5648

Signed-off-by: Manabu Sugimoto <Manabu.Sugimoto@sony.com>

src/tools/runk/libcontainer/src/activated_builder.rs

@@ -3,7 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 
-use crate::container::{create_linux_container, Container, ContainerLauncher};
+use crate::container::{load_linux_container, Container, ContainerLauncher};
 use crate::status::Status;
 use crate::utils::validate_spec;
 use anyhow::{anyhow, Result};
@@ -63,7 +63,7 @@ impl ActivatedContainer {
             logger,
             "enter ActivatedContainer::create_launcher {:?}", self
         );
-        let container = Container::load(&self.root, &self.id)?;
+        let mut container = Container::load(&self.root, &self.id)?;
 
         // If state is Created or Running, we can execute the process.
         if container.state != ContainerState::Created && container.state != ContainerState::Running
@@ -74,17 +74,21 @@ impl ActivatedContainer {
             ));
         }
 
-        let mut config = container.status.config;
-        let spec = config.spec.as_mut().unwrap();
+        let spec = container
+            .status
+            .config
+            .spec
+            .as_mut()
+            .ok_or_else(|| anyhow!("spec config was not present"))?;
         self.adapt_exec_spec(spec, container.status.pid, logger)?;
         debug!(logger, "adapted spec: {:?}", spec);
         validate_spec(spec, &self.console_socket)?;
 
-        debug!(logger, "create LinuxContainer with config: {:?}", config);
-        // Maybe we should move some properties from status into LinuxContainer,
-        // like pid, process_start_time, created, cgroup_manager, etc. But it works now.
-        let runner =
-            create_linux_container(&self.id, &self.root, config, self.console_socket, logger)?;
+        debug!(
+            logger,
+            "load LinuxContainer with config: {:?}", &container.status.config
+        );
+        let runner = load_linux_container(&container.status, self.console_socket, logger)?;
 
         Ok(ContainerLauncher::new(
             &self.id,

src/tools/runk/libcontainer/src/container.rs

@@ -35,6 +35,7 @@ pub const CONFIG_FILE_NAME: &str = "config.json";
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub enum ContainerAction {
     Create,
+    Start,
     Run,
 }
 
@@ -236,12 +237,12 @@ impl ContainerLauncher {
         if self.init {
             self.spawn_container(action, logger).await?;
         } else {
-            if action != ContainerAction::Run {
+            if action == ContainerAction::Create {
                 return Err(anyhow!(
                     "ContainerAction::Create is used for init-container only"
                 ));
             }
-            self.spawn_process(ContainerAction::Run, logger).await?;
+            self.spawn_process(action, logger).await?;
         }
         if let Some(pid_file) = self.pid_file.as_ref() {
             fs::write(
@@ -257,13 +258,15 @@ impl ContainerLauncher {
         // State root path root/id has been created in LinuxContainer::new(),
         // so we don't have to create it again.
 
+        // Spawn a new process in the container by using the agent's codes.
         self.spawn_process(action, logger).await?;
+
         let status = self.get_status()?;
         status.save()?;
         debug!(logger, "saved status is {:?}", status);
 
         // Clean up the fifo file created by LinuxContainer, which is used for block the created process.
-        if action == ContainerAction::Run {
+        if action == ContainerAction::Run || action == ContainerAction::Start {
             let fifo_path = get_fifo_path(&status);
             if fifo_path.exists() {
                 unlink(&fifo_path)?;
@@ -308,6 +311,9 @@ impl ContainerLauncher {
             ContainerAction::Create => {
                 self.runner.start(process).await?;
             }
+            ContainerAction::Start => {
+                self.runner.exec().await?;
+            }
             ContainerAction::Run => {
                 self.runner.run(process).await?;
             }
@@ -358,6 +364,33 @@ pub fn create_linux_container(
     Ok(container)
 }
 
+// Load rustjail's Linux container.
+// "uid_map_path" and "gid_map_path" are always empty, so they are not set.
+pub fn load_linux_container(
+    status: &Status,
+    console_socket: Option<PathBuf>,
+    logger: &Logger,
+) -> Result<LinuxContainer> {
+    let mut container = LinuxContainer::new(
+        &status.id,
+        &status
+            .root
+            .to_str()
+            .map(|s| s.to_string())
+            .ok_or_else(|| anyhow!("failed to convert a root path"))?,
+        status.config.clone(),
+        logger,
+    )?;
+    if let Some(socket_path) = console_socket.as_ref() {
+        container.set_console_socket(socket_path)?;
+    }
+
+    container.init_process_pid = status.pid;
+    container.init_process_start_time = status.process_start_time;
+    container.created = status.created.into();
+    Ok(container)
+}
+
 pub fn get_config_path<P: AsRef<Path>>(bundle: P) -> PathBuf {
     bundle.as_ref().join(CONFIG_FILE_NAME)
 }

src/tools/runk/libcontainer/src/created_builder.rs

@@ -0,0 +1,141 @@
+// Copyright 2022 Sony Group Corporation
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+use crate::container::{load_linux_container, Container, ContainerLauncher};
+use anyhow::{anyhow, Result};
+use derive_builder::Builder;
+use oci::ContainerState;
+use slog::{debug, Logger};
+use std::path::PathBuf;
+
+/// Used for start command. It will prepare the options used for starting a new container.
+#[derive(Default, Builder, Debug, Clone)]
+#[builder(build_fn(validate = "Self::validate"))]
+pub struct CreatedContainer {
+    id: String,
+    root: PathBuf,
+}
+
+impl CreatedContainerBuilder {
+    /// pre-validate before building CreatedContainer
+    fn validate(&self) -> Result<(), String> {
+        // ensure container exists
+        let id = self.id.as_ref().unwrap();
+        let root = self.root.as_ref().unwrap();
+        let path = root.join(id);
+        if !path.as_path().exists() {
+            return Err(format!("container {} does not exist", id));
+        }
+
+        Ok(())
+    }
+}
+
+impl CreatedContainer {
+    /// Create ContainerLauncher that can be used to start a process from an existing init container.
+    /// It reads the spec from status file of the init container.
+    pub fn create_launcher(self, logger: &Logger) -> Result<ContainerLauncher> {
+        debug!(logger, "enter CreatedContainer::create_launcher {:?}", self);
+        let container = Container::load(&self.root, &self.id)?;
+
+        if container.state != ContainerState::Created {
+            return Err(anyhow!(
+                "cannot start a container in the {:?} state",
+                container.state
+            ));
+        }
+
+        let config = container.status.config.clone();
+
+        debug!(
+            logger,
+            "Prepare LinuxContainer for starting with config: {:?}", config
+        );
+        let runner = load_linux_container(&container.status, None, logger)?;
+
+        Ok(ContainerLauncher::new(
+            &self.id,
+            &container.status.bundle,
+            &self.root,
+            true,
+            runner,
+            None,
+        ))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::status::Status;
+    use crate::utils::test_utils::*;
+    use nix::sys::stat::Mode;
+    use nix::unistd::{self, getpid};
+    use rustjail::container::EXEC_FIFO_FILENAME;
+    use scopeguard::defer;
+    use slog::o;
+    use std::fs::create_dir_all;
+    use std::path::Path;
+    use tempfile::tempdir;
+    use test_utils::skip_if_not_root;
+
+    fn create_created_container_dirs(root: &Path, id: &str, bundle: &Path) {
+        Status::create_dir(root, id).unwrap();
+        let fifo = root.join(id).join(EXEC_FIFO_FILENAME);
+        unistd::mkfifo(&fifo, Mode::from_bits(0o644).unwrap()).unwrap();
+        create_dir_all(bundle.join(TEST_ROOTFS_PATH)).unwrap();
+    }
+
+    #[test]
+    fn test_created_container_validate() {
+        let root = tempdir().unwrap();
+        let id = TEST_CONTAINER_ID.to_string();
+        let result = CreatedContainerBuilder::default()
+            .id(id)
+            .root(root.path().to_path_buf())
+            .build();
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_created_container_create_launcher() {
+        // create cgroup directory needs root permission
+        skip_if_not_root!();
+        let logger = slog::Logger::root(slog::Discard, o!());
+        let bundle_dir = tempdir().unwrap();
+        let root = tempdir().unwrap();
+        // Since tests are executed concurrently, container_id must be unique in tests with cgroup.
+        // Or the cgroup directory may be removed by other tests in advance.
+        let id = "test_created_container_create".to_string();
+        create_created_container_dirs(root.path(), &id, bundle_dir.path());
+        let pid = getpid().as_raw();
+
+        let mut spec = create_dummy_spec();
+        spec.root.as_mut().unwrap().path = bundle_dir
+            .path()
+            .join(TEST_ROOTFS_PATH)
+            .to_string_lossy()
+            .to_string();
+
+        let status = create_custom_dummy_status(&id, pid, root.path(), &spec);
+        status.save().unwrap();
+
+        // create empty cgroup directory to avoid is_pause failing
+        let cgroup = create_dummy_cgroup(Path::new(id.as_str()));
+        defer!(cgroup.delete().unwrap());
+
+        let launcher = CreatedContainerBuilder::default()
+            .id(id.clone())
+            .root(root.into_path())
+            .build()
+            .unwrap()
+            .create_launcher(&logger)
+            .unwrap();
+
+        assert!(launcher.init);
+        assert_eq!(launcher.runner.config.spec.unwrap(), spec);
+        assert_eq!(launcher.runner.id, id);
+    }
+}

src/tools/runk/libcontainer/src/lib.rs

@@ -6,6 +6,7 @@
 pub mod activated_builder;
 pub mod cgroup;
 pub mod container;
+pub mod created_builder;
 pub mod init_builder;
 pub mod status;
 pub mod utils;

src/tools/runk/src/commands/start.rs

@@ -3,34 +3,20 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 
-use crate::commands::state::get_container_state_name;
-use anyhow::{anyhow, Result};
-use libcontainer::container::{get_fifo_path, Container};
+use anyhow::Result;
+use libcontainer::{container::ContainerAction, created_builder::CreatedContainerBuilder};
 use liboci_cli::Start;
-use nix::unistd::unlink;
-use oci::ContainerState;
 use slog::{info, Logger};
-use std::{fs::OpenOptions, io::prelude::*, path::Path};
+use std::path::Path;
 
-pub fn run(opts: Start, state_root: &Path, logger: &Logger) -> Result<()> {
-    let container = Container::load(state_root, &opts.container_id)?;
-    if container.state != ContainerState::Created {
-        return Err(anyhow!(
-            "cannot start a container in the {} state",
-            get_container_state_name(container.state)
-        ));
-    };
+pub async fn run(opts: Start, root: &Path, logger: &Logger) -> Result<()> {
+    let mut launcher = CreatedContainerBuilder::default()
+        .id(opts.container_id)
+        .root(root.to_path_buf())
+        .build()?
+        .create_launcher(logger)?;
 
-    let fifo_path = get_fifo_path(&container.status);
-    let mut file = OpenOptions::new().write(true).open(&fifo_path)?;
-
-    file.write_all("0".as_bytes())?;
-
-    info!(&logger, "container started");
-
-    if fifo_path.exists() {
-        unlink(&fifo_path)?;
-    }
+    launcher.launch(ContainerAction::Start, logger).await?;
 
     info!(&logger, "start command finished successfully");
 

src/tools/runk/src/main.rs

@@ -72,7 +72,7 @@ async fn cmd_run(subcmd: SubCommand, root_path: &Path, logger: &Logger) -> Resul
     match subcmd {
         SubCommand::Standard(cmd) => match cmd {
             StandardCmd::Create(create) => commands::create::run(create, root_path, logger).await,
-            StandardCmd::Start(start) => commands::start::run(start, root_path, logger),
+            StandardCmd::Start(start) => commands::start::run(start, root_path, logger).await,
             StandardCmd::Delete(delete) => commands::delete::run(delete, root_path, logger).await,
             StandardCmd::State(state) => commands::state::run(state, root_path, logger),
         },