Skip to content

Commit

Permalink
fix: Refactor error propagation
Browse files Browse the repository at this point in the history
Refactors error propagation to avoid logging and printing misleading
error messages.

Signed-off-by: Jonathan Woollett-Light <jcawl@amazon.co.uk>
  • Loading branch information
Jonathan Woollett-Light authored and JonathanWoollett-Light committed Oct 20, 2023
1 parent bd55f7f commit 3806ded
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 9 additions & 4 deletions src/firecracker/src/api_server_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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),
}
}
8 changes: 6 additions & 2 deletions src/firecracker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ fn main() -> ExitCode {
eprintln!("Error: {err:?}");
ExitCode::from(err)
} else {
info!("Firecracker exited successfully");
ExitCode::SUCCESS
}
}
Expand Down Expand Up @@ -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(())
}
4 changes: 1 addition & 3 deletions tests/integration_tests/functional/test_cmd_line_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 3806ded

Please sign in to comment.