Configurable timeout for custom operation handler

[PradeepKiruvale]

Proposed changes

The custom operation must timeout if not completed successfully within the given timeout period.
The timeout will be passed as part of the operation file, for example as below.

[exec]
  topic = "c8y/s/ds"
  on_message = "511"
  command = "/usr/share/tedge-contrib/bin/handle_command.sh"
  timeout = 60

As in the above operation file, if the operation does not complete within mentioned timeout then it will be gracefully killed by sending the sigterm and will wait for some time.
If the operation is not stopped even after sending the sigterm then it will be killed forcefully by sending the sigkill.

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s)
• I ran cargo fmt as mentioned in CODING_GUIDELINES
• I used cargo clippy as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[didier-wenzek]

The whole timeout logic has to be rewritten. But fist, add tests for this new feature.

let outcome_fut = self.inner_child.wait_with_output();

// first attempt
tokio::select! {
    outcome  = outcome_fut => {
         return self.finalize(outcome)
    }
   _ = tokio::time::sleep(duration) => {
       send_term
   }
}


// 2nd attempt
tokio::select! {
    outcome  = outcome_fut => {
         return self.finalize(outcome)
    }
   _ = tokio::time::sleep(duration) => {
       send_kill
   }
}

outcome  = outcome_fut.wait();
return  self.finalize(outcome);

[github-actions]

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %
221	0	5	221	100

Passed Tests

Name	⏱️ Duration	Suite
Define Child device 1 ID	0.006 s	C8Y Child Alarms Rpi
Normal case when the child device does not exist on c8y cloud	4.2620000000000005 s	C8Y Child Alarms Rpi
Normal case when the child device already exists	0.893 s	C8Y Child Alarms Rpi
Reconciliation when the new alarm message arrives, restart the mapper	1.7389999999999999 s	C8Y Child Alarms Rpi
Reconciliation when the alarm that is cleared	65.957 s	C8Y Child Alarms Rpi
Prerequisite Parent	15.845 s	Child Conf Mgmt Plugin
Prerequisite Child	0.664 s	Child Conf Mgmt Plugin
Child device bootstrapping	16.656 s	Child Conf Mgmt Plugin
Snapshot from device	62.504 s	Child Conf Mgmt Plugin
Child device config update	61.884 s	Child Conf Mgmt Plugin
Configuration types should be detected on file change (without restarting service)	58.845 s	Inotify Crate
Check lock file existence in default folder	1.5659999999999998 s	Lock File
Check PID number in lock file	1.8090000000000002 s	Lock File
Check PID number in lock file after restarting the services	2.423 s	Lock File
Check starting same service twice	1.317 s	Lock File
Switch off lock file creation	2.02 s	Lock File
Set configuration when file exists	16.435 s	Configuration Operation
Set configuration when file does not exist	6.283 s	Configuration Operation
Set configuration with broken url	6.301 s	Configuration Operation
Get configuration	6.95 s	Configuration Operation
Get non existent configuration file	5.767 s	Configuration Operation
Get non existent configuration type	5.114 s	Configuration Operation
Update configuration plugin config via cloud	4.993 s	Configuration Operation
Modify configuration plugin config via local filesystem modify inplace	2.247 s	Configuration Operation
Modify configuration plugin config via local filesystem overwrite	4.877 s	Configuration Operation
Update configuration plugin config via local filesystem copy	3.651 s	Configuration Operation
Update configuration plugin config via local filesystem move (different directory)	5.444 s	Configuration Operation
Update configuration plugin config via local filesystem move (same directory)	5.907 s	Configuration Operation
Update the custom operation dynamically	60.621 s	Dynamically Reload Operation
Custom operation successful	73.082 s	Custom Operation
Custom operation fails	136.157 s	Custom Operation
Successful firmware operation	75.357 s	Firmware Operation
Install with empty firmware name	55.437 s	Firmware Operation
Prerequisite Parent	18.312 s	Firmware Operation Child Device
Prerequisite Child	8.368 s	Firmware Operation Child Device
Child device firmware update	6.936 s	Firmware Operation Child Device
Child device firmware update with cache	6.523 s	Firmware Operation Child Device
Firmware plugin supports restart via service manager #1932	5.241 s	Firmware Operation Child Device Retry
Update Inventory data via inventory.json	1.55 s	Inventory Update
Inventory includes the agent fragment with version information	1.155 s	Inventory Update
Retrieve a JWT tokens	47.61 s	Jwt Request
Mapper recovers and processes output of ongoing software update request	17.058 s	Recover And Publish Software Update Message
Check running collectd	1.251 s	Monitor Device Collectd
Is collectd publishing MQTT messages?	2.856 s	Monitor Device Collectd
Check thin-edge monitoring	4.1370000000000005 s	Monitor Device Collectd
Check grouping of measurements	8.858 s	Monitor Device Collectd
Main device registration	1.913 s	Device Registration
Child device registration	2.29 s	Device Registration
Supports restarting the device	124.221 s	Restart Device
Update tedge version from previous using Cumulocity	80.353 s	Tedge Self Update
Test if all c8y services are up	114.552 s	Service Monitoring
Test if all c8y services are down	120.972 s	Service Monitoring
Test if all c8y services are using configured service type	161.426 s	Service Monitoring
Test if all c8y services using default service type when service type configured as empty	95.155 s	Service Monitoring
Check health status of tedge-mapper-c8y service on broker stop start	26.496 s	Service Monitoring
Check health status of tedge-mapper-c8y service on broker restart	27.896 s	Service Monitoring
Check health status of child device service	25.236 s	Service Monitoring
Successful shell command with output	3.77 s	Shell Operation
Check Successful shell command with literal double quotes output	3.268 s	Shell Operation
Execute multiline shell command	3.417 s	Shell Operation
Failed shell command	3.265 s	Shell Operation
Software list should be populated during startup	44.422 s	Software
Install software via Cumulocity	46.933 s	Software
Software list should only show currently installed software and not candidates	42.374 s	Software
Child devices support sending simple measurements	1.528 s	Child Device Telemetry
Child devices support sending custom measurements	1.252 s	Child Device Telemetry
Child devices support sending custom events	1.379 s	Child Device Telemetry
Child devices support sending custom events overriding the type	1.201 s	Child Device Telemetry
Child devices support sending custom alarms #1699	3.197 s	Child Device Telemetry
Child devices support sending inventory data via c8y topic	0.896 s	Child Device Telemetry
Child device supports sending custom child device measurements directly to c8y	1.456 s	Child Device Telemetry
Check retained alarms	160.016 s	Raise Alarms
Thin-edge devices support sending simple measurements	1.388 s	Thin-Edge Device Telemetry
Thin-edge devices support sending simple measurements with custom type	1.219 s	Thin-Edge Device Telemetry
Thin-edge devices support sending custom measurements	1.22 s	Thin-Edge Device Telemetry
Thin-edge devices support sending custom events	3.63 s	Thin-Edge Device Telemetry
Thin-edge devices support sending custom events overriding the type	1.264 s	Thin-Edge Device Telemetry
Thin-edge devices support sending custom alarms #1699	1.443 s	Thin-Edge Device Telemetry
Thin-edge device supports sending custom Thin-edge device measurements directly to c8y	1.7389999999999999 s	Thin-Edge Device Telemetry
Thin-edge device support sending inventory data via c8y topic	1.476 s	Thin-Edge Device Telemetry
thin-edge components support a custom config-dir location via flags	27.196 s	Config Dir
Validate updated data path used by tedge-agent	0.366 s	Data Path Config
Validate updated data path used by c8y-firmware-plugin	10.811 s	Data Path Config
Validate updated data path used by tedge-agent	0.74 s	Log Path Config
Install thin-edge via apt	42.18 s	Install Apt
Install latest via script (from current branch)	27.082 s	Install Tedge
Install specific version via script (from current branch)	33.209 s	Install Tedge
Install latest tedge via script (from main branch)	21.251 s	Install Tedge
Install then uninstall latest tedge via script (from main branch)	43.465 s	Install Tedge
Support starting and stopping services	41.869 s	Service-Control
Supports a reconnect	63.577 s	Test-Commands
Supports disconnect then connect	43.864 s	Test-Commands
Update unknown setting	42.065 s	Test-Commands
Update known setting	34.757 s	Test-Commands
It checks MQTT messages using a pattern	84.074 s	Test-Mqtt
Stop c8y-configuration-plugin	0.194 s	Health C8Y-Configuration-Plugin
Update the service file	0.241 s	Health C8Y-Configuration-Plugin
Reload systemd files	0.256 s	Health C8Y-Configuration-Plugin
Start c8y-configuration-plugin	0.162 s	Health C8Y-Configuration-Plugin
Start watchdog service	10.326 s	Health C8Y-Configuration-Plugin
Check PID of c8y-configuration-plugin	0.093 s	Health C8Y-Configuration-Plugin
Kill the PID	0.189 s	Health C8Y-Configuration-Plugin
Recheck PID of c8y-configuration-plugin	6.422 s	Health C8Y-Configuration-Plugin
Compare PID change	0.001 s	Health C8Y-Configuration-Plugin
Stop watchdog service	0.101 s	Health C8Y-Configuration-Plugin
Remove entry from service file	0.079 s	Health C8Y-Configuration-Plugin
Stop c8y-log-plugin	0.282 s	Health C8Y-Log-Plugin
Update the service file	0.138 s	Health C8Y-Log-Plugin
Reload systemd files	0.521 s	Health C8Y-Log-Plugin
Start c8y-log-plugin	0.218 s	Health C8Y-Log-Plugin
Start watchdog service	10.213 s	Health C8Y-Log-Plugin
Check PID of c8y-log-plugin	0.092 s	Health C8Y-Log-Plugin
Kill the PID	0.147 s	Health C8Y-Log-Plugin
Recheck PID of c8y-log-plugin	6.466 s	Health C8Y-Log-Plugin
Compare PID change	0.004 s	Health C8Y-Log-Plugin
Stop watchdog service	0.194 s	Health C8Y-Log-Plugin
Remove entry from service file	0.118 s	Health C8Y-Log-Plugin
Stop tedge-mapper	0.086 s	Health Tedge Mapper C8Y
Update the service file	0.113 s	Health Tedge Mapper C8Y
Reload systemd files	0.279 s	Health Tedge Mapper C8Y
Start tedge-mapper	0.112 s	Health Tedge Mapper C8Y
Start watchdog service	10.222 s	Health Tedge Mapper C8Y
Check PID of tedge-mapper	0.083 s	Health Tedge Mapper C8Y
Kill the PID	0.205 s	Health Tedge Mapper C8Y
Recheck PID of tedge-mapper	6.384 s	Health Tedge Mapper C8Y
Compare PID change	0.001 s	Health Tedge Mapper C8Y
Stop watchdog service	0.099 s	Health Tedge Mapper C8Y
Remove entry from service file	0.111 s	Health Tedge Mapper C8Y
Stop tedge-agent	0.104 s	Health Tedge-Agent
Update the service file	0.115 s	Health Tedge-Agent
Reload systemd files	0.391 s	Health Tedge-Agent
Start tedge-agent	0.105 s	Health Tedge-Agent
Start watchdog service	10.178 s	Health Tedge-Agent
Check PID of tedge-mapper	0.092 s	Health Tedge-Agent
Kill the PID	0.154 s	Health Tedge-Agent
Recheck PID of tedge-agent	6.48 s	Health Tedge-Agent
Compare PID change	0.001 s	Health Tedge-Agent
Stop watchdog service	0.23 s	Health Tedge-Agent
Remove entry from service file	0.227 s	Health Tedge-Agent
Stop tedge-mapper-az	0.102 s	Health Tedge-Mapper-Az
Update the service file	0.078 s	Health Tedge-Mapper-Az
Reload systemd files	0.498 s	Health Tedge-Mapper-Az
Start tedge-mapper-az	0.139 s	Health Tedge-Mapper-Az
Start watchdog service	10.142 s	Health Tedge-Mapper-Az
Check PID of tedge-mapper-az	0.081 s	Health Tedge-Mapper-Az
Kill the PID	0.262 s	Health Tedge-Mapper-Az
Recheck PID of tedge-agent	6.611 s	Health Tedge-Mapper-Az
Compare PID change	0.002 s	Health Tedge-Mapper-Az
Stop watchdog service	0.134 s	Health Tedge-Mapper-Az
Remove entry from service file	0.162 s	Health Tedge-Mapper-Az
Stop tedge-mapper-collectd	0.135 s	Health Tedge-Mapper-Collectd
Update the service file	0.066 s	Health Tedge-Mapper-Collectd
Reload systemd files	0.247 s	Health Tedge-Mapper-Collectd
Start tedge-mapper-collectd	0.094 s	Health Tedge-Mapper-Collectd
Start watchdog service	10.111 s	Health Tedge-Mapper-Collectd
Check PID of tedge-mapper-collectd	0.169 s	Health Tedge-Mapper-Collectd
Kill the PID	0.344 s	Health Tedge-Mapper-Collectd
Recheck PID of tedge-mapper-collectd	6.859 s	Health Tedge-Mapper-Collectd
Compare PID change	0.023 s	Health Tedge-Mapper-Collectd
Stop watchdog service	0.441 s	Health Tedge-Mapper-Collectd
Remove entry from service file	0.251 s	Health Tedge-Mapper-Collectd
tedge-collectd-mapper health status	5.9719999999999995 s	Health Tedge-Mapper-Collectd
c8y-log-plugin health status	5.925 s	MQTT health endpoints
c8y-configuration-plugin health status	6.192 s	MQTT health endpoints
Publish on a local insecure broker	0.463 s	Basic Pub Sub
Publish on a local secure broker	2.55 s	Basic Pub Sub
Publish on a local secure broker with client authentication	2.434 s	Basic Pub Sub
Check remote mqtt broker #1773	3.745 s	Remote Mqtt Broker
Apply name filter	0.089 s	Filter Packages List Output
Apply maintainer filter	0.27 s	Filter Packages List Output
Apply both filters	0.239 s	Filter Packages List Output
No filters	0.177 s	Filter Packages List Output
Both filters but name filter as empty string	0.339 s	Filter Packages List Output
Both filters but maintainer filter as empty string	0.23 s	Filter Packages List Output
Both filters as empty string	0.276 s	Filter Packages List Output
Wrong package name	0.226 s	Improve Tedge Apt Plugin Error Messages
Wrong version	0.167 s	Improve Tedge Apt Plugin Error Messages
Wrong type	0.576 s	Improve Tedge Apt Plugin Error Messages
tedge_connect_test_positive	0.387 s	Tedge Connect Test
tedge_connect_test_negative	1.826 s	Tedge Connect Test
tedge_connect_test_sm_services	10.256 s	Tedge Connect Test
tedge_disconnect_test_sm_services	1.236 s	Tedge Connect Test
Install thin-edge.io	23.458 s	Call Tedge
call tedge -V	0.101 s	Call Tedge
call tedge -h	0.072 s	Call Tedge
call tedge -h -V	0.096 s	Call Tedge
call tedge help	0.086 s	Call Tedge
tedge config list	0.061 s	Call Tedge Config List
tedge config list --all	0.084 s	Call Tedge Config List
set/unset device.type	0.306 s	Call Tedge Config List
set/unset device.key_path	0.284 s	Call Tedge Config List
set/unset device.cert_path	0.301 s	Call Tedge Config List
set/unset c8y.root_cert_path	0.283 s	Call Tedge Config List
set/unset c8y.smartrest.templates	0.316 s	Call Tedge Config List
set/unset az.root_cert_path	0.273 s	Call Tedge Config List
set/unset aws.url	0.298 s	Call Tedge Config List
set/unset aws.root_cert_path	0.27 s	Call Tedge Config List
set/unset aws.mapper.timestamp	0.282 s	Call Tedge Config List
set/unset az.mapper.timestamp	0.362 s	Call Tedge Config List
set/unset mqtt.bind.address	0.426 s	Call Tedge Config List
set/unset mqtt.bind.port	0.36 s	Call Tedge Config List
set/unset http.bind.port	0.371 s	Call Tedge Config List
set/unset tmp.path	0.494 s	Call Tedge Config List
set/unset logs.path	0.412 s	Call Tedge Config List
set/unset run.path	0.538 s	Call Tedge Config List
set/unset firmware.child.update.timeout	0.537 s	Call Tedge Config List
set/unset c8y.url	0.71 s	Call Tedge Config List
set/unset az.url	0.576 s	Call Tedge Config List
set/unset mqtt.external.bind.port	0.646 s	Call Tedge Config List
mqtt.external.bind.address	0.517 s	Call Tedge Config List
mqtt.external.bind.interface	0.314 s	Call Tedge Config List
set/unset mqtt.external.ca_path	0.296 s	Call Tedge Config List
set/unset mqtt.external.cert_file	0.535 s	Call Tedge Config List
set/unset mqtt.external.key_file	0.317 s	Call Tedge Config List
set/unset software.plugin.default	0.279 s	Call Tedge Config List
Get Put Delete	2.297 s	Http File Transfer Api
Set keys should return value on stdout	0.116 s	Tedge Config Get
Unset keys should not return anything on stdout and warnings on stderr	0.228 s	Tedge Config Get
Invalid keys should not return anything on stdout and warnings on stderr	0.307 s	Tedge Config Get
Set configuration via environment variables	1.101 s	Tedge Config Get
Set unknown configuration via environment variables	0.087 s	Tedge Config Get

[didier-wenzek]

I would really avoid to add new config settings in tedge.toml. For many reasons:

• This will have to be re-implemented soon with the new config under implementation.
• This is not urgent. This is for default values. We can hard-code them. Users can set timeout values to each operations if not happy with the defaults.

[didier-wenzek]

What does it mean to have a timeout of 0 seconds?

[PradeepKiruvale]

These tests do not need any timeout options, but the API needs those parameters. I can make them optional, but this will make the code more complicated. So, I went this way.

[PradeepKiruvale]

I would really avoid to add new config settings in tedge.toml. For many reasons:

• This will have to be re-implemented soon with the new config under implementation.
• This is not urgent. This is for default values. We can hard-code them. Users can set timeout values to each operations if not happy with the defaults.

The user can set only graceful_timeout not the forceful_timeout. Then tests will have to wait for a long for forceful_timeout.

[didier-wenzek]

I would really avoid to add new config settings in tedge.toml. For many reasons:

• This will have to be re-implemented soon with the new config under implementation.
• This is not urgent. This is for default values. We can hard-code them. Users can set timeout values to each operations if not happy with the defaults.

The user can set only graceful_timeout not the forceful_timeout. Then tests will have to wait for a long for forceful_timeout.

You added OnMessageExec::forceful_timeout, so the users can now set forceful_timeout ;-)

For testing purpose, it makes sense to pass default timeout values when building OnMessageExec objects. But these defaults values can be hardcoded by the mapper with no need to read a config file.

[PradeepKiruvale]

I would really avoid to add new config settings in tedge.toml. For many reasons:

• This will have to be re-implemented soon with the new config under implementation.
• This is not urgent. This is for default values. We can hard-code them. Users can set timeout values to each operations if not happy with the defaults.

The user can set only graceful_timeout not the forceful_timeout. Then tests will have to wait for a long for forceful_timeout.

You added OnMessageExec::forceful_timeout, so the users can now set forceful_timeout ;-)

For testing purpose, it makes sense to pass default timeout values when building OnMessageExec objects. But these defaults values can be hardcoded by the mapper with no need to read a config file.

Passing can be done in case of unit tests, what about the integration tests?

[codecov-commenter]

Codecov Report

Merging #1917 (f9aaf77) into main (c2c60d9) will increase coverage by 0.4%.
The diff coverage is 94.5%.

❗ Current head f9aaf77 differs from pull request most recent head fcd4141. Consider uploading reports for the commit fcd4141 to get more accurate results

Additional details and impacted files

Impacted Files	Coverage Δ
crates/core/plugin_sm/src/operation_logs.rs	84.0% <0.0%> (ø)
crates/core/c8y_api/src/smartrest/operations.rs	82.0% <85.1%> (+8.0%)	⬆️
crates/common/logged_command/src/logged_command.rs	81.5% <88.4%> (+0.5%)	⬆️
crates/extensions/c8y_mapper_ext/src/converter.rs	79.1% <96.2%> (+2.7%)	⬆️
crates/extensions/c8y_mapper_ext/src/tests.rs	93.5% <97.1%> (+1.1%)	⬆️

... and 5 files with indirect coverage changes

[rina23q]

Left some comments.

[didier-wenzek]

The main point of this PR is now correct.

Still some small points to be fix. Most importantly the test in c8y mapper converter that is now green for bad reasons.

[didier-wenzek]

Why an Option ?

[PradeepKiruvale]

To send the signal if the child is present.

[didier-wenzek]

Approved - Thank you.