From 03bf1957f509e6059f95612f864a2926183e6476 Mon Sep 17 00:00:00 2001 From: ecpullen Date: Tue, 15 Mar 2022 21:42:29 +0000 Subject: [PATCH] Update `status` to display multiple test retries --- agent/test-agent/src/lib.rs | 16 +- .../src/bin/sonobuoy-test-agent/main.rs | 9 - testsys/src/run_aws_k8s.rs | 4 +- testsys/src/run_vmware.rs | 4 +- testsys/src/status.rs | 281 +++++------------- 5 files changed, 98 insertions(+), 216 deletions(-) diff --git a/agent/test-agent/src/lib.rs b/agent/test-agent/src/lib.rs index 6ca39ba80..87eba5c94 100644 --- a/agent/test-agent/src/lib.rs +++ b/agent/test-agent/src/lib.rs @@ -19,7 +19,7 @@ pub use k8s_client::ClientError; use log::info; use model::clients::TestClient; pub use model::{Configuration, TestResults}; -use model::{SecretName, SecretType}; +use model::{Outcome, SecretName, SecretType}; use std::collections::BTreeMap; use std::fmt::{Debug, Display}; use std::path::PathBuf; @@ -65,10 +65,16 @@ pub trait Runner: Sized + Send { /// Rerun a failed test. async fn rerun_failed( &mut self, - prev_test_result: &TestResults, + _prev_test_result: &TestResults, ) -> Result { info!("Tried to rerun test, but no retry method was defined."); - Ok(prev_test_result.clone()) + Ok(TestResults { + outcome: Outcome::Fail, + num_failed: 1, + num_passed: 0, + num_skipped: 0, + other_info: Some("No retry_failed method defined".to_string()), + }) } /// Cleans up prior to program exit. @@ -116,7 +122,9 @@ pub trait Client: Sized { /// Set the appropriate status field to represent that the test has started. async fn send_test_starting(&self) -> Result<(), Self::E>; - /// Set the appropriate status fields once the test has `TestResults` available. + /// Add a TestResults object to the CRDs array of TestResults without signaling that the test + /// is complete. This is used to send TestResults when some failures have occured and we are + /// going to re-run the failed test cases. async fn send_test_results(&self, results: TestResults) -> Result<(), Self::E>; /// Set the appropriate status fields once the test has finished. diff --git a/bottlerocket/agents/src/bin/sonobuoy-test-agent/main.rs b/bottlerocket/agents/src/bin/sonobuoy-test-agent/main.rs index e22983ef2..de5fd4b59 100644 --- a/bottlerocket/agents/src/bin/sonobuoy-test-agent/main.rs +++ b/bottlerocket/agents/src/bin/sonobuoy-test-agent/main.rs @@ -101,15 +101,6 @@ impl test_agent::Runner for SonobuoyTestRunner { setup_test_env(self, aws_secret_name).await?; } - if let Some(wireguard_secret_name) = &self.wireguard_secret_name { - // If a wireguard secret is specified, try to set up an wireguard connection with the - // wireguard configuration stored in the secret. - let wireguard_secret = self - .get_secret(wireguard_secret_name) - .context(error::SecretMissingSnafu)?; - setup_wireguard(&wireguard_secret).await?; - } - delete_sonobuoy(TEST_CLUSTER_KUBECONFIG_PATH).await?; decode_write_kubeconfig(&self.config.kubeconfig_base64, TEST_CLUSTER_KUBECONFIG_PATH) diff --git a/testsys/src/run_aws_k8s.rs b/testsys/src/run_aws_k8s.rs index 3bad7d902..976014cf1 100644 --- a/testsys/src/run_aws_k8s.rs +++ b/testsys/src/run_aws_k8s.rs @@ -131,7 +131,7 @@ pub(crate) struct RunAwsK8s { /// Allow the sonobuoy test agent to rerun failed test. #[structopt(long)] - allow_retries: Option, + retry_failed_attempts: Option, /// Perform an upgrade downgrade test. #[structopt(long, requires_all(&["starting-version", "upgrade-version", "migration-agent-image"]))] @@ -429,7 +429,7 @@ impl RunAwsK8s { cluster_resource_name.to_string(), ], depends_on, - retries: self.allow_retries, + retries: self.retry_failed_attempts, agent: Agent { name: "sonobuoy-test-agent".to_string(), image: self.test_agent_image.clone(), diff --git a/testsys/src/run_vmware.rs b/testsys/src/run_vmware.rs index f2cac1897..56cd6c2fb 100644 --- a/testsys/src/run_vmware.rs +++ b/testsys/src/run_vmware.rs @@ -144,7 +144,7 @@ pub(crate) struct RunVmware { /// Allow the sonobuoy test agent to rerun failed test. #[structopt(long)] - allow_retries: Option, + retry_failed_attempts: Option, /// Perform an upgrade downgrade test. #[structopt(long, requires_all(&["starting-version", "upgrade-version"]))] @@ -377,7 +377,7 @@ impl RunVmware { spec: TestSpec { resources: vec![vm_resource_name.to_string()], depends_on, - retries: self.allow_retries, + retries: self.retry_failed_attempts, agent: Agent { name: "vmware-sonobuoy-test-agent".to_string(), image: self.test_agent_image.clone(), diff --git a/testsys/src/status.rs b/testsys/src/status.rs index 00dd22199..2fd5202c3 100644 --- a/testsys/src/status.rs +++ b/testsys/src/status.rs @@ -6,11 +6,9 @@ use kube::core::object::HasStatus; use kube::{Api, Client, ResourceExt}; use model::clients::{CrdClient, ResourceClient, TestClient}; use model::constants::{LABEL_COMPONENT, NAMESPACE}; -use model::{Resource, TaskState, Test, TestUserState}; -use serde::{Deserialize, Serialize}; +use model::{Resource, TaskState, Test}; +use serde::Serialize; use snafu::ResultExt; -use std::collections::HashMap; -use std::fmt::Display; use structopt::StructOpt; use tabled::{Alignment, Column, Full, MaxWidth, Modify, Style, Table, Tabled}; use termion::terminal_size; @@ -36,24 +34,23 @@ impl Status { let tests_api = TestClient::new_from_k8s_client(k8s_client.clone()); let resources_api = ResourceClient::new_from_k8s_client(k8s_client.clone()); let pod_api = Api::::namespaced(k8s_client, NAMESPACE); - let mut status_results = StatusResults::new(); + let mut status_results = Results::default(); if self.controller { - status_results.controller_is_running = Some(is_controller_running(&pod_api).await?); + status_results.add_controller(is_controller_running(&pod_api).await?); } let tests = self.tests(&tests_api).await?; let resources = self.resources(&resources_api).await?; for test in tests { - let test_result = TestResult::from_test(&test); - status_results.add_test_result(test_result) + status_results.add_test(&test) } for resource in resources { - let resource_result = ResourceResult::from_resource(&resource); - status_results.add_resource_result(resource_result) + status_results.add_resource(&resource) } if !self.json { let (width, _) = terminal_size().ok().unwrap_or((120, 0)); - status_results.draw(width); + status_results.width = width; + status_results.draw(); } else { println!( "{}", @@ -126,153 +123,75 @@ async fn is_controller_running(pod_api: &Api) -> Result { Ok(true) } -#[derive(Debug, Serialize, Deserialize, Default)] -struct StatusResults { - tests: HashMap, - resources: HashMap, - controller_is_running: Option, -} - -#[derive(Debug, Serialize, Deserialize)] -struct TestResult { - name: String, - state: TestUserState, - passed: Option, - failed: Option, - skipped: Option, -} - -#[derive(Debug, Serialize, Deserialize)] -struct ResourceResult { - name: String, - create_state: TaskState, - delete_state: TaskState, +#[derive(Serialize)] +struct Results { + #[serde(skip_serializing)] + width: u16, + results: Vec, + #[serde(skip_serializing)] + min_name_width: u16, + #[serde(skip_serializing)] + min_object_width: u16, + #[serde(skip_serializing)] + min_state_width: u16, + #[serde(skip_serializing)] + min_passed_width: u16, + #[serde(skip_serializing)] + min_skipped_width: u16, + #[serde(skip_serializing)] + min_failed_width: u16, } -impl StatusResults { - fn new() -> Self { +impl Default for Results { + fn default() -> Self { Self { - tests: HashMap::new(), - resources: HashMap::new(), - controller_is_running: None, - } - } - - fn add_test_result(&mut self, test_result: TestResult) { - self.tests.insert(test_result.name.clone(), test_result); - } - - fn add_resource_result(&mut self, resource_result: ResourceResult) { - self.resources - .insert(resource_result.name.clone(), resource_result); - } - - fn draw(&self, width: u16) { - let mut results = Vec::new(); - if let Some(controller_running) = self.controller_is_running { - results.push(ResultRow { - name: "Controller".to_string(), - object_type: "Controller".to_string(), - state: if controller_running { "Running" } else { "" }.to_string(), - ..Default::default() - }) - } - for resource_result in self.resources.values() { - results.push(resource_result.into()); - } - for test_result in self.tests.values() { - results.push(test_result.into()); - } - let results_table = Results { - width, - results, - ..Default::default() - }; - - results_table.draw(); - } -} - -impl Display for StatusResults { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for result in self.tests.values() { - write!(f, "{}\n\n", result)?; - } - - for result in self.resources.values() { - write!(f, "{}\n\n", result)?; - } - - if let Some(running) = self.controller_is_running { - write!(f, "Controller Running: {}", running)?; + width: 100, + results: Vec::new(), + min_name_width: 4, + min_object_width: 4, + min_state_width: 5, + min_passed_width: 6, + min_skipped_width: 7, + min_failed_width: 6, } - - Ok(()) } } -impl TestResult { - fn from_test(test: &Test) -> Self { +impl Results { + /// Adds a new `ResultRow` for each `TestResults` in test, or a single row if no `TestsResults` are available. + fn add_test(&mut self, test: &Test) { let name = test.metadata.name.clone().unwrap_or_else(|| "".to_string()); - let mut passed = None; - let mut failed = None; - let mut skipped = None; - let test_user_state = test.test_user_state(); - if let Some(results) = test.agent_status().results.last() { - passed = Some(results.num_passed); - failed = Some(results.num_failed); - skipped = Some(results.num_skipped); - } - - Self { - name, - state: test_user_state, - passed, - failed, - skipped, - } - } -} - -impl Display for TestResult { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "Test Name: {}", self.name)?; - writeln!(f, "Test State: {}", self.state)?; - writeln!( - f, - "Passed: {}", - self.passed.map_or("".to_string(), |x| x.to_string()) - )?; - writeln!( - f, - "Failed: {}", - self.failed.map_or("".to_string(), |x| x.to_string()) - )?; - writeln!( - f, - "Skipped: {}", - self.skipped.map_or("".to_string(), |x| x.to_string()) - )?; - - Ok(()) - } -} - -impl From<&TestResult> for ResultRow { - fn from(test_result: &TestResult) -> ResultRow { - ResultRow { - name: test_result.name.clone(), - object_type: "Test".to_string(), - state: test_result.state.to_string(), - passed: test_result.passed, - skipped: test_result.skipped, - failed: test_result.failed, + let state = test.test_user_state().to_string(); + let results = &test.agent_status().results; + if results.is_empty() { + self.results.push(ResultRow { + name, + object_type: "Test".to_string(), + state, + passed: None, + skipped: None, + failed: None, + }) + } else { + for (test_count, result) in results.iter().enumerate() { + let retry_name = if test_count == 0 { + name.clone() + } else { + format!("{}-retry-{}", name, test_count) + }; + self.results.push(ResultRow { + name: retry_name, + object_type: "Test".to_string(), + state: state.clone(), + passed: Some(result.num_passed), + skipped: Some(result.num_skipped), + failed: Some(result.num_failed), + }); + } } } -} -impl ResourceResult { - fn from_resource(resource: &Resource) -> Self { + fn add_resource(&mut self, resource: &Resource) { let name = resource.name(); let mut create_state = TaskState::Unknown; let mut delete_state = TaskState::Unknown; @@ -280,68 +199,32 @@ impl ResourceResult { create_state = status.creation.task_state; delete_state = status.destruction.task_state; } + let state = match delete_state { + TaskState::Unknown => create_state, + _ => delete_state, + }; - Self { + self.results.push(ResultRow { name, - create_state, - delete_state, - } - } -} - -impl Display for ResourceResult { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "Resource Name: {}", self.name)?; - writeln!(f, "Resource Creation State: {:?}", self.create_state)?; - writeln!(f, "Resource Deletion State: {:?}", self.delete_state)?; - Ok(()) - } -} - -impl From<&ResourceResult> for ResultRow { - fn from(resource_result: &ResourceResult) -> ResultRow { - let state = match resource_result.delete_state { - TaskState::Unknown => resource_result.create_state, - _ => resource_result.delete_state, - }; - ResultRow { - name: resource_result.name.clone(), object_type: "Resource".to_string(), state: state.to_string(), passed: None, skipped: None, failed: None, - } + }); } -} - -struct Results { - width: u16, - results: Vec, - min_name_width: u16, - min_object_width: u16, - min_state_width: u16, - min_passed_width: u16, - min_skipped_width: u16, - min_failed_width: u16, -} -impl Default for Results { - fn default() -> Self { - Self { - width: 100, - results: Vec::new(), - min_name_width: 4, - min_object_width: 4, - min_state_width: 5, - min_passed_width: 6, - min_skipped_width: 7, - min_failed_width: 6, - } + fn add_controller(&mut self, running: bool) { + self.results.push(ResultRow { + name: "Controller".to_string(), + object_type: "Controller".to_string(), + state: if running { "Running" } else { "" }.to_string(), + passed: None, + skipped: None, + failed: None, + }); } -} -impl Results { fn draw(&self) { if self.width < self.min_name_width @@ -396,7 +279,7 @@ impl Results { } } -#[derive(Tabled, Default, Clone)] +#[derive(Tabled, Default, Clone, Serialize)] struct ResultRow { #[header("NAME")] name: String,