OTA-2488: Make Virtual Secondary use the new configuration way. #1246

mike-sul · 2019-06-28T15:21:29Z

Signed-off-by: Mike Sul ext-mykhaylo.sul@here.com

codecov-io · 2019-06-28T16:21:26Z

Codecov Report

Merging #1246 into master will decrease coverage by 0.11%.
The diff coverage is 84.14%.

@@            Coverage Diff             @@
##           master    #1246      +/-   ##
==========================================
- Coverage   79.49%   79.37%   -0.12%     
==========================================
  Files         168      167       -1     
  Lines       10144    10134      -10     
==========================================
- Hits         8064     8044      -20     
- Misses       2080     2090      +10

Impacted Files	Coverage Δ
src/libaktualizr/uptane/isotpsecondary.h	`0% <ø> (ø)`	⬆️
src/aktualizr_primary/secondary_config.h	`100% <ø> (ø)`	⬆️
src/libaktualizr/config/config.h	`100% <ø> (ø)`	⬆️
src/libaktualizr/primary/aktualizr.h	`100% <ø> (ø)`	⬆️
src/libaktualizr/config/config.cc	`93.63% <ø> (-1.08%)`	⬇️
src/libaktualizr/primary/sotauptaneclient.h	`100% <ø> (ø)`	⬆️
src/libaktualizr/uptane/isotpsecondary.cc	`0% <0%> (ø)`	⬆️
src/aktualizr_primary/secondary.cc	`84.21% <0%> (-3.47%)`	⬇️
...c/virtual_secondary/partialverificationsecondary.h	`77.77% <100%> (ø)`
src/virtual_secondary/virtualsecondary.cc	`100% <100%> (ø)`	⬆️
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12f8558...cef4c6a. Read the comment docs.

…e the old SecondaryConfig from libaktualizr Signed-off-by: Mike Sul <ext-mykhaylo.sul@here.com>

mike-sul · 2019-07-01T08:38:12Z

VirtualSecondary has been moved out of libaktualizr, as well as ManagedSecondary and PartialVerification since the first is used just by the virtual one and the second one is not used by any component or use case at all;
the new approach to secondaries configuration was applied to the virtual one;
the old approach to secondaries configuration has been removed;
SecondaryInterface has been freed from dependency on the old SecondaryConfig containing each secondary type specifics;
Documentation as well as meta-updater have been updated accordingly

config/sota-local.toml

src/virtual_secondary/CMakeLists.txt

src/aktualizr_primary/CMakeLists.txt

src/aktualizr_primary/secondary_config.h

src/libaktualizr/config/config.h

lbonn · 2019-07-03T10:03:15Z

src/libaktualizr/primary/aktualizr_test.cc

-  Aktualizr aktualizr(conf, storage, http);
+  UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
+  UptaneTestCommon::addDefaultSecondary(conf, temp_dir2, "sec_serial2", "sec_hwid2");
+  if (boost::filesystem::exists(conf.uptane.secondary_config_file)) {


Why is this if in an unit test? Shouldn't it fail if the file does not exist?

Ah I saw that a similar code lies in uptane_common_test.h. If I understand correctly, this is a test to verify that adding a secondary works after aktualizr has been instantiated.

If so, a small explanatory comment would be helpful.

It is redundant in the test, we actually should verify if the secondary instantiated successfully.

It's required in uptane_common_test.h as this code is shared among many tests including those that does not use secondaries. I will add explanatory comment.

lbonn · 2019-07-03T10:13:35Z

src/virtual_secondary/managedsecondary.h

@@ -1,5 +1,5 @@
-#ifndef UPTANE_MANAGEDSECONDARY_H_
-#define UPTANE_MANAGEDSECONDARY_H_
+#ifndef PRIMARY_MANAGEDSECONDARY_H_


include-guard was not updated. VIRTUAL_SECONDARY_MANAGEDSECONDARY_H_?

Actually, I did, PRIMARY_MANAGEDSECONDARY_H_ is the one I intended to apply, the first word denotes the namespace.

Ok, my bad that's from our naming conventions and it ends up being a bit confusing here.
It could be prefixed by the directory name but it would get quite big in this case. Or we could keep that.

lbonn · 2019-07-03T10:15:45Z

src/virtual_secondary/virtualsecondary.h

@@ -1,21 +1,35 @@
-#ifndef UPTANE_VIRTUALSECONDARY_H_
-#define UPTANE_VIRTUALSECONDARY_H_
+#ifndef PRIMARY_VIRTUALSECONDARY_H_


Same problem with include guard.

Signed-off-by: Mike Sul <ext-mykhaylo.sul@here.com>

mike-sul · 2019-07-03T14:02:14Z

Updated according to the latest comments

mike-sul force-pushed the refact/OTA-2488/virtual-secondary-refactory branch from c59aa46 to ea2fb03 Compare June 28, 2019 16:21

mike-sul force-pushed the refact/OTA-2488/virtual-secondary-refactory branch from ea2fb03 to 007fd98 Compare June 28, 2019 16:53

OTA-2488: Make Virtual Secondary use the new configuration way. Remov…

5146f19

…e the old SecondaryConfig from libaktualizr Signed-off-by: Mike Sul <ext-mykhaylo.sul@here.com>

mike-sul force-pushed the refact/OTA-2488/virtual-secondary-refactory branch 2 times, most recently from bfe08ca to f79c04a Compare July 1, 2019 08:18

mike-sul force-pushed the refact/OTA-2488/virtual-secondary-refactory branch 2 times, most recently from f55f554 to f8db802 Compare July 1, 2019 09:39

mike-sul requested review from pattivacek, lbonn and eu-siemann July 1, 2019 11:55

mike-sul mentioned this pull request Jul 1, 2019

OTA-2488: Remove SOTA_SECONDARY_CONFIG_DIR configuration variable advancedtelematic/meta-updater#533

Merged

mike-sul force-pushed the refact/OTA-2488/virtual-secondary-refactory branch from f8db802 to 5bb0a62 Compare July 1, 2019 13:42

mike-sul mentioned this pull request Jul 1, 2019

OTA-2488: Update the virtual secondary configuration part of the doc advancedtelematic/ats-garage-docs#90

Merged

lbonn reviewed Jul 3, 2019

View reviewed changes

OTA-2488: Move Managed and Virtual secondaries out of Uptane namespace

cef4c6a

Signed-off-by: Mike Sul <ext-mykhaylo.sul@here.com>

mike-sul force-pushed the refact/OTA-2488/virtual-secondary-refactory branch from 5bb0a62 to cef4c6a Compare July 3, 2019 13:10

lbonn approved these changes Jul 4, 2019

View reviewed changes

lbonn merged commit 9c592cf into master Jul 4, 2019

lbonn deleted the refact/OTA-2488/virtual-secondary-refactory branch July 4, 2019 10:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OTA-2488: Make Virtual Secondary use the new configuration way. #1246

OTA-2488: Make Virtual Secondary use the new configuration way. #1246

mike-sul commented Jun 28, 2019

codecov-io commented Jun 28, 2019 •

edited

Loading

mike-sul commented Jul 1, 2019 •

edited

Loading

lbonn Jul 3, 2019

lbonn Jul 3, 2019

mike-sul Jul 3, 2019

lbonn Jul 3, 2019

mike-sul Jul 3, 2019

lbonn Jul 3, 2019

lbonn Jul 3, 2019

mike-sul commented Jul 3, 2019

OTA-2488: Make Virtual Secondary use the new configuration way. #1246

OTA-2488: Make Virtual Secondary use the new configuration way. #1246

Conversation

mike-sul commented Jun 28, 2019

codecov-io commented Jun 28, 2019 • edited Loading

Codecov Report

mike-sul commented Jul 1, 2019 • edited Loading

lbonn Jul 3, 2019

Choose a reason for hiding this comment

lbonn Jul 3, 2019

Choose a reason for hiding this comment

mike-sul Jul 3, 2019

Choose a reason for hiding this comment

lbonn Jul 3, 2019

Choose a reason for hiding this comment

mike-sul Jul 3, 2019

Choose a reason for hiding this comment

lbonn Jul 3, 2019

Choose a reason for hiding this comment

lbonn Jul 3, 2019

Choose a reason for hiding this comment

mike-sul commented Jul 3, 2019

codecov-io commented Jun 28, 2019 •

edited

Loading

mike-sul commented Jul 1, 2019 •

edited

Loading