update_engine: Report result of enterprise update invalidation Report success/fail attempts of update invalidations initiated by the enterprise policy. BUG=b:275530794 TEST=unit tests Change-Id: I08095ac0058a2ad9dc4df2a894a57dc07ac66675 Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/4887246 Tested-by: Artyom Chen <artyomchen@google.com> Reviewed-by: Jae Hoon Kim <kimjae@chromium.org> Commit-Queue: Artyom Chen <artyomchen@google.com>
diff --git a/common/fake_hardware.h b/common/fake_hardware.h index 0c15e02..65a6897 100644 --- a/common/fake_hardware.h +++ b/common/fake_hardware.h
@@ -286,7 +286,17 @@ return rootfs_verification_enabled_; } - bool ResetFWTryNextSlot() override { return reset_fw_try_next_slot_ = true; } + bool ResetFWTryNextSlot() override { + if (fail_reset_fw_try_next_slot_) { + return false; + } + + return reset_fw_try_next_slot_ = true; + } + + void SetFailResetFwTryNextSlot(bool value) { + fail_reset_fw_try_next_slot_ = value; + } bool IsFWTryNextSlotReset() const { return reset_fw_try_next_slot_; } private: @@ -319,6 +329,7 @@ mutable std::map<std::string, std::string> partition_timestamps_; bool rootfs_verification_enabled_{false}; bool reset_fw_try_next_slot_{false}; + bool fail_reset_fw_try_next_slot_{false}; }; } // namespace chromeos_update_engine
diff --git a/common/fake_prefs.cc b/common/fake_prefs.cc index 275667e..66296bd 100644 --- a/common/fake_prefs.cc +++ b/common/fake_prefs.cc
@@ -94,8 +94,10 @@ } bool FakePrefs::Delete(const string& key) { - if (values_.find(key) == values_.end()) - return false; + if (values_.find(key) == values_.end()) { + return true; + } + values_.erase(key); const auto observers_for_key = observers_.find(key); if (observers_for_key != observers_.end()) {
diff --git a/common/metrics_reporter_interface.h b/common/metrics_reporter_interface.h index 739489d..89a200a 100644 --- a/common/metrics_reporter_interface.h +++ b/common/metrics_reporter_interface.h
@@ -202,6 +202,12 @@ // |UpdateEngine.UpdateInvalidated| virtual void ReportInvalidatedUpdate(bool success) = 0; + // Reports whether or not the enterprise update invalidation is completed + // successfully. + // + // |UpdateEngine.EnterpriseUpdateInvalidatedResult| + virtual void ReportEnterpriseUpdateInvalidatedResult(bool success) = 0; + // Helper function to report the source of installation data. The following // metrics are reported: //
diff --git a/common/metrics_reporter_stub.h b/common/metrics_reporter_stub.h index e697285..d049226 100644 --- a/common/metrics_reporter_stub.h +++ b/common/metrics_reporter_stub.h
@@ -84,6 +84,8 @@ void ReportInvalidatedUpdate(bool success) override {} + void ReportEnterpriseUpdateInvalidatedResult(bool success) override {} + void ReportInstallDateProvisioningSource(int source, int max) override {} void ReportInternalErrorCode(ErrorCode error_code) override {}
diff --git a/common/mock_metrics_reporter.h b/common/mock_metrics_reporter.h index 89fcacd..50e7d56 100644 --- a/common/mock_metrics_reporter.h +++ b/common/mock_metrics_reporter.h
@@ -82,6 +82,11 @@ MOCK_METHOD1(ReportInvalidatedUpdate, void(bool success)); + MOCK_METHOD(void, + ReportEnterpriseUpdateInvalidatedResult, + (bool success), + (override)); + MOCK_METHOD2(ReportInstallDateProvisioningSource, void(int source, int max)); MOCK_METHOD1(ReportInternalErrorCode, void(ErrorCode error_code));
diff --git a/cros/metrics_reporter_omaha.cc b/cros/metrics_reporter_omaha.cc index c245960..76d684a 100644 --- a/cros/metrics_reporter_omaha.cc +++ b/cros/metrics_reporter_omaha.cc
@@ -125,11 +125,13 @@ "UpdateEngine.CertificateCheck.Download"; // UpdateEngine.* metrics. +const char kMetricEnterpriseUpdateInvalidatedResult[] = + "UpdateEngine.EnterpriseUpdateInvalidatedResult"; const char kMetricFailedUpdateCount[] = "UpdateEngine.FailedUpdateCount"; const char kMetricInstallDateProvisioningSource[] = "UpdateEngine.InstallDateProvisioningSource"; -const char kMetricTimeToRebootMinutes[] = "UpdateEngine.TimeToRebootMinutes"; const char kMetricInvalidatedUpdate[] = "UpdateEngine.UpdateInvalidated"; +const char kMetricTimeToRebootMinutes[] = "UpdateEngine.TimeToRebootMinutes"; // UpdateEngine.ConsecutiveUpdate.* metrics. const char kMetricConsecutiveUpdateCount[] = @@ -508,6 +510,12 @@ metrics_lib_->SendBoolToUMA(metric, successful); } +void MetricsReporterOmaha::ReportEnterpriseUpdateInvalidatedResult( + bool success) { + string metric = metrics::kMetricEnterpriseUpdateInvalidatedResult; + metrics_lib_->SendBoolToUMA(metric, success); +} + void MetricsReporterOmaha::ReportInstallDateProvisioningSource(int source, int max) { metrics_lib_->SendEnumToUMA(metrics::kMetricInstallDateProvisioningSource,
diff --git a/cros/metrics_reporter_omaha.h b/cros/metrics_reporter_omaha.h index ed120e5..4c927b0 100644 --- a/cros/metrics_reporter_omaha.h +++ b/cros/metrics_reporter_omaha.h
@@ -97,10 +97,11 @@ extern const char kMetricKernelMaxRollforwardSetSuccess[]; // UpdateEngine.* metrics. +extern const char kMetricEnterpriseUpdateInvalidatedResult[]; extern const char kMetricFailedUpdateCount[]; extern const char kMetricInstallDateProvisioningSource[]; -extern const char kMetricTimeToRebootMinutes[]; extern const char kMetricInvalidatedUpdate[]; +extern const char kMetricTimeToRebootMinutes[]; // UpdateEngine.ConsecutiveUpdate.* metrics. extern const char kMetricConsecutiveUpdateCount[]; @@ -166,6 +167,8 @@ void ReportInvalidatedUpdate(bool success) override; + void ReportEnterpriseUpdateInvalidatedResult(bool success) override; + void ReportInstallDateProvisioningSource(int source, int max) override; void ReportInternalErrorCode(ErrorCode error_code) override;
diff --git a/cros/metrics_reporter_omaha_unittest.cc b/cros/metrics_reporter_omaha_unittest.cc index 5d658f5..894fbd2 100644 --- a/cros/metrics_reporter_omaha_unittest.cc +++ b/cros/metrics_reporter_omaha_unittest.cc
@@ -456,6 +456,42 @@ reporter_.ReportTimeToReboot(time_to_reboot_minutes); } +TEST_F(MetricsReporterOmahaTest, ReportInvalidatedUpdateSuccess) { + EXPECT_CALL(*mock_metrics_lib_, + SendBoolToUMA(metrics::kMetricInvalidatedUpdate, true)) + .Times(1); + + reporter_.ReportInvalidatedUpdate(true); +} + +TEST_F(MetricsReporterOmahaTest, ReportInvalidatedUpdateFailure) { + EXPECT_CALL(*mock_metrics_lib_, + SendBoolToUMA(metrics::kMetricInvalidatedUpdate, false)) + .Times(1); + + reporter_.ReportInvalidatedUpdate(false); +} + +TEST_F(MetricsReporterOmahaTest, + ReportEnterpriseUpdateInvalidatedResultSuccess) { + EXPECT_CALL( + *mock_metrics_lib_, + SendBoolToUMA(metrics::kMetricEnterpriseUpdateInvalidatedResult, true)) + .Times(1); + + reporter_.ReportEnterpriseUpdateInvalidatedResult(true); +} + +TEST_F(MetricsReporterOmahaTest, + ReportEnterpriseUpdateInvalidatedResultFailure) { + EXPECT_CALL( + *mock_metrics_lib_, + SendBoolToUMA(metrics::kMetricEnterpriseUpdateInvalidatedResult, false)) + .Times(1); + + reporter_.ReportEnterpriseUpdateInvalidatedResult(false); +} + TEST_F(MetricsReporterOmahaTest, ReportInstallDateProvisioningSource) { int source = 2; int max = 5;
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc index 29c8bc8..14a44f5 100644 --- a/cros/update_attempter.cc +++ b/cros/update_attempter.cc
@@ -227,7 +227,10 @@ status_ == UpdateStatus::UPDATED_NEED_REBOOT) { LOG(INFO) << "Received enterprise update invalidation signal. " << "Invalidating the pending update."; - InvalidateUpdate(); + bool invalidation_result = InvalidateUpdate(); + SystemState::Get() + ->metrics_reporter() + ->ReportEnterpriseUpdateInvalidatedResult(invalidation_result); ResetUpdateStatus(); } @@ -1760,10 +1763,10 @@ return ret_value; } -void UpdateAttempter::InvalidateUpdate() { +bool UpdateAttempter::InvalidateUpdate() { if (!GetBootTimeAtUpdate(nullptr)) { LOG(INFO) << "No previous update available to invalidate."; - return; + return true; } LOG(INFO) << "Invalidating previous update."; @@ -1793,6 +1796,8 @@ } SystemState::Get()->metrics_reporter()->ReportInvalidatedUpdate(success); + + return success; } void UpdateAttempter::DownloadComplete() {
diff --git a/cros/update_attempter.h b/cros/update_attempter.h index 20ac9d2..5206e8b 100644 --- a/cros/update_attempter.h +++ b/cros/update_attempter.h
@@ -126,7 +126,7 @@ // Resets the boot slot and update markers to invalidate a previously existing // update if one is available. - void InvalidateUpdate(); + bool InvalidateUpdate(); // Returns the current status in the out param. Returns true on success. virtual bool GetStatus(update_engine::UpdateEngineStatus* out_status); @@ -394,6 +394,8 @@ FRIEND_TEST(UpdateAttempterTest, AfterRestartSkipsUpdateInvalidationIfNotIdle); FRIEND_TEST(UpdateAttempterTest, AfterUpdateInvalidatesUpdate); + FRIEND_TEST(UpdateAttempterTest, AfterUpdateInvalidatesUpdateMetrics); + FRIEND_TEST(UpdateAttempterTest, AfterUpdateInvalidatesUpdateFailureMetrics); FRIEND_TEST(UpdateAttempterTest, AfterUpdateSubscribesInvalidatesUpdate); FRIEND_TEST(UpdateAttempterTest, AfterUpdateSkipsUpdateInvalidationIfNonEnterprise);
diff --git a/cros/update_attempter_unittest.cc b/cros/update_attempter_unittest.cc index ac7a693..f5d2f89 100644 --- a/cros/update_attempter_unittest.cc +++ b/cros/update_attempter_unittest.cc
@@ -3006,6 +3006,58 @@ EXPECT_FALSE(attempter_.GetBootTimeAtUpdate(nullptr)); } +TEST_F(UpdateAttempterTest, AfterUpdateInvalidatesUpdateMetrics) { + // Disables the updates via the enterprise policy. + chromeos_update_manager::FakeDevicePolicyProvider* device_policy_provider = + FakeSystemState::Get() + ->fake_update_manager() + ->state() + ->device_policy_provider(); + device_policy_provider->var_is_enterprise_enrolled()->reset(new bool(true)); + device_policy_provider->var_update_disabled()->reset(new bool(true)); + EXPECT_CALL(*FakeSystemState::Get()->mock_metrics_reporter(), + ReportEnterpriseUpdateInvalidatedResult(true)) + .Times(1); + + // Complete an update cycle. + pd_params_.should_update_completed_be_called = true; + pd_params_.expected_exit_status = UpdateStatus::UPDATED_NEED_REBOOT; + TestProcessingDone(); + ASSERT_TRUE(attempter_.GetBootTimeAtUpdate(nullptr)); + + ASSERT_TRUE(attempter_.enterprise_update_invalidation_check_scheduled_); + loop_.RunOnce(false); + + EXPECT_FALSE(attempter_.enterprise_update_invalidation_check_scheduled_); + EXPECT_FALSE(attempter_.GetBootTimeAtUpdate(nullptr)); +} + +TEST_F(UpdateAttempterTest, AfterUpdateInvalidatesUpdateFailureMetrics) { + // Disables the updates via the enterprise policy. + chromeos_update_manager::FakeDevicePolicyProvider* device_policy_provider = + FakeSystemState::Get() + ->fake_update_manager() + ->state() + ->device_policy_provider(); + device_policy_provider->var_is_enterprise_enrolled()->reset(new bool(true)); + device_policy_provider->var_update_disabled()->reset(new bool(true)); + FakeSystemState::Get()->fake_hardware()->SetFailResetFwTryNextSlot(true); + EXPECT_CALL(*FakeSystemState::Get()->mock_metrics_reporter(), + ReportEnterpriseUpdateInvalidatedResult(false)) + .Times(1); + + // Complete an update cycle. + pd_params_.should_update_completed_be_called = true; + pd_params_.expected_exit_status = UpdateStatus::UPDATED_NEED_REBOOT; + TestProcessingDone(); + ASSERT_TRUE(attempter_.GetBootTimeAtUpdate(nullptr)); + ASSERT_TRUE(attempter_.enterprise_update_invalidation_check_scheduled_); + loop_.RunOnce(false); + + EXPECT_FALSE(attempter_.enterprise_update_invalidation_check_scheduled_); + EXPECT_FALSE(attempter_.GetBootTimeAtUpdate(nullptr)); +} + TEST_F(UpdateAttempterTest, AfterUpdateSubscribesInvalidatesUpdate) { // First enable the updates. chromeos_update_manager::FakeDevicePolicyProvider* device_policy_provider =