diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 699a7862d..f939e30b3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -58,7 +58,7 @@ jobs: - name: Run unit tests run: RUST_LOG=debug cargo nextest run - name: Run clippy code checks - run: cargo clippy --all-targets --no-deps --all-features -- -D warnings + run: just clippy - name: Prevent docker.io images in test run: just check-test-images - name: Check copyright headers @@ -89,7 +89,7 @@ jobs: - name: Create code coverage html report run: | rustup component add llvm-tools-preview - tools/generate_test_coverage_report.sh test --html + just coverage - uses: actions/upload-artifact@v4.3.3 with: name: code-coverage @@ -163,8 +163,7 @@ jobs: steps: - uses: actions/checkout@v4.1.1 - run: | - mkdir -p dist - oft trace $(find . -type d -name "src" -o -name "doc") -a swdd,utest,itest,stest,impl -o html -f dist/req_tracing_report.html || true + just trace-requirements dist/req_tracing_report.html - uses: actions/upload-artifact@v4.3.3 with: name: requirement-tracing-report diff --git a/.vscode/launch.json b/.vscode/launch.json index 5a523a74d..d14cdcddf 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -38,7 +38,7 @@ "kind": "bin" } }, - "args": [], + "args": ["--name", "agent_A"], "cwd": "${workspaceFolder}" }, { diff --git a/agent/doc/swdesign/README.md b/agent/doc/swdesign/README.md index 6a30777a5..da598ed69 100644 --- a/agent/doc/swdesign/README.md +++ b/agent/doc/swdesign/README.md @@ -750,11 +750,11 @@ Needs: - utest ##### RuntimeManager handles existing workloads replace updated Workloads -`swdd~agent-existing-workloads-replace-updated~2` +`swdd~agent-existing-workloads-replace-updated~3` Status: approved -When the agent handles existing workloads, for each found existing workload which is requested to be started and either the workload's configuration has changed or the workload is not running, the RuntimeManager shall do the following: +When the agent handles existing workloads, for each found existing workload which is requested to be started and either the workload's configuration has changed or the workload is not in state running or succeeded, the RuntimeManager shall do the following: - request the RuntimeFacade to delete the existing workload - request the RuntimeFacade to create the workload @@ -771,6 +771,23 @@ Needs: - utest - stest +##### RuntimeManager handles existing workloads and reuses unmodified Workloads +`swdd~agent-existing-workloads-reuse-unmodified~1` + +Status: approved + +When the agent handles existing workloads, for each found existing workload which is requested to be started and the workload's configuration has not changed and the workload is in state succeeded, the RuntimeManager shall request the RuntimeFacade to reuse the existing workload. + +Rationale: Starting an existing, succeeded workload is much faster than deleting and creating a workload. If an existing workload is in the failed state, it is not reused because its file system might be corrupted. + +Tags: +- RuntimeManager + +Needs: +- impl +- utest +- stest + ##### RuntimeManager handles existing workloads deletes unneeded workloads `swdd~agent-existing-workloads-delete-unneeded~1` @@ -1837,11 +1854,11 @@ Needs: - utest ##### Podman create workload runs the workload object -`swdd~podman-create-workload-runs-workload~1` +`swdd~podman-create-workload-runs-workload~2` Status: approved -When the podman runtime connector is called to create workload, the podman runtime connector shall: +When the podman runtime connector is called to create a workload and no existing workload id is provided, the podman runtime connector shall: * pull the workload image specified in the runtime configuration if the image is not already available locally * create the container @@ -1856,6 +1873,27 @@ Needs: - utest - stest +##### Podman create workload starts an existing the workload object +`swdd~podman-create-workload-starts-existing-workload~1` + +Status: approved + +When the podman runtime connector is called to create a workload and an existing workload id is provided, the podman runtime connector shall: + +* start the existing container +* start a `GenericPollingStateChecker` to check the workload state + +Rationale: +Starting a stopped container is much faster than creating a new container bundle and starting that. Short startup times are ususally crucial for automotive. + +Tags: +- PodmanRuntimeConnector + +Needs: +- impl +- utest +- stest + ##### Podman create workload returns workload id `swdd~podman-create-workload-returns-workload-id~1` @@ -1891,11 +1929,11 @@ Needs: - utest ##### Podman create workload creates labels -`swdd~podman-create-workload-creates-labels~1` +`swdd~podman-create-workload-creates-labels~2` Status: approved -When the podman runtime connector is called to create workload, the podman runtime connector shall create following labels in the workload: +When the podman runtime connector is called without an existing workload id to create a new workload, the podman runtime connector shall create following labels in the workload: * `name` as the key and workload execution name as the value * `agent` as the key and the agent name where the workload is being created as the value @@ -1908,11 +1946,11 @@ Needs: - utest ##### Podman create workload sets optionally container name -`swdd~podman-create-workload-sets-optionally-container-name~1` +`swdd~podman-create-workload-sets-optionally-container-name~2` Status: approved -When the podman runtime connector is called to create workload and the workload name is not set in the runtime configuration, +When the podman runtime connector is called is called without an existing workload id to create a new workload and the workload name is not set in the runtime configuration, the podman runtime connector shall set the workload execution name as the workload name. Tags: diff --git a/agent/src/generic_polling_state_checker.rs b/agent/src/generic_polling_state_checker.rs index eca07ced4..98d6e6e89 100644 --- a/agent/src/generic_polling_state_checker.rs +++ b/agent/src/generic_polling_state_checker.rs @@ -13,7 +13,7 @@ // SPDX-License-Identifier: Apache-2.0 use async_trait::async_trait; -use std::time::Duration; +use std::{str::FromStr, time::Duration}; use tokio::{task::JoinHandle, time}; use crate::{ @@ -34,7 +34,7 @@ pub struct GenericPollingStateChecker { #[async_trait] impl StateChecker for GenericPollingStateChecker where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, { // [impl->swdd~agent-provides-generic-state-checker-implementation~1] fn start_checker( diff --git a/agent/src/runtime_connectors/mod.rs b/agent/src/runtime_connectors/mod.rs index cb0354c4f..f8fdb5659 100644 --- a/agent/src/runtime_connectors/mod.rs +++ b/agent/src/runtime_connectors/mod.rs @@ -21,7 +21,9 @@ pub(crate) mod podman; pub(crate) mod podman_kube; mod runtime_connector; -pub use runtime_connector::{OwnableRuntime, RuntimeConnector, RuntimeError}; +pub use runtime_connector::{ + OwnableRuntime, ReusableWorkloadState, RuntimeConnector, RuntimeError, +}; #[cfg(test)] pub use runtime_connector::test; diff --git a/agent/src/runtime_connectors/podman/podman_runtime.rs b/agent/src/runtime_connectors/podman/podman_runtime.rs index 609de0bc9..45841bbc2 100644 --- a/agent/src/runtime_connectors/podman/podman_runtime.rs +++ b/agent/src/runtime_connectors/podman/podman_runtime.rs @@ -12,18 +12,21 @@ // // SPDX-License-Identifier: Apache-2.0 -use std::{fmt::Display, path::PathBuf}; +use std::{fmt::Display, path::PathBuf, str::FromStr}; use async_trait::async_trait; use common::{ - objects::{AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec, WorkloadState}, + objects::{AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec}, std_extensions::UnreachableOption, }; use crate::{ generic_polling_state_checker::GenericPollingStateChecker, - runtime_connectors::{RuntimeConnector, RuntimeError, RuntimeStateGetter, StateChecker}, + runtime_connectors::{ + podman_cli::PodmanStartConfig, ReusableWorkloadState, RuntimeConnector, RuntimeError, + RuntimeStateGetter, StateChecker, + }, workload_state::WorkloadStateSender, }; @@ -55,6 +58,13 @@ impl Display for PodmanWorkloadId { } } +impl FromStr for PodmanWorkloadId { + type Err = String; + fn from_str(s: &str) -> Result { + Ok(PodmanWorkloadId { id: s.to_string() }) + } +} + #[async_trait] // [impl->swdd~podman-implements-runtime-state-getter~1] impl RuntimeStateGetter for PodmanStateGetter { @@ -95,15 +105,16 @@ impl PodmanRuntime { async fn workload_instance_names_to_workload_states( &self, workload_instance_names: &Vec, - ) -> Result, RuntimeError> { - let mut workload_states = Vec::::default(); + ) -> Result, RuntimeError> { + let mut workload_states = Vec::::default(); for instance_name in workload_instance_names { - match PodmanCli::list_states_by_id(&self.get_workload_id(instance_name).await?.id).await - { - Ok(Some(execution_state)) => workload_states.push(WorkloadState { - instance_name: instance_name.clone(), + let workload_id = &self.get_workload_id(instance_name).await?.id; + match PodmanCli::list_states_by_id(workload_id).await { + Ok(Some(execution_state)) => workload_states.push(ReusableWorkloadState::new( + instance_name.clone(), execution_state, - }), + Some(workload_id.to_string()), + )), Ok(None) => { return Err(RuntimeError::List(format!( "Could not get execution state for workload '{}'", @@ -128,7 +139,7 @@ impl RuntimeConnector for PodmanRu async fn get_reusable_workloads( &self, agent_name: &AgentName, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { // [impl->swdd~podman-list-of-existing-workloads-uses-labels~1] let res = PodmanCli::list_workload_names_by_label("agent", agent_name.get()) .await @@ -145,24 +156,39 @@ impl RuntimeConnector for PodmanRu .await } - // [impl->swdd~podman-create-workload-runs-workload~1] + // [impl->swdd~podman-create-workload-runs-workload~2] + // [impl->swdd~podman-create-workload-starts-existing-workload~1] async fn create_workload( &self, workload_spec: WorkloadSpec, + reusable_workload_id: Option, control_interface_path: Option, update_state_tx: WorkloadStateSender, ) -> Result<(PodmanWorkloadId, GenericPollingStateChecker), RuntimeError> { let workload_cfg = PodmanRuntimeConfig::try_from(&workload_spec) .map_err(|err| RuntimeError::Create(err.into()))?; - match PodmanCli::podman_run( - workload_cfg.into(), - &workload_spec.instance_name.to_string(), - workload_spec.instance_name.agent_name(), - control_interface_path, - ) - .await - { + let cli_result = match reusable_workload_id { + Some(workload_id) => { + let start_config = PodmanStartConfig { + general_options: workload_cfg.general_options, + container_id: workload_id.id, + }; + PodmanCli::podman_start(start_config, &workload_spec.instance_name.to_string()) + .await + } + None => { + PodmanCli::podman_run( + workload_cfg.into(), + &workload_spec.instance_name.to_string(), + workload_spec.instance_name.agent_name(), + control_interface_path, + ) + .await + } + }; + + match cli_result { Ok(workload_id) => { log::debug!( "The workload '{}' has been created with internal id '{}'", @@ -180,7 +206,7 @@ impl RuntimeConnector for PodmanRu } Err(err) => { // [impl->swdd~podman-create-workload-deletes-failed-container~1] - log::debug!("Creating container failed, cleaning up. Error: '{err}'"); + log::debug!("Creating/starting container failed, cleaning up. Error: '{err}'"); match PodmanCli::remove_workloads_by_id(&workload_spec.instance_name.to_string()) .await { @@ -266,6 +292,7 @@ impl RuntimeConnector for PodmanRu #[cfg(test)] mod tests { use std::path::PathBuf; + use std::str::FromStr; use common::objects::{ generate_test_workload_spec_with_param, AgentName, ExecutionState, WorkloadInstanceName, @@ -327,10 +354,9 @@ mod tests { .await .unwrap(); - assert_eq!(res.len(), 2); assert_eq!( res.iter() - .map(|x| x.instance_name.clone()) + .map(|x| x.workload_state.instance_name.clone()) .collect::>(), vec![ "container1.hash.dummy_agent".try_into().unwrap(), @@ -376,7 +402,7 @@ mod tests { ); } - // [utest->swdd~podman-create-workload-runs-workload~1] + // [utest->swdd~podman-create-workload-runs-workload~2] #[tokio::test] async fn utest_create_workload_success() { let _guard = MOCKALL_CONTEXT_SYNC.get_lock_async().await; @@ -398,6 +424,7 @@ mod tests { let res = podman_runtime .create_workload( workload_spec, + None, Some(PathBuf::from("run_folder")), state_change_tx, ) @@ -409,6 +436,44 @@ mod tests { assert_eq!(workload_id.id, "test_id".to_string()); } + // [utest->swdd~podman-create-workload-starts-existing-workload~1] + #[tokio::test] + async fn utest_create_workload_with_existing_workload_id_success() { + let _guard = MOCKALL_CONTEXT_SYNC.get_lock_async().await; + + let reusable_workload_id = "test_id"; + + let start_context = PodmanCli::podman_start_context(); + start_context + .expect() + .returning(|start_config, _| Ok(start_config.container_id)); + + let resest_cache_context = PodmanCli::reset_ps_cache_context(); + resest_cache_context.expect().return_const(()); + + let workload_spec = generate_test_workload_spec_with_param( + AGENT_NAME.to_string(), + WORKLOAD_1_NAME.to_string(), + PODMAN_RUNTIME_NAME.to_string(), + ); + let (state_change_tx, _state_change_rx) = tokio::sync::mpsc::channel(BUFFER_SIZE); + + let podman_runtime = PodmanRuntime {}; + let res = podman_runtime + .create_workload( + workload_spec, + Some(PodmanWorkloadId::from_str(reusable_workload_id).unwrap()), + Some(PathBuf::from("run_folder")), + state_change_tx, + ) + .await; + + let (workload_id, _checker) = res.unwrap(); + + // [utest->swdd~podman-create-workload-returns-workload-id~1] + assert_eq!(workload_id.id, reusable_workload_id); + } + // [utest->swdd~podman-state-getter-reset-cache~1] #[tokio::test] async fn utest_state_getter_resets_cache() { @@ -444,6 +509,7 @@ mod tests { let res = podman_runtime .create_workload( workload_spec, + None, Some(PathBuf::from("run_folder")), state_change_tx, ) @@ -499,6 +565,7 @@ mod tests { let res = podman_runtime .create_workload( workload_spec, + None, Some(PathBuf::from("run_folder")), state_change_tx, ) @@ -533,6 +600,7 @@ mod tests { let res = podman_runtime .create_workload( workload_spec, + None, Some(PathBuf::from("run_folder")), state_change_tx, ) @@ -558,6 +626,7 @@ mod tests { let res = podman_runtime .create_workload( workload_spec, + None, Some(PathBuf::from("run_folder")), state_change_tx, ) diff --git a/agent/src/runtime_connectors/podman_cli.rs b/agent/src/runtime_connectors/podman_cli.rs index 4aa0869f5..4861a4865 100644 --- a/agent/src/runtime_connectors/podman_cli.rs +++ b/agent/src/runtime_connectors/podman_cli.rs @@ -51,6 +51,12 @@ pub struct PodmanRunConfig { pub command_args: Vec, } +#[derive(Debug, PartialEq, Eq)] +pub struct PodmanStartConfig { + pub general_options: Vec, + pub container_id: String, +} + impl From for ContainerState { fn from(value: PodmanContainerInfo) -> Self { match value.state.to_lowercase().as_str() { @@ -319,7 +325,7 @@ impl PodmanCli { // We store workload name as a label (and use them from there). // Therefore we do insist on container names in particular format. // - // [impl->swdd~podman-create-workload-sets-optionally-container-name~1] + // [impl->swdd~podman-create-workload-sets-optionally-container-name~2] args.append(&mut vec!["--name".into(), workload_name.to_string()]); args.append(&mut run_config.command_options); @@ -337,7 +343,7 @@ impl PodmanCli { ); } - // [impl->swdd~podman-create-workload-creates-labels~1] + // [impl->swdd~podman-create-workload-creates-labels~2] args.push(format!("--label=name={workload_name}")); args.push(format!("--label=agent={agent}")); args.push(run_config.image); @@ -354,6 +360,31 @@ impl PodmanCli { Ok(id) } + pub async fn podman_start( + start_config: PodmanStartConfig, + workload_name: &str, + ) -> Result { + log::debug!( + "Starting the workload '{}' with id '{}'", + workload_name, + start_config.container_id + ); + + let mut args = start_config.general_options; + + args.push("start".into()); + + args.push(start_config.container_id); + + let id = CliCommand::new(PODMAN_CMD) + .args(&args.iter().map(|x| &**x).collect::>()) + .exec() + .await? + .trim() + .to_string(); + Ok(id) + } + // [impl->swdd~podmancli-uses-container-state-cache~1] pub async fn list_states_by_id(workload_id: &str) -> Result, String> { let ps_result = LAST_PS_RESULT.get().await; @@ -968,8 +999,8 @@ mod tests { assert!(matches!(res, Err(msg) if msg.starts_with("Could not parse podman output") )); } - // [utest->swdd~podman-create-workload-creates-labels~1] - // [utest->swdd~podman-create-workload-sets-optionally-container-name~1] + // [utest->swdd~podman-create-workload-creates-labels~2] + // [utest->swdd~podman-create-workload-sets-optionally-container-name~2] // [utest->swdd~podman-create-workload-mounts-fifo-files~1] #[tokio::test] async fn utest_run_container_success_no_options() { @@ -1031,7 +1062,7 @@ mod tests { assert!(matches!(res, Err(msg) if msg == SAMPLE_ERROR_MESSAGE)); } - // [utest->swdd~podman-create-workload-sets-optionally-container-name~1] + // [utest->swdd~podman-create-workload-sets-optionally-container-name~2] // [utest->swdd~podman-create-workload-mounts-fifo-files~1] #[tokio::test] async fn utest_run_container_success_with_options() { @@ -1075,6 +1106,52 @@ mod tests { assert_eq!(res, Ok("test_id".to_string())); } + // [utest->swdd~podman-create-workload-starts-existing-workload~1] + #[tokio::test] + async fn utest_start_container_success() { + let _guard = MOCKALL_CONTEXT_SYNC.get_lock_async().await; + + const ID: &str = "test_id"; + + super::CliCommand::reset(); + super::CliCommand::new_expect( + "podman", + super::CliCommand::default() + .expect_args(&["--remote", "start", ID]) + .exec_returns(Ok(ID.to_string())), + ); + + let start_config = super::PodmanStartConfig { + general_options: vec!["--remote".into()], + container_id: ID.into(), + }; + let res = PodmanCli::podman_start(start_config, "test_workload_name").await; + assert_eq!(res, Ok(ID.to_string())); + } + + // [utest->swdd~podman-create-workload-starts-existing-workload~1] + #[tokio::test] + async fn utest_start_container_fail() { + let _guard = MOCKALL_CONTEXT_SYNC.get_lock_async().await; + + static ID: &str = "unknown_id"; + + super::CliCommand::reset(); + super::CliCommand::new_expect( + "podman", + super::CliCommand::default() + .expect_args(&["start", ID]) + .exec_returns(Err(SAMPLE_ERROR_MESSAGE.into())), + ); + + let start_config = super::PodmanStartConfig { + general_options: vec![], + container_id: ID.into(), + }; + let res = PodmanCli::podman_start(start_config, "test_workload_name").await; + assert_eq!(res, Err(SAMPLE_ERROR_MESSAGE.to_string())); + } + // [utest->swdd~podman-state-getter-maps-state~3] // [utest->swdd~podmancli-container-state-cache-refresh~1] #[tokio::test] diff --git a/agent/src/runtime_connectors/podman_kube/podman_kube_runtime.rs b/agent/src/runtime_connectors/podman_kube/podman_kube_runtime.rs index c6ef5abca..f2b3d6530 100644 --- a/agent/src/runtime_connectors/podman_kube/podman_kube_runtime.rs +++ b/agent/src/runtime_connectors/podman_kube/podman_kube_runtime.rs @@ -12,11 +12,9 @@ // // SPDX-License-Identifier: Apache-2.0 -use std::{cmp::min, fmt::Display, path::PathBuf}; +use std::{cmp::min, fmt::Display, path::PathBuf, str::FromStr}; -use common::objects::{ - AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec, WorkloadState, -}; +use common::objects::{AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec}; use async_trait::async_trait; use futures_util::TryFutureExt; @@ -30,7 +28,8 @@ use crate::runtime_connectors::podman_cli::PodmanCli; use crate::{ generic_polling_state_checker::GenericPollingStateChecker, runtime_connectors::{ - podman_cli, RuntimeConnector, RuntimeError, RuntimeStateGetter, StateChecker, + podman_cli, ReusableWorkloadState, RuntimeConnector, RuntimeError, RuntimeStateGetter, + StateChecker, }, workload_state::WorkloadStateSender, }; @@ -65,20 +64,30 @@ impl Display for PodmanKubeWorkloadId { } } +impl FromStr for PodmanKubeWorkloadId { + type Err = String; + fn from_str(_s: &str) -> Result { + // Not supported as we cannot restart workloads + Err("Not supported for PodmanKubeWorkloadId".to_string()) + } +} + impl PodmanKubeRuntime { async fn workload_instance_names_to_workload_states( &self, workload_instance_names: &Vec, - ) -> Result, RuntimeError> { - let mut workload_states = Vec::::default(); + ) -> Result, RuntimeError> { + let mut workload_states = Vec::::default(); for instance_name in workload_instance_names { let execution_state = self .get_state(&self.get_workload_id(instance_name).await?) .await; - workload_states.push(WorkloadState { - instance_name: instance_name.clone(), + workload_states.push(ReusableWorkloadState::new( + instance_name.clone(), execution_state, - }); + // For the podman-kube runtime we cannot recreate/restart workloads and thus return None as as workload_id + None, + )); } Ok(workload_states) } @@ -96,7 +105,7 @@ impl RuntimeConnector for Podm async fn get_reusable_workloads( &self, agent_name: &AgentName, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let name_filter = format!( "{}{}$", agent_name.get_filter_suffix(), @@ -133,6 +142,7 @@ impl RuntimeConnector for Podm async fn create_workload( &self, workload_spec: WorkloadSpec, + _reusable_workload_id: Option, _control_interface_path: Option, update_state_tx: WorkloadStateSender, ) -> Result<(PodmanKubeWorkloadId, GenericPollingStateChecker), RuntimeError> { @@ -469,8 +479,24 @@ mod tests { let workloads = runtime.get_reusable_workloads(&SAMPLE_AGENT.into()).await; - assert!( - matches!(workloads, Ok(res) if res.iter().map(|x| x.instance_name.clone()).collect::>() == [workload_instance_1.try_into().unwrap(), workload_instance_2.try_into().unwrap()]) + let workloads = workloads.unwrap(); + + assert_eq!( + workloads + .iter() + .filter(|&x| x.workload_id.is_some()) + .count(), + 0 + ); + assert_eq!( + workloads + .iter() + .map(|x| x.workload_state.instance_name.clone()) + .collect::>(), + [ + workload_instance_1.try_into().unwrap(), + workload_instance_2.try_into().unwrap() + ] ); } @@ -515,7 +541,7 @@ mod tests { let workloads = runtime.get_reusable_workloads(&SAMPLE_AGENT.into()).await; assert!( - matches!(workloads, Ok(res) if res.iter().map(|x| x.instance_name.clone()).collect::>() == [workload_instance.try_into().unwrap()]) + matches!(workloads, Ok(res) if res.iter().map(|x| x.workload_state.instance_name.clone()).collect::>() == [workload_instance.try_into().unwrap()]) ); } @@ -555,7 +581,7 @@ mod tests { println!("{:?}", workloads); assert!( - matches!(workloads, Ok(res) if res.iter().map(|x| x.instance_name.clone()).collect::>() == [workload_instance.try_into().unwrap()]) + matches!(workloads, Ok(res) if res.iter().map(|x| x.workload_state.instance_name.clone()).collect::>() == [workload_instance.try_into().unwrap()]) ); } @@ -600,7 +626,9 @@ mod tests { ); let (sender, _) = tokio::sync::mpsc::channel(1); - let workload = runtime.create_workload(workload_spec, None, sender).await; + let workload = runtime + .create_workload(workload_spec, None, None, sender) + .await; // [utest->swdd~podman-kube-create-workload-returns-workload-id~1] assert!(matches!(workload, Ok((workload_id, _)) if workload_id.name == *WORKLOAD_INSTANCE_NAME && @@ -648,7 +676,9 @@ mod tests { ); let (sender, _) = tokio::sync::mpsc::channel(1); - let workload = runtime.create_workload(workload_spec, None, sender).await; + let workload = runtime + .create_workload(workload_spec, None, None, sender) + .await; assert!(matches!(workload, Ok((workload_id, _)) if workload_id.name == *WORKLOAD_INSTANCE_NAME && workload_id.manifest == SAMPLE_KUBE_CONFIG && @@ -695,7 +725,9 @@ mod tests { ); let (sender, _) = tokio::sync::mpsc::channel(1); - let workload = runtime.create_workload(workload_spec, None, sender).await; + let workload = runtime + .create_workload(workload_spec, None, None, sender) + .await; assert!(matches!(workload, Ok((workload_id, _)) if workload_id.name == *WORKLOAD_INSTANCE_NAME && workload_id.manifest == SAMPLE_KUBE_CONFIG && @@ -756,7 +788,9 @@ mod tests { ); let (sender, mut receiver) = tokio::sync::mpsc::channel(1); - let _workload = runtime.create_workload(workload_spec, None, sender).await; + let _workload = runtime + .create_workload(workload_spec, None, None, sender) + .await; receiver.recv().await; } @@ -791,7 +825,9 @@ mod tests { ); let (sender, _) = tokio::sync::mpsc::channel(1); - let workload = runtime.create_workload(workload_spec, None, sender).await; + let workload = runtime + .create_workload(workload_spec, None, None, sender) + .await; assert!(matches!(workload, Err(RuntimeError::Create(msg)) if msg == SAMPLE_ERROR)); } diff --git a/agent/src/runtime_connectors/runtime_connector.rs b/agent/src/runtime_connectors/runtime_connector.rs index 69b87d8a5..f2fbf5478 100644 --- a/agent/src/runtime_connectors/runtime_connector.rs +++ b/agent/src/runtime_connectors/runtime_connector.rs @@ -12,11 +12,13 @@ // // SPDX-License-Identifier: Apache-2.0 -use std::{fmt::Display, path::PathBuf}; +use std::{fmt::Display, path::PathBuf, str::FromStr}; use async_trait::async_trait; -use common::objects::{AgentName, WorkloadInstanceName, WorkloadSpec, WorkloadState}; +use common::objects::{ + AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec, WorkloadState, +}; use crate::{runtime_connectors::StateChecker, workload_state::WorkloadStateSender}; @@ -43,23 +45,46 @@ impl Display for RuntimeError { } } +#[derive(Debug, PartialEq)] +pub struct ReusableWorkloadState { + pub workload_state: WorkloadState, + pub workload_id: Option, +} + +impl ReusableWorkloadState { + pub fn new( + instance_name: WorkloadInstanceName, + execution_state: ExecutionState, + workload_id: Option, + ) -> ReusableWorkloadState { + ReusableWorkloadState { + workload_state: WorkloadState { + instance_name, + execution_state, + }, + workload_id, + } + } +} + // [impl->swdd~functions-required-by-runtime-connector~1] #[async_trait] pub trait RuntimeConnector: Sync + Send where StChecker: StateChecker + Send + Sync, - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, { fn name(&self) -> String; async fn get_reusable_workloads( &self, agent_name: &AgentName, - ) -> Result, RuntimeError>; + ) -> Result, RuntimeError>; async fn create_workload( &self, runtime_workload_config: WorkloadSpec, + reusable_workload_id: Option, control_interface_path: Option, update_state_tx: WorkloadStateSender, ) -> Result<(WorkloadId, StChecker), RuntimeError>; @@ -82,7 +107,7 @@ where pub trait OwnableRuntime: RuntimeConnector where StChecker: StateChecker + Send + Sync, - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, { fn to_owned(&self) -> Box>; } @@ -91,7 +116,7 @@ impl OwnableRuntime for R where R: RuntimeConnector + Clone + 'static, StChecker: StateChecker + Send + Sync, - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, { fn to_owned(&self) -> Box> { Box::new(self.clone()) @@ -111,13 +136,11 @@ pub mod test { use std::{collections::VecDeque, path::PathBuf, sync::Arc}; use async_trait::async_trait; - use common::objects::{ - AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec, WorkloadState, - }; + use common::objects::{AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec}; use tokio::sync::Mutex; use crate::{ - runtime_connectors::{RuntimeStateGetter, StateChecker}, + runtime_connectors::{ReusableWorkloadState, RuntimeStateGetter, StateChecker}, workload_state::WorkloadStateSender, }; @@ -175,7 +198,7 @@ pub mod test { #[derive(Debug)] pub enum RuntimeCall { - GetReusableWorkloads(AgentName, Result, RuntimeError>), + GetReusableWorkloads(AgentName, Result, RuntimeError>), CreateWorkload( WorkloadSpec, Option, @@ -293,7 +316,7 @@ pub mod test { async fn get_reusable_workloads( &self, agent_name: &AgentName, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { match self.get_expected_call().await { RuntimeCall::GetReusableWorkloads(expected_agent_name, result) if expected_agent_name == *agent_name => @@ -310,6 +333,7 @@ pub mod test { async fn create_workload( &self, runtime_workload_config: WorkloadSpec, + _reusable_workload_id: Option, control_interface_path: Option, _update_state_tx: WorkloadStateSender, ) -> Result<(String, StubStateChecker), RuntimeError> { diff --git a/agent/src/runtime_connectors/runtime_facade.rs b/agent/src/runtime_connectors/runtime_facade.rs index 203c96b7c..69076c5be 100644 --- a/agent/src/runtime_connectors/runtime_facade.rs +++ b/agent/src/runtime_connectors/runtime_facade.rs @@ -12,9 +12,11 @@ // // SPDX-License-Identifier: Apache-2.0 +use std::str::FromStr; + use async_trait::async_trait; use common::{ - objects::{AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec, WorkloadState}, + objects::{AgentName, ExecutionState, WorkloadInstanceName, WorkloadSpec}, std_extensions::IllegalStateResult, }; #[cfg(test)] @@ -27,7 +29,8 @@ use crate::control_interface::ControlInterface; use crate::control_interface::control_interface_info::ControlInterfaceInfo; use crate::{ - runtime_connectors::{OwnableRuntime, RuntimeError, StateChecker}, + runtime_connectors::{OwnableRuntime, ReusableWorkloadState, RuntimeError, StateChecker}, + workload_operation::ReusableWorkloadSpec, workload_state::{WorkloadStateSender, WorkloadStateSenderInterface}, }; @@ -46,11 +49,11 @@ pub trait RuntimeFacade: Send + Sync + 'static { async fn get_reusable_workloads( &self, agent_name: &AgentName, - ) -> Result, RuntimeError>; + ) -> Result, RuntimeError>; fn create_workload( &self, - runtime_workload: WorkloadSpec, + runtime_workload: ReusableWorkloadSpec, control_interface_info: Option, update_state_tx: &WorkloadStateSender, ) -> Workload; @@ -71,7 +74,7 @@ pub trait RuntimeFacade: Send + Sync + 'static { } pub struct GenericRuntimeFacade< - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync, > { runtime: Box>, @@ -79,7 +82,7 @@ pub struct GenericRuntimeFacade< impl GenericRuntimeFacade where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { pub fn new(runtime: Box>) -> Self { @@ -89,7 +92,7 @@ where #[async_trait] impl< - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, > RuntimeFacade for GenericRuntimeFacade { @@ -97,7 +100,7 @@ impl< async fn get_reusable_workloads( &self, agent_name: &AgentName, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { log::debug!( "Searching for reusable '{}' workloads on agent '{}'.", self.runtime.name(), @@ -109,13 +112,13 @@ impl< // [impl->swdd~agent-create-workload~2] fn create_workload( &self, - workload_spec: WorkloadSpec, + reusable_workload_spec: ReusableWorkloadSpec, control_interface_info: Option, update_state_tx: &WorkloadStateSender, ) -> Workload { let (_task_handle, workload) = Self::create_workload_non_blocking( self, - workload_spec, + reusable_workload_spec, control_interface_info, update_state_tx, ); @@ -156,17 +159,29 @@ impl< } impl< - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, > GenericRuntimeFacade { // [impl->swdd~agent-create-workload~2] fn create_workload_non_blocking( &self, - workload_spec: WorkloadSpec, + reusable_workload_spec: ReusableWorkloadSpec, control_interface_info: Option, update_state_tx: &WorkloadStateSender, ) -> (JoinHandle<()>, Workload) { + let workload_spec = reusable_workload_spec.workload_spec; + let workload_id = match reusable_workload_spec.workload_id { + Some(id) => match WorkloadId::from_str(&id) { + Ok(id) => Some(id), + Err(_) => { + log::warn!("Cannot decode workload id '{}'", id); + None + } + }, + None => None, + }; + let runtime = self.runtime.to_owned(); let update_state_tx = update_state_tx.clone(); let workload_name = workload_spec.instance_name.workload_name().to_owned(); @@ -223,6 +238,7 @@ impl< let control_loop_state = ControlLoopState::builder() .workload_spec(workload_spec) + .workload_id(workload_id) .control_interface_path(control_interface_path) .workload_state_sender(update_state_tx) .runtime(runtime) @@ -381,7 +397,6 @@ mod tests { use common::objects::{ generate_test_workload_spec_with_control_interface_access, generate_test_workload_spec_with_param, ExecutionState, WorkloadInstanceName, - WorkloadState, }; use crate::{ @@ -391,9 +406,10 @@ mod tests { }, runtime_connectors::{ runtime_connector::test::{MockRuntimeConnector, RuntimeCall, StubStateChecker}, - GenericRuntimeFacade, OwnableRuntime, RuntimeFacade, + GenericRuntimeFacade, OwnableRuntime, ReusableWorkloadState, RuntimeFacade, }, workload::{ControlLoopState, MockWorkload, MockWorkloadControlLoop}, + workload_operation::ReusableWorkloadSpec, workload_state::assert_execution_state_sequence, }; @@ -413,10 +429,11 @@ mod tests { .workload_name(WORKLOAD_1_NAME) .build(); - let workload_state = WorkloadState { - instance_name: workload_instance_name.clone(), - execution_state: ExecutionState::initial(), - }; + let workload_state = ReusableWorkloadState::new( + workload_instance_name.clone(), + ExecutionState::initial(), + None, + ); runtime_mock .expect(vec![RuntimeCall::GetReusableWorkloads( @@ -437,7 +454,7 @@ mod tests { .await .unwrap() .iter() - .map(|x| x.instance_name.clone()) + .map(|x| x.workload_state.instance_name.clone()) .collect::>(), vec![workload_instance_name] ); @@ -454,10 +471,15 @@ mod tests { .get_lock_async() .await; - let workload_spec = generate_test_workload_spec_with_control_interface_access( - AGENT_NAME.to_string(), - WORKLOAD_1_NAME.to_string(), - RUNTIME_NAME.to_string(), + const WORKLOAD_ID: &str = "workload_id_1"; + + let reusable_workload_spec = ReusableWorkloadSpec::new( + generate_test_workload_spec_with_control_interface_access( + AGENT_NAME.to_string(), + WORKLOAD_1_NAME.to_string(), + RUNTIME_NAME.to_string(), + ), + Some(WORKLOAD_ID.to_string()), ); let control_interface_mock = MockControlInterface::default(); @@ -481,7 +503,7 @@ mod tests { control_interface_info_mock .expect_get_instance_name() .once() - .return_const(workload_spec.instance_name.clone()); + .return_const(reusable_workload_spec.workload_spec.instance_name.clone()); control_interface_info_mock .expect_move_authorizer() @@ -514,7 +536,7 @@ mod tests { .return_once(|_: ControlLoopState| ()); let (task_handle, _workload) = test_runtime_facade.create_workload_non_blocking( - workload_spec.clone(), + reusable_workload_spec.clone(), Some(control_interface_info_mock), &wl_state_sender, ); diff --git a/agent/src/runtime_connectors/state_checker.rs b/agent/src/runtime_connectors/state_checker.rs index fe16f504d..0ceb99148 100644 --- a/agent/src/runtime_connectors/state_checker.rs +++ b/agent/src/runtime_connectors/state_checker.rs @@ -12,6 +12,8 @@ // // SPDX-License-Identifier: Apache-2.0 +use std::str::FromStr; + use async_trait::async_trait; use common::objects::{ExecutionState, WorkloadSpec}; @@ -26,7 +28,7 @@ use crate::workload_state::WorkloadStateSender; #[cfg_attr(test, automock)] pub trait RuntimeStateGetter: Send + Sync + 'static where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, { // [impl->swdd~allowed-workload-states~2] async fn get_state(&self, workload_id: &WorkloadId) -> ExecutionState; @@ -36,7 +38,7 @@ where #[async_trait] pub trait StateChecker where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, { fn start_checker( workload_spec: &WorkloadSpec, diff --git a/agent/src/runtime_manager.rs b/agent/src/runtime_manager.rs index 514ba2008..6eb8c656e 100644 --- a/agent/src/runtime_manager.rs +++ b/agent/src/runtime_manager.rs @@ -38,7 +38,7 @@ use crate::workload_scheduler::scheduler::WorkloadScheduler; use crate::workload_state::workload_state_store::WorkloadStateStore; use crate::{ runtime_connectors::RuntimeFacade, - workload_operation::WorkloadOperation, + workload_operation::{ReusableWorkloadSpec, WorkloadOperation}, workload_state::{WorkloadStateSender, WorkloadStateSenderInterface}, }; @@ -50,13 +50,29 @@ use mockall::automock; fn flatten( mut runtime_workload_map: HashMap>, -) -> Vec { +) -> Vec { runtime_workload_map .drain() - .flat_map(|(_, mut v)| v.drain().map(|(_, y)| y).collect::>()) + .flat_map(|(_, mut v)| { + v.drain() + .map(|(_, y)| ReusableWorkloadSpec::new(y, None)) + .collect::>() + }) .collect::>() } +pub trait ToReusableWorkloadSpecs { + fn into_reusable_workload_specs(self) -> Vec; +} + +impl ToReusableWorkloadSpecs for Vec { + fn into_reusable_workload_specs(self) -> Vec { + self.into_iter() + .map(|w| ReusableWorkloadSpec::new(w, None)) + .collect() + } +} + pub struct RuntimeManager { agent_name: AgentName, run_folder: PathBuf, @@ -105,7 +121,7 @@ impl RuntimeManager { pub async fn execute_workloads( &mut self, - added_workloads: Vec, + added_workloads: Vec, deleted_workloads: Vec, workload_state_db: &WorkloadStateStore, ) { @@ -126,7 +142,7 @@ impl RuntimeManager { // [impl->swdd~agent-initial-list-existing-workloads~1] pub async fn handle_server_hello( &mut self, - mut added_workloads: Vec, + added_workloads: Vec, workload_state_db: &WorkloadStateStore, ) { log::info!( @@ -134,11 +150,11 @@ impl RuntimeManager { added_workloads.len() ); - added_workloads = self + let new_added_workloads = self .resume_and_remove_from_added_workloads(added_workloads) .await; - self.execute_workloads(added_workloads, vec![], workload_state_db) + self.execute_workloads(new_added_workloads, vec![], workload_state_db) .await; } @@ -155,7 +171,10 @@ impl RuntimeManager { deleted_workloads.len() ); - self.execute_workloads(added_workloads, deleted_workloads, workload_state_db) + let new_added_workloads: Vec = + added_workloads.into_reusable_workload_specs(); + + self.execute_workloads(new_added_workloads, deleted_workloads, workload_state_db) .await; } @@ -185,7 +204,7 @@ impl RuntimeManager { async fn resume_and_remove_from_added_workloads( &mut self, added_workloads: Vec, - ) -> Vec { + ) -> Vec { log::debug!("Handling initial workload list."); // create a list per runtime @@ -221,7 +240,9 @@ impl RuntimeManager { runtime_name, ); - for workload_state in workload_states { + for reusable_workload_state in workload_states { + let workload_state = reusable_workload_state.workload_state; + let workload_id = reusable_workload_state.workload_id; if let Some(new_workload_spec) = added_workloads_per_runtime .get_mut(runtime_name) .and_then(|map| { @@ -254,16 +275,31 @@ impl RuntimeManager { &self.update_state_tx, ), ); + } else if Self::is_reusable_workload( + &workload_state, + &workload_id, + &new_instance_name, + ) { + // [impl->swdd~agent-existing-workloads-reuse-unmodified~1] + + log::info!( + "Re-starting workload '{}'", + new_instance_name.workload_name() + ); + + new_added_workloads.push(ReusableWorkloadSpec::new( + new_workload_spec, + workload_id, + )); } else { - // [impl->swdd~agent-existing-workloads-replace-updated~2] + // [impl->swdd~agent-existing-workloads-replace-updated~3] log::info!( "Replacing existing workload '{}'.", workload_state.instance_name.workload_name() ); - /* Temporary workaround until direct start of bundles is implemented to prevent - workload states from being overwritten by the delete. The decoupled create and a potential enqueue + /* This prevents workload states from being overwritten by the delete. The decoupled create and a potential enqueue on unmet inter-workload dependencies might run earlier than the delete and the delete overwrites the pending workload states.*/ const REPORT_WORKLOAD_STATES_FOR_WORKLOAD: bool = false; @@ -272,7 +308,8 @@ impl RuntimeManager { &self.update_state_tx, REPORT_WORKLOAD_STATES_FOR_WORKLOAD, ); - new_added_workloads.push(new_workload_spec); + new_added_workloads + .push(ReusableWorkloadSpec::new(new_workload_spec, None)); } } else { // No added workload matches the found running one => delete it @@ -310,21 +347,39 @@ impl RuntimeManager { .eq(new_instance_name) } + fn is_reusable_workload( + workload_state_existing_workload: &WorkloadState, + workload_id_existing_workload: &Option, + new_instance_name: &WorkloadInstanceName, + ) -> bool { + workload_state_existing_workload + .execution_state + .is_succeeded() + && workload_id_existing_workload.is_some() + && workload_state_existing_workload + .instance_name + .eq(new_instance_name) + } + // [impl->swdd~agent-transforms-update-workload-message-to-workload-operations~1] fn transform_into_workload_operations( &self, - added_workloads: Vec, + added_workloads: Vec, deleted_workloads: Vec, ) -> Vec { let mut workload_operations: Vec = Vec::new(); // transform into a hashmap to be able to search for updates // [impl->swdd~agent-updates-deleted-and-added-workloads~1] - let mut added_workloads: HashMap = added_workloads + let mut added_workloads: HashMap = added_workloads .into_iter() - .map(|workload_spec| { + .map(|reusable_workload_spec| { ( - workload_spec.instance_name.workload_name().to_owned(), - workload_spec, + reusable_workload_spec + .workload_spec + .instance_name + .workload_name() + .to_owned(), + reusable_workload_spec, ) }) .collect(); @@ -336,7 +391,7 @@ impl RuntimeManager { { // [impl->swdd~agent-updates-deleted-and-added-workloads~1] workload_operations.push(WorkloadOperation::Update( - updated_workload, + updated_workload.workload_spec, deleted_workload, )); } else { @@ -345,8 +400,11 @@ impl RuntimeManager { } } - for (_, workload_spec) in added_workloads { - let workload_name = workload_spec.instance_name.workload_name(); + for (_, reusable_workload_spec) in added_workloads { + let workload_name = reusable_workload_spec + .workload_spec + .instance_name + .workload_name(); if self.workloads.contains_key(workload_name) { log::warn!( "Added workload '{}' already exists. Updating without considering delete dependencies.", @@ -354,6 +412,7 @@ impl RuntimeManager { ); // We know this workload, seems the server is sending it again, try an update // [impl->swdd~agent-update-on-add-known-workload~1] + let workload_spec = reusable_workload_spec.workload_spec; let instance_name = workload_spec.instance_name.clone(); workload_operations.push(WorkloadOperation::Update( workload_spec, @@ -364,7 +423,7 @@ impl RuntimeManager { )); } else { // [impl->swdd~agent-added-creates-workload~1] - workload_operations.push(WorkloadOperation::Create(workload_spec)); + workload_operations.push(WorkloadOperation::Create(reusable_workload_spec)); } } @@ -374,9 +433,9 @@ impl RuntimeManager { async fn execute_workload_operations(&mut self, workload_operations: Vec) { for wl_operation in workload_operations { match wl_operation { - WorkloadOperation::Create(workload_spec) => { + WorkloadOperation::Create(reusable_workload_spec) => { // [impl->swdd~agent-executes-create-workload-operation~1] - self.add_workload(workload_spec).await + self.add_workload(reusable_workload_spec).await } WorkloadOperation::Update(new_workload_spec, _) => { // [impl->swdd~agent-executes-update-workload-operation~1] @@ -394,7 +453,8 @@ impl RuntimeManager { } } - async fn add_workload(&mut self, workload_spec: WorkloadSpec) { + async fn add_workload(&mut self, reusable_workload_spec: ReusableWorkloadSpec) { + let workload_spec = &reusable_workload_spec.workload_spec; let workload_name = workload_spec.instance_name.workload_name().to_owned(); // [impl->swdd~agent-control-interface-created-for-eligible-workloads~1] let control_interface_info = if workload_spec.needs_control_interface() { @@ -417,7 +477,7 @@ impl RuntimeManager { if let Some(runtime) = self.runtime_map.get(&workload_spec.runtime) { // [impl->swdd~agent-executes-create-workload-operation~1] let workload = runtime.create_workload( - workload_spec, + reusable_workload_spec, control_interface_info, &self.update_state_tx, ); @@ -495,7 +555,8 @@ impl RuntimeManager { workload_name ); // [impl->swdd~agent-add-on-update-missing-workload~1] - self.add_workload(workload_spec).await; + self.add_workload(ReusableWorkloadSpec::new(workload_spec, None)) + .await; } } @@ -528,8 +589,10 @@ mod tests { authorizer::MockAuthorizer, control_interface_info::MockControlInterfaceInfo, MockControlInterface, }; - use crate::runtime_connectors::{MockRuntimeFacade, RuntimeError}; + use crate::runtime_connectors::{MockRuntimeFacade, ReusableWorkloadState, RuntimeError}; + use crate::runtime_manager::ToReusableWorkloadSpecs; use crate::workload::{MockWorkload, WorkloadError}; + use crate::workload_operation::ReusableWorkloadSpec; use crate::workload_scheduler::scheduler::MockWorkloadScheduler; use crate::workload_state::workload_state_store::MockWorkloadStateStore; use crate::workload_state::WorkloadStateReceiver; @@ -619,8 +682,8 @@ mod tests { let added_workloads = vec![new_workload_access.clone(), new_workload_no_access.clone()]; let workload_operations = vec![ - WorkloadOperation::Create(new_workload_access), - WorkloadOperation::Create(new_workload_no_access), + WorkloadOperation::Create(ReusableWorkloadSpec::new(new_workload_access, None)), + WorkloadOperation::Create(ReusableWorkloadSpec::new(new_workload_no_access, None)), ]; let mut mock_workload_scheduler = MockWorkloadScheduler::default(); @@ -698,7 +761,10 @@ mod tests { ); let added_workloads = vec![workload_with_unknown_runtime.clone()]; - let workload_operations = vec![WorkloadOperation::Create(workload_with_unknown_runtime)]; + let workload_operations = vec![WorkloadOperation::Create(ReusableWorkloadSpec::new( + workload_with_unknown_runtime, + None, + ))]; let mut mock_workload_scheduler = MockWorkloadScheduler::default(); mock_workload_scheduler @@ -755,7 +821,9 @@ mod tests { ); let added_workloads = vec![workload.clone()]; - let workload_operations = vec![WorkloadOperation::Create(workload)]; + let workload_operations = vec![WorkloadOperation::Create(ReusableWorkloadSpec::new( + workload, None, + ))]; let mut mock_workload_scheduler = MockWorkloadScheduler::default(); mock_workload_scheduler .expect_enqueue_filtered_workload_operations() @@ -783,8 +851,12 @@ mod tests { runtime_facade_mock .expect_create_workload() .once() - .withf(|workload_spec, control_interface, to_server| { - workload_spec.instance_name.workload_name() == WORKLOAD_1_NAME + .withf(|reusable_workload_spec, control_interface, to_server| { + reusable_workload_spec + .workload_spec + .instance_name + .workload_name() + == WORKLOAD_1_NAME && control_interface.is_some() && !to_server.is_closed() }) @@ -894,16 +966,17 @@ mod tests { ); let existing_workload_instance_name = existing_workload.instance_name.clone(); - let workload_state_running = WorkloadState { - instance_name: existing_workload_instance_name, - execution_state: ExecutionState::running(), - }; + let resuable_workload_state_running = ReusableWorkloadState::new( + existing_workload_instance_name, + ExecutionState::running(), + None, + ); let mut runtime_facade_mock = MockRuntimeFacade::new(); runtime_facade_mock .expect_get_reusable_workloads() .once() - .return_once(|_| Box::pin(async { Ok(vec![workload_state_running]) })); + .return_once(|_| Box::pin(async { Ok(vec![resuable_workload_state_running]) })); runtime_facade_mock .expect_resume_workload() @@ -927,7 +1000,7 @@ mod tests { assert!(runtime_manager.workloads.contains_key(WORKLOAD_1_NAME)); } - // [utest->swdd~agent-existing-workloads-replace-updated~2] + // [utest->swdd~agent-existing-workloads-replace-updated~3] #[tokio::test] async fn utest_replace_existing_workload_with_different_config() { let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC @@ -955,16 +1028,17 @@ mod tests { .agent_name(AGENT_NAME) .build(); - let workload_state_running = WorkloadState { - instance_name: existing_workload_with_other_config, - execution_state: ExecutionState::running(), - }; + let reusable_workload_state_running = ReusableWorkloadState::new( + existing_workload_with_other_config, + ExecutionState::running(), + None, + ); let mut runtime_facade_mock = MockRuntimeFacade::new(); runtime_facade_mock .expect_get_reusable_workloads() .once() - .return_once(|_| Box::pin(async { Ok(vec![workload_state_running]) })); + .return_once(|_| Box::pin(async { Ok(vec![reusable_workload_state_running]) })); runtime_facade_mock .expect_delete_workload() @@ -978,7 +1052,8 @@ mod tests { ) .build(); - let expected_new_added_workloads = added_workloads.clone(); + let expected_new_added_workloads: Vec = + added_workloads.clone().into_reusable_workload_specs(); let new_added_workloads = runtime_manager .resume_and_remove_from_added_workloads(added_workloads) .await; @@ -987,7 +1062,7 @@ mod tests { assert!(!runtime_manager.workloads.contains_key(WORKLOAD_1_NAME)); } - // [utest->swdd~agent-existing-workloads-replace-updated~2] + // [utest->swdd~agent-existing-workloads-replace-updated~3] #[tokio::test] async fn utest_replace_existing_not_running_workload() { let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC @@ -1007,16 +1082,17 @@ mod tests { .once() .return_once(|_| MockWorkloadScheduler::default()); - let workload_state_succeeded = WorkloadState { - instance_name: existing_workload.instance_name, - execution_state: ExecutionState::succeeded(), - }; + let resuable_workload_state_succeeded = ReusableWorkloadState::new( + existing_workload.instance_name, + ExecutionState::succeeded(), + None, + ); let mut runtime_facade_mock = MockRuntimeFacade::new(); runtime_facade_mock .expect_get_reusable_workloads() .once() - .return_once(|_| Box::pin(async { Ok(vec![workload_state_succeeded]) })); + .return_once(|_| Box::pin(async { Ok(vec![resuable_workload_state_succeeded]) })); runtime_facade_mock .expect_delete_workload() .once() @@ -1029,7 +1105,8 @@ mod tests { ) .build(); - let expected_added_workloads = added_workloads.clone(); + let expected_added_workloads: Vec = + added_workloads.clone().into_reusable_workload_specs(); let new_added_workloads = runtime_manager .resume_and_remove_from_added_workloads(added_workloads) .await; @@ -1038,6 +1115,62 @@ mod tests { assert!(!runtime_manager.workloads.contains_key(WORKLOAD_1_NAME)); } + // [utest->swdd~agent-existing-workloads-reuse-unmodified~1] + #[tokio::test] + async fn utest_reuse_existing_succeeded_workload() { + let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC + .get_lock_async() + .await; + + let existing_workload = generate_test_workload_spec_with_control_interface_access( + AGENT_NAME.to_string(), + WORKLOAD_1_NAME.to_string(), + RUNTIME_NAME.to_string(), + ); + + let added_workloads = vec![existing_workload.clone()]; + + let mock_workload_scheduler_context = MockWorkloadScheduler::new_context(); + mock_workload_scheduler_context + .expect() + .once() + .return_once(|_| MockWorkloadScheduler::default()); + + const WORKLOAD_ID: &str = "workload_id_1"; + let resuable_workload_state_succeeded = ReusableWorkloadState::new( + existing_workload.instance_name.clone(), + ExecutionState::succeeded(), + Some(WORKLOAD_ID.to_string()), + ); + + let mut runtime_facade_mock = MockRuntimeFacade::new(); + runtime_facade_mock + .expect_get_reusable_workloads() + .once() + .return_once(|_| Box::pin(async { Ok(vec![resuable_workload_state_succeeded]) })); + + runtime_facade_mock.expect_delete_workload().never(); + + let (_, mut runtime_manager, _) = RuntimeManagerBuilder::default() + .with_runtime( + RUNTIME_NAME, + Box::new(runtime_facade_mock) as Box, + ) + .build(); + + let expected_new_added_workloads: Vec = added_workloads + .clone() + .into_iter() + .map(|w| ReusableWorkloadSpec::new(w, Some(WORKLOAD_ID.to_string()))) + .collect(); + let new_added_workloads = runtime_manager + .resume_and_remove_from_added_workloads(added_workloads) + .await; + + assert_eq!(expected_new_added_workloads, new_added_workloads); + assert!(!runtime_manager.workloads.contains_key(WORKLOAD_1_NAME)); + } + // [utest->swdd~agent-existing-workloads-delete-unneeded~1] #[tokio::test] async fn utest_handle_update_workload_initial_call_delete_unneeded() { @@ -1075,10 +1208,11 @@ mod tests { .once() .return_once(|_| { Box::pin(async move { - Ok(vec![WorkloadState { - instance_name: existing_workload_with_other_config, - ..Default::default() - }]) + Ok(vec![ReusableWorkloadState::new( + existing_workload_with_other_config, + ExecutionState::default(), + None, + )]) }) }); @@ -1187,7 +1321,9 @@ mod tests { WORKLOAD_1_NAME.to_string(), RUNTIME_NAME.to_string(), ); - runtime_manager.add_workload(workload_spec_no_access).await; + runtime_manager + .add_workload(ReusableWorkloadSpec::new(workload_spec_no_access, None)) + .await; control_interface_info_new_context.expect().never(); let workload_spec_has_access = generate_test_workload_spec_with_control_interface_access( @@ -1195,10 +1331,12 @@ mod tests { WORKLOAD_1_NAME.to_string(), RUNTIME_NAME.to_string(), ); - runtime_manager.add_workload(workload_spec_has_access).await; + runtime_manager + .add_workload(ReusableWorkloadSpec::new(workload_spec_has_access, None)) + .await; } - // [utest->swdd~agent-existing-workloads-replace-updated~2] + // [utest->swdd~agent-existing-workloads-replace-updated~3] #[tokio::test] async fn utest_handle_update_workload_initial_call_replace_workload_with_unfulfilled_dependencies( ) { @@ -1241,10 +1379,11 @@ mod tests { .once() .return_once(|_| { Box::pin(async { - Ok(vec![WorkloadState { - instance_name: existing_workload_with_other_config, - ..Default::default() - }]) + Ok(vec![ReusableWorkloadState::new( + existing_workload_with_other_config, + ExecutionState::default(), + None, + )]) }) }); @@ -1385,7 +1524,7 @@ mod tests { let workload_operations = vec![ WorkloadOperation::Delete(deleted_workload.clone()), - WorkloadOperation::Create(new_workload.clone()), + WorkloadOperation::Create(ReusableWorkloadSpec::new(new_workload.clone(), None)), ]; let mut mock_workload_scheduler = MockWorkloadScheduler::default(); mock_workload_scheduler @@ -1412,8 +1551,12 @@ mod tests { runtime_facade_mock .expect_create_workload() .once() - .withf(|workload_spec, control_interface, to_server| { - workload_spec.instance_name.workload_name() == WORKLOAD_2_NAME + .withf(|reusable_workload_spec, control_interface, to_server| { + reusable_workload_spec + .workload_spec + .instance_name + .workload_name() + == WORKLOAD_2_NAME && control_interface.is_some() && !to_server.is_closed() }) @@ -1471,7 +1614,10 @@ mod tests { let deleted_workload = generate_test_deleted_workload(AGENT_NAME.to_string(), WORKLOAD_1_NAME.to_string()); - let workload_operations = vec![WorkloadOperation::Create(new_workload.clone())]; + let workload_operations = vec![WorkloadOperation::Create(ReusableWorkloadSpec::new( + new_workload.clone(), + None, + ))]; let mut mock_workload_scheduler = MockWorkloadScheduler::default(); mock_workload_scheduler .expect_enqueue_filtered_workload_operations() @@ -1611,7 +1757,10 @@ mod tests { RUNTIME_NAME.to_string(), ); - let workload_operations = vec![WorkloadOperation::Create(new_workload.clone())]; + let workload_operations = vec![WorkloadOperation::Create(ReusableWorkloadSpec::new( + new_workload.clone(), + None, + ))]; let mut mock_workload_scheduler = MockWorkloadScheduler::default(); mock_workload_scheduler .expect_enqueue_filtered_workload_operations() @@ -1628,8 +1777,12 @@ mod tests { runtime_facade_mock .expect_create_workload() .once() - .withf(|workload_spec, control_interface, to_server| { - workload_spec.instance_name.workload_name() == WORKLOAD_1_NAME + .withf(|resuable_workload_spec, control_interface, to_server| { + resuable_workload_spec + .workload_spec + .instance_name + .workload_name() + == WORKLOAD_1_NAME && control_interface.is_some() && !to_server.is_closed() }) @@ -2053,7 +2206,10 @@ mod tests { ); workload_spec.control_interface_access = generate_test_control_interface_access(); - let next_workload_operations = vec![WorkloadOperation::Create(workload_spec)]; + let next_workload_operations = vec![WorkloadOperation::Create(ReusableWorkloadSpec::new( + workload_spec, + None, + ))]; let mut mock_workload_scheduler = MockWorkloadScheduler::default(); mock_workload_scheduler .expect_next_workload_operations() @@ -2275,13 +2431,16 @@ mod tests { WORKLOAD_1_NAME.to_owned(), RUNTIME_NAME.to_owned(), ); - let added_workloads = vec![new_workload.clone()]; + let added_workloads = vec![ReusableWorkloadSpec::new(new_workload.clone(), None)]; let deleted_workloads = vec![]; let workload_operations = runtime_manager.transform_into_workload_operations(added_workloads, deleted_workloads); assert_eq!( - vec![WorkloadOperation::Create(new_workload)], + vec![WorkloadOperation::Create(ReusableWorkloadSpec::new( + new_workload, + None + ))], workload_operations ); } @@ -2335,7 +2494,7 @@ mod tests { WORKLOAD_1_NAME.to_owned(), RUNTIME_NAME.to_owned(), ); - let added_workloads = vec![new_workload.clone()]; + let added_workloads = vec![ReusableWorkloadSpec::new(new_workload.clone(), None)]; let deleted_workload = generate_test_deleted_workload(AGENT_NAME.to_owned(), WORKLOAD_1_NAME.to_owned()); let deleted_workloads = vec![deleted_workload.clone()]; @@ -2387,7 +2546,10 @@ mod tests { WORKLOAD_1_NAME.to_owned(), RUNTIME_NAME.to_owned(), ); - let workload_operations = vec![WorkloadOperation::Create(new_workload)]; + let workload_operations = vec![WorkloadOperation::Create(ReusableWorkloadSpec::new( + new_workload, + None, + ))]; runtime_manager .execute_workload_operations(workload_operations) .await; diff --git a/agent/src/workload/control_loop_state.rs b/agent/src/workload/control_loop_state.rs index a42dfeffe..e64899b38 100644 --- a/agent/src/workload/control_loop_state.rs +++ b/agent/src/workload/control_loop_state.rs @@ -18,10 +18,11 @@ use crate::workload_state::{WorkloadStateReceiver, WorkloadStateSender}; use crate::BUFFER_SIZE; use common::objects::{WorkloadInstanceName, WorkloadSpec, WorkloadState}; use std::path::PathBuf; +use std::str::FromStr; pub struct ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { pub workload_spec: WorkloadSpec, @@ -39,7 +40,7 @@ where impl ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { pub fn builder() -> ControlLoopStateBuilder { @@ -53,10 +54,11 @@ where pub struct ControlLoopStateBuilder where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { workload_spec: Option, + workload_id: Option, control_interface_path: Option, workload_state_sender: Option, runtime: Option>>, @@ -67,12 +69,13 @@ where impl ControlLoopStateBuilder where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { pub fn new() -> Self { ControlLoopStateBuilder { workload_spec: None, + workload_id: None, control_interface_path: None, workload_state_sender: None, runtime: None, @@ -87,6 +90,11 @@ where self } + pub fn workload_id(mut self, workload_id: Option) -> Self { + self.workload_id = workload_id; + self + } + pub fn control_interface_path(mut self, control_interface_path: Option) -> Self { self.control_interface_path = control_interface_path; self @@ -122,7 +130,7 @@ where .workload_spec .ok_or_else(|| "WorkloadSpec is not set".to_string())?, control_interface_path: self.control_interface_path, - workload_id: None, + workload_id: self.workload_id, state_checker: None, to_agent_workload_state_sender: self .workload_state_sender diff --git a/agent/src/workload/workload_control_loop.rs b/agent/src/workload/workload_control_loop.rs index fa98803ec..94762f961 100644 --- a/agent/src/workload/workload_control_loop.rs +++ b/agent/src/workload/workload_control_loop.rs @@ -19,6 +19,7 @@ use common::objects::{ExecutionState, RestartPolicy, WorkloadInstanceName, Workl use common::std_extensions::IllegalStateResult; use futures_util::Future; use std::path::PathBuf; +use std::str::FromStr; #[cfg(not(test))] const MAX_RETRIES: usize = 20; @@ -70,7 +71,7 @@ impl WorkloadControlLoop { pub async fn run( mut control_loop_state: ControlLoopState, ) where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { loop { @@ -190,7 +191,7 @@ impl WorkloadControlLoop { control_loop_state: ControlLoopState, ) -> ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { log::debug!( @@ -239,7 +240,7 @@ impl WorkloadControlLoop { error_msg: String, ) -> ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { log::info!( @@ -265,7 +266,7 @@ impl WorkloadControlLoop { error_msg: String, ) -> ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { control_loop_state.workload_id = None; @@ -319,7 +320,7 @@ impl WorkloadControlLoop { func_on_error: ErrorFunc, ) -> ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, Fut: Future> + 'static, ErrorFunc: FnOnce(ControlLoopState, WorkloadInstanceName, String) -> Fut @@ -331,6 +332,7 @@ impl WorkloadControlLoop { .runtime .create_workload( control_loop_state.workload_spec.clone(), + control_loop_state.workload_id.clone(), control_loop_state.control_interface_path.clone(), control_loop_state .state_checker_workload_state_sender @@ -371,7 +373,7 @@ impl WorkloadControlLoop { mut control_loop_state: ControlLoopState, ) -> Option> where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { Self::send_workload_state_to_agent( @@ -430,7 +432,7 @@ impl WorkloadControlLoop { control_interface_path: Option, ) -> ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { Self::send_workload_state_to_agent( @@ -505,7 +507,7 @@ impl WorkloadControlLoop { instance_name: WorkloadInstanceName, ) -> ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { if Self::is_same_workload(control_loop_state.instance_name(), &instance_name) @@ -529,7 +531,7 @@ impl WorkloadControlLoop { mut control_loop_state: ControlLoopState, ) -> ControlLoopState where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static, { let workload_name = control_loop_state.instance_name().workload_name(); @@ -590,7 +592,7 @@ mockall::mock! { control_loop_state: ControlLoopState, ) where - WorkloadId: ToString + Send + Sync + 'static, + WorkloadId: ToString + FromStr + Clone + Send + Sync + 'static, StChecker: StateChecker + Send + Sync + 'static; } } diff --git a/agent/src/workload_operation.rs b/agent/src/workload_operation.rs index 621b514ee..f1a39643d 100644 --- a/agent/src/workload_operation.rs +++ b/agent/src/workload_operation.rs @@ -14,10 +14,25 @@ use common::objects::{DeletedWorkload, WorkloadSpec}; +#[derive(Debug, Clone, PartialEq)] +pub struct ReusableWorkloadSpec { + pub workload_spec: WorkloadSpec, + pub workload_id: Option, +} + +impl ReusableWorkloadSpec { + pub fn new(workload_spec: WorkloadSpec, workload_id: Option) -> ReusableWorkloadSpec { + ReusableWorkloadSpec { + workload_spec, + workload_id, + } + } +} + #[derive(Debug, Clone, PartialEq)] // [impl->swdd~agent-transforms-update-workload-message-to-workload-operations~1] pub enum WorkloadOperation { - Create(WorkloadSpec), + Create(ReusableWorkloadSpec), Update(WorkloadSpec, DeletedWorkload), UpdateDeleteOnly(DeletedWorkload), Delete(DeletedWorkload), diff --git a/agent/src/workload_scheduler/scheduler.rs b/agent/src/workload_scheduler/scheduler.rs index e42fd9573..01f12926d 100644 --- a/agent/src/workload_scheduler/scheduler.rs +++ b/agent/src/workload_scheduler/scheduler.rs @@ -14,11 +14,13 @@ #[cfg_attr(test, mockall_double::double)] use crate::workload_scheduler::dependency_state_validator::DependencyStateValidator; -use crate::workload_state::{WorkloadStateSender, WorkloadStateSenderInterface}; +use crate::{ + workload_operation::ReusableWorkloadSpec, + workload_state::{WorkloadStateSender, WorkloadStateSenderInterface}, +}; use common::objects::{DeletedWorkload, ExecutionState, WorkloadInstanceName, WorkloadSpec}; use std::{collections::HashMap, fmt::Display}; - use crate::workload_operation::WorkloadOperation; #[cfg_attr(test, mockall_double::double)] use crate::workload_state::workload_state_store::WorkloadStateStore; @@ -28,7 +30,7 @@ use mockall::automock; #[derive(Debug, Clone, PartialEq)] enum PendingEntry { - Create(WorkloadSpec), + Create(ReusableWorkloadSpec), Delete(DeletedWorkload), UpdateCreate(WorkloadSpec, DeletedWorkload), UpdateDelete(WorkloadSpec, DeletedWorkload), @@ -188,22 +190,29 @@ impl WorkloadScheduler { // [impl->swdd~agent-enqueues-unfulfilled-create~1] async fn enqueue_pending_create( &mut self, - new_workload_spec: WorkloadSpec, + new_workload_spec: ReusableWorkloadSpec, workload_state_db: &WorkloadStateStore, notify_on_new_entry: bool, ) -> Vec { let mut ready_workload_operations = Vec::new(); // [impl->swdd~workload-ready-to-create-on-fulfilled-dependencies~1] - if DependencyStateValidator::create_fulfilled(&new_workload_spec, workload_state_db) { + if DependencyStateValidator::create_fulfilled( + &new_workload_spec.workload_spec, + workload_state_db, + ) { ready_workload_operations.push(WorkloadOperation::Create(new_workload_spec)); } else { if notify_on_new_entry { - self.report_pending_create_state(&new_workload_spec.instance_name) + self.report_pending_create_state(&new_workload_spec.workload_spec.instance_name) .await; } self.put_on_queue( - new_workload_spec.instance_name.workload_name().to_owned(), + new_workload_spec + .workload_spec + .instance_name + .workload_name() + .to_owned(), PendingEntry::Create(new_workload_spec), ); } @@ -335,7 +344,7 @@ mod tests { use super::WorkloadScheduler; use crate::{ - workload_operation::WorkloadOperation, + workload_operation::{ReusableWorkloadSpec, WorkloadOperation}, workload_scheduler::{ dependency_state_validator::MockDependencyStateValidator, scheduler::PendingEntry, }, @@ -370,7 +379,10 @@ mod tests { RUNTIME.to_owned(), ); - let workload_operations = vec![WorkloadOperation::Create(pending_workload.clone())]; + let pending_reusable_workload = ReusableWorkloadSpec::new(pending_workload, None); + + let workload_operations = + vec![WorkloadOperation::Create(pending_reusable_workload.clone())]; let ready_workload_operations = workload_scheduler .enqueue_filtered_workload_operations( @@ -380,7 +392,7 @@ mod tests { .await; let expected_workload_state = generate_test_workload_state_with_workload_spec( - &pending_workload.clone(), + &pending_reusable_workload.workload_spec.clone(), ExecutionState::waiting_to_start(), ); @@ -393,9 +405,12 @@ mod tests { .await ); - assert!(workload_scheduler - .queue - .contains_key(pending_workload.instance_name.workload_name())); + assert!(workload_scheduler.queue.contains_key( + pending_reusable_workload + .workload_spec + .instance_name + .workload_name() + )); assert!(ready_workload_operations.is_empty()); } @@ -415,10 +430,13 @@ mod tests { .expect() .return_const(true); - let ready_workload = generate_test_workload_spec_with_param( - AGENT_A.to_owned(), - WORKLOAD_NAME_1.to_owned(), - RUNTIME.to_owned(), + let ready_workload = ReusableWorkloadSpec::new( + generate_test_workload_spec_with_param( + AGENT_A.to_owned(), + WORKLOAD_NAME_1.to_owned(), + RUNTIME.to_owned(), + ), + None, ); let workload_operations = vec![WorkloadOperation::Create(ready_workload.clone())]; @@ -1170,13 +1188,17 @@ mod tests { .expect() .return_const(false); - let pending_workload_spec = generate_test_workload_spec_with_param( - AGENT_A.to_owned(), - WORKLOAD_NAME_1.to_owned(), - RUNTIME.to_owned(), + let pending_workload_spec = ReusableWorkloadSpec::new( + generate_test_workload_spec_with_param( + AGENT_A.to_owned(), + WORKLOAD_NAME_1.to_owned(), + RUNTIME.to_owned(), + ), + None, ); - let instance_name_create_workload = pending_workload_spec.instance_name.clone(); + let instance_name_create_workload = + pending_workload_spec.workload_spec.instance_name.clone(); workload_scheduler.queue.insert( instance_name_create_workload.workload_name().to_owned(), @@ -1209,13 +1231,17 @@ mod tests { .expect() .return_const(false); - let pending_workload_spec = generate_test_workload_spec_with_param( - AGENT_A.to_owned(), - WORKLOAD_NAME_1.to_owned(), - RUNTIME.to_owned(), + let pending_workload_spec = ReusableWorkloadSpec::new( + generate_test_workload_spec_with_param( + AGENT_A.to_owned(), + WORKLOAD_NAME_1.to_owned(), + RUNTIME.to_owned(), + ), + None, ); - let instance_name_create_workload = pending_workload_spec.instance_name.clone(); + let instance_name_create_workload = + pending_workload_spec.workload_spec.instance_name.clone(); workload_scheduler.queue.insert( instance_name_create_workload.workload_name().to_owned(), @@ -1449,14 +1475,21 @@ mod tests { .expect() .return_const(true); - let ready_workload_spec = generate_test_workload_spec_with_param( - AGENT_A.to_owned(), - WORKLOAD_NAME_1.to_owned(), - RUNTIME.to_owned(), + let ready_workload_spec = ReusableWorkloadSpec::new( + generate_test_workload_spec_with_param( + AGENT_A.to_owned(), + WORKLOAD_NAME_1.to_owned(), + RUNTIME.to_owned(), + ), + None, ); workload_scheduler.queue.insert( - ready_workload_spec.instance_name.workload_name().to_owned(), + ready_workload_spec + .workload_spec + .instance_name + .workload_name() + .to_owned(), PendingEntry::Create(ready_workload_spec.clone()), ); diff --git a/ank/doc/swdesign/README.md b/ank/doc/swdesign/README.md index 74615056c..7ee75bb5d 100644 --- a/ank/doc/swdesign/README.md +++ b/ank/doc/swdesign/README.md @@ -551,7 +551,6 @@ Tags: - CliCommands Needs: -- swdd - impl - utest diff --git a/common/src/objects/workload_state.rs b/common/src/objects/workload_state.rs index 14df9e728..6a6a72c00 100644 --- a/common/src/objects/workload_state.rs +++ b/common/src/objects/workload_state.rs @@ -501,7 +501,7 @@ impl Display for ExecutionState { #[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(default, rename_all = "camelCase")] pub struct WorkloadState { - // [impl->swdd~common-workload-state-identification~1s] + // [impl->swdd~common-workload-state-identification~1] pub instance_name: WorkloadInstanceName, pub execution_state: ExecutionState, } diff --git a/doc/docs/development/requirement-tracing.md b/doc/docs/development/requirement-tracing.md index 0778c5fb1..258456ed8 100644 --- a/doc/docs/development/requirement-tracing.md +++ b/doc/docs/development/requirement-tracing.md @@ -5,7 +5,7 @@ The Eclipse Ankaios project provides requirement tracing using the [OpenFastTrace](https://github.com/itsallcode/openfasttrace) requirement tracing suite. The dev container already includes the required tooling. To generate a requirement tracing report call: ```shell -tools/generate_oft_html_report.sh +just trace-requirements ``` Afterwards the HTML report is available under `build/req/req_tracing_report.html` and shows the current coverage state. diff --git a/justfile b/justfile index 5ce492f9d..3b28bd62e 100644 --- a/justfile +++ b/justfile @@ -12,11 +12,13 @@ # # SPDX-License-Identifier: Apache-2.0 -all: check-test-images check-licenses test stest build-release +all: check-test-images check-licenses clippy test stest build-release +# Perform debug build build: cargo build +# Perform release build build-release: cargo build --release @@ -25,6 +27,7 @@ clean: ./tools/dev_scripts/ankaios-clean rm -rf build +# Check licenses of dependencies check-licenses: cargo deny check licenses @@ -32,15 +35,30 @@ check-licenses: check-test-images: test -z "$(find tests/resources/configs -type f -exec grep -H -P 'image: (?!ghcr\.io/|image_typo:latest)' {} \;)" +# Check for the presence of a copyright header check-copyright-headers: - ./tools/check_copyright_headers.sh + ./tools/check_copyright_headers.sh +# Run unit tests test: cargo nextest run # Build debug and run all system tests stest: build stest-only -# only execute the stests without building -stest-only: - ./tools/run_robot_tests.sh tests +# Only execute the stests without building +stest-only tests="tests": + ./tools/run_robot_tests.sh {{ tests }} + +# Run clippy code checks +clippy: + cargo clippy --all-targets --no-deps --all-features -- -D warnings + +# Generate test coverage report +coverage: + tools/generate_test_coverage_report.sh test --html + +# Create requirement tracing report +trace-requirements report="build/req/req_tracing_report.html": + mkdir -p $(dirname "{{ report }}") + oft trace $(find . -type d \( -name "src" -o -name "doc" -o -name "tests" \) -not -path './doc') -a swdd,impl,utest,itest,stest -o html -f "{{ report }}" || true diff --git a/tests/resources/ankaios.resource b/tests/resources/ankaios.resource index 6c091c7e7..8f3e068d7 100644 --- a/tests/resources/ankaios.resource +++ b/tests/resources/ankaios.resource @@ -30,6 +30,7 @@ ${CURRENT_RESULT} ${EMPTY} ${SERVER_PROCESS_HANDLE} ${EMPTY} ${TEST_FOLDER_NAME} ${EMPTY} &{ANKAIOS_INSTANCE_NAME_TO_PODMAN_ID_MAPPING} +&{ANKAIOS_WORKLOAD_NAME_TO_PODMAN_ID_MAPPING} ${ANKAIOS_VERSION} ${EMPTY} *** Keywords *** @@ -62,6 +63,7 @@ Clean up Ankaios Clean up Podman Run Command rm -rf %{ANKAIOS_TEMP} timeout=20 Set Variable ${ANKAIOS_INSTANCE_NAME_TO_PODMAN_ID_MAPPING} {} + Set Variable ${ANKAIOS_WORKLOAD_NAME_TO_PODMAN_ID_MAPPING} {} Remove Environment Variable ANKSERVER_CA_PEM Remove Environment Variable ANKSERVER_CRT_PEM Remove Environment Variable ANKSERVER_KEY_PEM @@ -404,6 +406,9 @@ podman shall have assigned a container id for workload "${workload_name}" on age Set To Dictionary ${ANKAIOS_INSTANCE_NAME_TO_PODMAN_ID_MAPPING} ${ankaios_instance_name}=${container_id} Set Global Variable ${ANKAIOS_INSTANCE_NAME_TO_PODMAN_ID_MAPPING} Log ANKAIOS_INSTANCE_NAME_TO_PODMAN_ID_MAPPING = '${ANKAIOS_INSTANCE_NAME_TO_PODMAN_ID_MAPPING}' + Set To Dictionary ${ANKAIOS_WORKLOAD_NAME_TO_PODMAN_ID_MAPPING} ${workload_name}=${container_id} + Set Global Variable ${ANKAIOS_WORKLOAD_NAME_TO_PODMAN_ID_MAPPING} + Log ANKAIOS_WORKLOAD_NAME_TO_PODMAN_ID_MAPPING = '${ANKAIOS_WORKLOAD_NAME_TO_PODMAN_ID_MAPPING}' podman kube shall have assigned an id for pod "${pod_name}" of workload "${workload_name}" on agent "${agent_name}" within "${timeout}" seconds volumes for "${workload_name}" shall exist on "${agent_name}" within "${timeout}" seconds @@ -483,6 +488,50 @@ the container of workload "${workload_name}" shall have a different id but same Should Be True ${id_changed} msg=Workload '${workload_name}' has the same id '${workload_id}' and the same configuration on podman! +the container of workload "${workload_name}" shall have a different id on the podman runtime + ${id_changed}= Set Variable ${False} + ${start_time}= Get Time Secs + WHILE True + ${current_secs}= Get Time Secs + ${elapsed_secs}= Evaluate ${current_secs} - ${start_time} + IF ${elapsed_secs} >= 20 BREAK + ${current_workload_id} ${current_ankaios_instance_name} Get Container Id And Name By Workload Name From Podman ${workload_name} + IF '${current_workload_id}' == '' or '${current_ankaios_instance_name}' == '' + CONTINUE + END + + ${workload_id}= Get From Dictionary ${ANKAIOS_WORKLOAD_NAME_TO_PODMAN_ID_MAPPING} ${workload_name} + + IF '${current_workload_id}' != '${workload_id}' + ${id_changed}= Set Variable ${True} + BREAK + END + END + + Should Be True ${id_changed} msg=Workload '${workload_name}' has the same id '${workload_id}' on podman! + +the container of workload "${workload_name}" shall have the same id and same configuration on the podman runtime + ${id_changed}= Set Variable ${True} + ${start_time}= Get Time Secs + WHILE True + ${current_secs}= Get Time Secs + ${elapsed_secs}= Evaluate ${current_secs} - ${start_time} + IF ${elapsed_secs} >= 20 BREAK + ${current_workload_id} ${current_ankaios_instance_name} Get Container Id And Name By Workload Name From Podman ${workload_name} + IF '${current_workload_id}' == '' or '${current_ankaios_instance_name}' == '' + CONTINUE + END + + ${old_container_id}= Get From Dictionary ${ANKAIOS_INSTANCE_NAME_TO_PODMAN_ID_MAPPING} ${current_ankaios_instance_name} + + IF '${current_workload_id}' == '${old_container_id}' + ${id_changed}= Set Variable ${False} + BREAK + END + END + + Should Not Be True ${id_changed} msg=Workload '${workload_name}' has a different '${old_container_id}' on podman! + the pod "${pod_name}" of workload "${workload_name}" shall have a different id but same configuration on the podman kube runtime ${id_changed}= Set Variable ${False} ${start_time}= Get Time Secs diff --git a/tests/resources/configs/device_restart_with_dependencies_modified_web_service_init.yaml b/tests/resources/configs/device_restart_with_dependencies_modified_web_service_init.yaml new file mode 100644 index 000000000..8af82541c --- /dev/null +++ b/tests/resources/configs/device_restart_with_dependencies_modified_web_service_init.yaml @@ -0,0 +1,29 @@ +apiVersion: v0.1 +workloads: + web_service_init: + runtime: podman + agent: agent_A + restartPolicy: NEVER + dependencies: + filesystem_init: ADD_COND_SUCCEEDED + runtimeConfig: | + image: ghcr.io/eclipse-ankaios/tests/alpine:latest + commandOptions: [ "--entrypoint", "/bin/sleep", "-e", "ENV=TEST" ] + commandArgs: [ "2" ] + filesystem_init: + runtime: podman + agent: agent_A + restartPolicy: NEVER + runtimeConfig: | + image: ghcr.io/eclipse-ankaios/tests/alpine:latest + commandOptions: [ "--entrypoint", "/bin/sleep" ] + commandArgs: [ "1" ] + web_service: + runtime: podman + agent: agent_B + restartPolicy: NEVER + dependencies: + filesystem_init: ADD_COND_SUCCEEDED + runtimeConfig: | + image: ghcr.io/eclipse-ankaios/tests/nginx:alpine-slim + commandOptions: ["-p", "8081:80"] diff --git a/tests/stests/workloads/create_workload_podman.robot b/tests/stests/workloads/create_workload_podman.robot index 014e69b3a..9b21fff51 100644 --- a/tests/stests/workloads/create_workload_podman.robot +++ b/tests/stests/workloads/create_workload_podman.robot @@ -24,7 +24,7 @@ Resource ../../resources/variables.resource *** Test Cases *** # [stest->swdd~agent-supports-podman~2] -# [stest->swdd~podman-create-workload-runs-workload~1] +# [stest->swdd~podman-create-workload-runs-workload~2] Test Ankaios Podman create workloads [Setup] Run Keywords Setup Ankaios @@ -47,7 +47,7 @@ Test Ankaios Podman create workloads [Teardown] Clean up Ankaios # [stest->swdd~agent-supports-podman~2] -# [stest->swdd~podman-create-workload-sets-optionally-container-name~1] +# [stest->swdd~podman-create-workload-sets-optionally-container-name~2] Test Ankaios Podman create a container with custom name [Setup] Run Keywords Setup Ankaios diff --git a/tests/stests/workloads/device_restart_with_dependencies.robot b/tests/stests/workloads/device_restart_with_dependencies.robot index f9f9ac95b..31084f5e5 100644 --- a/tests/stests/workloads/device_restart_with_dependencies.robot +++ b/tests/stests/workloads/device_restart_with_dependencies.robot @@ -21,9 +21,11 @@ Resource ../../resources/ankaios.resource Resource ../../resources/variables.resource *** Test Cases *** -# [stest->swdd~agent-existing-workloads-replace-updated~2] +# [stest->swdd~agent-existing-workloads-replace-updated~3] # [stest->swdd~agent-handles-workloads-with-fulfilled-dependencies~1] # [stest->swdd~agent-enqueues-unfulfilled-create~1] +# [stest->swdd~agent-existing-workloads-reuse-unmodified~1] +# [stest->swdd~podman-create-workload-starts-existing-workload~1] Test Ankaios restarts exited workloads on device restart with considering inter-workload dependencies [Documentation] Restart not running workloads after a device restart with considering inter-workload dependencies [Setup] Run Keywords Setup Ankaios @@ -43,13 +45,13 @@ Test Ankaios restarts exited workloads on device restart with considering inter- And Ankaios agent with name "agent_B" is terminated And Podman has terminated all existing containers # Restart of Ankaios on full device restart - And Ankaios server is started with config "${CONFIGS_DIR}/device_restart_with_dependencies.yaml" + And Ankaios server is started with config "${CONFIGS_DIR}/device_restart_with_dependencies_modified_web_service_init.yaml" And Ankaios agent is started with name "agent_B" And Ankaios agent is started with name "agent_A" # Asserts Then the workload "web_service" shall have the execution state "Pending(WaitingToStart)" on agent "agent_B" And the workload "web_service" shall have the execution state "Running(Ok)" on agent "agent_B" - And the container of workload "filesystem_init" shall have a different id but same configuration on the podman runtime - And the container of workload "web_service_init" shall have a different id but same configuration on the podman runtime - And the container of workload "web_service" shall have a different id but same configuration on the podman runtime + And the container of workload "filesystem_init" shall have the same id and same configuration on the podman runtime + And the container of workload "web_service_init" shall have a different id on the podman runtime + And the container of workload "web_service" shall have the same id and same configuration on the podman runtime [Teardown] Clean up Ankaios diff --git a/tools/generate_oft_html_report.sh b/tools/generate_oft_html_report.sh deleted file mode 100755 index cbb221fe3..000000000 --- a/tools/generate_oft_html_report.sh +++ /dev/null @@ -1,26 +0,0 @@ -#!/bin/bash - -# Copyright (c) 2023 Elektrobit Automotive GmbH -# -# This program and the accompanying materials are made available under the -# terms of the Apache License, Version 2.0 which is available at -# https://www.apache.org/licenses/LICENSE-2.0. -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -# -# SPDX-License-Identifier: Apache-2.0 - -set -e - -script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) -base_dir="$script_dir/.." -build_dir="$base_dir/build/req" - -mkdir -p "$build_dir" - -# shellcheck disable=SC2046 -oft trace $(find "$base_dir" -type d -name "src" -o -name "doc" -o -name "tests") -a swdd,impl,utest,itest,stest -o html -f "$build_dir/req_tracing_report.html"