update_engine: Add PayloadState Exclusion Logic
|PayloadState| will exclude payloads based on specific update failures.
This is to prevent critical platform updates from being blocked by less
critical updates (e.g. DLCs). A layer of robustness is added in
protecting CrOS devices from falling off the update train.
Some important changes to mention:
- Only during updates will update_engine exclude non-critical payloads
- |OmahaRequestAction|, the current precursor |Action| to
|OmahaResponseHandlerAction|, during a update will exclude
faulty/excluded payloads prior to setting the |OmahaResponse| as an
output object for suqsequent bonded |Action| to consume
- When all payloads are excluded for an update, the |ErrorCode| will
be indicated as |OmahaResponseInvalid| as this case is not a valid
Omaha response update_engine should ever run into because non-critical
updates must tag alongside a critical update
BUG=chromium:928805
TEST=FEATURES=test emerge-$B update_engine update_engine-client
Change-Id: I0551a228d0b84defb4d59966e8ed46a5d9278d60
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2190237
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Auto-Submit: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 4a0afcf..bf9aed4 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -23,9 +23,11 @@
#include <gtest/gtest.h>
#include "update_engine/common/constants.h"
+#include "update_engine/common/excluder_interface.h"
#include "update_engine/common/fake_clock.h"
#include "update_engine/common/fake_hardware.h"
#include "update_engine/common/fake_prefs.h"
+#include "update_engine/common/mock_excluder.h"
#include "update_engine/common/mock_prefs.h"
#include "update_engine/common/prefs.h"
#include "update_engine/common/test_utils.h"
@@ -44,6 +46,7 @@
using testing::NiceMock;
using testing::Return;
using testing::SetArgPointee;
+using testing::StrictMock;
namespace chromeos_update_engine {
@@ -1012,10 +1015,6 @@
NiceMock<MockPrefs>* mock_powerwash_safe_prefs =
fake_system_state.mock_powerwash_safe_prefs();
- EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
-
- // Verify pre-conditions are good.
- EXPECT_TRUE(payload_state.GetRollbackVersion().empty());
// Mock out the os version and make sure it's blacklisted correctly.
string rollback_version = "2345.0.0";
@@ -1023,6 +1022,11 @@
params.Init(rollback_version, "", false);
fake_system_state.set_request_params(¶ms);
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+ // Verify pre-conditions are good.
+ EXPECT_TRUE(payload_state.GetRollbackVersion().empty());
+
EXPECT_CALL(*mock_powerwash_safe_prefs,
SetString(kPrefsRollbackVersion, rollback_version));
payload_state.Rollback();
@@ -1353,15 +1357,15 @@
PayloadState payload_state;
FakeSystemState fake_system_state;
- EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
- SetupPayloadStateWith2Urls(
- "Hash6437", true, false, &payload_state, &response);
-
// Mock the request to a request where the delta was disabled.
OmahaRequestParams params(&fake_system_state);
params.set_delta_okay(false);
fake_system_state.set_request_params(¶ms);
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+ SetupPayloadStateWith2Urls(
+ "Hash6437", true, false, &payload_state, &response);
+
// Simulate a successful download and update.
payload_state.DownloadComplete();
@@ -1658,6 +1662,9 @@
TEST(PayloadStateTest, NextPayloadResetsUrlIndex) {
PayloadState payload_state;
FakeSystemState fake_system_state;
+ StrictMock<MockExcluder> mock_excluder;
+ EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+ .WillOnce(Return(&mock_excluder));
EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
OmahaResponse response;
@@ -1682,4 +1689,93 @@
EXPECT_EQ(payload_state.GetCurrentUrl(), "http://test1b");
}
+TEST(PayloadStateTest, ExcludeNoopForNonExcludables) {
+ PayloadState payload_state;
+ FakeSystemState fake_system_state;
+ StrictMock<MockExcluder> mock_excluder;
+ EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+ .WillOnce(Return(&mock_excluder));
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+ OmahaResponse response;
+ response.packages.push_back(
+ {.payload_urls = {"http://test1a", "http://test2a"},
+ .size = 123456789,
+ .metadata_size = 58123,
+ .metadata_signature = "msign",
+ .hash = "hash",
+ .can_exclude = false});
+ payload_state.SetResponse(response);
+
+ EXPECT_CALL(mock_excluder, Exclude(_)).Times(0);
+ payload_state.ExcludeCurrentPayload();
+}
+
+TEST(PayloadStateTest, ExcludeOnlyCanExcludables) {
+ PayloadState payload_state;
+ FakeSystemState fake_system_state;
+ StrictMock<MockExcluder> mock_excluder;
+ EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+ .WillOnce(Return(&mock_excluder));
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+ OmahaResponse response;
+ response.packages.push_back(
+ {.payload_urls = {"http://test1a", "http://test2a"},
+ .size = 123456789,
+ .metadata_size = 58123,
+ .metadata_signature = "msign",
+ .hash = "hash",
+ .can_exclude = true});
+ payload_state.SetResponse(response);
+
+ EXPECT_CALL(mock_excluder, Exclude(utils::GetExclusionName("http://test1a")))
+ .WillOnce(Return(true));
+ payload_state.ExcludeCurrentPayload();
+}
+
+TEST(PayloadStateTest, IncrementFailureExclusionTest) {
+ PayloadState payload_state;
+ FakeSystemState fake_system_state;
+ StrictMock<MockExcluder> mock_excluder;
+ EXPECT_CALL(*fake_system_state.mock_update_attempter(), GetExcluder())
+ .WillOnce(Return(&mock_excluder));
+ EXPECT_TRUE(payload_state.Initialize(&fake_system_state));
+
+ OmahaResponse response;
+ // Critical package.
+ response.packages.push_back(
+ {.payload_urls = {"http://crit-test1a", "http://crit-test2a"},
+ .size = 123456789,
+ .metadata_size = 58123,
+ .metadata_signature = "msign",
+ .hash = "hash",
+ .can_exclude = false});
+ // Non-critical package.
+ response.packages.push_back(
+ {.payload_urls = {"http://test1a", "http://test2a"},
+ .size = 123456789,
+ .metadata_size = 58123,
+ .metadata_signature = "msign",
+ .hash = "hash",
+ .can_exclude = true});
+ response.max_failure_count_per_url = 2;
+ payload_state.SetResponse(response);
+
+ // Critical package won't be excluded.
+ // Increment twice as failure count allowed per URL is set to 2.
+ payload_state.IncrementFailureCount();
+ payload_state.IncrementFailureCount();
+
+ EXPECT_TRUE(payload_state.NextPayload());
+
+ // First increment failure should not exclude.
+ payload_state.IncrementFailureCount();
+
+ // Second increment failure should exclude.
+ EXPECT_CALL(mock_excluder, Exclude(utils::GetExclusionName("http://test1a")))
+ .WillOnce(Return(true));
+ payload_state.IncrementFailureCount();
+}
+
} // namespace chromeos_update_engine