From ca2e8e1746ebde081cd1a3624dd9643e7160f59e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20M=C3=A4rkle?= Date: Thu, 19 Dec 2024 17:07:46 +0100 Subject: [PATCH] CARL -> Fix Ethernet Bridges and Executors being duplicated in PeerConfiguration every time the cluster got redeployed. --- Cargo.lock | 1 + .../src/actions/peers/assign_cluster.rs | 6 +- opendut-types/Cargo.toml | 1 + opendut-types/src/peer/configuration/mod.rs | 71 ++++++++++++++++++- .../src/peer/configuration/parameter/value.rs | 17 ++--- 5 files changed, 77 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 64f43c03..ec3467c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4083,6 +4083,7 @@ dependencies = [ name = "opendut-types" version = "0.4.0-alpha" dependencies = [ + "anyhow", "base64 0.22.1", "brotli", "glob", diff --git a/opendut-carl/src/actions/peers/assign_cluster.rs b/opendut-carl/src/actions/peers/assign_cluster.rs index 96e976b0..634f8045 100644 --- a/opendut-carl/src/actions/peers/assign_cluster.rs +++ b/opendut-carl/src/actions/peers/assign_cluster.rs @@ -57,7 +57,7 @@ pub async fn assign_cluster(params: AssignClusterParams) -> Result<(), AssignClu .unwrap_or_default(); for executor in Clone::clone(&peer_descriptor.executors).executors.into_iter() { - peer_configuration.insert(executor, ParameterTarget::Present); //TODO not always Present + peer_configuration.set(executor, ParameterTarget::Present); //TODO not always Present } { @@ -65,7 +65,7 @@ pub async fn assign_cluster(params: AssignClusterParams) -> Result<(), AssignClu .unwrap_or(options.bridge_name_default); let bridge = EthernetBridge { name: bridge }; - peer_configuration.insert(bridge, ParameterTarget::Present); //TODO not always Present + peer_configuration.set(bridge, ParameterTarget::Present); //TODO not always Present } peer_configuration @@ -189,7 +189,7 @@ mod tests { executors: vec![], ethernet_bridges: vec![], }; - peer_configuration.insert(EthernetBridge { name: NetworkInterfaceName::try_from("br-opendut-1")? }, ParameterTarget::Present); + peer_configuration.set(EthernetBridge { name: NetworkInterfaceName::try_from("br-opendut-1")? }, ParameterTarget::Present); let received = receiver.recv().await.unwrap() .message.unwrap(); diff --git a/opendut-types/Cargo.toml b/opendut-types/Cargo.toml index 3e1db82b..38fe3a48 100644 --- a/opendut-types/Cargo.toml +++ b/opendut-types/Cargo.toml @@ -29,6 +29,7 @@ prost = { workspace = true } strum_macros = "0.25.3" [dev-dependencies] +anyhow = { workspace = true } googletest = { workspace = true } indoc = { workspace = true } serde_yaml = { workspace = true } diff --git a/opendut-types/src/peer/configuration/mod.rs b/opendut-types/src/peer/configuration/mod.rs index b15acbd4..9585a586 100644 --- a/opendut-types/src/peer/configuration/mod.rs +++ b/opendut-types/src/peer/configuration/mod.rs @@ -18,7 +18,7 @@ pub struct PeerConfiguration { //TODO migrate more parameters } impl PeerConfiguration { - pub fn insert(&mut self, value: T, target: ParameterTarget) { + pub fn set(&mut self, value: T, target: ParameterTarget) { let parameter = Parameter { id: value.parameter_identifier(), dependencies: vec![], //TODO @@ -26,7 +26,72 @@ impl PeerConfiguration { value, }; - T::peer_configuration_field(self) - .push(parameter); //TODO don't insert, if already contained? Or maybe set the previous element absent? + let parameters = T::peer_configuration_field(self); + + parameters.retain(|existing_parameter| { + existing_parameter.id != parameter.id + }); + + parameters.push(parameter); + } +} + + +#[cfg(test)] +mod tests { + use crate::peer::executor::{ExecutorId, ExecutorKind, ResultsUrl}; + use crate::util::net::NetworkInterfaceName; + use super::*; + + #[test] + fn should_replace_a_previous_parameter_when_it_is_set_another_time() -> anyhow::Result<()> { + + let parameter_value = EthernetBridge { name: NetworkInterfaceName::try_from("br-opendut")? }; + + let mut testee = PeerConfiguration { + executors: vec![], + ethernet_bridges: vec![], + }; + testee.set(parameter_value.clone(), ParameterTarget::Present); + + + testee.set(parameter_value.clone(), ParameterTarget::Present); + assert_eq!(testee.ethernet_bridges.len(), 1); + + testee.set(parameter_value.clone(), ParameterTarget::Absent); + assert_eq!(testee.ethernet_bridges.len(), 1); + assert_eq!(testee.ethernet_bridges[0].target, ParameterTarget::Absent); + + Ok(()) + } + + + #[test] + fn should_update_the_value_of_a_parameter() -> anyhow::Result<()> { + + let parameter_value = ExecutorDescriptor { + id: ExecutorId::random(), + kind: ExecutorKind::Executable, + results_url: Some(ResultsUrl::try_from("https://example.com")?), + }; + + let mut testee = PeerConfiguration { + executors: vec![], + ethernet_bridges: vec![], + }; + testee.set(parameter_value.clone(), ParameterTarget::Present); + + + let expected = None; + let parameter_value = ExecutorDescriptor { + results_url: expected.clone(), + ..parameter_value + }; + + testee.set(parameter_value, ParameterTarget::Present); + assert_eq!(testee.executors.len(), 1); + assert_eq!(testee.executors[0].value.results_url, expected); + + Ok(()) } } diff --git a/opendut-types/src/peer/configuration/parameter/value.rs b/opendut-types/src/peer/configuration/parameter/value.rs index 2b18f42f..6be4d773 100644 --- a/opendut-types/src/peer/configuration/parameter/value.rs +++ b/opendut-types/src/peer/configuration/parameter/value.rs @@ -1,7 +1,7 @@ use crate::peer::configuration::parameter::{Parameter, ParameterId}; use crate::peer::configuration::PeerConfiguration; use crate::peer::ethernet::EthernetBridge; -use crate::peer::executor::{ExecutorDescriptor, ExecutorKind}; +use crate::peer::executor::ExecutorDescriptor; use crate::OPENDUT_UUID_NAMESPACE; use std::any::Any; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -40,16 +40,7 @@ pub trait ParameterValue: Any + Hash + Sized { impl ParameterValue for ExecutorDescriptor { fn parameter_identifier(&self) -> ParameterId { - let mut hasher = DefaultHasher::new(); //ID not stable across Rust releases - match &self.kind { - ExecutorKind::Executable => self.kind.hash(&mut hasher), - ExecutorKind::Container { name, .. } => name.hash(&mut hasher), - } - self.results_url.hash(&mut hasher); - let id = hasher.finish(); - - let id = Uuid::new_v5(&OPENDUT_UUID_NAMESPACE, &id.to_le_bytes()); - ParameterId(id) + ParameterId(self.id.uuid) } fn peer_configuration_field(peer_configuration: &mut PeerConfiguration) -> &mut Vec> { &mut peer_configuration.executors @@ -75,7 +66,7 @@ impl ParameterValue for EthernetBridge { mod tests { use super::*; use crate::peer::configuration::ParameterTarget; - use crate::peer::executor::ExecutorId; + use crate::peer::executor::{ExecutorId, ExecutorKind}; #[test] fn insert_value_in_peer_configuration() { @@ -90,7 +81,7 @@ mod tests { results_url: None }; let target = ParameterTarget::Present; - peer_configuration.insert(value.clone(), target); + peer_configuration.set(value.clone(), target); assert_eq!(peer_configuration.executors.len(), 1);