This repository has been archived by the owner on May 21, 2024. It is now read-only.
Fix/ota 2451/empty targets #1186
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's taken care of by SotaUptaneClient already in a generic fashion for all package managers. Signed-off-by: Patrick Vacek <[email protected]>
Signed-off-by: Patrick Vacek <[email protected]>
This should prevent libaktualizr from getting stuck just because of unexpected data in the manifest that gets rejected from the server. Signed-off-by: Patrick Vacek <[email protected]>
Previously, we could get left in an unexpected state if an update was interrupted and then resumed after receiving empty targets from the Director. libaktualizr could know about an update that was no longer in the metadata, which made verification and correlation ID tracking difficult. Now we don't write the empty targets metadata to the database if we still have a non-empty version available. We still check the latest (potentially empty) targets for expiration. This way we can keep better track of the last thing the server told us to install. Included is a test that verifies that we do the right thing in exactly these circumstances. A key component is making sure we never send a manifest with an empty correlation ID, which the server will reject and thus should no longer ever happen. Signed-off-by: Patrick Vacek <[email protected]>
This makes managing a repo with aktualizr-repo much easier. Signed-off-by: Patrick Vacek <[email protected]>
Signed-off-by: Patrick Vacek <[email protected]>
Instead of chain-including all of gtest, this just includes a single macro to enable FRIEND_TEST to work as intended. Signed-off-by: Patrick Vacek <[email protected]>
Signed-off-by: Patrick Vacek <[email protected]>
pattivacek
force-pushed
the
fix/OTA-2451/empty-targets
branch
from
c199cf1 to
d0d726c
Compare
lbonn
reviewed
Apr 18, 2019
| @@ -17,5 +17,7 @@ add_aktualizr_test(NAME aktualizr SOURCES aktualizr_test.cc PROJECT_WORKING_DIRE | |||
| add_dependencies(t_aktualizr uptane_repo_full_no_correlation_id) | |||
|
|
|||
| add_aktualizr_test(NAME reportqueue SOURCES reportqueue_test.cc PROJECT_WORKING_DIRECTORY) | |||
| add_aktualizr_test(NAME emptytargets SOURCES empty_targets_test.cc PROJECT_WORKING_DIRECTORY | |||
There was a problem hiding this comment.
Inconsequential but this source file is introduced in a later commit.
There was a problem hiding this comment.
Oops. If anything else comes up, I can fix that, but I'm not going to rebase just for that.
lbonn
approved these changes
Apr 18, 2019
|
Apparently gitlab had an error pulling from github, but refreshing seems to have done the job. |
mike-sul
reviewed
Apr 18, 2019
| @@ -29,6 +29,7 @@ TEST(PackageManagerFake, FinalizeAfterReboot) { | |||
| Uptane::Target target("pkg", {Uptane::Hash(Uptane::Hash::Type::kSha256, "hash")}, 1, ""); | |||
| auto result = fakepm.install(target); | |||
| EXPECT_EQ(result.result_code, data::ResultCode::Numeric::kNeedCompletion); | |||
| storage->savePrimaryInstalledVersion(target, InstalledVersionUpdateMode::kPending); | |||
There was a problem hiding this comment.
I know this is out of scope of this PR just noticed that savePrimaryInstalledVersion() neither returns error code nor throws exception in case of failure.
There was a problem hiding this comment.
Yes, that's one of the places that I've mentioned this morning. Feel free to fix it here, of course, or you can wait for me to prepare a PR with few more similar fixes.
Codecov Report
@@ Coverage Diff @@
## master #1186 +/- ##
==========================================
+ Coverage 77.91% 77.97% +0.05%
==========================================
Files 170 170
Lines 10003 10005 +2
==========================================
+ Hits 7794 7801 +7
+ Misses 2209 2204 -5
Continue to review full report at Codecov.
|
cajun-rat
added a commit
to cajun-rat/aktualizr
that referenced
this pull request
Dec 31, 2023
This removes the distinction between 'the last director targets the server sent us' and 'the last _non-empty_ targets the server sent us'. This change was introduced in 2019 by advancedtelematic#1186 (aka #02deeb5), we believe to deal with the old director that needed the client to always send back a end-state manifest for any correlationid it gave out. Unfortunately this doesn't work with offline updates. Consider the following steps: 1) The user sends an online update to a device 2) Aktualizr receives a non-empty targets, and installs it 3) The user plugs in a device and installs an offline update 4) Aktualizr sends the manifest to the server 5) The server recognises that the device has been updated offline, and starts sending empty targets to mean 'I don't know what you should be running, carry on as before' Under the old behaviour, Aktualizr would ignore the empty targets and try and install the whatever was sent in step advancedtelematic#2. This also removes the targets_meta field from the UpdateCheck. This appears to be unused and doesn't have any tests. It was introduced in 2018 by advancedtelematic#981, aka Commit #c3ca9f52. We populate it by fishing the information out of the Sqlite storage, and this introduces a few more error paths in SotaUptaneClient.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two main fixes:
There are tests for both in addition to some other much smaller things found while working on this.