Incorporate review feedback

crates/libcontainer/src/process/container_init_process.rs

@@ -1,7 +1,7 @@
 use super::args::ContainerArgs;
 use crate::apparmor;
 use crate::syscall::Syscall;
-use crate::workload::{CompositeExecutor, Executor};
+use crate::workload::ExecutorManager;
 use crate::{
     capabilities, hooks, namespaces::Namespaces, process::channel, rootfs::RootFS,
     rootless::Rootless, seccomp, tty, utils,
@@ -415,7 +415,7 @@ pub fn container_init_process(
     }
 
     if proc.args().is_some() {
-        CompositeExecutor::default().exec(spec)
+        ExecutorManager::exec(spec)
     } else {
         bail!("on non-Windows, at least one process arg entry is required")
     }

crates/libcontainer/src/workload/default.rs

@@ -6,10 +6,12 @@ use oci_spec::runtime::Spec;
 
 use super::{Executor, EMPTY};
 
+const EXECUTOR_NAME: &str = "default";
+
 pub struct DefaultExecutor {}
 
 impl Executor for DefaultExecutor {
-    fn exec(&self, spec: &Spec) -> Result<()> {
+    fn exec(spec: &Spec) -> Result<()> {
         log::debug!("Executing workload with default handler");
         let args = spec
             .process()
@@ -35,11 +37,11 @@ impl Executor for DefaultExecutor {
         unreachable!();
     }
 
-    fn can_handle(&self, _: &Spec) -> Result<bool> {
+    fn can_handle(_: &Spec) -> Result<bool> {
         Ok(true)
     }
 
-    fn name(&self) -> &str {
-        "default"
+    fn name() -> &'static str {
+        EXECUTOR_NAME
     }
 }

crates/libcontainer/src/workload/mod.rs

@@ -13,65 +13,21 @@ static EMPTY: Vec<String> = Vec::new();
 
 pub trait Executor {
     /// Executes the workload
-    fn exec(&self, spec: &Spec) -> Result<()>;
+    fn exec(spec: &Spec) -> Result<()>;
     /// Checks if the handler is able to handle the workload
-    fn can_handle(&self, spec: &Spec) -> Result<bool>;
+    fn can_handle(spec: &Spec) -> Result<bool>;
     /// The name of the handler
-    fn name(&self) -> &str;
+    fn name() -> &'static str;
 }
-pub struct CompositeExecutor {
-    executors: Vec<Box<dyn Executor>>,
-}
-
-impl Executor for CompositeExecutor {
-    fn exec(&self, spec: &Spec) -> Result<()> {
-        for executor in &self.executors {
-            if executor
-                .can_handle(spec)
-                .with_context(|| format!("executor {} failed on selection", executor.name()))?
-            {
-                let result = executor.exec(spec);
-                if result.is_err() {
-                    let error_msg = if executor.name() == "default" {
-                        "executor default failed on exec. This might have been caused \
-                            by another handler not being able to match on your request"
-                            .to_string()
-                    } else {
-                        format!("executor {} failed on exec", executor.name())
-                    };
+pub struct ExecutorManager {}
 
-                    return result.context(error_msg);
-                } else {
-                    return Ok(());
-                }
-            }
+impl ExecutorManager {
+    pub fn exec(spec: &Spec) -> Result<()> {
+        #[cfg(feature = "wasm-wasmer")]
+        if WasmerExecutor::can_handle(spec)? {
+            return WasmerExecutor::exec(&spec).context("wasmer execution failed");
         }
 
-        unreachable!("no suitable execution handler has been registered");
-    }
-
-    fn can_handle(&self, spec: &Spec) -> Result<bool> {
-        Ok(self
-            .executors
-            .iter()
-            .any(|h| h.can_handle(spec).unwrap_or_default()))
-    }
-
-    fn name(&self) -> &str {
-        "composite"
-    }
-}
-
-impl Default for CompositeExecutor {
-    fn default() -> Self {
-        let handlers: Vec<Box<dyn Executor>> = vec![
-            #[cfg(feature = "wasm-wasmer")]
-            Box::new(WasmerExecutor {}),
-            Box::new(DefaultExecutor {}),
-        ];
-
-        Self {
-            executors: handlers,
-        }
+        DefaultExecutor::exec(spec).context("default execution failed")
     }
 }

crates/libcontainer/src/workload/wasmer.rs

@@ -5,10 +5,12 @@ use wasmer_wasi::WasiState;
 
 use super::{Executor, EMPTY};
 
+const EXECUTOR_NAME: &str = "wasmer";
+
 pub struct WasmerExecutor {}
 
 impl Executor for WasmerExecutor {
-    fn exec(&self, spec: &Spec) -> Result<()> {
+    fn exec(spec: &Spec) -> Result<()> {
         log::debug!("Executing workload with wasmer handler");
         let process = spec.process().as_ref();
 
@@ -59,7 +61,7 @@ impl Executor for WasmerExecutor {
         Ok(())
     }
 
-    fn can_handle(&self, spec: &Spec) -> Result<bool> {
+    fn can_handle(spec: &Spec) -> Result<bool> {
         if let Some(annotations) = spec.annotations() {
             if let Some(handler) = annotations.get("run.oci.handler") {
                 return Ok(handler == "wasm");
@@ -73,8 +75,8 @@ impl Executor for WasmerExecutor {
         Ok(false)
     }
 
-    fn name(&self) -> &str {
-        "wasmer"
+    fn name() -> &'static str {
+        EXECUTOR_NAME
     }
 }
 
@@ -93,8 +95,7 @@ mod tests {
             .build()
             .context("build spec")?;
 
-        let handler = WasmerExecutor {};
-        assert!(handler.can_handle(&spec).context("can handle")?);
+        assert!(WasmerExecutor::can_handle(&spec).context("can handle")?);
 
         Ok(())
     }
@@ -108,8 +109,7 @@ mod tests {
             .build()
             .context("build spec")?;
 
-        let handler = WasmerExecutor {};
-        assert!(handler.can_handle(&spec).context("can handle")?);
+        assert!(WasmerExecutor::can_handle(&spec).context("can handle")?);
 
         Ok(())
     }
@@ -118,8 +118,7 @@ mod tests {
     fn test_can_handle_no_execute() -> Result<()> {
         let spec = SpecBuilder::default().build().context("build spec")?;
 
-        let handler = WasmerExecutor {};
-        assert!(!handler.can_handle(&spec).context("can handle")?);
+        assert!(!WasmerExecutor::can_handle(&spec).context("can handle")?);
 
         Ok(())
     }