From 3806ded421abdf372382b83d6d98b1435d9b6ba3 Mon Sep 17 00:00:00 2001
From: Jonathan Woollett-Light <jcawl@amazon.co.uk>
Date: Thu, 19 Oct 2023 15:52:34 +0100
Subject: [PATCH] fix: Refactor error propagation

Refactors error propagation to avoid logging and printing misleading
error messages.

Signed-off-by: Jonathan Woollett-Light <jcawl@amazon.co.uk>
---
 CHANGELOG.md                                        |  4 ++++
 src/firecracker/src/api_server_adapter.rs           | 13 +++++++++----
 src/firecracker/src/main.rs                         |  8 ++++++--
 .../functional/test_cmd_line_start.py               |  4 +---
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0445db3e392..15ee1e04e09 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,10 @@
 - Simplified and clarified the removal policy of deprecated API elements
   to follow semantic versioning 2.0.0. For more information, please refer to
   [this GitHub discussion](https://github.com/firecracker-microvm/firecracker/discussions/4135).
+- [#4180](https://github.com/firecracker-microvm/firecracker/pull/4180):
+  Refactored error propagation to avoid logging and printing an error on
+  exits with a zero exit code. Now, on successful exit
+  "Firecracker exited successfully" is logged.
 
 ### Deprecated
 
diff --git a/src/firecracker/src/api_server_adapter.rs b/src/firecracker/src/api_server_adapter.rs
index 7a532463b86..b2cad171f3c 100644
--- a/src/firecracker/src/api_server_adapter.rs
+++ b/src/firecracker/src/api_server_adapter.rs
@@ -52,7 +52,7 @@ impl ApiServerAdapter {
         vm_resources: VmResources,
         vmm: Arc<Mutex<Vmm>>,
         event_manager: &mut EventManager,
-    ) -> FcExitCode {
+    ) -> Result<(), FcExitCode> {
         let api_adapter = Arc::new(Mutex::new(Self {
             api_event_fd,
             from_api,
@@ -64,10 +64,14 @@ impl ApiServerAdapter {
             event_manager
                 .run()
                 .expect("EventManager events driver fatal error");
-            if let Some(exit_code) = vmm.lock().unwrap().shutdown_exit_code() {
-                return exit_code;
+
+            match vmm.lock().unwrap().shutdown_exit_code() {
+                Some(FcExitCode::Ok) => break,
+                Some(exit_code) => return Err(exit_code),
+                None => continue,
             }
         }
+        Ok(())
     }
 
     fn handle_request(&mut self, req_action: VmmAction) {
@@ -245,7 +249,8 @@ pub(crate) fn run_with_api(
     api_thread.join().expect("Api thread should join");
 
     match result {
-        Ok(exit_code) => Err(ApiServerError::MicroVMStoppedWithoutError(exit_code)),
+        Ok(Ok(())) => Ok(()),
+        Ok(Err(exit_code)) => Err(ApiServerError::MicroVMStoppedWithoutError(exit_code)),
         Err(exit_error) => Err(exit_error),
     }
 }
diff --git a/src/firecracker/src/main.rs b/src/firecracker/src/main.rs
index af8c92ee7f0..a1f63d480bd 100644
--- a/src/firecracker/src/main.rs
+++ b/src/firecracker/src/main.rs
@@ -98,6 +98,7 @@ fn main() -> ExitCode {
         eprintln!("Error: {err:?}");
         ExitCode::from(err)
     } else {
+        info!("Firecracker exited successfully");
         ExitCode::SUCCESS
     }
 }
@@ -631,8 +632,11 @@ fn run_without_api(
             .run()
             .expect("Failed to start the event manager");
 
-        if let Some(exit_code) = vmm.lock().unwrap().shutdown_exit_code() {
-            return Err(RunWithoutApiError::Shutdown(exit_code));
+        match vmm.lock().unwrap().shutdown_exit_code() {
+            Some(FcExitCode::Ok) => break,
+            Some(exit_code) => return Err(RunWithoutApiError::Shutdown(exit_code)),
+            None => continue,
         }
     }
+    Ok(())
 }
diff --git a/tests/integration_tests/functional/test_cmd_line_start.py b/tests/integration_tests/functional/test_cmd_line_start.py
index b81bab4631c..8250d441710 100644
--- a/tests/integration_tests/functional/test_cmd_line_start.py
+++ b/tests/integration_tests/functional/test_cmd_line_start.py
@@ -170,9 +170,7 @@ def test_config_start_no_api_exit(uvm_plain, vm_config_file):
     time.sleep(3)  # Wait for shutdown
 
     # Check error log
-    test_microvm.check_log_message(
-        "RunWithoutApiError error: MicroVMStopped without an error: Ok"
-    )
+    test_microvm.check_log_message("Firecracker exited successfully")
 
 
 @pytest.mark.parametrize(