update_engine: Call dlcservice's Install/Update Completion DBus APIs
When an Install/Update completes, update_engine will now let dlcservice
know all the DLCs that were installed/updated + verified.
Update_engine will also track during install/update for DLCs which did
not install/update so dlcservice receives the correct list of DLC IDs.
BUG=chromium:1059126
TEST=FEATURES=test emerge-$B update_engine update_engine-client
Cq-Depend: chromium:2141191
Change-Id: Id57f66c7c6957d34870d27119d9a6482fe902503
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2146104
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Andrew Lassalle <andrewlassalle@chromium.org>
Auto-Submit: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/common/dlcservice_interface.h b/common/dlcservice_interface.h
index aa24105..3524d50 100644
--- a/common/dlcservice_interface.h
+++ b/common/dlcservice_interface.h
@@ -34,6 +34,14 @@
// On failure it returns false.
virtual bool GetInstalled(std::vector<std::string>* dlc_module_ids) = 0;
+ // Returns true if dlcservice successfully handled the install completion
+ // method call, otherwise false.
+ virtual bool InstallCompleted(const std::vector<std::string>& ids) = 0;
+
+ // Returns true if dlcservice successfully handled the update completion
+ // method call, otherwise false.
+ virtual bool UpdateCompleted(const std::vector<std::string>& ids) = 0;
+
protected:
DlcServiceInterface() = default;
diff --git a/common/dlcservice_stub.cc b/common/dlcservice_stub.cc
index c5f9306..3dcb2e0 100644
--- a/common/dlcservice_stub.cc
+++ b/common/dlcservice_stub.cc
@@ -33,4 +33,11 @@
return true;
}
+bool DlcServiceStub::InstallCompleted(const vector<string>& ids) {
+ return true;
+}
+bool DlcServiceStub::UpdateCompleted(const vector<string>& ids) {
+ return true;
+}
+
} // namespace chromeos_update_engine
diff --git a/common/dlcservice_stub.h b/common/dlcservice_stub.h
index 4e12c11..9b27971 100644
--- a/common/dlcservice_stub.h
+++ b/common/dlcservice_stub.h
@@ -32,6 +32,8 @@
// BootControlInterface overrides.
bool GetInstalled(std::vector<std::string>* dlc_module_ids) override;
+ bool InstallCompleted(const std::vector<std::string>& ids) override;
+ bool UpdateCompleted(const std::vector<std::string>& ids) override;
private:
DISALLOW_COPY_AND_ASSIGN(DlcServiceStub);
diff --git a/dlcservice_chromeos.cc b/dlcservice_chromeos.cc
index ad5806a..3c76b2a 100644
--- a/dlcservice_chromeos.cc
+++ b/dlcservice_chromeos.cc
@@ -16,6 +16,7 @@
#include "update_engine/dlcservice_chromeos.h"
+#include <brillo/errors/error.h>
#include <dlcservice/proto_bindings/dlcservice.pb.h>
// NOLINTNEXTLINE(build/include_alpha) "dbus-proxies.h" needs "dlcservice.pb.h"
#include <dlcservice/dbus-proxies.h>
@@ -28,6 +29,12 @@
namespace chromeos_update_engine {
+namespace {
+org::chromium::DlcServiceInterfaceProxy GetDlcServiceProxy() {
+ return {DBusConnection::Get()->GetDBus()};
+}
+} // namespace
+
std::unique_ptr<DlcServiceInterface> CreateDlcService() {
return std::make_unique<DlcServiceChromeOS>();
}
@@ -37,11 +44,8 @@
return false;
dlc_module_ids->clear();
- org::chromium::DlcServiceInterfaceProxy dlcservice_proxy(
- DBusConnection::Get()->GetDBus());
-
dlcservice::DlcModuleList dlc_module_list;
- if (!dlcservice_proxy.GetInstalled(&dlc_module_list, nullptr)) {
+ if (!GetDlcServiceProxy().GetInstalled(&dlc_module_list, nullptr)) {
LOG(ERROR) << "dlcservice does not return installed DLC module list.";
return false;
}
@@ -51,4 +55,24 @@
return true;
}
+bool DlcServiceChromeOS::InstallCompleted(const std::vector<std::string>& ids) {
+ brillo::ErrorPtr err;
+ if (!GetDlcServiceProxy().InstallCompleted(ids, &err)) {
+ LOG(ERROR) << "dlcservice failed to complete install. ErrCode="
+ << err->GetCode() << ", ErrMsg=" << err->GetMessage();
+ return false;
+ }
+ return true;
+}
+
+bool DlcServiceChromeOS::UpdateCompleted(const std::vector<std::string>& ids) {
+ brillo::ErrorPtr err;
+ if (!GetDlcServiceProxy().UpdateCompleted(ids, &err)) {
+ LOG(ERROR) << "dlcservice failed to complete updated. ErrCode="
+ << err->GetCode() << ", ErrMsg=" << err->GetMessage();
+ return false;
+ }
+ return true;
+}
+
} // namespace chromeos_update_engine
diff --git a/dlcservice_chromeos.h b/dlcservice_chromeos.h
index 73442e6..b56b495 100644
--- a/dlcservice_chromeos.h
+++ b/dlcservice_chromeos.h
@@ -32,13 +32,20 @@
DlcServiceChromeOS() = default;
~DlcServiceChromeOS() = default;
- // BootControlInterface overrides.
+ // DlcServiceInterface overrides.
+
// Will clear the |dlc_module_ids|, passed to be modified. Clearing by
// default has the added benefit of avoiding indeterminate behavior in the
// case that |dlc_module_ids| wasn't empty to begin which would lead to
// possible duplicates and cases when error was not checked it's still safe.
bool GetInstalled(std::vector<std::string>* dlc_module_ids) override;
+ // Call into dlcservice for it to mark the DLC IDs as being installed.
+ bool InstallCompleted(const std::vector<std::string>& ids) override;
+
+ // Call into dlcservice for it to mark the DLC IDs as being updated.
+ bool UpdateCompleted(const std::vector<std::string>& ids) override;
+
private:
DISALLOW_COPY_AND_ASSIGN(DlcServiceChromeOS);
};
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 50fe3cc..85699d8 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -782,6 +782,7 @@
if (params_->IsDlcAppId(app.id)) {
LOG(INFO) << "No update for <app> " << app.id
<< " but update continuing since a DLC.";
+ params_->SetDlcNoUpdate(app.id);
continue;
}
// Don't update if any app has status="noupdate".
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index 8863392..5267598 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -258,4 +258,11 @@
return dlc_apps_params().find(app_id) != dlc_apps_params().end();
}
+void OmahaRequestParams::SetDlcNoUpdate(const string& app_id) {
+ auto itr = dlc_apps_params_.find(app_id);
+ if (itr == dlc_apps_params_.end())
+ return;
+ itr->second.updated = false;
+}
+
} // namespace chromeos_update_engine
diff --git a/omaha_request_params.h b/omaha_request_params.h
index 14f3eaf..b33d0b1 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -76,6 +76,9 @@
int64_t ping_date_last_active;
int64_t ping_date_last_rollcall;
bool send_ping;
+ // |updated| is only used for DLCs to decide sending DBus message to
+ // dlcservice on an install/update completion.
+ bool updated = true;
};
// Setters and getters for the various properties.
@@ -232,6 +235,9 @@
// request parameters.
virtual bool IsDlcAppId(const std::string& app_id) const;
+ // If the App ID is a DLC App ID will set to no update.
+ void SetDlcNoUpdate(const std::string& app_id);
+
// Suggested defaults
static const char kOsVersion[];
static const int64_t kDefaultMinUpdateChecks = 0;
diff --git a/update_attempter.cc b/update_attempter.cc
index 29d256c..c45fe4f 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -1219,8 +1219,18 @@
}
}
+vector<string> UpdateAttempter::GetSuccessfulDlcIds() {
+ vector<string> dlc_ids;
+ for (const auto& pr : omaha_request_params_->dlc_apps_params())
+ if (pr.second.updated)
+ dlc_ids.push_back(pr.second.name);
+ return dlc_ids;
+}
+
void UpdateAttempter::ProcessingDoneInstall(const ActionProcessor* processor,
ErrorCode code) {
+ if (!system_state_->dlcservice()->InstallCompleted(GetSuccessfulDlcIds()))
+ LOG(WARNING) << "dlcservice didn't successfully handle install completion.";
SetStatusAndNotify(UpdateStatus::IDLE);
ScheduleUpdates();
LOG(INFO) << "DLC successfully installed, no reboot needed.";
@@ -1230,6 +1240,8 @@
ErrorCode code) {
WriteUpdateCompletedMarker();
+ if (!system_state_->dlcservice()->UpdateCompleted(GetSuccessfulDlcIds()))
+ LOG(WARNING) << "dlcservice didn't successfully handle update completion.";
SetStatusAndNotify(UpdateStatus::UPDATED_NEED_REBOOT);
ScheduleUpdates();
LOG(INFO) << "Update successfully applied, waiting to reboot.";
diff --git a/update_attempter.h b/update_attempter.h
index c364de3..3c6f4a1 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -292,6 +292,7 @@
FRIEND_TEST(UpdateAttempterTest, UpdateAttemptFlagsCachedAtUpdateStart);
FRIEND_TEST(UpdateAttempterTest, UpdateDeferredByPolicyTest);
FRIEND_TEST(UpdateAttempterTest, UpdateIsNotRunningWhenUpdateAvailable);
+ FRIEND_TEST(UpdateAttempterTest, GetSuccessfulDlcIds);
// Returns the special flags to be added to ErrorCode values based on the
// parameters used in the current update attempt.
@@ -450,6 +451,10 @@
// parameter on the |omaha_request_params_| object.
void CalculateDlcParams();
+ // Returns the list of DLC IDs that were installed/updated, excluding the ones
+ // which had "noupdate" in the Omaha response.
+ std::vector<std::string> GetSuccessfulDlcIds();
+
// Last status notification timestamp used for throttling. Use monotonic
// TimeTicks to ensure that notifications are sent even if the system clock is
// set back in the middle of an update.
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index d65a556..5849c38 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -19,7 +19,9 @@
#include <stdint.h>
#include <limits>
+#include <map>
#include <memory>
+#include <string>
#include <unordered_set>
#include <base/files/file_util.h>
@@ -64,12 +66,14 @@
using chromeos_update_manager::StagingSchedule;
using chromeos_update_manager::UpdateCheckParams;
using policy::DevicePolicy;
+using std::map;
using std::string;
using std::unique_ptr;
using std::unordered_set;
using std::vector;
using testing::_;
using testing::DoAll;
+using testing::ElementsAre;
using testing::Field;
using testing::InSequence;
using testing::Invoke;
@@ -135,16 +139,23 @@
UpdateStatus status = UpdateStatus::CHECKING_FOR_UPDATE;
ActionProcessor* processor = nullptr;
ErrorCode code = ErrorCode::kSuccess;
+ map<string, OmahaRequestParams::AppParams> dlc_apps_params;
// Expects:
const bool kExpectedIsInstall = false;
bool should_schedule_updates_be_called = true;
UpdateStatus expected_exit_status = UpdateStatus::IDLE;
+ bool should_install_completed_be_called = false;
+ bool should_update_completed_be_called = false;
+ vector<string> args_to_install_completed;
+ vector<string> args_to_update_completed;
};
class MockDlcService : public DlcServiceInterface {
public:
MOCK_METHOD1(GetInstalled, bool(vector<string>*));
+ MOCK_METHOD1(InstallCompleted, bool(const vector<string>&));
+ MOCK_METHOD1(UpdateCompleted, bool(const vector<string>&));
};
} // namespace
@@ -376,6 +387,22 @@
attempter_.DisableScheduleUpdates();
attempter_.is_install_ = pd_params_.is_install;
attempter_.status_ = pd_params_.status;
+ attempter_.omaha_request_params_->set_dlc_apps_params(
+ pd_params_.dlc_apps_params);
+
+ // Expects
+ if (pd_params_.should_install_completed_be_called)
+ EXPECT_CALL(mock_dlcservice_,
+ InstallCompleted(pd_params_.args_to_install_completed))
+ .WillOnce(Return(true));
+ else
+ EXPECT_CALL(mock_dlcservice_, InstallCompleted(_)).Times(0);
+ if (pd_params_.should_update_completed_be_called)
+ EXPECT_CALL(mock_dlcservice_,
+ UpdateCompleted(pd_params_.args_to_update_completed))
+ .WillOnce(Return(true));
+ else
+ EXPECT_CALL(mock_dlcservice_, UpdateCompleted(_)).Times(0);
// Invocation
attempter_.ProcessingDone(pd_params_.processor, pd_params_.code);
@@ -2001,6 +2028,25 @@
TEST_F(UpdateAttempterTest, ProcessingDoneUpdated) {
// GIVEN an update finished.
+ // THEN update_engine should call update completion.
+ pd_params_.should_update_completed_be_called = true;
+ // THEN need reboot since update applied.
+ pd_params_.expected_exit_status = UpdateStatus::UPDATED_NEED_REBOOT;
+ // THEN install indication should be false.
+
+ TestProcessingDone();
+}
+
+TEST_F(UpdateAttempterTest, ProcessingDoneUpdatedDlcFilter) {
+ // GIVEN an update finished.
+ // GIVEN DLC |AppParams| list.
+ auto dlc_1 = "dlc_1", dlc_2 = "dlc_2";
+ pd_params_.dlc_apps_params = {{dlc_1, {.name = dlc_1, .updated = false}},
+ {dlc_2, {.name = dlc_2}}};
+
+ // THEN update_engine should call update completion.
+ pd_params_.should_update_completed_be_called = true;
+ pd_params_.args_to_update_completed = {dlc_2};
// THEN need reboot since update applied.
pd_params_.expected_exit_status = UpdateStatus::UPDATED_NEED_REBOOT;
// THEN install indication should be false.
@@ -2012,6 +2058,25 @@
// GIVEN an install finished.
pd_params_.is_install = true;
+ // THEN update_engine should call install completion.
+ pd_params_.should_install_completed_be_called = true;
+ // THEN go idle.
+ // THEN install indication should be false.
+
+ TestProcessingDone();
+}
+
+TEST_F(UpdateAttempterTest, ProcessingDoneInstalledDlcFilter) {
+ // GIVEN an install finished.
+ pd_params_.is_install = true;
+ // GIVEN DLC |AppParams| list.
+ auto dlc_1 = "dlc_1", dlc_2 = "dlc_2";
+ pd_params_.dlc_apps_params = {{dlc_1, {.name = dlc_1, .updated = false}},
+ {dlc_2, {.name = dlc_2}}};
+
+ // THEN update_engine should call install completion.
+ pd_params_.should_install_completed_be_called = true;
+ pd_params_.args_to_install_completed = {dlc_2};
// THEN go idle.
// THEN install indication should be false.
@@ -2024,6 +2089,7 @@
// GIVEN a reporting error occurred.
pd_params_.status = UpdateStatus::REPORTING_ERROR_EVENT;
+ // THEN update_engine should not call install completion.
// THEN go idle.
// THEN install indication should be false.
@@ -2035,6 +2101,7 @@
// GIVEN an action error occured.
pd_params_.code = ErrorCode::kNoUpdate;
+ // THEN update_engine should not call update completion.
// THEN go idle.
// THEN install indication should be false.
@@ -2047,6 +2114,7 @@
// GIVEN an action error occured.
pd_params_.code = ErrorCode::kNoUpdate;
+ // THEN update_engine should not call install completion.
// THEN go idle.
// THEN install indication should be false.
@@ -2066,6 +2134,7 @@
pd_params_.expected_exit_status = UpdateStatus::REPORTING_ERROR_EVENT;
// THEN install indication should be false.
+ // THEN update_engine should not call update completion.
// THEN expect critical actions of |ScheduleErrorEventAction()|.
EXPECT_CALL(*processor_, EnqueueAction(Pointee(_))).Times(1);
EXPECT_CALL(*processor_, StartProcessing()).Times(1);
@@ -2089,6 +2158,7 @@
pd_params_.expected_exit_status = UpdateStatus::REPORTING_ERROR_EVENT;
// THEN install indication should be false.
+ // THEN update_engine should not call install completion.
// THEN expect critical actions of |ScheduleErrorEventAction()|.
EXPECT_CALL(*processor_, EnqueueAction(Pointee(_))).Times(1);
EXPECT_CALL(*processor_, StartProcessing()).Times(1);
@@ -2457,4 +2527,13 @@
EXPECT_FALSE(base::PathExists(metadata_path_dlc0));
}
+TEST_F(UpdateAttempterTest, GetSuccessfulDlcIds) {
+ auto dlc_1 = "1", dlc_2 = "2", dlc_3 = "3";
+ attempter_.omaha_request_params_->set_dlc_apps_params(
+ {{dlc_1, {.name = dlc_1, .updated = false}},
+ {dlc_2, {.name = dlc_2}},
+ {dlc_3, {.name = dlc_3, .updated = false}}});
+ EXPECT_THAT(attempter_.GetSuccessfulDlcIds(), ElementsAre(dlc_2));
+}
+
} // namespace chromeos_update_engine