Allow Policies to defer updates
The OmahaResponseHandlerAction consults with the Policy to confirm if
the generated InstallPlan can be actioned or not. If not, then the
UpdateAttempter reports to Omaha that the update has been deferred and
reverts to a status of IDLE.
This causes an API-visible transition through REPORTING_ERROR, before
ending at IDLE.
Bug: 66016687
Test: unit-tests, manual OTA
Change-Id: I5c31a04975586630dd6a9c373022f1b407bef4e8
(cherry picked from 8a5f6317a60c6e54917f115bdf190f46653a225a)
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index d23c14e..9c5fb4a 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -31,7 +31,11 @@
#include "update_engine/omaha_request_params.h"
#include "update_engine/payload_consumer/delta_performer.h"
#include "update_engine/payload_state_interface.h"
+#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/update_manager.h"
+using chromeos_update_manager::Policy;
+using chromeos_update_manager::UpdateManager;
using std::string;
namespace chromeos_update_engine {
@@ -158,7 +162,14 @@
chmod(deadline_file_.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
}
- completer.set_code(ErrorCode::kSuccess);
+ // Check the generated install-plan with the Policy to confirm that
+ // it can be applied at this time (or at all).
+ UpdateManager* const update_manager = system_state_->update_manager();
+ CHECK(update_manager);
+ auto ec = ErrorCode::kSuccess;
+ update_manager->PolicyRequest(
+ &Policy::UpdateCanBeApplied, &ec, &install_plan_);
+ completer.set_code(ec);
}
bool OmahaResponseHandlerAction::AreHashChecksMandatory(
diff --git a/omaha_response_handler_action.h b/omaha_response_handler_action.h
index 51dfa7a..2974841 100644
--- a/omaha_response_handler_action.h
+++ b/omaha_response_handler_action.h
@@ -89,6 +89,7 @@
friend class OmahaResponseHandlerActionTest;
FRIEND_TEST(UpdateAttempterTest, CreatePendingErrorEventResumedTest);
+ FRIEND_TEST(UpdateAttempterTest, UpdateDeferredByPolicyTest);
DISALLOW_COPY_AND_ASSIGN(OmahaResponseHandlerAction);
};
diff --git a/omaha_response_handler_action_unittest.cc b/omaha_response_handler_action_unittest.cc
index 935ae5d..0887e1f 100644
--- a/omaha_response_handler_action_unittest.cc
+++ b/omaha_response_handler_action_unittest.cc
@@ -20,6 +20,7 @@
#include <base/files/file_util.h>
#include <base/files/scoped_temp_dir.h>
+#include <brillo/message_loops/fake_message_loop.h>
#include <gtest/gtest.h>
#include "update_engine/common/constants.h"
@@ -29,11 +30,17 @@
#include "update_engine/fake_system_state.h"
#include "update_engine/mock_payload_state.h"
#include "update_engine/payload_consumer/payload_constants.h"
+#include "update_engine/update_manager/mock_policy.h"
using chromeos_update_engine::test_utils::System;
using chromeos_update_engine::test_utils::WriteFileString;
+using chromeos_update_manager::EvalStatus;
+using chromeos_update_manager::FakeUpdateManager;
+using chromeos_update_manager::MockPolicy;
using std::string;
+using testing::DoAll;
using testing::Return;
+using testing::SetArgPointee;
using testing::_;
namespace chromeos_update_engine {
@@ -58,6 +65,13 @@
const string& deadline_file,
InstallPlan* out);
+ // Pointer to the Action, valid after |DoTest|, released when the test is
+ // finished.
+ std::unique_ptr<OmahaResponseHandlerAction> action_;
+ // Captures the action's result code, for tests that need to directly verify
+ // it in non-success cases.
+ ErrorCode action_result_code_;
+
FakeSystemState fake_system_state_;
// "Hash+"
const brillo::Blob expected_hash_ = {0x48, 0x61, 0x73, 0x68, 0x2b};
@@ -99,6 +113,8 @@
const OmahaResponse& in,
const string& test_deadline_file,
InstallPlan* out) {
+ brillo::FakeMessageLoop loop(nullptr);
+ loop.SetAsCurrent();
ActionProcessor processor;
OmahaResponseHandlerActionProcessorDelegate delegate;
processor.set_delegate(&delegate);
@@ -123,15 +139,15 @@
EXPECT_CALL(*(fake_system_state_.mock_payload_state()), GetCurrentUrl())
.WillRepeatedly(Return(current_url));
- OmahaResponseHandlerAction response_handler_action(
+ action_.reset(new OmahaResponseHandlerAction(
&fake_system_state_,
- (test_deadline_file.empty() ?
- constants::kOmahaResponseDeadlineFile : test_deadline_file));
- BondActions(&feeder_action, &response_handler_action);
+ (test_deadline_file.empty() ? constants::kOmahaResponseDeadlineFile
+ : test_deadline_file)));
+ BondActions(&feeder_action, action_.get());
ObjectCollectorAction<InstallPlan> collector_action;
- BondActions(&response_handler_action, &collector_action);
+ BondActions(action_.get(), &collector_action);
processor.EnqueueAction(&feeder_action);
- processor.EnqueueAction(&response_handler_action);
+ processor.EnqueueAction(action_.get());
processor.EnqueueAction(&collector_action);
processor.StartProcessing();
EXPECT_TRUE(!processor.IsRunning())
@@ -139,6 +155,7 @@
if (out)
*out = collector_action.object();
EXPECT_TRUE(delegate.code_set_);
+ action_result_code_ = delegate.code_;
return delegate.code_ == ErrorCode::kSuccess;
}
@@ -480,4 +497,36 @@
EXPECT_EQ(in.system_version, install_plan.system_version);
}
+TEST_F(OmahaResponseHandlerActionTest, TestDeferredByPolicy) {
+ OmahaResponse in;
+ in.update_exists = true;
+ in.version = "a.b.c.d";
+ in.packages.push_back({.payload_urls = {"http://foo/the_update_a.b.c.d.tgz"},
+ .size = 12,
+ .hash = kPayloadHashHex});
+ // Setup the UpdateManager to disallow the update.
+ FakeClock fake_clock;
+ MockPolicy* mock_policy = new MockPolicy(&fake_clock);
+ FakeUpdateManager* fake_update_manager =
+ fake_system_state_.fake_update_manager();
+ fake_update_manager->set_policy(mock_policy);
+ EXPECT_CALL(*mock_policy, UpdateCanBeApplied(_, _, _, _, _))
+ .WillOnce(
+ DoAll(SetArgPointee<3>(ErrorCode::kOmahaUpdateDeferredPerPolicy),
+ Return(EvalStatus::kSucceeded)));
+ // Perform the Action. It should "fail" with kOmahaUpdateDeferredPerPolicy.
+ InstallPlan install_plan;
+ EXPECT_FALSE(DoTest(in, "", &install_plan));
+ EXPECT_EQ(ErrorCode::kOmahaUpdateDeferredPerPolicy, action_result_code_);
+ // Verify that DoTest() didn't set the output install plan.
+ EXPECT_EQ("", install_plan.version);
+ // Copy the underlying InstallPlan from the Action (like a real Delegate).
+ install_plan = action_->install_plan();
+ // Now verify the InstallPlan that was generated.
+ EXPECT_EQ(in.packages[0].payload_urls[0], install_plan.download_url);
+ EXPECT_EQ(expected_hash_, install_plan.payloads[0].hash);
+ EXPECT_EQ(1U, install_plan.target_slot);
+ EXPECT_EQ(in.version, install_plan.version);
+}
+
} // namespace chromeos_update_engine
diff --git a/update_attempter.cc b/update_attempter.cc
index 9a8900d..c7c0177 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -1012,7 +1012,28 @@
server_dictated_poll_interval_ =
std::max(0, omaha_request_action->GetOutputObject().poll_interval);
}
+ } else if (type == OmahaResponseHandlerAction::StaticType()) {
+ // Depending on the returned error code, note that an update is available.
+ if (code == ErrorCode::kOmahaUpdateDeferredPerPolicy ||
+ code == ErrorCode::kSuccess) {
+ // Note that the status will be updated to DOWNLOADING when some bytes
+ // get actually downloaded from the server and the BytesReceived
+ // callback is invoked. This avoids notifying the user that a download
+ // has started in cases when the server and the client are unable to
+ // initiate the download.
+ CHECK(action == response_handler_action_.get());
+ auto plan = response_handler_action_->install_plan();
+ UpdateLastCheckedTime();
+ new_version_ = plan.version;
+ new_system_version_ = plan.system_version;
+ new_payload_size_ = 0;
+ for (const auto& payload : plan.payloads)
+ new_payload_size_ += payload.size;
+ cpu_limiter_.StartLimiter();
+ SetStatusAndNotify(UpdateStatus::UPDATE_AVAILABLE);
+ }
}
+ // General failure cases.
if (code != ErrorCode::kSuccess) {
// If the current state is at or past the download phase, count the failure
// in case a switch to full update becomes necessary. Ignore network
@@ -1025,23 +1046,8 @@
CreatePendingErrorEvent(action, code);
return;
}
- // Find out which action completed.
- if (type == OmahaResponseHandlerAction::StaticType()) {
- // Note that the status will be updated to DOWNLOADING when some bytes get
- // actually downloaded from the server and the BytesReceived callback is
- // invoked. This avoids notifying the user that a download has started in
- // cases when the server and the client are unable to initiate the download.
- CHECK(action == response_handler_action_.get());
- const InstallPlan& plan = response_handler_action_->install_plan();
- UpdateLastCheckedTime();
- new_version_ = plan.version;
- new_system_version_ = plan.system_version;
- new_payload_size_ = 0;
- for (const auto& payload : plan.payloads)
- new_payload_size_ += payload.size;
- cpu_limiter_.StartLimiter();
- SetStatusAndNotify(UpdateStatus::UPDATE_AVAILABLE);
- } else if (type == DownloadAction::StaticType()) {
+ // Find out which action completed (successfully).
+ if (type == DownloadAction::StaticType()) {
SetStatusAndNotify(UpdateStatus::FINALIZING);
}
}
diff --git a/update_attempter.h b/update_attempter.h
index 4aac60a..6b2a7a7 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -259,6 +259,8 @@
FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest);
FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionTest);
FRIEND_TEST(UpdateAttempterTest, TargetVersionPrefixSetAndReset);
+ FRIEND_TEST(UpdateAttempterTest, UpdateDeferredByPolicyTest);
+ FRIEND_TEST(UpdateAttempterTest, UpdateIsNotRunningWhenUpdateAvailable);
FRIEND_TEST(UpdateAttempterTest, UpdateTest);
// CertificateChecker::Observer method.
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index dfeebcf..7ecc5a6 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -1063,4 +1063,59 @@
fake_system_state_.request_params()->target_version_prefix().empty());
}
+TEST_F(UpdateAttempterTest, UpdateDeferredByPolicyTest) {
+ // Construct an OmahaResponseHandlerAction that has processed an InstallPlan,
+ // but the update is being deferred by the Policy.
+ OmahaResponseHandlerAction* response_action =
+ new OmahaResponseHandlerAction(&fake_system_state_);
+ response_action->install_plan_.version = "a.b.c.d";
+ response_action->install_plan_.system_version = "b.c.d.e";
+ response_action->install_plan_.payloads.push_back(
+ {.size = 1234ULL, .type = InstallPayloadType::kFull});
+ attempter_.response_handler_action_.reset(response_action);
+ // Inform the UpdateAttempter that the OmahaResponseHandlerAction has
+ // completed, with the deferred-update error code.
+ attempter_.ActionCompleted(
+ nullptr, response_action, ErrorCode::kOmahaUpdateDeferredPerPolicy);
+ {
+ UpdateEngineStatus status;
+ attempter_.GetStatus(&status);
+ EXPECT_EQ(UpdateStatus::UPDATE_AVAILABLE, status.status);
+ EXPECT_EQ(response_action->install_plan_.version, status.new_version);
+ EXPECT_EQ(response_action->install_plan_.system_version,
+ status.new_system_version);
+ EXPECT_EQ(response_action->install_plan_.payloads[0].size,
+ status.new_size_bytes);
+ }
+ // An "error" event should have been created to tell Omaha that the update is
+ // being deferred.
+ EXPECT_TRUE(nullptr != attempter_.error_event_);
+ EXPECT_EQ(OmahaEvent::kTypeUpdateComplete, attempter_.error_event_->type);
+ EXPECT_EQ(OmahaEvent::kResultUpdateDeferred, attempter_.error_event_->result);
+ ErrorCode expected_code = static_cast<ErrorCode>(
+ static_cast<int>(ErrorCode::kOmahaUpdateDeferredPerPolicy) |
+ static_cast<int>(ErrorCode::kTestOmahaUrlFlag));
+ EXPECT_EQ(expected_code, attempter_.error_event_->error_code);
+ // End the processing
+ attempter_.ProcessingDone(nullptr, ErrorCode::kOmahaUpdateDeferredPerPolicy);
+ // Validate the state of the attempter.
+ {
+ UpdateEngineStatus status;
+ attempter_.GetStatus(&status);
+ EXPECT_EQ(UpdateStatus::REPORTING_ERROR_EVENT, status.status);
+ EXPECT_EQ(response_action->install_plan_.version, status.new_version);
+ EXPECT_EQ(response_action->install_plan_.system_version,
+ status.new_system_version);
+ EXPECT_EQ(response_action->install_plan_.payloads[0].size,
+ status.new_size_bytes);
+ }
+}
+
+TEST_F(UpdateAttempterTest, UpdateIsNotRunningWhenUpdateAvailable) {
+ EXPECT_FALSE(attempter_.IsUpdateRunningOrScheduled());
+ // Verify in-progress update with UPDATE_AVAILABLE is running
+ attempter_.status_ = UpdateStatus::UPDATE_AVAILABLE;
+ EXPECT_TRUE(attempter_.IsUpdateRunningOrScheduled());
+}
+
} // namespace chromeos_update_engine
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index 44f8821..12f417c 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -36,6 +36,7 @@
using chromeos_update_engine::ConnectionTethering;
using chromeos_update_engine::ConnectionType;
using chromeos_update_engine::ErrorCode;
+using chromeos_update_engine::InstallPlan;
using std::get;
using std::max;
using std::min;
@@ -324,6 +325,15 @@
return EvalStatus::kSucceeded;
}
+EvalStatus ChromeOSPolicy::UpdateCanBeApplied(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ ErrorCode* result,
+ InstallPlan* install_plan) const {
+ *result = ErrorCode::kSuccess;
+ return EvalStatus::kSucceeded;
+}
+
EvalStatus ChromeOSPolicy::UpdateCanStart(
EvaluationContext* ec,
State* state,
diff --git a/update_manager/chromeos_policy.h b/update_manager/chromeos_policy.h
index b4370c4..283bedc 100644
--- a/update_manager/chromeos_policy.h
+++ b/update_manager/chromeos_policy.h
@@ -59,6 +59,13 @@
EvaluationContext* ec, State* state, std::string* error,
UpdateCheckParams* result) const override;
+ EvalStatus UpdateCanBeApplied(
+ EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ chromeos_update_engine::ErrorCode* result,
+ chromeos_update_engine::InstallPlan* install_plan) const override;
+
EvalStatus UpdateCanStart(
EvaluationContext* ec,
State* state,
diff --git a/update_manager/default_policy.cc b/update_manager/default_policy.cc
index 9a5ce7e..5da1520 100644
--- a/update_manager/default_policy.cc
+++ b/update_manager/default_policy.cc
@@ -16,6 +16,9 @@
#include "update_engine/update_manager/default_policy.h"
+using chromeos_update_engine::ErrorCode;
+using chromeos_update_engine::InstallPlan;
+
namespace {
// A fixed minimum interval between consecutive allowed update checks. This
@@ -53,6 +56,15 @@
return EvalStatus::kAskMeAgainLater;
}
+EvalStatus DefaultPolicy::UpdateCanBeApplied(EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ ErrorCode* result,
+ InstallPlan* install_plan) const {
+ *result = ErrorCode::kSuccess;
+ return EvalStatus::kSucceeded;
+}
+
EvalStatus DefaultPolicy::UpdateCanStart(
EvaluationContext* ec,
State* state,
diff --git a/update_manager/default_policy.h b/update_manager/default_policy.h
index 3f41178..136ca35 100644
--- a/update_manager/default_policy.h
+++ b/update_manager/default_policy.h
@@ -69,6 +69,13 @@
EvaluationContext* ec, State* state, std::string* error,
UpdateCheckParams* result) const override;
+ EvalStatus UpdateCanBeApplied(
+ EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ chromeos_update_engine::ErrorCode* result,
+ chromeos_update_engine::InstallPlan* install_plan) const override;
+
EvalStatus UpdateCanStart(
EvaluationContext* ec, State* state, std::string* error,
UpdateDownloadParams* result,
diff --git a/update_manager/mock_policy.h b/update_manager/mock_policy.h
index 14470e9..8060bf8 100644
--- a/update_manager/mock_policy.h
+++ b/update_manager/mock_policy.h
@@ -36,6 +36,11 @@
testing::_))
.WillByDefault(testing::Invoke(
&default_policy_, &DefaultPolicy::UpdateCheckAllowed));
+ ON_CALL(*this,
+ UpdateCanBeApplied(
+ testing::_, testing::_, testing::_, testing::_, testing::_))
+ .WillByDefault(testing::Invoke(&default_policy_,
+ &DefaultPolicy::UpdateCanBeApplied));
ON_CALL(*this, UpdateCanStart(testing::_, testing::_, testing::_,
testing::_, testing::_))
.WillByDefault(testing::Invoke(
@@ -61,6 +66,13 @@
EvalStatus(EvaluationContext*, State*, std::string*,
UpdateCheckParams*));
+ MOCK_CONST_METHOD5(UpdateCanBeApplied,
+ EvalStatus(EvaluationContext*,
+ State*,
+ std::string*,
+ chromeos_update_engine::ErrorCode*,
+ chromeos_update_engine::InstallPlan*));
+
MOCK_CONST_METHOD5(UpdateCanStart,
EvalStatus(EvaluationContext*, State*, std::string*,
UpdateDownloadParams*, UpdateState));
diff --git a/update_manager/policy.h b/update_manager/policy.h
index fae1494..c2fc358 100644
--- a/update_manager/policy.h
+++ b/update_manager/policy.h
@@ -22,6 +22,7 @@
#include <vector>
#include "update_engine/common/error_code.h"
+#include "update_engine/payload_consumer/install_plan.h"
#include "update_engine/update_manager/evaluation_context.h"
#include "update_engine/update_manager/state.h"
@@ -204,6 +205,9 @@
if (reinterpret_cast<typeof(&Policy::UpdateCheckAllowed)>(
policy_method) == &Policy::UpdateCheckAllowed)
return class_name + "UpdateCheckAllowed";
+ if (reinterpret_cast<typeof(&Policy::UpdateCanBeApplied)>(policy_method) ==
+ &Policy::UpdateCanBeApplied)
+ return class_name + "UpdateCanBeApplied";
if (reinterpret_cast<typeof(&Policy::UpdateCanStart)>(
policy_method) == &Policy::UpdateCanStart)
return class_name + "UpdateCanStart";
@@ -235,6 +239,17 @@
EvaluationContext* ec, State* state, std::string* error,
UpdateCheckParams* result) const = 0;
+ // UpdateCanBeApplied returns whether the given |install_plan| can be acted
+ // on at this time. The reason for not applying is returned in |result|.
+ // The Policy may modify the passed-in |install_plan|, based on the
+ // implementation in the Policy and values provided by the EvaluationContext.
+ virtual EvalStatus UpdateCanBeApplied(
+ EvaluationContext* ec,
+ State* state,
+ std::string* error,
+ chromeos_update_engine::ErrorCode* result,
+ chromeos_update_engine::InstallPlan* install_plan) const = 0;
+
// Returns EvalStatus::kSucceeded if either an update can start being
// processed, or the attempt needs to be aborted. In cases where the update
// needs to wait for some condition to be satisfied, but none of the values
