From 9877bdf4280e8a3d71e4443956e5910aee14f0a8 Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Tue, 6 Aug 2024 14:23:47 +0300 Subject: [PATCH 01/15] Modified implementation of set_state and added unit tests --- ank/src/cli.rs | 16 +- ank/src/cli_commands/set_state.rs | 276 ++++++++++++++++-- doc/docs/reference/complete-state.md | 2 +- .../interworkload_dependencies.robot | 6 +- ...eject_state_with_cyclic_dependencies.robot | 2 +- .../retry_creation_of_workload_podman.robot | 8 +- .../state_operations_with_field_mask.robot | 2 +- .../workloads/update_workload_podman.robot | 8 +- 8 files changed, 281 insertions(+), 39 deletions(-) diff --git a/ank/src/cli.rs b/ank/src/cli.rs index 4dc94a16b..2f2a7757f 100644 --- a/ank/src/cli.rs +++ b/ank/src/cli.rs @@ -12,12 +12,26 @@ // // SPDX-License-Identifier: Apache-2.0 +// use std::{error::Error, fs::File, io}; use std::error::Error; use clap::{command, Parser, Subcommand}; use common::DEFAULT_SERVER_ADDRESS; +#[cfg(not(test))] +// pub fn open_manifest(file_path: &str) -> io::Result { +// match File::open(file_path) { +// Ok(open_file) => Ok((file_path.to_owned(), Box::new(open_file))), +// Err(err) => Err(err), +// } +// } +#[cfg(test)] +use self::tests::open_manifest_mock as open_manifest; + +// pub type InputSourcePair = (String, Box); +// pub type InputSources = Result, String>; + const ANK_SERVER_URL_ENV_KEY: &str = "ANK_SERVER_URL"; // [impl->swdd~cli-supports-server-url-cli-argument~1] @@ -137,7 +151,7 @@ pub enum SetCommands { #[arg(required = true)] object_field_mask: Vec, /// A file containing the new State Object Description in yaml format - #[arg(short = 'f', long = "file")] + #[arg(required = true)] state_object_file: Option, }, } diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index 7c548d603..99ace4b3f 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -16,6 +16,7 @@ use common::{ objects::{CompleteState, StoredWorkloadSpec}, state_manipulation::{Object, Path}, }; +use std::io::{self, Read}; #[cfg(not(test))] fn read_file_to_string(file: String) -> std::io::Result { @@ -54,6 +55,70 @@ fn add_default_workload_spec_per_update_mask( } } +// [impl->swdd~cli-supports-yaml-to-set-desired-state~1] +async fn process_inputs(reader: R, state_object_file: &str, temp_obj: &mut Object) { + match state_object_file { + "-" => { + let stdin = io::read_to_string(reader).unwrap_or_else(|error| { + output_and_error!("Could not read the state object file.\nError: {}", error) + }); + let value: serde_yaml::Value = serde_yaml::from_str(&stdin).unwrap_or_else(|error| { + output_and_error!("Could not convert to yaml Value.\nError: {}", error) + }); + *temp_obj = Object::try_from(&value).unwrap_or_else(|error| { + output_and_error!("Could not convert object.\n Error: {}", error) + }); + } + _ => { + let state_object_data = read_file_to_string(state_object_file.to_string()) + .unwrap_or_else(|error| { + output_and_error!("Could not read the state object file.\nError: {}", error) + }); + let value: serde_yaml::Value = + serde_yaml::from_str(&state_object_data).unwrap_or_else(|error| { + output_and_error!("Could not convert to yaml Value.\nError: {}", error) + }); + *temp_obj = Object::try_from(&value).unwrap_or_else(|error| { + output_and_error!("Could not convert object.\n Error: {}", error) + }); + } + } +} + +fn overwrite_using_field_mask( + complete_state_object: &mut Object, + object_field_mask: &Vec, + temp_obj: &Object, +) { + for field_mask in object_field_mask { + let path: Path = field_mask.into(); + + complete_state_object + .set( + &path, + temp_obj + .get(&path) + .ok_or(CliError::ExecutionError(format!( + "Specified update mask '{field_mask}' not found in the input config.", + ))) + .unwrap_or_else(|error| { + output_and_error!( + "Encountered error while overwritting using field mask. Error: {}", + error + ) + }) + .clone(), + ) + .map_err(|err| CliError::ExecutionError(err.to_string())) + .unwrap_or_else(|error| { + output_and_error!( + "Encountered error while overwritting using field mask. Error: {}", + error + ) + }); + } +} + impl CliCommands { pub async fn set_state( &mut self, @@ -67,33 +132,15 @@ impl CliCommands { ); let mut complete_state = CompleteState::default(); - if let Some(state_object_file) = state_object_file { - let state_object_data = - read_file_to_string(state_object_file).unwrap_or_else(|error| { - output_and_error!("Could not read the state object file.\nError: {}", error) - }); - let value: serde_yaml::Value = serde_yaml::from_str(&state_object_data)?; - let x = Object::try_from(&value)?; + let mut temp_obj: Object = Object::default(); - // This here is a workaround for the default workload specs + if let Some(state_object_file) = state_object_file { + process_inputs(io::stdin(), &state_object_file, &mut temp_obj).await; add_default_workload_spec_per_update_mask(&object_field_mask, &mut complete_state); // now overwrite with the values from the field mask let mut complete_state_object: Object = complete_state.try_into()?; - for field_mask in &object_field_mask { - let path: Path = field_mask.into(); - - complete_state_object - .set( - &path, - x.get(&path) - .ok_or(CliError::ExecutionError(format!( - "Specified update mask '{field_mask}' not found in the input config.", - )))? - .clone(), - ) - .map_err(|err| CliError::ExecutionError(err.to_string()))?; - } + overwrite_using_field_mask(&mut complete_state_object, &object_field_mask, &temp_obj); complete_state = complete_state_object.try_into()?; } @@ -117,9 +164,190 @@ impl CliCommands { ////////////////////////////////////////////////////////////////////////////// #[cfg(test)] mod tests { - use std::io; + use super::*; + use crate::cli_commands::server_connection::MockServerConnection; + use api::ank_base::UpdateStateSuccess; + use common::{ + // commands::UpdateStateSuccess, + objects::{CompleteState, RestartPolicy, State}, + state_manipulation::Object, + }; + use mockall::predicate::eq; + use serde_yaml::Value; + use std::{collections::HashMap, io::Cursor}; pub fn read_to_string_mock(_file: String) -> io::Result { - Ok("".into()) + Ok(_file) + } + + const RESPONSE_TIMEOUT_MS: u64 = 3000; + + const SAMPLE_CONFIG: &str = r#"desiredState: + workloads: + nginx: + agent: agent_A + tags: + - key: owner + value: Ankaios team + dependencies: {} + restartPolicy: ALWAYS + runtime: podman + runtimeConfig: | + image: docker.io/nginx:latest + commandOptions: ["-p", "8081:80"]"#; + + #[test] + fn utest_add_default_workload_spec_empty_update_mask() { + let update_mask = vec![]; + let mut complete_state = CompleteState::default(); + + add_default_workload_spec_per_update_mask(&update_mask, &mut complete_state); + + assert!(complete_state.desired_state.workloads.is_empty()); + } + + #[test] + fn utest_add_default_workload_spec_with_update_mask() { + let update_mask = vec![ + "desiredState.workloads.nginx.restartPolicy".to_string(), + "desiredState.workloads.nginx2.restartPolicy".to_string(), + "desiredState.workloads.nginx3".to_string(), + ]; + let mut complete_state = CompleteState::default(); + + add_default_workload_spec_per_update_mask(&update_mask, &mut complete_state); + + assert!(complete_state.desired_state.workloads.contains_key("nginx")); + assert!(complete_state + .desired_state + .workloads + .contains_key("nginx2")); + assert!(!complete_state + .desired_state + .workloads + .contains_key("nginx3")); + } + + #[test] + fn utest_add_default_workload_spec_invalid_path() { + let update_mask = vec!["invalid.path".to_string()]; + let mut complete_state = CompleteState::default(); + + add_default_workload_spec_per_update_mask(&update_mask, &mut complete_state); + + assert!(complete_state.desired_state.workloads.is_empty()); + } + + #[test] + fn utest_overwrite_using_field_mask() { + let workload_spec = StoredWorkloadSpec::default(); + let mut complete_state = CompleteState { + desired_state: State { + workloads: HashMap::from([("nginx".to_string(), workload_spec)]), + ..Default::default() + }, + ..Default::default() + }; + let mut complete_state_object: Object = complete_state.try_into().unwrap(); + let value: serde_yaml::Value = serde_yaml::from_str(SAMPLE_CONFIG).unwrap(); + let temp_object = Object::try_from(&value).unwrap(); + let update_mask = vec!["desiredState.workloads.nginx".to_string()]; + + overwrite_using_field_mask(&mut complete_state_object, &update_mask, &temp_object); + + complete_state = complete_state_object.try_into().unwrap(); + + assert!(complete_state.desired_state.workloads.contains_key("nginx")); + assert_eq!( + complete_state + .desired_state + .workloads + .get("nginx") + .unwrap() + .restart_policy, + RestartPolicy::Always + ) + } + + // [utest->swdd~cli-supports-yaml-to-set-desired-state~1] + #[tokio::test] + async fn utest_process_inputs_stdin() { + let input = SAMPLE_CONFIG; + let reader = Cursor::new(input); + let state_object_file = "-".to_string(); + let mut temp_obj = Object::default(); + + process_inputs(reader, &state_object_file, &mut temp_obj).await; + + let value: Value = serde_yaml::from_str(SAMPLE_CONFIG).unwrap(); + let expected_obj = Object::try_from(&value).unwrap(); + + assert_eq!(temp_obj, expected_obj); + } + + // [utest->swdd~cli-supports-yaml-to-set-desired-state~1] + #[tokio::test] + async fn utest_process_inputs_file() { + let state_object_file = SAMPLE_CONFIG.to_owned(); + let mut temp_obj = Object::default(); + println!("{:?}", state_object_file); + + process_inputs(io::empty(), &state_object_file, &mut temp_obj).await; + println!("{:?}", temp_obj); + + let value: Value = serde_yaml::from_str(SAMPLE_CONFIG).unwrap(); + let expected_obj = Object::try_from(&value).unwrap(); + println!("{:?}", expected_obj); + + assert_eq!(temp_obj, expected_obj); + } + + // [utest->swdd~cli-supports-yaml-to-set-desired-state~1] + #[tokio::test] + async fn utest_process_inputs_invalid_yaml() { + let input = "invalid yaml"; + let reader = Cursor::new(input); + let state_object_file = "-".to_string(); + let mut temp_obj = Object::default(); + + process_inputs(reader, &state_object_file, &mut temp_obj).await; + } + + // [utest->swdd~cli-provides-set-desired-state~1] + #[tokio::test] + async fn utest_set_state_ok() { + let update_mask = vec!["desiredState.workloads.nginx.restartPolicy".to_string()]; + let state_object_file = Some(SAMPLE_CONFIG.to_owned()); + + let workload_spec = StoredWorkloadSpec { + restart_policy: RestartPolicy::Always, + ..Default::default() + }; + let updated_state = CompleteState { + desired_state: State { + workloads: HashMap::from([("nginx".to_string(), workload_spec)]), + ..Default::default() + }, + ..Default::default() + }; + let mut mock_server_connection = MockServerConnection::default(); + mock_server_connection + .expect_update_state() + .with(eq(updated_state), eq(update_mask.clone())) + .return_once(|_, _| { + Ok(UpdateStateSuccess { + added_workloads: vec![], + deleted_workloads: vec![], + }) + }); + + let mut cmd = CliCommands { + _response_timeout_ms: RESPONSE_TIMEOUT_MS, + no_wait: true, + server_connection: mock_server_connection, + }; + + let set_state_result = cmd.set_state(update_mask, state_object_file).await; + assert!(set_state_result.is_ok()); } } diff --git a/doc/docs/reference/complete-state.md b/doc/docs/reference/complete-state.md index 62a40d5d1..bae31c2e2 100644 --- a/doc/docs/reference/complete-state.md +++ b/doc/docs/reference/complete-state.md @@ -125,7 +125,7 @@ The object field mask can be constructed using the field names of the [CompleteS commandOptions: ["-p", "8081:80"] ``` -3. Example `ank -k set state -f new-state.yaml desiredState.workloads.nginx.restartPolicy` changes the restart behavior of nginx workload to `NEVER`: +3. Example `ank set state desiredState.workloads.nginx.restartPolicy new-state.yaml` changes the restart behavior of nginx workload to `NEVER`: ```yaml title="new-state.yaml" desiredState: diff --git a/tests/stests/workloads/interworkload_dependencies.robot b/tests/stests/workloads/interworkload_dependencies.robot index cbd74e027..3e1b407dd 100644 --- a/tests/stests/workloads/interworkload_dependencies.robot +++ b/tests/stests/workloads/interworkload_dependencies.robot @@ -87,7 +87,7 @@ Test Ankaios CLI update workload with pending delete # Actions When user triggers "ank -k get state > ${new_state_yaml_file}" And user updates the state "${new_state_yaml_file}" with "desiredState.workloads.backend.runtimeConfig.commandOptions=['-p', '8084:80']" - And user triggers "ank -k --no-wait set state -f ${new_state_yaml_file} desiredState.workloads.backend" + And user triggers "ank -k --no-wait set state ${new_state_yaml_file} desiredState.workloads.backend" And the workload "backend" shall have the execution state "Stopping(WaitingToStop)" on agent "agent_A" within "20" seconds And user triggers "ank -k delete workload frontend" # Asserts @@ -110,9 +110,9 @@ Test Ankaios CLI update workload with pending create And Ankaios agent is started with name "agent_A" And the workload "after_backend" shall have the execution state "Succeeded(Ok)" on agent "agent_A" within "20" seconds # Actions - When user triggers "ank -k --no-wait set state -f ${new_state_yaml_file} desiredState.workloads.after_backend" + When user triggers "ank -k --no-wait set state ${new_state_yaml_file} desiredState.workloads.after_backend" And the workload "after_backend" shall have the execution state "Pending(WaitingToStart)" on agent "agent_A" within "3" seconds - And user triggers "ank -k set state -f ${new_state_yaml_file} desiredState.workloads.backend" + And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.backend" # Asserts Then the workload "backend" shall have the execution state "Succeeded(Ok)" on agent "agent_A" within "5" seconds And the workload "after_backend" shall have the execution state "Succeeded(Ok)" on agent "agent_A" within "5" seconds diff --git a/tests/stests/workloads/reject_state_with_cyclic_dependencies.robot b/tests/stests/workloads/reject_state_with_cyclic_dependencies.robot index bdab4d77a..1d7c64294 100644 --- a/tests/stests/workloads/reject_state_with_cyclic_dependencies.robot +++ b/tests/stests/workloads/reject_state_with_cyclic_dependencies.robot @@ -60,7 +60,7 @@ Test Ankaios CLI update state with cycle in interworkload dependencies is reject And Ankaios agent is started with name "agent_A" And all workloads of agent "agent_A" have an initial execution state # Actions - And user triggers "ank -k set state -f ${new_state_yaml_file} desiredState.workloads.workload_C" + And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.workload_C" # Asserts Then the workload "workload_C" shall not exist And podman shall not have a container for workload "workload_C" on agent "agentA" within "5" seconds diff --git a/tests/stests/workloads/retry_creation_of_workload_podman.robot b/tests/stests/workloads/retry_creation_of_workload_podman.robot index 21443f6c9..6d5eb623e 100644 --- a/tests/stests/workloads/retry_creation_of_workload_podman.robot +++ b/tests/stests/workloads/retry_creation_of_workload_podman.robot @@ -39,7 +39,7 @@ Test Ankaios Podman retry creation of a workload on creation failure And user triggers "ank -k delete workload hello1" And the workload "hello1" shall not exist on agent "agent_A" within "20" seconds And podman shall not have a container for workload "hello1" on agent "agent_A" within "20" seconds - And user triggers "ank -k set state -f ${new_state_yaml_file} desiredState.workloads.hello1" + And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" # Asserts Then the workload "hello1" shall have the execution state "Running(Ok)" from agent "agent_A" within "20" seconds [Teardown] Clean up Ankaios @@ -62,9 +62,9 @@ Test Ankaios Podman retry creation of a workload on creation failure intercepted And user triggers "ank -k delete workload hello1" And the workload "hello1" shall not exist on agent "agent_A" within "20" seconds And podman shall not have a container for workload "hello1" on agent "agent_A" within "20" seconds - And user triggers "ank -k set state -f ${new_state_yaml_file} desiredState.workloads.hello1" + And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" And user updates the state "${new_state_yaml_file}" with "desiredState.workloads.hello1.runtimeConfig.commandArgs=['3']" - And user triggers "ank -k set state -f ${new_state_yaml_file} desiredState.workloads.hello1" + And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" # Asserts Then the workload "hello1" shall have the execution state "Succeeded(Ok)" from agent "agent_A" within "20" seconds [Teardown] Clean up Ankaios @@ -87,7 +87,7 @@ Test Ankaios Podman retry creation of a workload on creation failure intercepted And user triggers "ank -k delete workload hello1" And the workload "hello1" shall not exist on agent "agent_A" within "20" seconds And podman shall not have a container for workload "hello1" on agent "agent_A" within "20" seconds - And user triggers "ank -k set state -f ${new_state_yaml_file} desiredState.workloads.hello1" + And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" And the user waits "1" seconds And user triggers "ank -k delete workload hello1" # Asserts diff --git a/tests/stests/workloads/state_operations_with_field_mask.robot b/tests/stests/workloads/state_operations_with_field_mask.robot index a6e384431..bd686b5e6 100644 --- a/tests/stests/workloads/state_operations_with_field_mask.robot +++ b/tests/stests/workloads/state_operations_with_field_mask.robot @@ -39,7 +39,7 @@ Test Ankaios CLI update workload And Ankaios agent is started with name "agent_A" And all workloads of agent "agent_A" have an initial execution state # Actions - And user triggers "ank -k --no-wait set state -f ${new_state_yaml_file} desiredState.workloads.simple.agent" + And user triggers "ank -k --no-wait set state ${new_state_yaml_file} desiredState.workloads.simple.agent" # Asserts Then the workload "simple" shall not exist on agent "agent_A" within "20" seconds And podman shall not have a container for workload "simple" on agent "agent_A" diff --git a/tests/stests/workloads/update_workload_podman.robot b/tests/stests/workloads/update_workload_podman.robot index 8d57bf957..0f3af1387 100644 --- a/tests/stests/workloads/update_workload_podman.robot +++ b/tests/stests/workloads/update_workload_podman.robot @@ -43,7 +43,7 @@ Test Ankaios CLI update workload # Actions When user triggers "ank -k get state > ${new_state_yaml_file}" And user updates the state "${new_state_yaml_file}" with "desiredState.workloads.nginx.runtimeConfig.commandOptions=['-p', '8082:80']" - And user triggers "ank -k set state -f ${new_state_yaml_file} desiredState.workloads.nginx" + And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.nginx" # Asserts Then the workload "nginx" shall have the execution state "Running(Ok)" on agent "agent_A" within "20" seconds And the command "curl localhost:8082" shall finish with exit code "0" within "10" seconds @@ -61,7 +61,7 @@ Test Ankaios Podman update workload from empty state # Actions When user triggers "ank -k get workloads" Then list of workloads shall be empty - When user triggers "ank -k set state --file ${CONFIGS_DIR}/update_state_create_one_workload.yaml desiredState.workloads" + When user triggers "ank -k set state ${CONFIGS_DIR}/update_state_create_one_workload.yaml desiredState.workloads" Then the workload "nginx" shall have the execution state "Running(Ok)" on agent "agent_A" within "20" seconds [Teardown] Clean up Ankaios @@ -77,7 +77,7 @@ Test Ankaios Podman Update workload with invalid api version # Actions When user triggers "ank -k get workloads" Then list of workloads shall be empty - When user triggers "ank -k set state --file ${CONFIGS_DIR}/update_state_invalid_version.yaml desiredState" + When user triggers "ank -k set state ${CONFIGS_DIR}/update_state_invalid_version.yaml desiredState" And user triggers "ank -k get workloads" Then list of workloads shall be empty @@ -95,7 +95,7 @@ Test Ankaios Podman Update workload with missing api version # Actions When user triggers "ank -k get workloads" Then list of workloads shall be empty - When user triggers "ank -k set state --file ${CONFIGS_DIR}/update_state_missing_version.yaml desiredState" + When user triggers "ank -k set state ${CONFIGS_DIR}/update_state_missing_version.yaml desiredState" And user triggers "ank -k get workloads" Then list of workloads shall be empty From 48fa003e48324f6c1912fb04d6e3b1c34b1ab8aa Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Tue, 6 Aug 2024 14:26:15 +0300 Subject: [PATCH 02/15] Remove unneeded comments --- ank/src/cli.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/ank/src/cli.rs b/ank/src/cli.rs index 2f2a7757f..4ea249951 100644 --- a/ank/src/cli.rs +++ b/ank/src/cli.rs @@ -19,19 +19,6 @@ use clap::{command, Parser, Subcommand}; use common::DEFAULT_SERVER_ADDRESS; -#[cfg(not(test))] -// pub fn open_manifest(file_path: &str) -> io::Result { -// match File::open(file_path) { -// Ok(open_file) => Ok((file_path.to_owned(), Box::new(open_file))), -// Err(err) => Err(err), -// } -// } -#[cfg(test)] -use self::tests::open_manifest_mock as open_manifest; - -// pub type InputSourcePair = (String, Box); -// pub type InputSources = Result, String>; - const ANK_SERVER_URL_ENV_KEY: &str = "ANK_SERVER_URL"; // [impl->swdd~cli-supports-server-url-cli-argument~1] From 81f586185e39a018e2c0107f34444265c0b8917f Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Tue, 6 Aug 2024 14:27:55 +0300 Subject: [PATCH 03/15] remove comment --- ank/src/cli.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/ank/src/cli.rs b/ank/src/cli.rs index 4ea249951..311fd30eb 100644 --- a/ank/src/cli.rs +++ b/ank/src/cli.rs @@ -12,7 +12,6 @@ // // SPDX-License-Identifier: Apache-2.0 -// use std::{error::Error, fs::File, io}; use std::error::Error; use clap::{command, Parser, Subcommand}; From 3077552028e76610efa5b5701c5640806432423e Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Mon, 12 Aug 2024 12:29:44 +0000 Subject: [PATCH 04/15] Fix failing stests --- tests/stests/workloads/interworkload_dependencies.robot | 6 +++--- .../workloads/retry_creation_of_workload_podman.robot | 6 +++--- .../stests/workloads/state_operations_with_field_mask.robot | 2 +- tests/stests/workloads/update_workload_podman.robot | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/stests/workloads/interworkload_dependencies.robot b/tests/stests/workloads/interworkload_dependencies.robot index 3e1b407dd..8ebd09ab0 100644 --- a/tests/stests/workloads/interworkload_dependencies.robot +++ b/tests/stests/workloads/interworkload_dependencies.robot @@ -87,7 +87,7 @@ Test Ankaios CLI update workload with pending delete # Actions When user triggers "ank -k get state > ${new_state_yaml_file}" And user updates the state "${new_state_yaml_file}" with "desiredState.workloads.backend.runtimeConfig.commandOptions=['-p', '8084:80']" - And user triggers "ank -k --no-wait set state ${new_state_yaml_file} desiredState.workloads.backend" + And user triggers "ank -k --no-wait set state desiredState.workloads.backend ${new_state_yaml_file}" And the workload "backend" shall have the execution state "Stopping(WaitingToStop)" on agent "agent_A" within "20" seconds And user triggers "ank -k delete workload frontend" # Asserts @@ -110,9 +110,9 @@ Test Ankaios CLI update workload with pending create And Ankaios agent is started with name "agent_A" And the workload "after_backend" shall have the execution state "Succeeded(Ok)" on agent "agent_A" within "20" seconds # Actions - When user triggers "ank -k --no-wait set state ${new_state_yaml_file} desiredState.workloads.after_backend" + When user triggers "ank -k --no-wait set state desiredState.workloads.after_backend ${new_state_yaml_file}" And the workload "after_backend" shall have the execution state "Pending(WaitingToStart)" on agent "agent_A" within "3" seconds - And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.backend" + And user triggers "ank -k set state desiredState.workloads.backend ${new_state_yaml_file}" # Asserts Then the workload "backend" shall have the execution state "Succeeded(Ok)" on agent "agent_A" within "5" seconds And the workload "after_backend" shall have the execution state "Succeeded(Ok)" on agent "agent_A" within "5" seconds diff --git a/tests/stests/workloads/retry_creation_of_workload_podman.robot b/tests/stests/workloads/retry_creation_of_workload_podman.robot index 6d5eb623e..a04eb4479 100644 --- a/tests/stests/workloads/retry_creation_of_workload_podman.robot +++ b/tests/stests/workloads/retry_creation_of_workload_podman.robot @@ -39,7 +39,7 @@ Test Ankaios Podman retry creation of a workload on creation failure And user triggers "ank -k delete workload hello1" And the workload "hello1" shall not exist on agent "agent_A" within "20" seconds And podman shall not have a container for workload "hello1" on agent "agent_A" within "20" seconds - And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" + And user triggers "ank -k set state desiredState.workloads.hello1 ${new_state_yaml_file}" # Asserts Then the workload "hello1" shall have the execution state "Running(Ok)" from agent "agent_A" within "20" seconds [Teardown] Clean up Ankaios @@ -64,7 +64,7 @@ Test Ankaios Podman retry creation of a workload on creation failure intercepted And podman shall not have a container for workload "hello1" on agent "agent_A" within "20" seconds And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" And user updates the state "${new_state_yaml_file}" with "desiredState.workloads.hello1.runtimeConfig.commandArgs=['3']" - And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" + And user triggers "ank -k set state desiredState.workloads.hello1 ${new_state_yaml_file}" # Asserts Then the workload "hello1" shall have the execution state "Succeeded(Ok)" from agent "agent_A" within "20" seconds [Teardown] Clean up Ankaios @@ -87,7 +87,7 @@ Test Ankaios Podman retry creation of a workload on creation failure intercepted And user triggers "ank -k delete workload hello1" And the workload "hello1" shall not exist on agent "agent_A" within "20" seconds And podman shall not have a container for workload "hello1" on agent "agent_A" within "20" seconds - And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.hello1" + And user triggers "ank -k set state desiredState.workloads.hello1 ${new_state_yaml_file}" And the user waits "1" seconds And user triggers "ank -k delete workload hello1" # Asserts diff --git a/tests/stests/workloads/state_operations_with_field_mask.robot b/tests/stests/workloads/state_operations_with_field_mask.robot index bd686b5e6..14697c4c7 100644 --- a/tests/stests/workloads/state_operations_with_field_mask.robot +++ b/tests/stests/workloads/state_operations_with_field_mask.robot @@ -39,7 +39,7 @@ Test Ankaios CLI update workload And Ankaios agent is started with name "agent_A" And all workloads of agent "agent_A" have an initial execution state # Actions - And user triggers "ank -k --no-wait set state ${new_state_yaml_file} desiredState.workloads.simple.agent" + And user triggers "ank -k --no-wait set state desiredState.workloads.simple.agent ${new_state_yaml_file}" # Asserts Then the workload "simple" shall not exist on agent "agent_A" within "20" seconds And podman shall not have a container for workload "simple" on agent "agent_A" diff --git a/tests/stests/workloads/update_workload_podman.robot b/tests/stests/workloads/update_workload_podman.robot index 0f3af1387..f5d06f394 100644 --- a/tests/stests/workloads/update_workload_podman.robot +++ b/tests/stests/workloads/update_workload_podman.robot @@ -43,7 +43,7 @@ Test Ankaios CLI update workload # Actions When user triggers "ank -k get state > ${new_state_yaml_file}" And user updates the state "${new_state_yaml_file}" with "desiredState.workloads.nginx.runtimeConfig.commandOptions=['-p', '8082:80']" - And user triggers "ank -k set state ${new_state_yaml_file} desiredState.workloads.nginx" + And user triggers "ank -k set state desiredState.workloads.nginx ${new_state_yaml_file}" # Asserts Then the workload "nginx" shall have the execution state "Running(Ok)" on agent "agent_A" within "20" seconds And the command "curl localhost:8082" shall finish with exit code "0" within "10" seconds @@ -61,7 +61,7 @@ Test Ankaios Podman update workload from empty state # Actions When user triggers "ank -k get workloads" Then list of workloads shall be empty - When user triggers "ank -k set state ${CONFIGS_DIR}/update_state_create_one_workload.yaml desiredState.workloads" + When user triggers "ank -k set state desiredState.workloads ${CONFIGS_DIR}/update_state_create_one_workload.yaml" Then the workload "nginx" shall have the execution state "Running(Ok)" on agent "agent_A" within "20" seconds [Teardown] Clean up Ankaios From ef95f2fdd04d6b3110b112695a5d6c0c63b925cd Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Tue, 13 Aug 2024 13:17:06 +0000 Subject: [PATCH 05/15] Fix review findings --- ank/src/cli.rs | 2 +- ank/src/cli_commands/set_state.rs | 105 +++++++++++++++++------------- 2 files changed, 59 insertions(+), 48 deletions(-) diff --git a/ank/src/cli.rs b/ank/src/cli.rs index 311fd30eb..e1b7d30ae 100644 --- a/ank/src/cli.rs +++ b/ank/src/cli.rs @@ -138,7 +138,7 @@ pub enum SetCommands { object_field_mask: Vec, /// A file containing the new State Object Description in yaml format #[arg(required = true)] - state_object_file: Option, + state_object_file: String, }, } diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index 99ace4b3f..2db44d533 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -56,40 +56,51 @@ fn add_default_workload_spec_per_update_mask( } // [impl->swdd~cli-supports-yaml-to-set-desired-state~1] -async fn process_inputs(reader: R, state_object_file: &str, temp_obj: &mut Object) { +async fn process_inputs(reader: R, state_object_file: &str) -> Result { match state_object_file { "-" => { - let stdin = io::read_to_string(reader).unwrap_or_else(|error| { - output_and_error!("Could not read the state object file.\nError: {}", error) - }); - let value: serde_yaml::Value = serde_yaml::from_str(&stdin).unwrap_or_else(|error| { - output_and_error!("Could not convert to yaml Value.\nError: {}", error) - }); - *temp_obj = Object::try_from(&value).unwrap_or_else(|error| { - output_and_error!("Could not convert object.\n Error: {}", error) - }); + let stdin = io::read_to_string(reader).map_err(|error| { + CliError::ExecutionError(format!( + "Could not read the state object file.\nError: {}", + error + )) + })?; + let value: serde_yaml::Value = serde_yaml::from_str(&stdin).map_err(|error| { + CliError::YamlSerialization(format!( + "Could not convert to yaml Value.\nError: {}", + error + )) + })?; + let temp_obj = Object::try_from(&value)?; + Ok(temp_obj) } _ => { - let state_object_data = read_file_to_string(state_object_file.to_string()) - .unwrap_or_else(|error| { - output_and_error!("Could not read the state object file.\nError: {}", error) - }); + let state_object_data = + read_file_to_string(state_object_file.to_string()).map_err(|error| { + CliError::ExecutionError(format!( + "Could not read the state object file.\nError: {}", + error + )) + })?; let value: serde_yaml::Value = - serde_yaml::from_str(&state_object_data).unwrap_or_else(|error| { - output_and_error!("Could not convert to yaml Value.\nError: {}", error) - }); - *temp_obj = Object::try_from(&value).unwrap_or_else(|error| { - output_and_error!("Could not convert object.\n Error: {}", error) - }); + serde_yaml::from_str(&state_object_data).map_err(|error| { + CliError::YamlSerialization(format!( + "Could not convert to yaml Value.\nError: {}", + error + )) + })?; + let temp_obj = Object::try_from(&value)?; + Ok(temp_obj) } } } +// [impl->swdd~cli-supports-yaml-to-set-desired-state~1] fn overwrite_using_field_mask( - complete_state_object: &mut Object, + mut complete_state_object: Object, object_field_mask: &Vec, temp_obj: &Object, -) { +) -> Result { for field_mask in object_field_mask { let path: Path = field_mask.into(); @@ -109,21 +120,17 @@ fn overwrite_using_field_mask( }) .clone(), ) - .map_err(|err| CliError::ExecutionError(err.to_string())) - .unwrap_or_else(|error| { - output_and_error!( - "Encountered error while overwritting using field mask. Error: {}", - error - ) - }); + .map_err(|err| CliError::ExecutionError(err.to_string()))?; } + + Ok(complete_state_object) } impl CliCommands { pub async fn set_state( &mut self, object_field_mask: Vec, - state_object_file: Option, + state_object_file: String, ) -> Result<(), CliError> { output_debug!( "Got: object_field_mask={:?} state_object_file={:?}", @@ -132,17 +139,15 @@ impl CliCommands { ); let mut complete_state = CompleteState::default(); - let mut temp_obj: Object = Object::default(); - if let Some(state_object_file) = state_object_file { - process_inputs(io::stdin(), &state_object_file, &mut temp_obj).await; - add_default_workload_spec_per_update_mask(&object_field_mask, &mut complete_state); + let temp_obj = process_inputs(io::stdin(), &state_object_file).await?; + add_default_workload_spec_per_update_mask(&object_field_mask, &mut complete_state); - // now overwrite with the values from the field mask - let mut complete_state_object: Object = complete_state.try_into()?; - overwrite_using_field_mask(&mut complete_state_object, &object_field_mask, &temp_obj); - complete_state = complete_state_object.try_into()?; - } + // now overwrite with the values from the field mask + let mut complete_state_object: Object = complete_state.try_into()?; + complete_state_object = + overwrite_using_field_mask(complete_state_object, &object_field_mask, &temp_obj)?; + complete_state = complete_state_object.try_into()?; output_debug!( "Send UpdateState request with the CompleteState {:?}", @@ -196,6 +201,7 @@ mod tests { image: docker.io/nginx:latest commandOptions: ["-p", "8081:80"]"#; + // [utest->swdd~cli-provides-set-desired-state~1] #[test] fn utest_add_default_workload_spec_empty_update_mask() { let update_mask = vec![]; @@ -206,6 +212,7 @@ mod tests { assert!(complete_state.desired_state.workloads.is_empty()); } + // [utest->swdd~cli-provides-set-desired-state~1] #[test] fn utest_add_default_workload_spec_with_update_mask() { let update_mask = vec![ @@ -228,6 +235,7 @@ mod tests { .contains_key("nginx3")); } + // [utest->swdd~cli-provides-set-desired-state~1] #[test] fn utest_add_default_workload_spec_invalid_path() { let update_mask = vec!["invalid.path".to_string()]; @@ -238,6 +246,7 @@ mod tests { assert!(complete_state.desired_state.workloads.is_empty()); } + // [utest->swdd~cli-provides-set-desired-state~1] #[test] fn utest_overwrite_using_field_mask() { let workload_spec = StoredWorkloadSpec::default(); @@ -253,7 +262,8 @@ mod tests { let temp_object = Object::try_from(&value).unwrap(); let update_mask = vec!["desiredState.workloads.nginx".to_string()]; - overwrite_using_field_mask(&mut complete_state_object, &update_mask, &temp_object); + complete_state_object = + overwrite_using_field_mask(complete_state_object, &update_mask, &temp_object).unwrap(); complete_state = complete_state_object.try_into().unwrap(); @@ -275,9 +285,8 @@ mod tests { let input = SAMPLE_CONFIG; let reader = Cursor::new(input); let state_object_file = "-".to_string(); - let mut temp_obj = Object::default(); - process_inputs(reader, &state_object_file, &mut temp_obj).await; + let temp_obj = process_inputs(reader, &state_object_file).await.unwrap(); let value: Value = serde_yaml::from_str(SAMPLE_CONFIG).unwrap(); let expected_obj = Object::try_from(&value).unwrap(); @@ -289,10 +298,11 @@ mod tests { #[tokio::test] async fn utest_process_inputs_file() { let state_object_file = SAMPLE_CONFIG.to_owned(); - let mut temp_obj = Object::default(); println!("{:?}", state_object_file); - process_inputs(io::empty(), &state_object_file, &mut temp_obj).await; + let temp_obj = process_inputs(io::empty(), &state_object_file) + .await + .unwrap(); println!("{:?}", temp_obj); let value: Value = serde_yaml::from_str(SAMPLE_CONFIG).unwrap(); @@ -308,16 +318,17 @@ mod tests { let input = "invalid yaml"; let reader = Cursor::new(input); let state_object_file = "-".to_string(); - let mut temp_obj = Object::default(); - process_inputs(reader, &state_object_file, &mut temp_obj).await; + let temp_obj = process_inputs(reader, &state_object_file).await; + + assert!(temp_obj.is_ok()); } // [utest->swdd~cli-provides-set-desired-state~1] #[tokio::test] async fn utest_set_state_ok() { let update_mask = vec!["desiredState.workloads.nginx.restartPolicy".to_string()]; - let state_object_file = Some(SAMPLE_CONFIG.to_owned()); + let state_object_file = SAMPLE_CONFIG.to_owned(); let workload_spec = StoredWorkloadSpec { restart_policy: RestartPolicy::Always, From f5e7904e4ddc7753eb829fa87656c0374dac0a76 Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Mon, 19 Aug 2024 07:50:01 +0000 Subject: [PATCH 06/15] Fix review findings --- ank/src/cli_commands/set_state.rs | 58 ++++++++++--------------------- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index 2db44d533..903a5a14c 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -22,30 +22,21 @@ use std::io::{self, Read}; fn read_file_to_string(file: String) -> std::io::Result { std::fs::read_to_string(file) } -use crate::{cli_error::CliError, output_and_error, output_debug}; +use crate::{cli_error::CliError, output_debug}; #[cfg(test)] use tests::read_to_string_mock as read_file_to_string; use super::CliCommands; -fn add_default_workload_spec_per_update_mask( - update_mask: &Vec, - complete_state: &mut CompleteState, -) { +fn create_state_with_default_workload_specs(update_mask: &[String]) -> CompleteState { + let mut complete_state = CompleteState::default(); + for field_mask in update_mask { let path: Path = field_mask.into(); // if we want to set an attribute of a workload create a default object for the workload - if path.parts().len() >= 4 - && path.parts()[0] == "desiredState" - && path.parts()[1] == "workloads" - { - let stored_workload = StoredWorkloadSpec { - agent: "".to_string(), - runtime: "".to_string(), - runtime_config: "".to_string(), - ..Default::default() - }; + if path.parts().len() >= 4 && field_mask.starts_with("desiredState.workloads.") { + let stored_workload = StoredWorkloadSpec::default(); complete_state .desired_state @@ -53,6 +44,8 @@ fn add_default_workload_spec_per_update_mask( .insert(path.parts()[2].to_string(), stored_workload); } } + + complete_state } // [impl->swdd~cli-supports-yaml-to-set-desired-state~1] @@ -61,36 +54,34 @@ async fn process_inputs(reader: R, state_object_file: &str) -> Result { let stdin = io::read_to_string(reader).map_err(|error| { CliError::ExecutionError(format!( - "Could not read the state object file.\nError: {}", + "Could not read the state object from stdin.\nError: '{}'", error )) })?; let value: serde_yaml::Value = serde_yaml::from_str(&stdin).map_err(|error| { CliError::YamlSerialization(format!( - "Could not convert to yaml Value.\nError: {}", + "Could not convert stdin input to yaml.\nError: '{}'", error )) })?; - let temp_obj = Object::try_from(&value)?; - Ok(temp_obj) + Ok(Object::try_from(&value)?) } _ => { let state_object_data = read_file_to_string(state_object_file.to_string()).map_err(|error| { CliError::ExecutionError(format!( - "Could not read the state object file.\nError: {}", + "Could not read the state object file.\nError: '{}'", error )) })?; let value: serde_yaml::Value = serde_yaml::from_str(&state_object_data).map_err(|error| { CliError::YamlSerialization(format!( - "Could not convert to yaml Value.\nError: {}", + "Could not convert state object file to yaml Value.\nError: '{}'", error )) })?; - let temp_obj = Object::try_from(&value)?; - Ok(temp_obj) + Ok(Object::try_from(&value)?) } } } @@ -111,13 +102,7 @@ fn overwrite_using_field_mask( .get(&path) .ok_or(CliError::ExecutionError(format!( "Specified update mask '{field_mask}' not found in the input config.", - ))) - .unwrap_or_else(|error| { - output_and_error!( - "Encountered error while overwritting using field mask. Error: {}", - error - ) - }) + )))? .clone(), ) .map_err(|err| CliError::ExecutionError(err.to_string()))?; @@ -138,10 +123,8 @@ impl CliCommands { state_object_file ); - let mut complete_state = CompleteState::default(); - let temp_obj = process_inputs(io::stdin(), &state_object_file).await?; - add_default_workload_spec_per_update_mask(&object_field_mask, &mut complete_state); + let mut complete_state = create_state_with_default_workload_specs(&object_field_mask); // now overwrite with the values from the field mask let mut complete_state_object: Object = complete_state.try_into()?; @@ -205,9 +188,8 @@ mod tests { #[test] fn utest_add_default_workload_spec_empty_update_mask() { let update_mask = vec![]; - let mut complete_state = CompleteState::default(); - add_default_workload_spec_per_update_mask(&update_mask, &mut complete_state); + let complete_state = create_state_with_default_workload_specs(&update_mask); assert!(complete_state.desired_state.workloads.is_empty()); } @@ -220,9 +202,8 @@ mod tests { "desiredState.workloads.nginx2.restartPolicy".to_string(), "desiredState.workloads.nginx3".to_string(), ]; - let mut complete_state = CompleteState::default(); - add_default_workload_spec_per_update_mask(&update_mask, &mut complete_state); + let complete_state = create_state_with_default_workload_specs(&update_mask); assert!(complete_state.desired_state.workloads.contains_key("nginx")); assert!(complete_state @@ -239,9 +220,8 @@ mod tests { #[test] fn utest_add_default_workload_spec_invalid_path() { let update_mask = vec!["invalid.path".to_string()]; - let mut complete_state = CompleteState::default(); - add_default_workload_spec_per_update_mask(&update_mask, &mut complete_state); + let complete_state = create_state_with_default_workload_specs(&update_mask); assert!(complete_state.desired_state.workloads.is_empty()); } From 930c682c8734daf3150ea0388c76111bbec2ea6b Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Mon, 19 Aug 2024 13:03:17 +0000 Subject: [PATCH 07/15] Add unit test for failing set state --- ank/src/cli_commands/set_state.rs | 65 +++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index 903a5a14c..da86d782b 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -30,18 +30,22 @@ use super::CliCommands; fn create_state_with_default_workload_specs(update_mask: &[String]) -> CompleteState { let mut complete_state = CompleteState::default(); + const WORKLOAD_ATTRIBUTE_LEVEL: usize = 4; + let workload_level_mask_parts = ["desiredState".to_string(), "workloads".to_string()]; + const WORKLOAD_NAME_POSITION: usize = 2; for field_mask in update_mask { let path: Path = field_mask.into(); // if we want to set an attribute of a workload create a default object for the workload - if path.parts().len() >= 4 && field_mask.starts_with("desiredState.workloads.") { - let stored_workload = StoredWorkloadSpec::default(); - - complete_state - .desired_state - .workloads - .insert(path.parts()[2].to_string(), stored_workload); + let mask_parts = path.parts(); + if mask_parts.len() >= WORKLOAD_ATTRIBUTE_LEVEL + && mask_parts.starts_with(&workload_level_mask_parts) + { + complete_state.desired_state.workloads.insert( + path.parts()[WORKLOAD_NAME_POSITION].to_string(), + StoredWorkloadSpec::default(), + ); } } @@ -124,21 +128,21 @@ impl CliCommands { ); let temp_obj = process_inputs(io::stdin(), &state_object_file).await?; - let mut complete_state = create_state_with_default_workload_specs(&object_field_mask); + let default_complete_state = create_state_with_default_workload_specs(&object_field_mask); // now overwrite with the values from the field mask - let mut complete_state_object: Object = complete_state.try_into()?; + let mut complete_state_object: Object = default_complete_state.try_into()?; complete_state_object = overwrite_using_field_mask(complete_state_object, &object_field_mask, &temp_obj)?; - complete_state = complete_state_object.try_into()?; + let new_complete_state = complete_state_object.try_into()?; output_debug!( "Send UpdateState request with the CompleteState {:?}", - complete_state + new_complete_state ); // [impl->swdd~cli-blocks-until-ankaios-server-responds-set-desired-state~2] - self.update_state_and_wait_for_complete(complete_state, object_field_mask) + self.update_state_and_wait_for_complete(new_complete_state, object_field_mask) .await } } @@ -341,4 +345,41 @@ mod tests { let set_state_result = cmd.set_state(update_mask, state_object_file).await; assert!(set_state_result.is_ok()); } + + #[tokio::test] + async fn utest_set_state_failed() { + let wrong_update_mask = vec!["desiredState.workloads.nginx.restartPolicies".to_string()]; + let state_object_file = SAMPLE_CONFIG.to_owned(); + + let workload_spec = StoredWorkloadSpec { + restart_policy: RestartPolicy::Always, + ..Default::default() + }; + let updated_state = CompleteState { + desired_state: State { + workloads: HashMap::from([("nginx".to_string(), workload_spec)]), + ..Default::default() + }, + ..Default::default() + }; + let mut mock_server_connection = MockServerConnection::default(); + mock_server_connection + .expect_update_state() + .with(eq(updated_state), eq(wrong_update_mask.clone())) + .return_once(|_, _| { + Ok(UpdateStateSuccess { + added_workloads: vec![], + deleted_workloads: vec![], + }) + }); + + let mut cmd = CliCommands { + _response_timeout_ms: RESPONSE_TIMEOUT_MS, + no_wait: true, + server_connection: mock_server_connection, + }; + + let set_state_result = cmd.set_state(wrong_update_mask, state_object_file).await; + assert!(set_state_result.is_err()); + } } From 22e61593bac7abb0a59627124e5f3dacf26bebfc Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Tue, 20 Aug 2024 06:54:54 +0000 Subject: [PATCH 08/15] Fix review findings --- ank/src/cli_commands/set_state.rs | 48 +++++++++---------------------- 1 file changed, 13 insertions(+), 35 deletions(-) diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index da86d782b..f7ee246c6 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -43,7 +43,7 @@ fn create_state_with_default_workload_specs(update_mask: &[String]) -> CompleteS && mask_parts.starts_with(&workload_level_mask_parts) { complete_state.desired_state.workloads.insert( - path.parts()[WORKLOAD_NAME_POSITION].to_string(), + mask_parts[WORKLOAD_NAME_POSITION].to_string(), StoredWorkloadSpec::default(), ); } @@ -116,6 +116,7 @@ fn overwrite_using_field_mask( } impl CliCommands { + // [impl->swdd-cli-supports-yaml-to-set-desired-state~1] pub async fn set_state( &mut self, object_field_mask: Vec, @@ -156,11 +157,13 @@ impl CliCommands { ////////////////////////////////////////////////////////////////////////////// #[cfg(test)] mod tests { - use super::*; + use super::{ + create_state_with_default_workload_specs, io, overwrite_using_field_mask, process_inputs, + CliCommands, StoredWorkloadSpec, + }; use crate::cli_commands::server_connection::MockServerConnection; use api::ank_base::UpdateStateSuccess; use common::{ - // commands::UpdateStateSuccess, objects::{CompleteState, RestartPolicy, State}, state_manipulation::Object, }; @@ -190,7 +193,7 @@ mod tests { // [utest->swdd~cli-provides-set-desired-state~1] #[test] - fn utest_add_default_workload_spec_empty_update_mask() { + fn utest_create_state_with_default_workload_specs_empty_update_mask() { let update_mask = vec![]; let complete_state = create_state_with_default_workload_specs(&update_mask); @@ -200,7 +203,7 @@ mod tests { // [utest->swdd~cli-provides-set-desired-state~1] #[test] - fn utest_add_default_workload_spec_with_update_mask() { + fn utest_create_state_with_default_workload_specs_with_update_mask() { let update_mask = vec![ "desiredState.workloads.nginx.restartPolicy".to_string(), "desiredState.workloads.nginx2.restartPolicy".to_string(), @@ -222,7 +225,7 @@ mod tests { // [utest->swdd~cli-provides-set-desired-state~1] #[test] - fn utest_add_default_workload_spec_invalid_path() { + fn utest_create_state_with_default_workload_specs_invalid_path() { let update_mask = vec!["invalid.path".to_string()]; let complete_state = create_state_with_default_workload_specs(&update_mask); @@ -282,16 +285,13 @@ mod tests { #[tokio::test] async fn utest_process_inputs_file() { let state_object_file = SAMPLE_CONFIG.to_owned(); - println!("{:?}", state_object_file); let temp_obj = process_inputs(io::empty(), &state_object_file) .await .unwrap(); - println!("{:?}", temp_obj); let value: Value = serde_yaml::from_str(SAMPLE_CONFIG).unwrap(); let expected_obj = Object::try_from(&value).unwrap(); - println!("{:?}", expected_obj); assert_eq!(temp_obj, expected_obj); } @@ -329,12 +329,7 @@ mod tests { mock_server_connection .expect_update_state() .with(eq(updated_state), eq(update_mask.clone())) - .return_once(|_, _| { - Ok(UpdateStateSuccess { - added_workloads: vec![], - deleted_workloads: vec![], - }) - }); + .return_once(|_, _| Ok(UpdateStateSuccess::default())); let mut cmd = CliCommands { _response_timeout_ms: RESPONSE_TIMEOUT_MS, @@ -346,32 +341,15 @@ mod tests { assert!(set_state_result.is_ok()); } + // [utest->swdd~cli-provides-set-desired-state~1] #[tokio::test] async fn utest_set_state_failed() { - let wrong_update_mask = vec!["desiredState.workloads.nginx.restartPolicies".to_string()]; + let wrong_update_mask = vec!["desiredState.workloads.notExistingWorkload".to_string()]; let state_object_file = SAMPLE_CONFIG.to_owned(); - - let workload_spec = StoredWorkloadSpec { - restart_policy: RestartPolicy::Always, - ..Default::default() - }; - let updated_state = CompleteState { - desired_state: State { - workloads: HashMap::from([("nginx".to_string(), workload_spec)]), - ..Default::default() - }, - ..Default::default() - }; let mut mock_server_connection = MockServerConnection::default(); mock_server_connection .expect_update_state() - .with(eq(updated_state), eq(wrong_update_mask.clone())) - .return_once(|_, _| { - Ok(UpdateStateSuccess { - added_workloads: vec![], - deleted_workloads: vec![], - }) - }); + .return_once(|_, _| Ok(UpdateStateSuccess::default())); let mut cmd = CliCommands { _response_timeout_ms: RESPONSE_TIMEOUT_MS, From c36cb50c9a5d84b1c0c5e6353b151f96bb0f4306 Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Tue, 20 Aug 2024 14:18:36 +0300 Subject: [PATCH 09/15] Improve error message --- ank/src/cli_commands/set_state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index f7ee246c6..207395233 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -74,14 +74,14 @@ async fn process_inputs(reader: R, state_object_file: &str) -> Result Date: Tue, 20 Aug 2024 14:21:09 +0300 Subject: [PATCH 10/15] Link swdd Co-authored-by: Oliver <42932060+inf17101@users.noreply.github.com> --- ank/src/cli_commands/set_state.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index 207395233..469cf7d0a 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -28,6 +28,7 @@ use tests::read_to_string_mock as read_file_to_string; use super::CliCommands; +// [impl->swdd~cli-supports-yaml-to-set-desired-state~1] fn create_state_with_default_workload_specs(update_mask: &[String]) -> CompleteState { let mut complete_state = CompleteState::default(); const WORKLOAD_ATTRIBUTE_LEVEL: usize = 4; From f4937ec708ff8e112191c5df4eb076b07577db3d Mon Sep 17 00:00:00 2001 From: HorjuRares Date: Tue, 20 Aug 2024 13:14:12 +0000 Subject: [PATCH 11/15] Update documentation --- doc/docs/reference/complete-state.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/docs/reference/complete-state.md b/doc/docs/reference/complete-state.md index bae31c2e2..45b55ae47 100644 --- a/doc/docs/reference/complete-state.md +++ b/doc/docs/reference/complete-state.md @@ -125,7 +125,7 @@ The object field mask can be constructed using the field names of the [CompleteS commandOptions: ["-p", "8081:80"] ``` -3. Example `ank set state desiredState.workloads.nginx.restartPolicy new-state.yaml` changes the restart behavior of nginx workload to `NEVER`: +3. Example `ank -k set state desiredState.workloads.nginx.restartPolicy new-state.yaml` changes the restart behavior of nginx workload to `NEVER`: ```yaml title="new-state.yaml" desiredState: From b8d2999e17ea585786a9efe23d21f222a5d15a9f Mon Sep 17 00:00:00 2001 From: Oliver <42932060+inf17101@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:42:55 +0200 Subject: [PATCH 12/15] Remove linkage in main set state --- ank/src/main.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ank/src/main.rs b/ank/src/main.rs index e1777a805..7d8a04246 100644 --- a/ank/src/main.rs +++ b/ank/src/main.rs @@ -124,8 +124,7 @@ async fn main() { object_field_mask, state_object_file ); - // [impl -> swdd~cli-provides-set-desired-state~1] - // [impl -> swdd~cli-blocks-until-ankaios-server-responds-set-desired-state~2] + if let Err(err) = cmd.set_state(object_field_mask, state_object_file).await { output_and_error!("Failed to set state: '{}'", err) } From 809370a450c234d550f879c44ad62785439907cb Mon Sep 17 00:00:00 2001 From: Oliver <42932060+inf17101@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:44:55 +0200 Subject: [PATCH 13/15] Adapt linkage in set_state --- ank/src/cli_commands/set_state.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index 469cf7d0a..32669773a 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -28,7 +28,6 @@ use tests::read_to_string_mock as read_file_to_string; use super::CliCommands; -// [impl->swdd~cli-supports-yaml-to-set-desired-state~1] fn create_state_with_default_workload_specs(update_mask: &[String]) -> CompleteState { let mut complete_state = CompleteState::default(); const WORKLOAD_ATTRIBUTE_LEVEL: usize = 4; @@ -91,7 +90,6 @@ async fn process_inputs(reader: R, state_object_file: &str) -> Resultswdd~cli-supports-yaml-to-set-desired-state~1] fn overwrite_using_field_mask( mut complete_state_object: Object, object_field_mask: &Vec, @@ -117,7 +115,7 @@ fn overwrite_using_field_mask( } impl CliCommands { - // [impl->swdd-cli-supports-yaml-to-set-desired-state~1] + // [impl->swdd~cli-provides-set-desired-state~1] pub async fn set_state( &mut self, object_field_mask: Vec, From abd5d1982284e1b38c36eda9b532771590daee58 Mon Sep 17 00:00:00 2001 From: Oliver <42932060+inf17101@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:50:40 +0200 Subject: [PATCH 14/15] Revert part of linkage in main --- ank/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ank/src/main.rs b/ank/src/main.rs index 7d8a04246..f5ddab5b2 100644 --- a/ank/src/main.rs +++ b/ank/src/main.rs @@ -125,6 +125,7 @@ async fn main() { state_object_file ); + // [impl->swdd~cli-blocks-until-ankaios-server-responds-set-desired-state~2] if let Err(err) = cmd.set_state(object_field_mask, state_object_file).await { output_and_error!("Failed to set state: '{}'", err) } From 0fbd89f19186d74cc9b753830279577bc89fb48c Mon Sep 17 00:00:00 2001 From: RaresHorju <36081886+HorjuRares@users.noreply.github.com> Date: Wed, 21 Aug 2024 08:49:49 +0300 Subject: [PATCH 15/15] Update unit test Co-authored-by: Oliver <42932060+inf17101@users.noreply.github.com> --- ank/src/cli_commands/set_state.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/ank/src/cli_commands/set_state.rs b/ank/src/cli_commands/set_state.rs index 32669773a..bbb9f0b76 100644 --- a/ank/src/cli_commands/set_state.rs +++ b/ank/src/cli_commands/set_state.rs @@ -211,11 +211,15 @@ mod tests { let complete_state = create_state_with_default_workload_specs(&update_mask); - assert!(complete_state.desired_state.workloads.contains_key("nginx")); - assert!(complete_state - .desired_state - .workloads - .contains_key("nginx2")); + assert_eq!( + complete_state.desired_state.workloads.get("nginx"), + Some(&StoredWorkloadSpec::default()) + ); + + assert_eq!( + complete_state.desired_state.workloads.get("nginx2"), + Some(&StoredWorkloadSpec::default()) + ); assert!(!complete_state .desired_state .workloads