This is an automated email from the ASF dual-hosted git repository. chhsiao pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit a17a536611ca38783bfdf86630664f86562ea98e Author: Chun-Hung Hsiao <[email protected]> AuthorDate: Wed May 15 21:54:20 2019 -0700 Notifies master `/api/v1` subscribers about recovered frameworks. If one subscribes to master's `/api/v1` endpoint after a master failover but before an agent reregistration, frameworks recovered through the agent registration should be notified to the subscriber, otherwise recovered tasks will have framework IDs referring to frameworks unknown to the subscriber. Review: https://reviews.apache.org/r/70651 --- src/common/protobuf_utils.cpp | 4 ---- src/master/master.cpp | 7 +++++++ src/tests/api_tests.cpp | 40 ++++++++++++++++++++++++++++++---------- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp index fc67c38..7778e7f 100644 --- a/src/common/protobuf_utils.cpp +++ b/src/common/protobuf_utils.cpp @@ -1438,10 +1438,6 @@ mesos::master::Event createTaskAdded(const Task& task) mesos::master::Event createFrameworkAdded( const mesos::internal::master::Framework& _framework) { - CHECK(_framework.active()); - CHECK(_framework.connected()); - CHECK(!_framework.recovered()); - mesos::master::Event event; event.set_type(mesos::master::Event::FRAMEWORK_ADDED); diff --git a/src/master/master.cpp b/src/master/master.cpp index ace05b2..fbde112 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -10893,6 +10893,13 @@ void Master::recoverFramework( Framework* framework = new Framework(this, flags, info); + // Send a `FRAMEWORK_ADDED` event to subscribers before adding recovered tasks + // so the framework ID referred by any succeeding `TASK_ADDED` event will be + // known to subscribers. + if (!subscribers.subscribed.empty()) { + subscribers.send(protobuf::master::event::createFrameworkAdded(*framework)); + } + // Add active operations, tasks, and executors to the framework. foreachvalue (Slave* slave, slaves.registered) { if (slave->tasks.contains(framework->id())) { diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp index 561ff20..3479ed3 100644 --- a/src/tests/api_tests.cpp +++ b/src/tests/api_tests.cpp @@ -2630,8 +2630,9 @@ TEST_P(MasterAPITest, SubscribersReceiveHealthUpdates) // This test verifies that subscribing to the 'api/v1' endpoint between -// a master failover and an agent re-registration won't cause the master -// to crash. See MESOS-8601. +// a master failover and an agent reregistration won't cause the master +// to crash, and frameworks recovered through agent reregistration will be +// broadcast to subscribers. See MESOS-8601 and MESOS-9785. TEST_P(MasterAPITest, MasterFailover) { ContentType contentType = GetParam(); @@ -2733,15 +2734,24 @@ TEST_P(MasterAPITest, MasterFailover) EXPECT_CALL(subscriber, subscribed(_)) .WillOnce(FutureArg<0>(&subscribedToMasterAPIEvents)); - // The agent re-registration should result in an `AGENT_ADDED` event - // and a `TASK_ADDED` event. - Future<Nothing> taskAdded; - EXPECT_CALL(subscriber, taskAdded(_)) - .WillOnce(FutureSatisfy(&taskAdded)); + // The agent re-registration should result in an `AGENT_ADDED` event, + // a `FRAMEWORK_ADDED` event and a `TASK_ADDED` event in order. + Sequence masterEventsSequence; - Future<Nothing> agentAdded; + Future<v1::master::Event::AgentAdded> agentAdded; EXPECT_CALL(subscriber, agentAdded(_)) - .WillOnce(FutureSatisfy(&agentAdded)); + .InSequence(masterEventsSequence) + .WillOnce(FutureArg<0>(&agentAdded)); + + Future<v1::master::Event::FrameworkAdded> frameworkAdded; + EXPECT_CALL(subscriber, frameworkAdded(_)) + .InSequence(masterEventsSequence) + .WillOnce(FutureArg<0>(&frameworkAdded)); + + Future<v1::master::Event::TaskAdded> taskAdded; + EXPECT_CALL(subscriber, taskAdded(_)) + .InSequence(masterEventsSequence) + .WillOnce(FutureArg<0>(&taskAdded)); // Create event stream after the master failover but before the agent // re-registration. We should see no framework, agent, task and @@ -2764,8 +2774,18 @@ TEST_P(MasterAPITest, MasterFailover) Clock::advance(slaveFlags.registration_backoff_factor); AWAIT_READY(slaveReregisteredMessage); - AWAIT_READY(taskAdded); + AWAIT_READY(agentAdded); + EXPECT_EQ(agentId, agentAdded->agent().agent_info().id()); + + AWAIT_READY(frameworkAdded); + EXPECT_EQ(frameworkId, frameworkAdded->framework().framework_info().id()); + EXPECT_FALSE(frameworkAdded->framework().active()); + EXPECT_FALSE(frameworkAdded->framework().connected()); + EXPECT_TRUE(frameworkAdded->framework().recovered()); + + AWAIT_READY(taskAdded); + EXPECT_EQ(task.task_id(), taskAdded->task().task_id()); }
