Commit 7da429e4 authored by Kentaro Hara's avatar Kentaro Hara Committed by Commit Bot

Revert "CrostiniManager stores OsRelease protos reported from containers."

This reverts commit 78d78792.

Reason for revert: This broke unit_tests (CrostiniManagerRestartTest.OsReleaseSetCorrectly
) on multiple bots.

https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/14819


Original change's description:
> CrostiniManager stores OsRelease protos reported from containers.
> 
> The information can change dynamically during a container session since the
> user could upgrade manually.
> 
> Bug: 930901
> Change-Id: I93fd4b227a2f93b26924c0eae0e8f005aea4e562
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1765088
> Auto-Submit: Nicholas Verne <nverne@chromium.org>
> Reviewed-by: Joel Hockey <joelhockey@chromium.org>
> Commit-Queue: Nicholas Verne <nverne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#689746}

TBR=timloh@chromium.org,joelhockey@chromium.org,nverne@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 930901
Change-Id: Ib55e556ff909ee3c39232c86c6303c9324be375a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768493Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarNicholas Verne <nverne@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690270}
parent 9ec0478e
...@@ -534,30 +534,6 @@ void CrostiniManager::SetContainerSshfsMounted(std::string vm_name, ...@@ -534,30 +534,6 @@ void CrostiniManager::SetContainerSshfsMounted(std::string vm_name,
} }
} }
void CrostiniManager::SetContainerOsRelease(
std::string vm_name,
std::string container_name,
const vm_tools::cicerone::OsRelease& os_release) {
container_os_releases_.emplace(ContainerId(vm_name, container_name),
os_release);
VLOG(1) << "vm_name: " << vm_name << " container_name: " << container_name;
VLOG(1) << "os_release.pretty_name " << os_release.pretty_name();
VLOG(1) << "os_release.name " << os_release.name();
VLOG(1) << "os_release.version " << os_release.version();
VLOG(1) << "os_release.version_id " << os_release.version_id();
VLOG(1) << "os_release.id " << os_release.id();
}
const vm_tools::cicerone::OsRelease* CrostiniManager::GetContainerOsRelease(
std::string vm_name,
std::string container_name) {
auto it = container_os_releases_.find(ContainerId(vm_name, container_name));
if (it != container_os_releases_.end()) {
return &it->second;
}
return nullptr;
}
base::Optional<ContainerInfo> CrostiniManager::GetContainerInfo( base::Optional<ContainerInfo> CrostiniManager::GetContainerInfo(
std::string vm_name, std::string vm_name,
std::string container_name) { std::string container_name) {
...@@ -2141,9 +2117,6 @@ void CrostiniManager::OnStartLxdContainer( ...@@ -2141,9 +2117,6 @@ void CrostiniManager::OnStartLxdContainer(
NOTREACHED(); NOTREACHED();
break; break;
} }
if (response->has_os_release()) {
SetContainerOsRelease(vm_name, container_name, response->os_release());
}
} }
void CrostiniManager::OnSetUpLxdContainerUser( void CrostiniManager::OnSetUpLxdContainerUser(
...@@ -2284,10 +2257,6 @@ void CrostiniManager::OnLxdContainerStarting( ...@@ -2284,10 +2257,6 @@ void CrostiniManager::OnLxdContainerStarting(
VLOG(1) << "Awaiting ContainerStarted signal from Garcon"; VLOG(1) << "Awaiting ContainerStarted signal from Garcon";
return; return;
} }
if (signal.has_os_release()) {
SetContainerOsRelease(signal.vm_name(), signal.container_name(),
signal.os_release());
}
// Find the callbacks to call, then erase them from the map. // Find the callbacks to call, then erase them from the map.
auto range = start_container_callbacks_.equal_range( auto range = start_container_callbacks_.equal_range(
std::make_tuple(signal.vm_name(), signal.container_name())); std::make_tuple(signal.vm_name(), signal.container_name()));
......
...@@ -481,12 +481,6 @@ class CrostiniManager : public KeyedService, ...@@ -481,12 +481,6 @@ class CrostiniManager : public KeyedService,
void SetContainerSshfsMounted(std::string vm_name, void SetContainerSshfsMounted(std::string vm_name,
std::string container_name, std::string container_name,
bool is_mounted); bool is_mounted);
void SetContainerOsRelease(std::string vm_name,
std::string container_name,
const vm_tools::cicerone::OsRelease& os_release);
const vm_tools::cicerone::OsRelease* GetContainerOsRelease(
std::string vm_name,
std::string container_name);
// Returns null if VM or container is not running. // Returns null if VM or container is not running.
base::Optional<ContainerInfo> GetContainerInfo(std::string vm_name, base::Optional<ContainerInfo> GetContainerInfo(std::string vm_name,
std::string container_name); std::string container_name);
...@@ -737,10 +731,6 @@ class CrostiniManager : public KeyedService, ...@@ -737,10 +731,6 @@ class CrostiniManager : public KeyedService,
// Running containers as keyed by vm name. // Running containers as keyed by vm name.
std::multimap<std::string, ContainerInfo> running_containers_; std::multimap<std::string, ContainerInfo> running_containers_;
// OsRelease protos keyed by ContainerId. We populate this map even if a
// container fails to start normally.
std::map<ContainerId, vm_tools::cicerone::OsRelease> container_os_releases_;
std::vector<RemoveCrostiniCallback> remove_crostini_callbacks_; std::vector<RemoveCrostiniCallback> remove_crostini_callbacks_;
base::ObserverList<LinuxPackageOperationProgressObserver>::Unchecked base::ObserverList<LinuxPackageOperationProgressObserver>::Unchecked
......
...@@ -3,9 +3,7 @@ ...@@ -3,9 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "chrome/browser/chromeos/crostini/crostini_manager.h" #include "chrome/browser/chromeos/crostini/crostini_manager.h"
#include <memory> #include <memory>
#include "base/base64.h" #include "base/base64.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
...@@ -930,30 +928,6 @@ TEST_F(CrostiniManagerRestartTest, IsContainerRunningFalseIfVmNotStarted) { ...@@ -930,30 +928,6 @@ TEST_F(CrostiniManagerRestartTest, IsContainerRunningFalseIfVmNotStarted) {
EXPECT_FALSE(crostini_manager()->GetContainerInfo(kVmName, kContainerName)); EXPECT_FALSE(crostini_manager()->GetContainerInfo(kVmName, kContainerName));
} }
TEST_F(CrostiniManagerRestartTest, OsReleaseSetCorrectly) {
vm_tools::cicerone::OsRelease os_release;
vm_tools::cicerone::StartLxdContainerResponse response;
response.set_status(vm_tools::cicerone::StartLxdContainerResponse::STARTED);
response.mutable_os_release()->CopyFrom(os_release);
fake_cicerone_client_->set_start_lxd_container_response(response);
restart_id_ = crostini_manager()->RestartCrostini(
kVmName, kContainerName,
base::BindOnce(&CrostiniManagerRestartTest::RestartCrostiniCallback,
base::Unretained(this), run_loop()->QuitClosure()),
this);
EXPECT_TRUE(crostini_manager()->IsRestartPending(restart_id_));
run_loop()->Run();
const auto* stored_os_release =
crostini_manager()->GetContainerOsRelease(kVmName, kContainerName);
EXPECT_NE(stored_os_release, nullptr);
// Sadly, we can't use MessageDifferencer here because we're using the LITE
// API in our protos.
EXPECT_EQ(os_release.SerializeAsString(),
stored_os_release->SerializeAsString());
}
TEST_F(CrostiniManagerRestartTest, RestartThenUninstall) { TEST_F(CrostiniManagerRestartTest, RestartThenUninstall) {
restart_id_ = crostini_manager()->RestartCrostini( restart_id_ = crostini_manager()->RestartCrostini(
kVmName, kContainerName, kVmName, kContainerName,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment