From 3d5095c78be9ea10e3bb297d33399ef6599307ac Mon Sep 17 00:00:00 2001 From: Alex Zenla Date: Sat, 23 Mar 2024 07:00:12 +0000 Subject: [PATCH] krata: reconcile improvements and better kratactl error experience --- crates/krata/proto/krata/common.proto | 15 +++--- crates/kratactl/src/cli/launch.rs | 7 ++- crates/kratactl/src/cli/list.rs | 23 ++++----- crates/kratactl/src/cli/pretty.rs | 12 +++-- crates/kratactl/src/cli/watch.rs | 69 ++++++++++++++++++++++----- crates/kratactl/src/console.rs | 2 +- crates/kratactl/src/format.rs | 7 +++ crates/kratad/src/control.rs | 6 +-- crates/kratad/src/event.rs | 1 + crates/kratad/src/reconcile/guest.rs | 32 ++++++------- crates/kratart/src/image/registry.rs | 2 +- hack/os/build.sh | 5 +- 12 files changed, 119 insertions(+), 62 deletions(-) diff --git a/crates/krata/proto/krata/common.proto b/crates/krata/proto/krata/common.proto index bba0601..cbf6e69 100644 --- a/crates/krata/proto/krata/common.proto +++ b/crates/krata/proto/krata/common.proto @@ -40,22 +40,23 @@ message GuestErrorInfo { enum GuestStatus { GUEST_STATUS_UNKNOWN = 0; - GUEST_STATUS_START = 1; + GUEST_STATUS_STARTING = 1; GUEST_STATUS_STARTED = 2; GUEST_STATUS_EXITED = 3; - GUEST_STATUS_DESTROY = 4; + GUEST_STATUS_DESTROYING = 4; GUEST_STATUS_DESTROYED = 5; + GUEST_STATUS_FAILED = 6; } message GuestState { GuestStatus status = 1; - GuestExitInfo exit_info = 2; - GuestErrorInfo error_info = 3; + GuestNetworkState network = 2; + GuestExitInfo exit_info = 3; + GuestErrorInfo error_info = 4; } message Guest { string id = 1; - GuestState state = 2; - GuestSpec spec = 3; - GuestNetworkState network = 4; + GuestSpec spec = 2; + GuestState state = 3; } diff --git a/crates/kratactl/src/cli/launch.rs b/crates/kratactl/src/cli/launch.rs index 30e34d7..71ced84 100644 --- a/crates/kratactl/src/cli/launch.rs +++ b/crates/kratactl/src/cli/launch.rs @@ -94,7 +94,12 @@ async fn wait_guest_started(id: &str, events: EventStream) -> Result<()> { }; if let Some(ref error) = state.error_info { - error!("guest error: {}", error.message); + if state.status() == GuestStatus::Failed { + error!("launch failed: {}", error.message); + std::process::exit(1); + } else { + error!("guest error: {}", error.message); + } } if state.status() == GuestStatus::Destroyed { diff --git a/crates/kratactl/src/cli/list.rs b/crates/kratactl/src/cli/list.rs index 676196e..74d90ab 100644 --- a/crates/kratactl/src/cli/list.rs +++ b/crates/kratactl/src/cli/list.rs @@ -11,14 +11,13 @@ use tonic::{transport::Channel, Request}; use crate::{ events::EventStream, - format::{proto2dynamic, proto2kv}, + format::{kv2line, proto2dynamic, proto2kv}, }; use super::pretty::guest_state_text; -#[derive(ValueEnum, Clone, Default, Debug, PartialEq, Eq)] +#[derive(ValueEnum, Clone, Debug, PartialEq, Eq)] enum ListFormat { - #[default] CliTable, Json, JsonPretty, @@ -29,7 +28,7 @@ enum ListFormat { #[derive(Parser)] pub struct ListCommand { - #[arg(short, long)] + #[arg(short, long, default_value = "cli-table")] format: ListFormat, } @@ -87,13 +86,15 @@ impl ListCommand { table.push_row(&header)?; for guest in guests { let ipv4 = guest - .network + .state .as_ref() + .and_then(|x| x.network.as_ref()) .map(|x| x.ipv4.as_str()) .unwrap_or("unknown"); let ipv6 = guest - .network + .state .as_ref() + .and_then(|x| x.network.as_ref()) .map(|x| x.ipv6.as_str()) .unwrap_or("unknown"); let Some(spec) = guest.spec else { @@ -112,7 +113,7 @@ impl ListCommand { table.push_row_string(&vec![ spec.name, guest.id, - format!("{}", guest_state_text(guest.state.unwrap_or_default())), + format!("{}", guest_state_text(guest.state.as_ref())), ipv4.to_string(), ipv6.to_string(), image, @@ -129,13 +130,7 @@ impl ListCommand { fn print_key_value(&self, guests: Vec) -> Result<()> { for guest in guests { let kvs = proto2kv(guest)?; - println!( - "{}", - kvs.iter() - .map(|(k, v)| format!("{}={}", k, v)) - .collect::>() - .join(" ") - ); + println!("{}", kv2line(kvs),); } Ok(()) } diff --git a/crates/kratactl/src/cli/pretty.rs b/crates/kratactl/src/cli/pretty.rs index d7b2f66..3e8b39b 100644 --- a/crates/kratactl/src/cli/pretty.rs +++ b/crates/kratactl/src/cli/pretty.rs @@ -2,17 +2,19 @@ use krata::common::{GuestState, GuestStatus}; pub fn guest_status_text(status: GuestStatus) -> String { match status { - GuestStatus::Destroy => "destroying", - GuestStatus::Destroyed => "destroyed", - GuestStatus::Start => "starting", - GuestStatus::Exited => "exited", + GuestStatus::Starting => "starting", GuestStatus::Started => "started", + GuestStatus::Destroying => "destroying", + GuestStatus::Destroyed => "destroyed", + GuestStatus::Exited => "exited", + GuestStatus::Failed => "failed", _ => "unknown", } .to_string() } -pub fn guest_state_text(state: GuestState) -> String { +pub fn guest_state_text(state: Option<&GuestState>) -> String { + let state = state.cloned().unwrap_or_default(); let mut text = guest_status_text(state.status()); if let Some(exit) = state.exit_info { diff --git a/crates/kratactl/src/cli/watch.rs b/crates/kratactl/src/cli/watch.rs index ec7d569..5e9b04c 100644 --- a/crates/kratactl/src/cli/watch.rs +++ b/crates/kratactl/src/cli/watch.rs @@ -1,11 +1,27 @@ use anyhow::Result; -use clap::Parser; -use krata::control::watch_events_reply::Event; +use clap::{Parser, ValueEnum}; +use krata::{common::Guest, control::watch_events_reply::Event}; +use prost_reflect::ReflectMessage; +use serde_json::Value; -use crate::{cli::pretty::guest_status_text, events::EventStream}; +use crate::{ + cli::pretty::guest_state_text, + events::EventStream, + format::{kv2line, proto2dynamic, proto2kv}, +}; + +#[derive(ValueEnum, Clone, Debug, PartialEq, Eq)] +enum WatchFormat { + Simple, + Json, + KeyValue, +} #[derive(Parser)] -pub struct WatchCommand {} +pub struct WatchCommand { + #[arg(short, long, default_value = "simple")] + format: WatchFormat, +} impl WatchCommand { pub async fn run(self, events: EventStream) -> Result<()> { @@ -14,15 +30,46 @@ impl WatchCommand { let event = stream.recv().await?; match event { Event::GuestChanged(changed) => { - if let Some(guest) = changed.guest { - println!( - "event=guest.changed guest={} status={}", - guest.id, - guest_status_text(guest.state.unwrap_or_default().status()) - ); - } + let guest = changed.guest.clone(); + self.print_event("guest.changed", changed, guest)?; } } } } + + fn print_event( + &self, + typ: &str, + event: impl ReflectMessage, + guest: Option, + ) -> Result<()> { + match self.format { + WatchFormat::Simple => { + if let Some(guest) = guest { + println!( + "{} guest={} status=\"{}\"", + typ, + guest.id, + guest_state_text(guest.state.as_ref()).replace('"', "\\\"") + ); + } + } + + WatchFormat::Json => { + let message = proto2dynamic(event)?; + let mut value = serde_json::to_value(&message)?; + if let Value::Object(ref mut map) = value { + map.insert("event.type".to_string(), Value::String(typ.to_string())); + } + println!("{}", serde_json::to_string(&value)?); + } + + WatchFormat::KeyValue => { + let mut map = proto2kv(event)?; + map.insert("event.type".to_string(), typ.to_string()); + println!("{}", kv2line(map),); + } + } + Ok(()) + } } diff --git a/crates/kratactl/src/console.rs b/crates/kratactl/src/console.rs index ece348b..802782b 100644 --- a/crates/kratactl/src/console.rs +++ b/crates/kratactl/src/console.rs @@ -87,7 +87,7 @@ impl StdioConsoleStream { } let status = state.status(); - if status == GuestStatus::Destroy || status == GuestStatus::Destroyed { + if status == GuestStatus::Destroying || status == GuestStatus::Destroyed { return Some(10); } } diff --git a/crates/kratactl/src/format.rs b/crates/kratactl/src/format.rs index 5fcca42..e01915a 100644 --- a/crates/kratactl/src/format.rs +++ b/crates/kratactl/src/format.rs @@ -49,3 +49,10 @@ pub fn proto2kv(proto: impl ReflectMessage) -> Result> { Ok(map) } + +pub fn kv2line(map: HashMap) -> String { + map.iter() + .map(|(k, v)| format!("{}=\"{}\"", k, v.replace('"', "\\\""))) + .collect::>() + .join(" ") +} diff --git a/crates/kratad/src/control.rs b/crates/kratad/src/control.rs index 5b619cc..2e1cb20 100644 --- a/crates/kratad/src/control.rs +++ b/crates/kratad/src/control.rs @@ -100,12 +100,12 @@ impl ControlService for RuntimeControlService { guest: Some(Guest { id: uuid.to_string(), state: Some(GuestState { - status: GuestStatus::Start.into(), + status: GuestStatus::Starting.into(), + network: None, exit_info: None, error_info: None, }), spec: Some(spec), - network: None, }), }, ) @@ -152,7 +152,7 @@ impl ControlService for RuntimeControlService { .into()); } - guest.state.as_mut().unwrap().status = GuestStatus::Destroy.into(); + guest.state.as_mut().unwrap().status = GuestStatus::Destroying.into(); self.guests .update(uuid, entry) .await diff --git a/crates/kratad/src/event.rs b/crates/kratad/src/event.rs index 90e0f3d..3243620 100644 --- a/crates/kratad/src/event.rs +++ b/crates/kratad/src/event.rs @@ -121,6 +121,7 @@ impl DaemonEventGenerator { guest.state = Some(GuestState { status: GuestStatus::Exited.into(), + network: guest.state.clone().unwrap_or_default().network, exit_info: Some(GuestExitInfo { code }), error_info: None, }); diff --git a/crates/kratad/src/reconcile/guest.rs b/crates/kratad/src/reconcile/guest.rs index ced7565..cfad6c1 100644 --- a/crates/kratad/src/reconcile/guest.rs +++ b/crates/kratad/src/reconcile/guest.rs @@ -53,7 +53,7 @@ impl GuestReconciler { } }, - _ = sleep(Duration::from_secs(30)) => { + _ = sleep(Duration::from_secs(5)) => { if let Err(error) = self.reconcile_runtime(false).await { error!("runtime reconciler failed: {}", error); } @@ -79,10 +79,9 @@ impl GuestReconciler { None => { let mut state = stored_guest.state.as_mut().cloned().unwrap_or_default(); if state.status() == GuestStatus::Started { - state.status = GuestStatus::Start.into(); + state.status = GuestStatus::Starting.into(); } stored_guest.state = Some(state); - stored_guest.network = None; } Some(runtime) => { @@ -93,18 +92,18 @@ impl GuestReconciler { } else { state.status = GuestStatus::Started.into(); } - stored_guest.state = Some(state); - stored_guest.network = Some(GuestNetworkState { + state.network = Some(GuestNetworkState { ipv4: runtime.ipv4.map(|x| x.ip().to_string()).unwrap_or_default(), ipv6: runtime.ipv6.map(|x| x.ip().to_string()).unwrap_or_default(), }); + stored_guest.state = Some(state); } } let changed = *stored_guest != previous_guest; - self.guests.update(uuid, stored_guest_entry).await?; if changed || initial { + self.guests.update(uuid, stored_guest_entry).await?; if let Err(error) = self.reconcile(uuid).await { error!("failed to reconcile guest {}: {}", uuid, error); } @@ -134,8 +133,8 @@ impl GuestReconciler { }))?; let result = match guest.state.as_ref().map(|x| x.status()).unwrap_or_default() { - GuestStatus::Start => self.start(uuid, guest).await, - GuestStatus::Destroy | GuestStatus::Exited => self.destroy(uuid, guest).await, + GuestStatus::Starting => self.start(uuid, guest).await, + GuestStatus::Destroying | GuestStatus::Exited => self.destroy(uuid, guest).await, _ => Ok(false), }; @@ -143,6 +142,7 @@ impl GuestReconciler { Ok(changed) => changed, Err(error) => { guest.state = Some(guest.state.as_mut().cloned().unwrap_or_default()); + guest.state.as_mut().unwrap().status = GuestStatus::Failed.into(); guest.state.as_mut().unwrap().error_info = Some(GuestErrorInfo { message: error.to_string(), }); @@ -152,8 +152,8 @@ impl GuestReconciler { info!("reconciled guest {}", uuid); - let destroyed = - guest.state.as_ref().map(|x| x.status()).unwrap_or_default() == GuestStatus::Destroyed; + let status = guest.state.as_ref().map(|x| x.status()).unwrap_or_default(); + let destroyed = status == GuestStatus::Destroyed || status == GuestStatus::Failed; if changed { let event = DaemonEvent::GuestChanged(GuestChangedEvent { @@ -205,12 +205,12 @@ impl GuestReconciler { }) .await?; info!("started guest {}", uuid); - guest.network = Some(GuestNetworkState { - ipv4: info.ipv4.map(|x| x.ip().to_string()).unwrap_or_default(), - ipv6: info.ipv6.map(|x| x.ip().to_string()).unwrap_or_default(), - }); guest.state = Some(GuestState { status: GuestStatus::Started.into(), + network: Some(GuestNetworkState { + ipv4: info.ipv4.map(|x| x.ip().to_string()).unwrap_or_default(), + ipv6: info.ipv6.map(|x| x.ip().to_string()).unwrap_or_default(), + }), exit_info: None, error_info: None, }); @@ -219,13 +219,13 @@ impl GuestReconciler { async fn destroy(&self, uuid: Uuid, guest: &mut Guest) -> Result { if let Err(error) = self.runtime.destroy(uuid).await { - warn!("failed to destroy runtime guest {}: {}", uuid, error); + trace!("failed to destroy runtime guest {}: {}", uuid, error); } info!("destroyed guest {}", uuid); - guest.network = None; guest.state = Some(GuestState { status: GuestStatus::Destroyed.into(), + network: None, exit_info: None, error_info: None, }); diff --git a/crates/kratart/src/image/registry.rs b/crates/kratart/src/image/registry.rs index a013c78..e40a5cf 100644 --- a/crates/kratart/src/image/registry.rs +++ b/crates/kratart/src/image/registry.rs @@ -110,7 +110,7 @@ impl OciRegistryClient { if !response.status().is_success() { return Err(anyhow!( - "failed to send request to {}: status {}", + "request to {} failed: status {}", req.build()?.url(), response.status() )); diff --git a/hack/os/build.sh b/hack/os/build.sh index 3d8f883..1410354 100755 --- a/hack/os/build.sh +++ b/hack/os/build.sh @@ -15,15 +15,14 @@ TARGET_OS_DIR="${TARGET_DIR}/os" mkdir -p "${TARGET_OS_DIR}" cp "${TARGET_DIR}/dist/krata_${KRATA_VERSION}_${TARGET_ARCH}.apk" "${TARGET_OS_DIR}/krata-${TARGET_ARCH}.apk" -DOCKER_FLAGS="" +DOCKER_FLAGS="--platform linux/${TARGET_ARCH_ALT}" if [ -t 0 ] then - DOCKER_FLAGS="-it" + DOCKER_FLAGS="${DOCKER_FLAGS} -it" fi if [ "${CROSS_COMPILE}" = "1" ] then - DOCKER_FLAGS="${DOCKER_FLAGS} --platform linux/${TARGET_ARCH_ALT}" docker run --privileged --rm tonistiigi/binfmt --install all fi