Merge pull request #2620 from Bravo555/fix/2584/services-shown-as-chi…

…ld-devices

fixed services shown as child devices #2584

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 {

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(

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<Option<DiscoverOp>, 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 {

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<Message> = 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,

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