From e96bad7e74f25005bd4a6516308b014f02dc6fac Mon Sep 17 00:00:00 2001 From: Marcel Guzik Date: Wed, 24 Jan 2024 18:59:35 +0000 Subject: [PATCH 1/2] fixed services shown as child devices #2584 Added a workaround for #2584 by changing status update smartrest message to 102. Also fixed #2606 by properly checking in the fs event handler if the path of the event is actually inside the c8y operations path. Signed-off-by: Marcel Guzik --- crates/core/tedge_api/src/entity_store.rs | 16 ++-- crates/extensions/c8y_mapper_ext/src/actor.rs | 2 +- .../c8y_mapper_ext/src/dynamic_discovery.rs | 6 ++ .../c8y_mapper_ext/src/service_monitor.rs | 76 +++++++++++-------- 4 files changed, 61 insertions(+), 39 deletions(-) diff --git a/crates/core/tedge_api/src/entity_store.rs b/crates/core/tedge_api/src/entity_store.rs index 5ecc8e2d1ca..9a6e7c57bf2 100644 --- a/crates/core/tedge_api/src/entity_store.rs +++ b/crates/core/tedge_api/src/entity_store.rs @@ -714,16 +714,22 @@ pub enum EntityType { Service, } -impl Display for EntityType { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl EntityType { + pub fn as_str(&self) -> &str { match self { - EntityType::MainDevice => write!(f, "device"), - EntityType::ChildDevice => write!(f, "child-device"), - EntityType::Service => write!(f, "service"), + EntityType::MainDevice => "device", + EntityType::ChildDevice => "child-device", + EntityType::Service => "service", } } } +impl Display for EntityType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + impl EntityMetadata { /// Creates a entity metadata for the main device. pub fn main_device(device_id: String) -> Self { diff --git a/crates/extensions/c8y_mapper_ext/src/actor.rs b/crates/extensions/c8y_mapper_ext/src/actor.rs index f315185f0e8..1c102362754 100644 --- a/crates/extensions/c8y_mapper_ext/src/actor.rs +++ b/crates/extensions/c8y_mapper_ext/src/actor.rs @@ -201,7 +201,7 @@ impl C8yMapperActor { | FsWatchEvent::FileDeleted(path) | FsWatchEvent::Modified(path) | FsWatchEvent::DirectoryDeleted(path) => { - match process_inotify_events(&path, file_event) { + match process_inotify_events(&self.converter.ops_dir, &path, file_event) { Ok(Some(discovered_ops)) => { self.mqtt_publisher .send( diff --git a/crates/extensions/c8y_mapper_ext/src/dynamic_discovery.rs b/crates/extensions/c8y_mapper_ext/src/dynamic_discovery.rs index f39c9bb4703..912ed12a5ac 100644 --- a/crates/extensions/c8y_mapper_ext/src/dynamic_discovery.rs +++ b/crates/extensions/c8y_mapper_ext/src/dynamic_discovery.rs @@ -28,6 +28,7 @@ pub enum DynamicDiscoverOpsError { } pub fn process_inotify_events( + c8y_operations_dir: &Path, path: &Path, mask: FsWatchEvent, ) -> Result, DynamicDiscoverOpsError> { @@ -40,6 +41,11 @@ pub fn process_inotify_events( .parent() .ok_or_else(|| DynamicDiscoverOpsError::NotAnOperation(path.to_path_buf()))?; + // only files inside `/etc/tedge/operations/c8y` directory are valid c8y operations + if !parent_dir.starts_with(c8y_operations_dir) { + return Ok(None); + } + if is_valid_operation_name(operation_name) { match mask { FsWatchEvent::FileDeleted(_) => Ok(Some(DiscoverOp { diff --git a/crates/extensions/c8y_mapper_ext/src/service_monitor.rs b/crates/extensions/c8y_mapper_ext/src/service_monitor.rs index 1dfb6e7f143..4c6642ec95b 100644 --- a/crates/extensions/c8y_mapper_ext/src/service_monitor.rs +++ b/crates/extensions/c8y_mapper_ext/src/service_monitor.rs @@ -4,6 +4,7 @@ use serde::Serialize; use tedge_api::entity_store::EntityMetadata; use tedge_api::entity_store::EntityType; use tedge_mqtt_ext::Message; +use tracing::error; #[derive(Deserialize, Serialize, Debug, Default)] pub struct HealthStatus { @@ -25,13 +26,11 @@ pub fn convert_health_status_message( return vec![]; } - let mut mqtt_messages: Vec = Vec::new(); - // If not Bridge health status // FIXME: can also match "device/bridge//" or "/device/main/service/my_custom_bridge" // should match ONLY the single mapper bridge if entity.topic_id.as_str().contains("bridge") { - return mqtt_messages; + return vec![]; } let HealthStatus { @@ -42,16 +41,27 @@ pub fn convert_health_status_message( health_status = "unknown".into(); } - // FIXME: `ancestors_external_ids` gives external ids starting from the parent, but for health - // we need XID of current device as well - let mut external_ids = vec![entity.external_id.as_ref().to_string()]; - external_ids.extend_from_slice(ancestors_external_ids); - let status_message = - smartrest::inventory::service_status_update_message(&external_ids, &health_status); - - mqtt_messages.push(status_message); + let display_name = entity + .other + .get("name") + .and_then(|v| v.as_str()) + .or(entity.topic_id.default_service_name()) + .unwrap_or(entity.external_id.as_ref()); + + let display_type = entity + .other + .get("type") + .and_then(|v| v.as_str()) + .expect("display type should be inserted for every service in the converter"); + + let Ok(status_message) = + // smartrest::inventory::service_status_update_message(&external_ids, &health_status); + smartrest::inventory::service_creation_message(entity.external_id.as_ref(), display_name, display_type, &health_status, ancestors_external_ids) else { + error!("Can't create 102 for service status update"); + return vec![]; + }; - mqtt_messages + vec![status_message] } #[cfg(test)] @@ -66,57 +76,57 @@ mod tests { "test_device", "te/device/main/service/tedge-mapper-c8y/status/health", r#"{"pid":"1234","status":"up"}"#, - "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", - r#"104,up"#; - "service monitoring thin-edge device" + "c8y/s/us", + r#"102,test_device:device:main:service:tedge-mapper-c8y,service,tedge-mapper-c8y,up"#; + "service-monitoring-thin-edge-device" )] #[test_case( "test_device", "te/device/child/service/tedge-mapper-c8y/status/health", r#"{"pid":"1234","status":"up"}"#, - "c8y/s/us/test_device:device:child/test_device:device:child:service:tedge-mapper-c8y", - r#"104,up"#; - "service monitoring thin-edge child device" + "c8y/s/us/test_device:device:child", + r#"102,test_device:device:child:service:tedge-mapper-c8y,service,tedge-mapper-c8y,up"#; + "service-monitoring-thin-edge-child-device" )] #[test_case( "test_device", "te/device/main/service/tedge-mapper-c8y/status/health", r#"{"pid":"123456"}"#, - "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", - r#"104,unknown"#; - "service monitoring thin-edge no status" + "c8y/s/us", + r#"102,test_device:device:main:service:tedge-mapper-c8y,service,tedge-mapper-c8y,unknown"#; + "service-monitoring-thin-edge-no-status" )] #[test_case( "test_device", "te/device/main/service/tedge-mapper-c8y/status/health", r#"{"status":""}"#, - "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", - r#"104,unknown"#; - "service monitoring empty status" + "c8y/s/us", + r#"102,test_device:device:main:service:tedge-mapper-c8y,service,tedge-mapper-c8y,unknown"#; + "service-monitoring-empty-status" )] #[test_case( "test_device", "te/device/main/service/tedge-mapper-c8y/status/health", "{}", - "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", - r#"104,unknown"#; - "service monitoring empty health message" + "c8y/s/us", + r#"102,test_device:device:main:service:tedge-mapper-c8y,service,tedge-mapper-c8y,unknown"#; + "service-monitoring-empty-health-message" )] #[test_case( "test_device", "te/device/main/service/tedge-mapper-c8y/status/health", r#"{"status":"up,down"}"#, - "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", - r#"104,"up,down""#; - "service monitoring type with comma health message" + "c8y/s/us", + r#"102,test_device:device:main:service:tedge-mapper-c8y,service,tedge-mapper-c8y,"up,down""#; + "service-monitoring-type-with-comma-health-message" )] #[test_case( "test_device", "te/device/main/service/tedge-mapper-c8y/status/health", r#"{"status":"up\"down"}"#, - "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", - r#"104,"up""down""#; - "service monitoring double quotes health message" + "c8y/s/us", + r#"102,test_device:device:main:service:tedge-mapper-c8y,service,tedge-mapper-c8y,"up""down""#; + "service-monitoring-double-quotes-health-message" )] fn translate_health_status_to_c8y_service_monitoring_message( device_name: &str, From 4e715cdb3b8aaff5c5c1c441ffbd8ec0205f6cd8 Mon Sep 17 00:00:00 2001 From: Marcel Guzik Date: Fri, 26 Jan 2024 12:27:51 +0000 Subject: [PATCH 2/2] int-test no unexpected child devices are created --- .../tests/cumulocity/bootstrap.robot | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 tests/RobotFramework/tests/cumulocity/bootstrap.robot diff --git a/tests/RobotFramework/tests/cumulocity/bootstrap.robot b/tests/RobotFramework/tests/cumulocity/bootstrap.robot new file mode 100644 index 00000000000..0e9a0e18cef --- /dev/null +++ b/tests/RobotFramework/tests/cumulocity/bootstrap.robot @@ -0,0 +1,41 @@ +*** Settings *** +Resource ../../resources/common.resource + +Library Cumulocity +Library ThinEdgeIO +Library DateTime + +Test Teardown Get Logs + +*** Test Cases *** + +No unexpected child devices created with service autostart + [Tags] \#2584 + ${DEVICE_SN}= Setup skip_bootstrap=True + Execute Command test -f ./bootstrap.sh && ./bootstrap.sh --no-connect || true + Execute Command systemctl start mosquitto + Execute Command systemctl start tedge-agent + Execute Command systemctl start tedge-mapper-c8y + Execute Command test -f ./bootstrap.sh && ./bootstrap.sh --no-install --no-secure || true + Device Should Exist ${DEVICE_SN} + + # wait for messages to be processed + Sleep 15s + + # Assert that there are no child devices present. + Cumulocity.Device Should Not Have Any Child Devices + +No unexpected child devices created without service autostart + [Tags] \#2606 + ${DEVICE_SN}= Setup + Device Should Exist ${DEVICE_SN} + + # Touching the operations directories should not create child devices + Execute Command touch /etc/tedge/operations + Execute Command touch /etc/tedge/operations/c8y + + # wait for fs event to be detected + Sleep 5s + + # Assert that there are no child devices present. + Cumulocity.Device Should Not Have Any Child Devices