Fix remaining test failures from built-in bridge #2879

jarhodes314 · 2024-05-15T09:28:50Z

Proposed changes

This PR fixes some remaining issues with the built-in bridge so that the tests work with the built-in bridge enabled.

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

Follow up tasks for built-in bridge #2794

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

codecov · 2024-05-15T09:40:39Z

Codecov Report

Attention: Patch coverage is 22.42991% with 166 lines in your changes are missing coverage. Please review.

Project coverage is 77.8%. Comparing base (d52bbdf) to head (299627a).
Report is 3 commits behind head on main.

Additional details and impacted files

Files	Coverage Δ
...core/tedge/src/cli/disconnect/disconnect_bridge.rs	`32.8% <100.0%> (-0.5%)`	⬇️
crates/extensions/tedge_mqtt_bridge/src/config.rs	`96.1% <100.0%> (ø)`
crates/extensions/tedge_mqtt_bridge/src/health.rs	`95.0% <100.0%> (+0.1%)`	⬆️
crates/core/c8y_api/src/smartrest/inventory.rs	`58.8% <94.1%> (+1.9%)`	⬆️
crates/core/tedge/src/cli/common.rs	`0.0% <0.0%> (ø)`
crates/extensions/tedge_mqtt_bridge/src/lib.rs	`89.1% <96.0%> (-0.1%)`	⬇️
crates/extensions/tedge_mqtt_bridge/src/topics.rs	`71.4% <20.0%> (-20.3%)`	⬇️
crates/core/tedge/src/cli/refresh_bridges.rs	`0.0% <0.0%> (ø)`
crates/core/tedge/src/cli/connect/command.rs	`0.0% <0.0%> (ø)`
crates/core/tedge_mapper/src/c8y/mapper.rs	`0.0% <0.0%> (ø)`

... and 5 files with indirect coverage changes

github-actions · 2024-05-15T09:55:34Z

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
444	0	3	444	100	1h1m32.791408999s

jarhodes314 · 2024-05-17T15:01:12Z

crates/core/tedge_mapper/src/c8y/mapper.rs

@@ -72,7 +74,36 @@ impl TEdgeComponent for CumulocityMapper {
                tc.forward_from_remote(topic, local_prefix.clone(), "")?;
            }

-            tc.forward_from_local("#", local_prefix, "")?;


I don't think this was genuinely problematic as Cumulocity doesn't send us our own messages back, but I've changed the config to match the current bridge config in any case

… Using '/bin/systemctl' as the default service manager No bridges to refresh. to assert ownership of certificate and delete unwanted mosquitto configurations Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

…file Signed-off-by: James Rhodes <jarhodes314@gmail.com>

…of mosquitto config Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

…dition to new logic Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

…reserved Signed-off-by: James Rhodes <jarhodes314@gmail.com>

…and flakiness Signed-off-by: James Rhodes <jarhodes314@gmail.com>

…enabling built in bridge Signed-off-by: James Rhodes <jarhodes314@gmail.com>

didier-wenzek

Approved. A nice step forward!

Once this PR merged, it would be good to re-run all the tests under a draft PR with the default value for mqtt.bridge.built_in set to true.

didier-wenzek · 2024-05-30T08:23:50Z

crates/extensions/tedge_mqtt_bridge/src/lib.rs

@@ -375,6 +377,7 @@ enum Status {
 impl Status {
    fn json(self) -> &'static str {
        match self {
+            // TODO Robot test that I make it to Cumulocity


I don't get this TODO. What has to be done?

didier-wenzek · 2024-05-30T08:28:53Z

crates/core/tedge/src/cli/connect/command.rs

+    // This will forcefully create directory structure if it doesn't exist, we should find better way to do it, maybe config should deal with it?
+    create_directories(dir_path)?;


Yeah. The directories should be created by tedge init and tedge connect should fail if given a path with unknown parents.

jarhodes314 temporarily deployed to Test Pull Request May 15, 2024 09:28 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 15, 2024 09:36 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request May 15, 2024 11:57 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 15, 2024 12:03 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request May 15, 2024 15:22 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 15, 2024 15:28 — with GitHub Actions Failure

jarhodes314 requested a deployment to Test Pull Request May 15, 2024 15:43 — with GitHub Actions Waiting

jarhodes314 force-pushed the enable-bridge-for-robot-tests branch from dc35b3f to 6dd0e52 Compare May 15, 2024 15:47

jarhodes314 temporarily deployed to Test Pull Request May 15, 2024 15:47 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 15, 2024 15:54 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request May 15, 2024 16:15 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 15, 2024 16:20 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request May 16, 2024 13:16 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 16, 2024 13:17 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request May 16, 2024 17:14 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 16, 2024 17:20 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request May 16, 2024 18:27 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 16, 2024 18:36 — with GitHub Actions Failure

jarhodes314 requested a deployment to Test Pull Request May 17, 2024 09:31 — with GitHub Actions Waiting

jarhodes314 force-pushed the enable-bridge-for-robot-tests branch from 2ecba4d to fc33478 Compare May 17, 2024 09:32

jarhodes314 temporarily deployed to Test Pull Request May 17, 2024 09:32 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 17, 2024 09:41 — with GitHub Actions Failure

jarhodes314 requested a deployment to Test Pull Request May 17, 2024 14:49 — with GitHub Actions Waiting

jarhodes314 temporarily deployed to Test Pull Request May 17, 2024 14:51 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 17, 2024 14:57 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request May 17, 2024 14:58 — with GitHub Actions Inactive

jarhodes314 commented May 17, 2024

View reviewed changes

jarhodes314 had a problem deploying to Test Auto May 17, 2024 15:21 — with GitHub Actions Failure

jarhodes314 added 9 commits May 21, 2024 09:44

Modify The system config file '/etc/tedge/system.toml' doesn't exist.…

c755244

… Using '/bin/systemctl' as the default service manager No bridges to refresh. to assert ownership of certificate and delete unwanted mosquitto configurations Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Ignore unconfigured clouds in refresh-bridges

c99ba66

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Modify bridge clean-up to truncate, rather than delete bridge config …

1c4accc

…file Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Update built-in bridge tests to check health status and ignore state …

89db10f

…of mosquitto config Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Fix service status reporting when the mapper goes down

7fb77a9

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Improve mapping rules and restore original service status logic in ad…

1d00724

…dition to new logic Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Fix tests

884807b

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Update handling of built-in bridge to reflect the config file being p…

bbff089

…reserved Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Remove unnecessary restart of tedge-mapper that was causing slowness …

30b0e61

…and flakiness Signed-off-by: James Rhodes <jarhodes314@gmail.com>

jarhodes314 force-pushed the enable-bridge-for-robot-tests branch from c299254 to 30b0e61 Compare May 21, 2024 08:44

jarhodes314 temporarily deployed to Test Pull Request May 21, 2024 08:44 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 21, 2024 09:09 — with GitHub Actions Failure

Merge branch 'main' into enable-bridge-for-robot-tests

df59548

jarhodes314 temporarily deployed to Test Pull Request May 29, 2024 14:31 — with GitHub Actions Inactive

jarhodes314 had a problem deploying to Test Auto May 29, 2024 14:37 — with GitHub Actions Failure

Update test suite to give more helpful error message and fix test by …

299627a

…enabling built in bridge Signed-off-by: James Rhodes <jarhodes314@gmail.com>

jarhodes314 temporarily deployed to Test Pull Request May 29, 2024 15:07 — with GitHub Actions Inactive

jarhodes314 temporarily deployed to Test Auto May 29, 2024 15:13 — with GitHub Actions Inactive

jarhodes314 changed the title ~~Enable the built-in bridge for robot testing~~ Fix remaining test failures from built-in bridge May 29, 2024

jarhodes314 marked this pull request as ready for review May 29, 2024 16:30

jarhodes314 requested review from reubenmiller, albinsuresh, rina23q and a team as code owners May 29, 2024 16:30

didier-wenzek approved these changes May 30, 2024

View reviewed changes

jarhodes314 added this pull request to the merge queue May 31, 2024

Merged via the queue into thin-edge:main with commit ba2b00a May 31, 2024
32 checks passed

jarhodes314 mentioned this pull request May 31, 2024

Enable built-in bridge for testing #2911

Draft

11 tasks

reubenmiller added the theme:mqtt Theme: mqtt and mosquitto related topics label Jun 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix remaining test failures from built-in bridge #2879

Fix remaining test failures from built-in bridge #2879

jarhodes314 commented May 15, 2024 •

edited

codecov bot commented May 15, 2024 •

edited

github-actions bot commented May 15, 2024 •

edited

jarhodes314 May 17, 2024

didier-wenzek left a comment

didier-wenzek May 30, 2024

didier-wenzek May 30, 2024

		// This will forcefully create directory structure if it doesn't exist, we should find better way to do it, maybe config should deal with it?
		create_directories(dir_path)?;

Fix remaining test failures from built-in bridge #2879

Fix remaining test failures from built-in bridge #2879

Conversation

jarhodes314 commented May 15, 2024 • edited

Proposed changes

Types of changes

Paste Link to the issue

Checklist

Further comments

codecov bot commented May 15, 2024 • edited

Codecov Report

github-actions bot commented May 15, 2024 • edited

Robot Results

jarhodes314 May 17, 2024

Choose a reason for hiding this comment

didier-wenzek left a comment

Choose a reason for hiding this comment

didier-wenzek May 30, 2024

Choose a reason for hiding this comment

didier-wenzek May 30, 2024

Choose a reason for hiding this comment

jarhodes314 commented May 15, 2024 •

edited

codecov bot commented May 15, 2024 •

edited

github-actions bot commented May 15, 2024 •

edited