return run and cleanup both errors on create failed (#2998)

* fix(libcontainer): combine multi error when create container failed

When container creation fails and triggers a cleanup process that also
fails, the original creation error is overwritten by the cleanup error.
This change ensures that both the primary creation error and the
subsequent cleanup error are captured and reported, providing better
visibility into the failure sequence.

Signed-off-by: xujihui1985 <[email protected]>

* fix: fix lint error

Signed-off-by: xujihui1985 <[email protected]>

* refactor(libcontainer): add CreateContainerError

add CreateContainerError to encapsulate both create error and cleanup
error, so that caller will understand what happened

Signed-off-by: xujihui1985 <[email protected]>

* refactor(libcontainer): use tuple instead of struct

change createContainerError to tuple instead of struct

Signed-off-by: xujihui1985 <[email protected]>

---------

Signed-off-by: xujihui1985 <[email protected]>

crates/libcontainer/src/container/builder_impl.rs

@@ -10,7 +10,7 @@ use nix::unistd::Pid;
 use oci_spec::runtime::Spec;
 
 use super::{Container, ContainerStatus};
-use crate::error::{LibcontainerError, MissingSpecError};
+use crate::error::{CreateContainerError, LibcontainerError, MissingSpecError};
 use crate::notify_socket::NotifyListener;
 use crate::process::args::{ContainerArgs, ContainerType};
 use crate::process::intel_rdt::delete_resctrl_subdirectory;
@@ -67,15 +67,21 @@ impl ContainerBuilderImpl {
             Err(outer) => {
                 // Only the init container should be cleaned up in the case of
                 // an error.
-                if matches!(self.container_type, ContainerType::InitContainer) {
-                    self.cleanup_container()?;
-                }
+                let cleanup_err = if self.is_init_container() {
+                    self.cleanup_container().err()
+                } else {
+                    None
+                };
 
-                Err(outer)
+                Err(CreateContainerError::new(outer, cleanup_err).into())
             }
         }
     }
 
+    fn is_init_container(&self) -> bool {
+        matches!(self.container_type, ContainerType::InitContainer)
+    }
+
     fn run_container(&mut self) -> Result<Pid, LibcontainerError> {
         let linux = self.spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
         let cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.container_id);

crates/libcontainer/src/error.rs

@@ -62,6 +62,8 @@ pub enum LibcontainerError {
     CgroupGet(#[from] libcgroups::common::GetCgroupSetupError),
     #[error[transparent]]
     Checkpoint(#[from] crate::container::CheckpointError),
+    #[error[transparent]]
+    CreateContainerError(#[from] CreateContainerError),
 
     // Catch all errors that are not covered by the above
     #[error("syscall error")]
@@ -97,3 +99,54 @@ pub enum ErrInvalidSpec {
     #[error("invalid scheduler config for process")]
     Scheduler,
 }
+
+#[derive(Debug, thiserror::Error)]
+pub struct CreateContainerError(Box<LibcontainerError>, Option<Box<LibcontainerError>>);
+
+impl CreateContainerError {
+    pub(crate) fn new(
+        run_error: LibcontainerError,
+        cleanup_error: Option<LibcontainerError>,
+    ) -> Self {
+        Self(Box::new(run_error), cleanup_error.map(Box::new))
+    }
+}
+
+impl std::fmt::Display for CreateContainerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "failed to create container: {}", self.0)?;
+        if let Some(cleanup_err) = &self.1 {
+            write!(f, ". error during cleanup: {}", cleanup_err)?;
+        }
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use libcgroups::common::CreateCgroupSetupError;
+
+    use super::{CreateContainerError, ErrInvalidID};
+
+    #[test]
+    fn test_create_container() {
+        let create_container_err =
+            CreateContainerError::new(CreateCgroupSetupError::NonDefault.into(), None);
+        let msg = format!("{}", create_container_err);
+        assert_eq!(
+            "failed to create container: non default cgroup root not supported",
+            msg
+        );
+
+        let create_container_err = CreateContainerError::new(
+            CreateCgroupSetupError::NonDefault.into(),
+            Some(ErrInvalidID::Empty.into()),
+        );
+        let msg = format!("{}", create_container_err);
+        assert_eq!(
+            "failed to create container: non default cgroup root not supported. \
+         error during cleanup: container id can't be empty",
+            msg
+        );
+    }
+}