This is an automated email from the ASF dual-hosted git repository. chhsiao pushed a commit to branch 1.7.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 30dbd9644a9f8b7078b7e81618c87f1a82c02971 Author: Chun-Hung Hsiao <chhs...@mesosphere.io> AuthorDate: Thu Dec 6 20:20:46 2018 -0800 Fixed `AgentResourceProviderConfigApiTest.Update` for `vendor` field. The introduction of the `Resource.DiskInfo.Source.vendor` changes the resource comparison and thus breaks this test. This patch fixes the test and make it more robust. NOTE: The updated test will fail unless the previous patch (which sets up the `vendor` field) is also applied. Review: https://reviews.apache.org/r/69521 --- .../agent_resource_provider_config_api_tests.cpp | 51 ++++++++++++++++------ 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/tests/agent_resource_provider_config_api_tests.cpp b/src/tests/agent_resource_provider_config_api_tests.cpp index e6a68ba..f55953e 100644 --- a/src/tests/agent_resource_provider_config_api_tests.cpp +++ b/src/tests/agent_resource_provider_config_api_tests.cpp @@ -85,18 +85,19 @@ public: // This extra closure is necessary in order to use `ASSERT_*`, as // these macros require a void return type. - [&]() { - // Randomize the plugin name so we get a clean work directory for - // each created config. + [&] { + // Randomize the plugin name so every created config uses its own CSI + // plugin instance. We use this to check if the config has been updated in + // some tests. const string testCsiPluginName = - "test_csi_plugin_" + - strings::remove(id::UUID::random().toString(), "-"); + "local_" + strings::remove(id::UUID::random().toString(), "-"); const string testCsiPluginPath = path::join(tests::flags.build_dir, "src", "test-csi-plugin"); const string testCsiPluginWorkDir = path::join(sandbox.get(), testCsiPluginName); + ASSERT_SOME(os::mkdir(testCsiPluginWorkDir)); Try<string> resourceProviderConfig = strings::format( @@ -508,10 +509,12 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update) slaveFlags.resource_provider_config_dir = resourceProviderConfigDir; // Generate a pre-existing config. + const string volumes = "volume1:4GB"; const string configPath = path::join(resourceProviderConfigDir, "test.json"); + ASSERT_SOME(os::write( configPath, - stringify(JSON::protobuf(createResourceProviderInfo("volume1:4GB"))))); + stringify(JSON::protobuf(createResourceProviderInfo(volumes))))); Future<SlaveRegisteredMessage> slaveRegisteredMessage = FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _); @@ -544,14 +547,22 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update) Future<vector<Offer>> oldOffers; EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource( - std::bind(&Resources::hasResourceProvider, lambda::_1)))) + &Resources::hasResourceProvider))) .WillOnce(FutureArg<1>(&oldOffers)); driver.start(); // Wait for an offer having the old provider resource. AWAIT_READY(oldOffers); - ASSERT_FALSE(oldOffers->empty()); + ASSERT_EQ(1u, oldOffers->size()); + + Resource oldResource = *Resources(oldOffers->at(0).resources()) + .filter(&Resources::hasResourceProvider) + .begin(); + + ASSERT_TRUE(oldResource.has_disk()); + ASSERT_TRUE(oldResource.disk().has_source()); + ASSERT_TRUE(oldResource.disk().source().has_vendor()); Future<OfferID> rescinded; @@ -564,7 +575,8 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update) std::bind(&Resources::hasResourceProvider, lambda::_1)))) .WillOnce(FutureArg<1>(&newOffers)); - ResourceProviderInfo info = createResourceProviderInfo("volume1:2GB"); + // Create a new config that serves the same volumes with a different vendor. + ResourceProviderInfo info = createResourceProviderInfo(volumes); AWAIT_EXPECT_RESPONSE_STATUS_EQ( http::OK().status, @@ -585,6 +597,7 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update) Try<ResourceProviderInfo> _info = ::protobuf::parse<ResourceProviderInfo>(json.get()); + ASSERT_SOME(_info); EXPECT_EQ(_info.get(), info); @@ -593,10 +606,22 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update) // Wait for an offer having the new provider resource. AWAIT_READY(newOffers); - - // The new provider resource is smaller than the old provider resource. - EXPECT_FALSE(Resources(newOffers->at(0).resources()).contains( - oldOffers->at(0).resources())); + ASSERT_EQ(1u, newOffers->size()); + + Resource newResource = *Resources(newOffers->at(0).resources()) + .filter(&Resources::hasResourceProvider) + .begin(); + + ASSERT_TRUE(newResource.has_disk()); + ASSERT_TRUE(newResource.disk().has_source()); + ASSERT_TRUE(newResource.disk().source().has_vendor()); + + // The resource from the new resource provider has the same provider ID but a + // different vendor as that from the old one. + EXPECT_EQ(oldResource.provider_id(), newResource.provider_id()); + EXPECT_NE( + oldResource.disk().source().vendor(), + newResource.disk().source().vendor()); }