Add address/port config settings for crates using MQTT clients #1789

Bravo555 · 2023-03-03T10:56:24Z

Proposed changes

As described in #1773, config settings for addresses and port numbers were made separate for MQTT clients and brokers, as often connect and bind addresses are different. This change will help in scenarios where the MQTT broker is located outside of the local network, e.g. when using containers or when using a hostname.

Additionally c8y_configuration_plugin, c8y_firmware_plugin, and c8y_log_plugin, which didnt's use any MQTT address/host config settings, and were hardcoded to default to localhost, now use MqttClientAddressSetting and MqttClientPortSetting settings.

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

Enable easier configuration of tedge components to connect to an external mqtt broker #1773

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

github-actions · 2023-03-03T13:33:27Z

Robot Results

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

Passed Tests

Name	⏱️ Duration	Suite
Define Child device 1 ID	0.011 s	`C8Y Child Alarms Rpi`
Normal case when the child device does not exist on c8y cloud	2.904 s	`C8Y Child Alarms Rpi`
Normal case when the child device already exists	1.046 s	`C8Y Child Alarms Rpi`
Reconciliation when the new alarm message arrives, restart the mapper	1.671 s	`C8Y Child Alarms Rpi`
Reconciliation when the alarm that is cleared	6.074 s	`C8Y Child Alarms Rpi`
Prerequisite Parent	23.651 s	`Child Conf Mgmt Plugin`
Prerequisite Child	0.317 s	`Child Conf Mgmt Plugin`
Child device bootstrapping	18.454 s	`Child Conf Mgmt Plugin`
Snapshot from device	26.979 s	`Child Conf Mgmt Plugin`
Child device config update	17.47 s	`Child Conf Mgmt Plugin`
Configuration types should be detected on file change (without restarting service)	66.27 s	`Inotify Crate`
Child devices support sending simple measurements	64.165 s	`Child Device Telemetry`
Child devices support sending custom measurements	57.738 s	`Child Device Telemetry`
Child devices support sending custom events	51.887 s	`Child Device Telemetry`
Child devices support sending custom events overriding the type	36.464 s	`Child Device Telemetry`
Child devices support sending custom alarms #1699	45.505 s	`Child Device Telemetry`
Child devices support sending inventory data via c8y topic	26.659 s	`Child Device Telemetry`
Main device support sending inventory data via c8y topic	30.112 s	`Child Device Telemetry`
Successful firmware operation	81.786 s	`Firmware Operation`
Install with empty firmware name	60.468 s	`Firmware Operation`
Supports restarting the device	104.656 s	`Restart Device`
Update tedge version from previous using Cumulocity	97.334 s	`Tedge Self Update`
Successful shell command with output	4.632 s	`Shell Operation`
Check Successful shell command with literal double quotes output	4.168 s	`Shell Operation`
Execute multiline shell command	3.73 s	`Shell Operation`
Failed shell command	3.622 s	`Shell Operation`
Software list should be populated during startup	64.612 s	`Software`
Install software via Cumulocity	88.976 s	`Software`
Software list should only show currently installed software and not candidates	54.528 s	`Software`
Stop tedge-agent service	0.371 s	`Log Path Config`
Customize the log path	0.156 s	`Log Path Config`
Initialize tedge-agent	0.189 s	`Log Path Config`
Check created folders	0.127 s	`Log Path Config`
Remove created custom folders	0.15 s	`Log Path Config`
Install latest via script (from current branch)	36.471 s	`Install Tedge`
Install specific version via script (from current branch)	19.565 s	`Install Tedge`
Install latest tedge via script (from main branch)	27.826999999999998 s	`Install Tedge`
Support starting and stopping services	48.296 s	`Service-Control`
Supports a reconnect	53.267 s	`Test-Commands`
Supports disconnect then connect	56.947 s	`Test-Commands`
Update unknown setting	39.332 s	`Test-Commands`
Update known setting	31.748 s	`Test-Commands`
Stop c8y-configuration-plugin	0.247 s	`Health C8Y-Configuration-Plugin`
Update the service file	0.212 s	`Health C8Y-Configuration-Plugin`
Reload systemd files	0.814 s	`Health C8Y-Configuration-Plugin`
Start c8y-configuration-plugin	0.283 s	`Health C8Y-Configuration-Plugin`
Start watchdog service	10.405 s	`Health C8Y-Configuration-Plugin`
Check PID of c8y-configuration-plugin	0.138 s	`Health C8Y-Configuration-Plugin`
Kill the PID	0.254 s	`Health C8Y-Configuration-Plugin`
Recheck PID of c8y-configuration-plugin	0.202 s	`Health C8Y-Configuration-Plugin`
Compare PID change	0.001 s	`Health C8Y-Configuration-Plugin`
Stop watchdog service	0.161 s	`Health C8Y-Configuration-Plugin`
Remove entry from service file	0.18 s	`Health C8Y-Configuration-Plugin`
Stop c8y-log-plugin	0.148 s	`Health C8Y-Log-Plugin`
Update the service file	0.145 s	`Health C8Y-Log-Plugin`
Reload systemd files	0.981 s	`Health C8Y-Log-Plugin`
Start c8y-log-plugin	0.206 s	`Health C8Y-Log-Plugin`
Start watchdog service	10.342 s	`Health C8Y-Log-Plugin`
Check PID of c8y-log-plugin	0.158 s	`Health C8Y-Log-Plugin`
Kill the PID	0.274 s	`Health C8Y-Log-Plugin`
Recheck PID of c8y-log-plugin	2.591 s	`Health C8Y-Log-Plugin`
Compare PID change	0.009 s	`Health C8Y-Log-Plugin`
Stop watchdog service	0.352 s	`Health C8Y-Log-Plugin`
Remove entry from service file	0.249 s	`Health C8Y-Log-Plugin`
Stop tedge-mapper	0.316 s	`Health Tedge Mapper C8Y`
Update the service file	0.299 s	`Health Tedge Mapper C8Y`
Reload systemd files	1.409 s	`Health Tedge Mapper C8Y`
Start tedge-mapper	0.548 s	`Health Tedge Mapper C8Y`
Start watchdog service	10.424 s	`Health Tedge Mapper C8Y`
Check PID of tedge-mapper	0.139 s	`Health Tedge Mapper C8Y`
Kill the PID	0.152 s	`Health Tedge Mapper C8Y`
Recheck PID of tedge-mapper	2.269 s	`Health Tedge Mapper C8Y`
Compare PID change	0.001 s	`Health Tedge Mapper C8Y`
Stop watchdog service	0.23 s	`Health Tedge Mapper C8Y`
Remove entry from service file	0.107 s	`Health Tedge Mapper C8Y`
Stop tedge-agent	0.343 s	`Health Tedge-Agent`
Update the service file	0.266 s	`Health Tedge-Agent`
Reload systemd files	0.731 s	`Health Tedge-Agent`
Start tedge-agent	0.212 s	`Health Tedge-Agent`
Start watchdog service	10.42 s	`Health Tedge-Agent`
Check PID of tedge-mapper	0.104 s	`Health Tedge-Agent`
Kill the PID	0.163 s	`Health Tedge-Agent`
Recheck PID of tedge-agent	2.267 s	`Health Tedge-Agent`
Compare PID change	0.002 s	`Health Tedge-Agent`
Stop watchdog service	0.16 s	`Health Tedge-Agent`
Remove entry from service file	0.147 s	`Health Tedge-Agent`
Stop tedge-mapper-az	0.385 s	`Health Tedge-Mapper-Az`
Update the service file	0.242 s	`Health Tedge-Mapper-Az`
Reload systemd files	1.015 s	`Health Tedge-Mapper-Az`
Start tedge-mapper-az	0.27 s	`Health Tedge-Mapper-Az`
Start watchdog service	10.372 s	`Health Tedge-Mapper-Az`
Check PID of tedge-mapper-az	0.18 s	`Health Tedge-Mapper-Az`
Kill the PID	0.247 s	`Health Tedge-Mapper-Az`
Recheck PID of tedge-agent	2.274 s	`Health Tedge-Mapper-Az`
Compare PID change	0.001 s	`Health Tedge-Mapper-Az`
Stop watchdog service	0.212 s	`Health Tedge-Mapper-Az`
Remove entry from service file	0.307 s	`Health Tedge-Mapper-Az`
Stop tedge-mapper-collectd	0.207 s	`Health Tedge-Mapper-Collectd`
Update the service file	0.173 s	`Health Tedge-Mapper-Collectd`
Reload systemd files	0.834 s	`Health Tedge-Mapper-Collectd`
Start tedge-mapper-collectd	0.204 s	`Health Tedge-Mapper-Collectd`
Start watchdog service	10.349 s	`Health Tedge-Mapper-Collectd`
Check PID of tedge-mapper-collectd	0.179 s	`Health Tedge-Mapper-Collectd`
Kill the PID	0.212 s	`Health Tedge-Mapper-Collectd`
Recheck PID of tedge-mapper-collectd	2.317 s	`Health Tedge-Mapper-Collectd`
Compare PID change	0.001 s	`Health Tedge-Mapper-Collectd`
Stop watchdog service	0.289 s	`Health Tedge-Mapper-Collectd`
Remove entry from service file	0.252 s	`Health Tedge-Mapper-Collectd`
c8y-log-plugin health status	5.992 s	`MQTT health endpoints`
c8y-configuration-plugin health status	5.931 s	`MQTT health endpoints`
Wrong package name	0.499 s	`Improve Tedge Apt Plugin Error Messages`
Wrong version	0.349 s	`Improve Tedge Apt Plugin Error Messages`
Wrong type	0.669 s	`Improve Tedge Apt Plugin Error Messages`
tedge_connect_test_positive	0.826 s	`Tedge Connect Test`
tedge_connect_test_negative	0.627 s	`Tedge Connect Test`
tedge_connect_test_sm_services	9.478 s	`Tedge Connect Test`
tedge_disconnect_test_sm_services	0.439 s	`Tedge Connect Test`
Install thin-edge.io	20.813 s	`Call Tedge`
call tedge -V	0.37 s	`Call Tedge`
call tedge -h	0.229 s	`Call Tedge`
call tedge -h -V	0.224 s	`Call Tedge`
call tedge help	0.301 s	`Call Tedge`
tedge config list	0.14 s	`Call Tedge Config List`
tedge config list --all	0.138 s	`Call Tedge Config List`
set/unset device.type	0.794 s	`Call Tedge Config List`
set/unset device.key.path	1.506 s	`Call Tedge Config List`
set/unset device.cert.path	0.906 s	`Call Tedge Config List`
set/unset c8y.root.cert.path	0.927 s	`Call Tedge Config List`
set/unset c8y.smartrest.templates	0.951 s	`Call Tedge Config List`
set/unset az.root.cert.path	1.014 s	`Call Tedge Config List`
set/unset az.mapper.timestamp	1.142 s	`Call Tedge Config List`
set/unset mqtt.bind_address	0.489 s	`Call Tedge Config List`
set/unset mqtt.port	0.374 s	`Call Tedge Config List`
set/unset tmp.path	0.41 s	`Call Tedge Config List`
set/unset logs.path	0.492 s	`Call Tedge Config List`
set/unset run.path	0.439 s	`Call Tedge Config List`
Get Put Delete	4.8149999999999995 s	`Http File Transfer Api`
Set keys should return value on stdout	0.29 s	`Tedge Config Get`
Unset keys should not return anything on stdout and warnings on stderr	0.735 s	`Tedge Config Get`
Invalid keys should not return anything on stdout and warnings on stderr	0.633 s	`Tedge Config Get`

reubenmiller

Looks good from a functional point of view. I was able to set the mqtt client host/port independently of other binding settings, and was able to use a hostname instead of an IP 🥇

I will let one of the other reviewers approve the code changes.

didier-wenzek

Approved.

crates/common/tedge_config/src/tedge_config_cli/config_setting.rs

crates/common/tedge_config/src/tedge_config_cli/tedge_config_dto.rs

crates/common/tedge_config/src/tedge_config_cli/settings.rs

crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs

crates/common/tedge_config/src/tedge_config_cli/tedge_config_dto.rs

Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>

As described in thin-edge#1773, config settings for addresses and port numbers were made separate for MQTT clients and brokers, as often connect and bind addresses are different. This change will help in scenarios where the MQTT broker is located outside of the local network, e.g. when using containers or when using a hostname. Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>

c8y_configuration_plugin, c8y_firmware_plugin, and c8y_log_plugin didnt's use any MQTT address/host config settings, but were hardcoded to default to localhost. Now they use MqttClientAddressSetting and MqttClientPortSetting settings. Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>

Bravo555 · 2023-03-06T16:26:20Z

Applied suggestions and also changed MqttClientAddress to MqttClientHost because it can also contain a hostname.

…edge#1789) * Added impl TryFrom<&str> for IpAddress Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com> * Add MQTT client address and port config options As described in thin-edge#1773, config settings for addresses and port numbers were made separate for MQTT clients and brokers, as often connect and bind addresses are different. This change will help in scenarios where the MQTT broker is located outside of the local network, e.g. when using containers or when using a hostname. Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com> * Make c8y plugins use client address/port settings c8y_configuration_plugin, c8y_firmware_plugin, and c8y_log_plugin didnt's use any MQTT address/host config settings, but were hardcoded to default to localhost. Now they use MqttClientAddressSetting and MqttClientPortSetting settings. Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com> --------- Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>

Bravo555 requested review from albinsuresh, didier-wenzek and reubenmiller March 3, 2023 10:57

Bravo555 temporarily deployed to Test Pull Request March 3, 2023 11:03 — with GitHub Actions Inactive

reubenmiller reviewed Mar 3, 2023

View reviewed changes

didier-wenzek approved these changes Mar 3, 2023

View reviewed changes

crates/common/tedge_config/src/tedge_config_cli/config_setting.rs Outdated Show resolved Hide resolved

crates/common/tedge_config/src/tedge_config_cli/tedge_config_dto.rs Outdated Show resolved Hide resolved

didier-wenzek mentioned this pull request Mar 3, 2023

Support setting tedge configuration via environment variables #1783

Closed

albinsuresh approved these changes Mar 6, 2023

View reviewed changes

Bravo555 had a problem deploying to Test Pull Request March 6, 2023 16:05 — with GitHub Actions Failure

Bravo555 force-pushed the tedge-config-mqtt branch from ea8c9ff to adfcc49 Compare March 6, 2023 16:17

Bravo555 added 3 commits March 6, 2023 17:24

Added impl TryFrom<&str> for IpAddress

1e35d80

Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>

Bravo555 force-pushed the tedge-config-mqtt branch from adfcc49 to 8402b86 Compare March 6, 2023 16:24

Bravo555 temporarily deployed to Test Pull Request March 6, 2023 16:34 — with GitHub Actions Inactive

reubenmiller merged commit d7df62d into thin-edge:main Mar 6, 2023

didier-wenzek mentioned this pull request Mar 13, 2023

Gather the pain points on implementing IIoT components and composing them #1406

Closed

Bravo555 deleted the tedge-config-mqtt branch June 12, 2023 15:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add address/port config settings for crates using MQTT clients #1789

Add address/port config settings for crates using MQTT clients #1789

Bravo555 commented Mar 3, 2023

github-actions bot commented Mar 3, 2023 •

edited

Loading

reubenmiller left a comment

didier-wenzek left a comment

Bravo555 commented Mar 6, 2023

Add address/port config settings for crates using MQTT clients #1789

Add address/port config settings for crates using MQTT clients #1789

Conversation

Bravo555 commented Mar 3, 2023

Proposed changes

Types of changes

Paste Link to the issue

Checklist

Further comments

github-actions bot commented Mar 3, 2023 • edited Loading

Robot Results

Passed Tests

reubenmiller left a comment

Choose a reason for hiding this comment

didier-wenzek left a comment

Choose a reason for hiding this comment

Bravo555 commented Mar 6, 2023

github-actions bot commented Mar 3, 2023 •

edited

Loading