Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Secondary implementation improvement #1529

Merged
merged 2 commits into from
Jan 23, 2020
Merged

Secondary implementation improvement #1529

merged 2 commits into from
Jan 23, 2020

Conversation

mike-sul
Copy link
Collaborator

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

@codecov-io
Copy link

codecov-io commented Jan 21, 2020 •
edited

Codecov Report

Merging #1529 into master will increase coverage by 0.07%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1529      +/-   ##
==========================================
+ Coverage   81.85%   81.93%   +0.07%     
==========================================
  Files         186      186              
  Lines       11403    11424      +21     
==========================================
+ Hits         9334     9360      +26     
+ Misses       2069     2064       -5
Impacted Files Coverage Δ
src/aktualizr_secondary/aktualizr_secondary.h 100% <ø> (ø) ⬆️
src/libaktualizr/uptane/manifest.h 100% <ø> (ø) ⬆️
src/libaktualizr/uptane/manifest.cc 94.87% <100%> (+0.13%) ⬆️
...aktualizr_secondary/aktualizr_secondary_factory.cc 95.23% <100%> (+0.5%) ⬆️
src/aktualizr_secondary/update_agent_ostree.cc 83.33% <100%> (+0.98%) ⬆️
src/aktualizr_secondary/aktualizr_secondary.cc 76.56% <73.91%> (+2.22%) ⬆️
src/libaktualizr/package_manager/ostreemanager.cc 79.02% <0%> (-1.13%) ⬇️
src/libaktualizr/storage/sqlstorage.cc 76.94% <0%> (-0.76%) ⬇️
src/libaktualizr/primary/sotauptaneclient.cc 90.01% <0%> (-0.14%) ⬇️
... and 5 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 4ad5eb2...c8814a9. Read the comment docs.

@mike-sul mike-sul force-pushed the test/OTA-4143/secondary-file-update-test branch 2 times, most recently from 2fefa57 to c8f68d5 Compare January 21, 2020 21:29
@mike-sul mike-sul changed the title Secondary updated file and manifest check in tests Secondary implementation improvement Jan 22, 2020
pattivacek
pattivacek reviewed Jan 22, 2020
View reviewed changes
Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but CI ain't happy.

src/aktualizr_secondary/update_agent_ostree.cc

// TODO: consider more elegant way of storing currently installed target name
// usage of the SQLStorage and ostree implementions aimed for Primary is
// a quite overhead for Secondary
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would you propose? We can make a ticket to reconsider this, but if we have to use the same storage and OSTree manager classes, it doesn't seem like much of an extra burden here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I have a few ideas, better to talk/discuss it during a call/meeting.

@mike-sul
Copy link
Collaborator Author

Looks good but CI ain't happy.

Yeah, I am working on it. I tested this fix on QEMU and it does work for me. Also, I am going to add a fix for the second minor issue into this PR (2) Secondary outputs an incorrect hash value just after applying the pending hash/version into logs, but it does not break anything)

@mike-sul mike-sul force-pushed the test/OTA-4143/secondary-file-update-test branch 3 times, most recently from 0c42544 to b1d8cdb Compare January 22, 2020 14:48
@mike-sul
Copy link
Collaborator Author

mike-sul commented Jan 22, 2020 •
edited by pattivacek

@patrickvacek I verified this version of QEMU multiple times with different value of OSTREE_BRANCHNAME and different reboot sequence, looks like it works and fixes the issue.

Mike Sul added 2 commits January 22, 2020 17:31
OTA-4143: Check an updated file and a manifest
635dcca 
Signed-off-by: Mike Sul <ext-mykhaylo.sul@here.com>
Correct secondary target name/filepath in a manifest
c8814a9 
Fix the issue with an incorrect value of filepath/target_name in a
device manifest if GARAGE_TARGET_NAME/OSTREE_BRANCHNAME != SOTA_HARDWARE_ID

Signed-off-by: Mike Sul <ext-mykhaylo.sul@here.com>
@mike-sul mike-sul force-pushed the test/OTA-4143/secondary-file-update-test branch from b1d8cdb to c8814a9 Compare January 22, 2020 15:33
@pattivacek pattivacek merged commit 9258111 into master Jan 23, 2020
@pattivacek pattivacek deleted the test/OTA-4143/secondary-file-update-test branch January 23, 2020 08:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pattivacek pattivacek pattivacek approved these changes

@lbonn lbonn Awaiting requested review from lbonn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mike-sul @codecov-io @pattivacek