This repository has been archived by the owner on May 21, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 61
Add automatic removal of old installed targets #1318
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
41de002
Fix a missing include
lbonn abfb0fe
Systematic `isTypeOf` for objects in events.h
lbonn 395bdc0
Remove some zealous throws in uptane-generator
lbonn 85b4162
Keep a log of installed versions
lbonn 0ea8674
Split loadInstalledVersions into two
lbonn a2a8a40
Add a simple autoclean feature for aktualizr daemon
lbonn 33d1973
Document autoclean and release log features
lbonn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc | ||
SAVEPOINT MIGRATION; | ||
|
||
ALTER TABLE installed_versions RENAME TO installed_versions_old; | ||
CREATE TABLE installed_versions(id INTEGER PRIMARY KEY, ecu_serial TEXT NOT NULL, sha256 TEXT NOT NULL, name TEXT NOT NULL, hashes TEXT NOT NULL, length INTEGER NOT NULL DEFAULT 0, correlation_id TEXT NOT NULL DEFAULT '', is_current INTEGER NOT NULL CHECK (is_current IN (0,1)) DEFAULT 0, is_pending INTEGER NOT NULL CHECK (is_pending IN (0,1)) DEFAULT 0, was_installed INTEGER NOT NULL CHECK (was_installed IN (0,1)) DEFAULT 0); | ||
INSERT INTO installed_versions(ecu_serial, sha256, name, hashes, length, correlation_id, is_current, is_pending, was_installed) SELECT installed_versions_old.ecu_serial, installed_versions_old.sha256, installed_versions_old.name, installed_versions_old.hashes, installed_versions_old.length, installed_versions_old.correlation_id, installed_versions_old.is_current, installed_versions_old.is_pending, 1 FROM installed_versions_old ORDER BY rowid; | ||
|
||
DROP TABLE installed_versions_old; | ||
|
||
DELETE FROM version; | ||
INSERT INTO version VALUES(20); | ||
|
||
RELEASE MIGRATION; |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc | ||
SAVEPOINT ROLLBACK_MIGRATION; | ||
|
||
CREATE TABLE installed_versions_migrate(ecu_serial TEXT NOT NULL, sha256 TEXT NOT NULL, name TEXT NOT NULL, hashes TEXT NOT NULL, length INTEGER NOT NULL DEFAULT 0, correlation_id TEXT NOT NULL DEFAULT '', is_current INTEGER NOT NULL CHECK (is_current IN (0,1)) DEFAULT 0, is_pending INTEGER NOT NULL CHECK (is_pending IN (0,1)) DEFAULT 0, UNIQUE(ecu_serial, sha256, name)); | ||
INSERT INTO installed_versions_migrate(ecu_serial, sha256, name, hashes, length, correlation_id, is_current, is_pending) SELECT installed_versions.ecu_serial, installed_versions.sha256, installed_versions.name, installed_versions.hashes, installed_versions.length, installed_versions.correlation_id, installed_versions.is_current, installed_versions.is_pending FROM installed_versions; | ||
|
||
DROP TABLE installed_versions; | ||
ALTER TABLE installed_versions_migrate RENAME TO installed_versions; | ||
|
||
DELETE FROM version; | ||
INSERT INTO version VALUES(19); | ||
lbonn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
RELEASE ROLLBACK_MIGRATION; |
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
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
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
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
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
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
#ifndef AKTUALIZR_H_ | ||
#define AKTUALIZR_H_ | ||
|
||
#include <atomic> | ||
#include <future> | ||
#include <memory> | ||
|
||
#include <boost/signals2.hpp> | ||
|
@@ -82,6 +82,19 @@ class Aktualizr { | |
*/ | ||
std::future<result::Download> Download(const std::vector<Uptane::Target>& updates); | ||
|
||
/** | ||
* Get log of installations. The log is indexed for every ECU and contains | ||
* every change of versions ordered by time. It may contain duplicates in | ||
* case of rollbacks. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this still true? Won't anything rolled back erase the prior generation number for that target? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What you describe is close to the old behavior. |
||
* @return installation log | ||
*/ | ||
struct InstallationLogEntry { | ||
Uptane::EcuSerial ecu; | ||
std::vector<Uptane::Target> installs; | ||
}; | ||
using InstallationLog = std::vector<InstallationLogEntry>; | ||
InstallationLog GetInstallationLog(); | ||
|
||
/** | ||
* Get list of targets currently in storage. This is intended to be used with | ||
* DeleteStoredTarget and targets are not guaranteed to be verified and | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
#include <set> | ||
|
||
#include "aktualizr_helpers.h" | ||
|
||
void targets_autoclean_cb(Aktualizr &aktualizr, const std::shared_ptr<event::BaseEvent> &event) { | ||
if (!event->isTypeOf<event::AllInstallsComplete>()) { | ||
return; | ||
} | ||
|
||
std::vector<Uptane::Target> installed_targets = aktualizr.GetStoredTargets(); | ||
std::vector<bool> to_remove(installed_targets.size(), true); | ||
|
||
Aktualizr::InstallationLog log = aktualizr.GetInstallationLog(); | ||
|
||
// keep the last two installed targets for each ecu | ||
for (const Aktualizr::InstallationLogEntry &entry : log) { | ||
auto start = entry.installs.size() >= 2 ? entry.installs.end() - 2 : entry.installs.begin(); | ||
for (auto it = start; it != entry.installs.end(); it++) { | ||
auto fit = std::find_if(installed_targets.begin(), installed_targets.end(), | ||
[&it](const Uptane::Target &t2) { return it->sha256Hash() == t2.sha256Hash(); }); | ||
|
||
if (fit == installed_targets.end()) { | ||
continue; | ||
} | ||
|
||
size_t rem_idx = static_cast<size_t>(fit - installed_targets.begin()); | ||
to_remove[rem_idx] = false; | ||
} | ||
} | ||
|
||
for (size_t k = 0; k < installed_targets.size(); k++) { | ||
if (to_remove[k]) { | ||
aktualizr.DeleteStoredTarget(installed_targets[k]); | ||
} | ||
} | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
#ifndef AKTUALIZR_HELPERS_H_ | ||
#define AKTUALIZR_HELPERS_H_ | ||
|
||
#include <memory> | ||
#include "aktualizr.h" | ||
|
||
/* | ||
* Signal handler to remove old targets just after an installation completes | ||
* | ||
* To be attached with Aktualizr::SetSignalHandler | ||
*/ | ||
void targets_autoclean_cb(Aktualizr &aktualizr, const std::shared_ptr<event::BaseEvent> &event); | ||
|
||
#endif // AKTUALIZR_HELPERS_H_ |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is
was_installed
actually used for? I'm not quite following why that is needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
installed_versions has is_current and is_pending columns to distinguish between what's currently running and staged from the rest.
was_installed is used to distinguish historical versions that were never current from the rest. ie: you set something to pending and end up installing some other version. The actual log would only contain versions that have actually been installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional note: we assume that all versions have been installed when migrating (hence, the 1). It seems to be better than reporting none.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just determine if it was installed if the generation is > 0 (or >= 0)? Or does the generation value get set even if the target is pending, such that if it isn't actually installed, you'd still want to know that it was pending at one point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea but in this code, generation was also incremented for pending version.
In the end I realized it was still complex so I removed the generation field entirely and relies on id which should have sufficient guarantees for this purpose.
was_installed is still there in the new code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't even realize that
id
would probably work fine. Makes sense if you're copying entries for rollbacks/reinstalls. I think that means this is good to go, then.