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 2018/12/18 14:30:31 UTC
[mesos] branch master updated (1d1af19 -> 49fc7fc)
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 1d1af19 Added 'brew install xz' to the macOS getting started documentation.
new 9dea6df Added missing code to set resource provider and agent ids.
new 49fc7fc Validated that resource providers use correct ID in operation states.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/common/protobuf_utils.cpp | 8 +
src/resource_provider/manager.cpp | 8 +
src/tests/master_slave_reconciliation_tests.cpp | 7 +
src/tests/master_tests.cpp | 129 ----------------
src/tests/resource_provider_manager_tests.cpp | 3 +
.../storage_local_resource_provider_tests.cpp | 163 +++++++++++++++++++++
6 files changed, 189 insertions(+), 129 deletions(-)
[mesos] 02/02: Validated that resource providers use correct ID in
operation states.
Posted by bb...@apache.org.
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
commit 49fc7fcf1e26d803aa0775f66f36763e39d695e9
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Tue Dec 18 15:08:18 2018 +0100
Validated that resource providers use correct ID in operation states.
We expect resource providers to set their IDs in operation status
messages. This patch adds some assertion that the IDs match our
expectations, and adjusts some test code to honor these assumptions.
Review: https://reviews.apache.org/r/69572/
---
src/resource_provider/manager.cpp | 8 ++++++++
src/tests/master_slave_reconciliation_tests.cpp | 7 +++++++
src/tests/resource_provider_manager_tests.cpp | 3 +++
3 files changed, 18 insertions(+)
diff --git a/src/resource_provider/manager.cpp b/src/resource_provider/manager.cpp
index 103f545..65852c6 100644
--- a/src/resource_provider/manager.cpp
+++ b/src/resource_provider/manager.cpp
@@ -882,13 +882,21 @@ void ResourceProviderManagerProcess::updateOperationStatus(
ResourceProvider* resourceProvider,
const Call::UpdateOperationStatus& update)
{
+ CHECK_EQ(update.status().resource_provider_id(), resourceProvider->info.id());
+
ResourceProviderMessage::UpdateOperationStatus body;
body.update.mutable_status()->CopyFrom(update.status());
body.update.mutable_operation_uuid()->CopyFrom(update.operation_uuid());
+
if (update.has_framework_id()) {
body.update.mutable_framework_id()->CopyFrom(update.framework_id());
}
+
if (update.has_latest_status()) {
+ CHECK_EQ(
+ update.latest_status().resource_provider_id(),
+ resourceProvider->info.id());
+
body.update.mutable_latest_status()->CopyFrom(update.latest_status());
}
diff --git a/src/tests/master_slave_reconciliation_tests.cpp b/src/tests/master_slave_reconciliation_tests.cpp
index de6e382..979048c 100644
--- a/src/tests/master_slave_reconciliation_tests.cpp
+++ b/src/tests/master_slave_reconciliation_tests.cpp
@@ -497,6 +497,13 @@ TEST_F(
updateOperationStatus->mutable_operation_uuid()->CopyFrom(
operationUuid);
+ ASSERT_TRUE(resourceProvider->info.has_id())
+ << "Asked to reconcile before subscription was finished";
+
+ updateOperationStatus->mutable_status()
+ ->mutable_resource_provider_id()
+ ->CopyFrom(resourceProvider->info.id());
+
resourceProvider->send(call);
}
};
diff --git a/src/tests/resource_provider_manager_tests.cpp b/src/tests/resource_provider_manager_tests.cpp
index b61c50f..7049698 100644
--- a/src/tests/resource_provider_manager_tests.cpp
+++ b/src/tests/resource_provider_manager_tests.cpp
@@ -372,6 +372,8 @@ TEST_P(ResourceProviderManagerHttpApiTest, UpdateOperationStatus)
resourceProviderId = evolve(message->subscribe->info.id());
}
+ ASSERT_SOME(resourceProviderId);
+
// Then, send an operation status update to the manager.
{
v1::FrameworkID frameworkId;
@@ -379,6 +381,7 @@ TEST_P(ResourceProviderManagerHttpApiTest, UpdateOperationStatus)
mesos::v1::OperationStatus status;
status.set_state(mesos::v1::OperationState::OPERATION_FINISHED);
+ status.mutable_resource_provider_id()->CopyFrom(resourceProviderId.get());
mesos::v1::UUID operationUUID = evolve(protobuf::createUUID());;
[mesos] 01/02: Added missing code to set resource provider and
agent ids.
Posted by bb...@apache.org.
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
commit 9dea6df03761ea0c34704268e0e50c79121fb9a7
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Tue Dec 18 15:08:17 2018 +0100
Added missing code to set resource provider and agent ids.
The helper function to set up the message was set up to receive
arguments, but we missed implementing the code making use of the values.
This went undiscovered as previously only used a mock resource provider.
Review: https://reviews.apache.org/r/69569/
---
src/common/protobuf_utils.cpp | 8 +
src/tests/master_tests.cpp | 129 ----------------
.../storage_local_resource_provider_tests.cpp | 163 +++++++++++++++++++++
3 files changed, 171 insertions(+), 129 deletions(-)
diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp
index 583f0ab..a0159fe 100644
--- a/src/common/protobuf_utils.cpp
+++ b/src/common/protobuf_utils.cpp
@@ -467,6 +467,14 @@ OperationStatus createOperationStatus(
status.mutable_uuid()->CopyFrom(protobuf::createUUID(uuid.get()));
}
+ if (slaveId.isSome()) {
+ status.mutable_slave_id()->CopyFrom(slaveId.get());
+ }
+
+ if (resourceProviderId.isSome()) {
+ status.mutable_resource_provider_id()->CopyFrom(resourceProviderId.get());
+ }
+
return status;
}
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index 50b905b..a67e3db 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -9224,135 +9224,6 @@ TEST_F(MasterTest, OperationUpdateDuringFailover)
}
-// This test verifies that operation status updates contain the
-// agent ID and resource provider ID of originating providers.
-TEST_F(MasterTest, OperationUpdateResourceProvider)
-{
- Clock::pause();
-
- master::Flags masterFlags = CreateMasterFlags();
-
- Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
- ASSERT_SOME(master);
-
- slave::Flags slaveFlags = CreateSlaveFlags();
-
- Future<UpdateSlaveMessage> updateSlaveMessage =
- FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
-
- StandaloneMasterDetector detector(master.get()->pid);
- Try<Owned<cluster::Slave>> slave = StartSlave(&detector, slaveFlags);
- ASSERT_SOME(slave);
-
- Clock::advance(slaveFlags.registration_backoff_factor);
-
- AWAIT_READY(updateSlaveMessage);
-
- // Register a resource provider with the agent.
- mesos::v1::ResourceProviderInfo resourceProviderInfo;
- resourceProviderInfo.set_type("org.apache.mesos.resource_provider.test");
- resourceProviderInfo.set_name("test");
-
- v1::Resources resourceProviderResources = v1::createDiskResource(
- "200", "*", None(), None(), v1::createDiskSourceRaw(None(), "profile"));
-
- v1::MockResourceProvider resourceProvider(
- resourceProviderInfo, resourceProviderResources);
-
- Owned<EndpointDetector> endpointDetector(
- resource_provider::createEndpointDetector(slave.get()->pid));
-
- updateSlaveMessage = FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
-
- resourceProvider.start(endpointDetector, ContentType::PROTOBUF);
-
- AWAIT_READY(updateSlaveMessage);
-
- const v1::AgentID agentId = evolve(updateSlaveMessage->slave_id());
-
- ASSERT_TRUE(resourceProvider.info.has_id());
- const v1::ResourceProviderID resourceProviderId = resourceProvider.info.id();
-
- // Start a framework to operate on offers.
- auto scheduler = std::make_shared<v1::MockHTTPScheduler>();
-
- EXPECT_CALL(*scheduler, connected(_))
- .WillOnce(v1::scheduler::SendSubscribe(v1::DEFAULT_FRAMEWORK_INFO));
-
- v1::scheduler::TestMesos driver(
- master.get()->pid, ContentType::PROTOBUF, scheduler);
-
- Future<Event::Subscribed> subscribed;
- EXPECT_CALL(*scheduler, subscribed(_, _))
- .WillOnce(FutureArg<1>(&subscribed));
-
- EXPECT_CALL(*scheduler, heartbeat(_))
- .WillRepeatedly(Return()); // Ignore heartbeats.
-
- Future<Event::Offers> offers;
- EXPECT_CALL(*scheduler, offers(_, _))
- .WillOnce(FutureArg<1>(&offers));
-
- AWAIT_READY(subscribed);
-
- const v1::FrameworkID& frameworkId = subscribed->framework_id();
-
- Clock::settle();
- Clock::advance(masterFlags.allocation_interval);
-
- AWAIT_READY(offers);
-
- Future<Event::UpdateOperationStatus> updateOperationStatus;
- EXPECT_CALL(*scheduler, updateOperationStatus(_, _))
- .WillOnce(FutureArg<1>(&updateOperationStatus));
-
- ASSERT_FALSE(offers->offers().empty());
- const v1::Offer& offer = offers->offers(0);
-
- // Perform an operation against the resource provider resources.
- Option<v1::Resource> resource;
- foreach (const v1::Resource& resource_, offer.resources()) {
- if (resource_.has_provider_id()) {
- resource = resource_;
- break;
- }
- }
-
- ASSERT_SOME(resource);
-
- {
- Call call;
- call.mutable_framework_id()->CopyFrom(frameworkId);
- call.set_type(Call::ACCEPT);
-
- Call::Accept* accept = call.mutable_accept();
- accept->add_offer_ids()->CopyFrom(offer.id());
-
- v1::Offer::Operation* operation = accept->add_operations();
- operation->set_type(v1::Offer::Operation::CREATE_DISK);
- operation->mutable_id()->set_value("create_disk");
-
- v1::Offer::Operation::CreateDisk* createDisk =
- operation->mutable_create_disk();
- createDisk->mutable_source()->CopyFrom(resource.get());
- createDisk->set_target_type(v1::Resource::DiskInfo::Source::MOUNT);
-
- driver.send(call);
- }
-
- AWAIT_READY(updateOperationStatus);
-
- const v1::OperationStatus& status = updateOperationStatus->status();
- ASSERT_EQ("create_disk", status.operation_id().value());
-
- ASSERT_TRUE(status.has_agent_id());
- EXPECT_EQ(agentId, status.agent_id());
-
- ASSERT_TRUE(status.has_resource_provider_id());
- EXPECT_EQ(resourceProviderId, status.resource_provider_id());
-}
-
-
// Tests that the master correctly drops an operation if the operation's 'id'
// field is set and the operation affects resources not managed by a resource
// provider.
diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp
index 7887bd5..e8ed20f 100644
--- a/src/tests/storage_local_resource_provider_tests.cpp
+++ b/src/tests/storage_local_resource_provider_tests.cpp
@@ -3362,6 +3362,169 @@ TEST_F(
}
+// This test verifies that operation status updates contain the
+// agent ID and resource provider ID of originating providers.
+TEST_F(StorageLocalResourceProviderTest, OperationUpdate)
+{
+ Clock::pause();
+
+ const string profilesPath = path::join(sandbox.get(), "profiles.json");
+
+ ASSERT_SOME(
+ os::write(profilesPath, createDiskProfileMapping({{"test", None()}})));
+
+ loadUriDiskProfileAdaptorModule(profilesPath);
+
+ setupResourceProviderConfig(Gigabytes(4));
+
+ master::Flags masterFlags = CreateMasterFlags();
+ Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
+ ASSERT_SOME(master);
+
+ Owned<MasterDetector> detector = master.get()->createDetector();
+
+ slave::Flags flags = CreateSlaveFlags();
+ flags.disk_profile_adaptor = URI_DISK_PROFILE_ADAPTOR_NAME;
+
+ // Since the local resource provider daemon is started 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.
+ //
+ // NOTE: The order of the two `FUTURE_PROTOBUF`s is reversed because
+ // Google Mock will search the expectations in reverse order.
+ Future<UpdateSlaveMessage> updateSlave2 =
+ FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+ Future<UpdateSlaveMessage> updateSlave1 =
+ FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+
+ Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), flags);
+ ASSERT_SOME(slave);
+
+ // Advance the clock to trigger agent registration.
+ Clock::advance(flags.registration_backoff_factor);
+
+ AWAIT_READY(updateSlave1);
+
+ // 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(updateSlave2);
+ ASSERT_TRUE(updateSlave2->has_resource_providers());
+ ASSERT_EQ(1, updateSlave2->resource_providers().providers_size());
+
+ Clock::pause();
+
+ // Register a framework to exercise an operation.
+ v1::FrameworkInfo frameworkInfo = v1::DEFAULT_FRAMEWORK_INFO;
+ frameworkInfo.set_roles(0, "storage");
+
+ auto scheduler = std::make_shared<v1::MockHTTPScheduler>();
+
+ EXPECT_CALL(*scheduler, connected(_))
+ .WillOnce(v1::scheduler::SendSubscribe(frameworkInfo));
+
+ Future<v1::scheduler::Event::Subscribed> subscribed;
+ EXPECT_CALL(*scheduler, subscribed(_, _))
+ .WillOnce(FutureArg<1>(&subscribed));
+
+ EXPECT_CALL(*scheduler, heartbeat(_))
+ .WillRepeatedly(Return()); // Ignore heartbeats.
+
+ // Decline offers that do not contain wanted resources.
+ EXPECT_CALL(*scheduler, offers(_, _))
+ .WillRepeatedly(v1::scheduler::DeclineOffers());
+
+ Future<v1::scheduler::Event::Offers> offers;
+
+ auto isRaw = [](const v1::Resource& r) {
+ return r.has_disk() &&
+ r.disk().has_source() &&
+ r.disk().source().has_profile() &&
+ r.disk().source().type() == v1::Resource::DiskInfo::Source::RAW;
+ };
+
+ EXPECT_CALL(
+ *scheduler, offers(_, v1::scheduler::OffersHaveAnyResource(isRaw)))
+ .WillOnce(FutureArg<1>(&offers));
+
+ v1::scheduler::TestMesos mesos(
+ master.get()->pid,
+ ContentType::PROTOBUF,
+ scheduler);
+
+ AWAIT_READY(subscribed);
+
+ const v1::FrameworkID& frameworkId = subscribed->framework_id();
+
+ // NOTE: If the framework has not declined an unwanted offer yet when
+ // the master updates the agent with the RAW disk resource, the new
+ // allocation triggered by this update won't generate an allocatable
+ // offer due to no CPU and memory resources. So here we first settle
+ // the clock to ensure that the unwanted offer has been declined, then
+ // advance the clock to trigger another allocation.
+ Clock::settle();
+ Clock::advance(masterFlags.allocation_interval);
+
+ AWAIT_READY(offers);
+ ASSERT_FALSE(offers->offers().empty());
+
+ const v1::Offer& offer = offers->offers(0);
+
+ const v1::AgentID& agentId = offer.agent_id();
+
+ Option<v1::Resource> source;
+ Option<mesos::v1::ResourceProviderID> resourceProviderId;
+ foreach (const v1::Resource& resource, offer.resources()) {
+ if (isRaw(resource)) {
+ source = resource;
+
+ ASSERT_TRUE(resource.has_provider_id());
+ resourceProviderId = resource.provider_id();
+
+ break;
+ }
+ }
+
+ ASSERT_SOME(source);
+ ASSERT_SOME(resourceProviderId);
+
+ Future<v1::scheduler::Event::UpdateOperationStatus> update;
+
+ EXPECT_CALL(*scheduler, updateOperationStatus(_, _))
+ .WillOnce(FutureArg<1>(&update))
+ .WillRepeatedly(Return()); // Ignore subsequent updates.
+
+ // Create a volume.
+ v1::OperationID operationId;
+ operationId.set_value("operation");
+
+ mesos.send(v1::createCallAccept(
+ frameworkId,
+ offer,
+ {v1::CREATE_DISK(
+ source.get(),
+ v1::Resource::DiskInfo::Source::MOUNT,
+ None(),
+ operationId)}));
+
+ AWAIT_READY(update);
+
+ ASSERT_EQ(operationId, update->status().operation_id());
+ ASSERT_EQ(
+ mesos::v1::OperationState::OPERATION_FINISHED, update->status().state());
+ ASSERT_TRUE(update->status().has_uuid());
+
+ ASSERT_TRUE(update->status().has_agent_id());
+ EXPECT_EQ(agentId, update->status().agent_id());
+
+ ASSERT_TRUE(update->status().has_resource_provider_id());
+ EXPECT_EQ(resourceProviderId.get(), update->status().resource_provider_id());
+}
+
+
// This test verifies that storage local resource provider properly
// reports metrics related to operation states.
// TODO(chhsiao): Currently there is no way to test the `pending` metric for