Allow Policies to defer updates am: 23bd339a82
am: f8be59e442
Change-Id: I3b102c17b777ba68013beaba5e06e37a150e5cad
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 da04372..b32b626 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;
@@ -325,6 +326,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