Merge pull request #112 from Synicix/logging_patch

Added logging to PodResult

Author: eywalker

src/core/error.rs

@@ -122,6 +122,7 @@ impl fmt::Debug for OrcaError {
         match &self.kind {
             Kind::AgentCommunicationFailure { backtrace, .. }
             | Kind::FailedToStartPod { backtrace, .. }
+            | Kind::FailedToExtractRunInfo { backtrace, .. }
             | Kind::IncompletePacket { backtrace, .. }
             | Kind::InvalidPath { backtrace, .. }
             | Kind::InvalidIndex { backtrace, .. }

src/uniffi/error.rs

@@ -31,6 +31,13 @@ pub(crate) enum Kind {
         source: Box<dyn Error + Send + Sync>,
         backtrace: Option<Backtrace>,
     },
+    #[snafu(display(
+        "Failed to extract run info from the container image file: {container_name}."
+    ))]
+    FailedToExtractRunInfo {
+        container_name: String,
+        backtrace: Option<Backtrace>,
+    },
     #[snafu(display(
         "Missing expected output file or dir with key {packet_key} at path {path:?} for pod job (hash: {pod_job_hash})."
     ))]

src/uniffi/model/pod.rs

@@ -204,6 +204,8 @@ pub struct PodResult {
     pub created: u64,
     /// Time in epoch when terminated in seconds.
     pub terminated: u64,
+    /// Logs about stdout and stderr, where stderr is append at the end
+    pub logs: String,
 }
 
 impl PodResult {
@@ -220,6 +222,7 @@ impl PodResult {
         created: u64,
         terminated: u64,
         namespace_lookup: &HashMap<String, PathBuf>,
+        logs: String,
     ) -> Result<Self> {
         let output_packet = pod_job
             .pod
@@ -276,6 +279,7 @@ impl PodResult {
             status,
             created,
             terminated,
+            logs,
         };
         Ok(Self {
             hash: hash_buffer(to_yaml(&pod_result_no_hash)?),

src/uniffi/orchestrator/docker.rs

@@ -12,7 +12,9 @@ use crate::{
 use async_trait;
 use bollard::{
     Docker,
-    container::{RemoveContainerOptions, StartContainerOptions, WaitContainerOptions},
+    container::{
+        LogOutput, LogsOptions, RemoveContainerOptions, StartContainerOptions, WaitContainerOptions,
+    },
     errors::Error::DockerContainerWaitError,
     image::{CreateImageOptions, ImportImageOptions},
 };
@@ -70,6 +72,9 @@ impl Orchestrator for LocalDockerOrchestrator {
     ) -> Result<PodResult> {
         ASYNC_RUNTIME.block_on(self.get_result(pod_run, namespace_lookup))
     }
+    fn get_logs_blocking(&self, pod_run: &PodRun) -> Result<String> {
+        ASYNC_RUNTIME.block_on(self.get_logs(pod_run))
+    }
     #[expect(
         clippy::try_err,
         reason = r#"
@@ -263,8 +268,65 @@ impl Orchestrator for LocalDockerOrchestrator {
                 ),
             })?,
             namespace_lookup,
+            self.get_logs(pod_run).await?,
         )
     }
+
+    async fn get_logs(&self, pod_run: &PodRun) -> Result<String> {
+        let mut std_out = Vec::new();
+        let mut std_err = Vec::new();
+
+        self.api
+            .logs::<String>(
+                &pod_run.assigned_name,
+                Some(LogsOptions {
+                    stdout: true,
+                    stderr: true,
+                    ..Default::default()
+                }),
+            )
+            .try_collect::<Vec<_>>()
+            .await?
+            .iter()
+            .for_each(|log_output| match log_output {
+                LogOutput::StdOut { message } => {
+                    std_out.extend(message.to_vec());
+                }
+                LogOutput::StdErr { message } => {
+                    std_err.extend(message.to_vec());
+                }
+                LogOutput::StdIn { .. } | LogOutput::Console { .. } => {
+                    // Ignore stdin logs, as they are not relevant for our use case
+                }
+            });
+
+        let mut logs = String::from_utf8_lossy(&std_out).to_string();
+        if !std_err.is_empty() {
+            logs.push_str("\nSTDERR:\n");
+            logs.push_str(&String::from_utf8_lossy(&std_err));
+        }
+
+        // Check for errors in the docker state, if exist, attach it to logs
+        // This is for when the container exits immediately due to a bad command or similar
+        let error = self
+            .api
+            .inspect_container(&pod_run.assigned_name, None)
+            .await?
+            .state
+            .context(selector::FailedToExtractRunInfo {
+                container_name: &pod_run.assigned_name,
+            })?
+            .error
+            .context(selector::FailedToExtractRunInfo {
+                container_name: &pod_run.assigned_name,
+            })?;
+
+        if !error.is_empty() {
+            logs.push_str(&error);
+        }
+
+        Ok(logs)
+    }
 }
 
 #[uniffi::export]

src/uniffi/orchestrator/mod.rs

@@ -121,6 +121,10 @@ pub trait Orchestrator: Send + Sync + fmt::Debug {
         pod_run: &PodRun,
         namespace_lookup: &HashMap<String, PathBuf>,
     ) -> Result<PodResult>;
+    /// Get the logs for a specific pod run.
+    /// # Errors
+    /// Will return `Err` if there is an issue getting logs.
+    fn get_logs_blocking(&self, pod_run: &PodRun) -> Result<String>;
     /// How to asynchronously start containers with an alternate image.
     ///
     /// # Errors
@@ -170,6 +174,9 @@ pub trait Orchestrator: Send + Sync + fmt::Debug {
         pod_run: &PodRun,
         namespace_lookup: &HashMap<String, PathBuf>,
     ) -> Result<PodResult>;
+
+    /// Get the logs for a specific pod run.
+    async fn get_logs(&self, pod_run: &PodRun) -> Result<String>;
 }
 /// Orchestration execution agent daemon and client.
 pub mod agent;

tests/fixture/mod.rs

@@ -157,6 +157,7 @@ pub fn pod_result_style(
         1_737_922_307,
         1_737_925_907,
         namespace_lookup,
+        "Example logs".to_owned(),
     )
 }
 

tests/model.rs

@@ -104,7 +104,7 @@ fn pod_job_to_yaml() -> Result<()> {
 fn hash_pod_result() -> Result<()> {
     pretty_assert_eq!(
         pod_result_style(&NAMESPACE_LOOKUP_READ_ONLY)?.hash,
-        "e752d86d4fc5435bfa4564ba951851530f5cf2228586c2f488c2ca9e7bcc7ed1",
+        "0bc0f17230cf78dbf3054c6887f42abad8b48382efee2ffe1adc79f0ae02fd1a",
         "Hash didn't match."
     );
     Ok(())
@@ -134,6 +134,7 @@ fn pod_result_to_yaml() -> Result<()> {
             status: Completed
             created: 1737922307
             terminated: 1737925907
+            logs: Example logs
         "},
         "YAML serialization didn't match."
     );

tests/orchestrator.rs

@@ -295,3 +295,44 @@ async fn verify_pod_result_not_running() -> Result<()> {
     );
     Ok(())
 }
+
+#[test]
+fn logs() -> Result<()> {
+    execute_wrapper(|orchestrator, namespace_lookup| {
+        let pod_job = pod_job_custom(
+            pod_custom(
+                "alpine:3.14",
+                vec!["bin/sh".into(), "-c".into(), "echo \"hi\"".into()],
+                HashMap::new(),
+            )?,
+            HashMap::new(),
+            namespace_lookup,
+        )?;
+
+        let pod_run = orchestrator.start_blocking(&pod_job, namespace_lookup)?;
+        let pod_result = orchestrator.get_result_blocking(&pod_run, namespace_lookup)?;
+
+        assert_eq!(
+            pod_result.status,
+            PodStatus::Completed,
+            "Pod status is not completed"
+        );
+
+        assert_eq!(orchestrator.get_logs_blocking(&pod_run)?, "hi\n");
+        assert_eq!(
+            orchestrator
+                .get_result_blocking(&pod_run, namespace_lookup)?
+                .logs,
+            "hi\n"
+        );
+
+        orchestrator.delete_blocking(&pod_run)?;
+
+        assert!(
+            !orchestrator.list_blocking()?.contains(&pod_run),
+            "Unexpected container remains."
+        );
+
+        Ok(())
+    })
+}