You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2019/09/16 08:46:56 UTC
[mesos] branch master updated: Removed race in
`StorageLocalResourceProviderTest.Update`.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
The following commit(s) were added to refs/heads/master by this push:
new a1a2411 Removed race in `StorageLocalResourceProviderTest.Update`.
a1a2411 is described below
commit a1a2411da60ac5d01bccca220b4782a19bada50b
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Thu Sep 12 10:20:13 2019 +0200
Removed race in `StorageLocalResourceProviderTest.Update`.
The agent might take some time before registering with the master which
could have lead to unexpected retries in this test which the clock was
resumed. This patch simplifies the test so we do not rely on
`UpdateSlaveMessage` anymore.
Review: https://reviews.apache.org/r/71445/
---
.../storage_local_resource_provider_tests.cpp | 87 +---------------------
1 file changed, 2 insertions(+), 85 deletions(-)
diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp
index 089aa97..d4a73c3 100644
--- a/src/tests/storage_local_resource_provider_tests.cpp
+++ b/src/tests/storage_local_resource_provider_tests.cpp
@@ -6825,7 +6825,7 @@ TEST_P(StorageLocalResourceProviderTest, Update)
ASSERT_SOME(
os::write(profilesPath, createDiskProfileMapping({{"test", None()}})));
- loadUriDiskProfileAdaptorModule(profilesPath);
+ loadUriDiskProfileAdaptorModule(profilesPath, Duration::max());
const string mockCsiEndpoint =
"unix://" + path::join(sandbox.get(), "mock_csi.sock");
@@ -6852,77 +6852,37 @@ TEST_P(StorageLocalResourceProviderTest, Update)
process::Queue<Nothing> listVolumesCalls;
if (GetParam() == csi::v0::API_VERSION) {
EXPECT_CALL(plugin, ListVolumes(_, _, A<csi::v0::ListVolumesResponse*>()))
- .WillOnce(Invoke([&](
- grpc::ServerContext* context,
- const csi::v0::ListVolumesRequest* request,
- csi::v0::ListVolumesResponse* response) {
- listVolumesCalls.put({});
- return grpc::Status::OK;
- }))
.WillRepeatedly(Invoke([&](
grpc::ServerContext* context,
const csi::v0::ListVolumesRequest* request,
csi::v0::ListVolumesResponse* response) {
- csi::v0::Volume* volume = response->add_entries()->mutable_volume();
- volume->set_capacity_bytes(Bytes(1024).bytes());
- volume->set_id("volume1");
-
listVolumesCalls.put({});
return grpc::Status::OK;
}));
EXPECT_CALL(plugin, GetCapacity(_, _, A<csi::v0::GetCapacityResponse*>()))
- .WillOnce(Invoke([&](
- grpc::ServerContext* context,
- const csi::v0::GetCapacityRequest* request,
- csi::v0::GetCapacityResponse* response) {
- getCapacityCalls.put({});
- return grpc::Status::OK;
- }))
.WillRepeatedly(Invoke([&](
grpc::ServerContext* context,
const csi::v0::GetCapacityRequest* request,
csi::v0::GetCapacityResponse* response) {
- response->set_available_capacity(Bytes(1024).bytes());
-
getCapacityCalls.put({});
return grpc::Status::OK;
}));
} else if (GetParam() == csi::v1::API_VERSION) {
EXPECT_CALL(plugin, ListVolumes(_, _, A<csi::v1::ListVolumesResponse*>()))
- .WillOnce(Invoke([&](
- grpc::ServerContext* context,
- const csi::v1::ListVolumesRequest* request,
- csi::v1::ListVolumesResponse* response) {
- listVolumesCalls.put({});
- return grpc::Status::OK;
- }))
.WillRepeatedly(Invoke([&](
grpc::ServerContext* context,
const csi::v1::ListVolumesRequest* request,
csi::v1::ListVolumesResponse* response) {
- csi::v1::Volume* volume = response->add_entries()->mutable_volume();
- volume->set_capacity_bytes(Bytes(1024).bytes());
- volume->set_volume_id("volume1");
-
listVolumesCalls.put({});
return grpc::Status::OK;
}));
EXPECT_CALL(plugin, GetCapacity(_, _, A<csi::v1::GetCapacityResponse*>()))
- .WillOnce(Invoke([&](
- grpc::ServerContext* context,
- const csi::v1::GetCapacityRequest* request,
- csi::v1::GetCapacityResponse* response) {
- getCapacityCalls.put({});
- return grpc::Status::OK;
- }))
.WillRepeatedly(Invoke([&](
grpc::ServerContext* context,
const csi::v1::GetCapacityRequest* request,
csi::v1::GetCapacityResponse* response) {
- response->set_available_capacity(Bytes(1024).bytes());
-
getCapacityCalls.put({});
return grpc::Status::OK;
}));
@@ -6934,22 +6894,6 @@ TEST_P(StorageLocalResourceProviderTest, Update)
Future<Nothing> getCapacity1 = getCapacityCalls.get();
Future<Nothing> getCapacity2 = getCapacityCalls.get();
-
- // Since the local resource provider daemon gets subscribed after the agent
- // is registered, it is guaranteed that the slave will send two
- // `UpdateSlaveMessage`s, where the latter one contains resources from
- // the storage local resource provider. After that a single update
- // will be send since they underlying provider resources got changed.
- //
- // NOTE: The order of the two `FUTURE_PROTOBUF`s is reversed because
- // Google Mock will search the expectations in reverse order.
- Future<UpdateSlaveMessage> updateSlave3 =
- FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
- Future<UpdateSlaveMessage> updateSlave2 =
- FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
- Future<UpdateSlaveMessage> updateSlave1 =
- FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
-
slave::Flags slaveFlags = CreateSlaveFlags();
slaveFlags.disk_profile_adaptor = URI_DISK_PROFILE_ADAPTOR_NAME;
@@ -6959,21 +6903,13 @@ TEST_P(StorageLocalResourceProviderTest, Update)
// Advance the clock to trigger agent registration and prevent retry.
Clock::advance(slaveFlags.registration_backoff_factor);
- AWAIT_READY(updateSlave1);
- ASSERT_TRUE(updateSlave1->has_resource_providers());
- ASSERT_TRUE(updateSlave1->resource_providers().providers().empty());
-
// NOTE: We need to resume the clock so that the resource provider can
// periodically check if the CSI endpoint socket has been created by
// the plugin container, which runs in another Linux process.
Clock::resume();
- AWAIT_READY(getCapacity1);
AWAIT_READY(listVolumes1);
-
- AWAIT_READY(updateSlave2);
- ASSERT_TRUE(updateSlave2->has_resource_providers());
- ASSERT_FALSE(updateSlave2->resource_providers().providers().empty());
+ AWAIT_READY(getCapacity1);
Clock::pause();
@@ -6983,25 +6919,6 @@ TEST_P(StorageLocalResourceProviderTest, Update)
AWAIT_READY(listVolumes2);
AWAIT_READY(getCapacity2);
- ASSERT_TRUE(updateSlave3.isPending());
-
- // Advance the clock so the SLRP polls again.
- Future<Nothing> listVolumes3 = listVolumesCalls.get();
- Future<Nothing> getCapacity3 = getCapacityCalls.get();
-
- Clock::settle();
- Clock::advance(reconciliationInterval);
-
- AWAIT_READY(listVolumes3);
- AWAIT_READY(getCapacity3);
- AWAIT_READY(updateSlave3);
- ASSERT_TRUE(updateSlave3->has_resource_providers());
- ASSERT_FALSE(updateSlave3->resource_providers().providers().empty());
-
- // Resource changes are reported and the resource version changes.
- ASSERT_NE(
- updateSlave2->resource_providers().providers(0).resource_version_uuid(),
- updateSlave3->resource_providers().providers(0).resource_version_uuid());
}
} // namespace tests {