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]>

crates/libcontainer/src/container/builder_impl.rs

@@ -9,7 +9,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;
@@ -60,18 +60,21 @@ impl ContainerBuilderImpl {
             Err(outer) => {
                 // Only the init container should be cleaned up in the case of
                 // an error.
-                let mut errors = vec![outer];
-                if matches!(self.container_type, ContainerType::InitContainer) {
-                    if let Err(e) = self.cleanup_container() {
-                        errors.push(e);
-                    }
-                }
+                let cleanup_err = if self.is_init_container() {
+                    self.cleanup_container().err()
+                } else {
+                    None
+                };
 
-                Err(LibcontainerError::MultiError(errors.into()))
+                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

@@ -63,7 +63,7 @@ pub enum LibcontainerError {
     #[error[transparent]]
     Checkpoint(#[from] crate::container::CheckpointError),
     #[error[transparent]]
-    MultiError(#[from] MultiError),
+    CreateContainerError(#[from] CreateContainerError),
 
     // Catch all errors that are not covered by the above
     #[error("syscall error")]
@@ -101,58 +101,59 @@ pub enum ErrInvalidSpec {
 }
 
 #[derive(Debug, thiserror::Error)]
-pub struct MultiError(Vec<LibcontainerError>);
+pub struct CreateContainerError {
+    #[source]
+    run_error: Box<LibcontainerError>,
+    cleanup_error: Option<Box<LibcontainerError>>,
+}
 
-impl std::fmt::Display for MultiError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self.0.len() {
-            0 => write!(f, ""),
-            1 => std::fmt::Display::fmt(&self.0[0], f),
-            _ => {
-                writeln!(f, "multiple errors occurred while processing the request:")?;
-                for (index, error) in self.0.iter().enumerate() {
-                    writeln!(f, "{}: {}", index + 1, error)?;
-                }
-                Ok(())
-            }
+impl CreateContainerError {
+    pub(crate) fn new(
+        run_error: LibcontainerError,
+        cleanup_error: Option<LibcontainerError>,
+    ) -> Self {
+        Self {
+            run_error: Box::new(run_error),
+            cleanup_error: cleanup_error.map(Box::new),
         }
     }
 }
 
-impl From<Vec<LibcontainerError>> for MultiError {
-    fn from(value: Vec<LibcontainerError>) -> Self {
-        Self(value)
+impl std::fmt::Display for CreateContainerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "failed to create container: {}", self.run_error)?;
+        if let Some(cleanup_err) = &self.cleanup_error {
+            write!(f, ". error during cleanup: {}", cleanup_err)?;
+        }
+        Ok(())
     }
 }
 
 #[cfg(test)]
 mod tests {
     use libcgroups::common::CreateCgroupSetupError;
 
-    use super::{LibcontainerError, MultiError};
+    use super::{CreateContainerError, ErrInvalidID};
 
     #[test]
-    fn test_multi_error() {
-        let errs = Vec::<LibcontainerError>::new();
-        let multi_err: MultiError = errs.into();
-        let msg = format!("{}", multi_err);
-        assert_eq!("", msg);
-
-        let err1 = LibcontainerError::CgroupCreate(CreateCgroupSetupError::NonDefault);
-        let errs = vec![err1];
-        let multi_err: MultiError = errs.into();
-        let msg = format!("{:#}", multi_err);
-        assert_eq!("non default cgroup root not supported", msg);
+    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 err1 = LibcontainerError::CgroupCreate(CreateCgroupSetupError::NonDefault);
-        let err2 = LibcontainerError::InvalidID(super::ErrInvalidID::Empty);
-        let errs = vec![err1, err2];
-        let multi_err: MultiError = errs.into();
-        let msg = format!("{:#}", multi_err);
-        let expect = r#"multiple errors occurred while processing the request:
-1: non default cgroup root not supported
-2: container id can't be empty
-"#;
-        assert_eq!(expect, 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
+        );
     }
 }