Added TaskStatus label decorator hook for Slave. This allows Slave modules to expose some information to the frameworks as well as Mesos-DNS via state.json.
Review: https://reviews.apache.org/r/36580 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a434ecc1 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a434ecc1 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a434ecc1 Branch: refs/heads/master Commit: a434ecc14c9ebb4cb7dcb23c59306fff187ab9b5 Parents: bbd3104 Author: Kapil Arya <[email protected]> Authored: Tue Jul 21 13:29:55 2015 -0700 Committer: Benjamin Hindman <[email protected]> Committed: Tue Jul 21 13:29:58 2015 -0700 ---------------------------------------------------------------------- include/mesos/hook.hpp | 11 ++++ src/examples/test_hook_module.cpp | 25 ++++++++- src/hook/manager.cpp | 31 +++++++++-- src/hook/manager.hpp | 4 ++ src/slave/slave.cpp | 7 +++ src/tests/hook_tests.cpp | 99 ++++++++++++++++++++++++++++++++++ 6 files changed, 172 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/a434ecc1/include/mesos/hook.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/hook.hpp b/include/mesos/hook.hpp index 0995c24..bb5a635 100644 --- a/include/mesos/hook.hpp +++ b/include/mesos/hook.hpp @@ -79,6 +79,17 @@ public: { return Nothing(); } + + // This hook is called from within slave when it receives a status + // update from the executor. A module implementing the hook creates + // and returns a set of labels. These labels overwrite the existing + // labels on the TaskStatus. + virtual Result<Labels> slaveTaskStatusLabelDecorator( + const FrameworkID& frameworkId, + const TaskStatus& status) + { + return None(); + } }; } // namespace mesos { http://git-wip-us.apache.org/repos/asf/mesos/blob/a434ecc1/src/examples/test_hook_module.cpp ---------------------------------------------------------------------- diff --git a/src/examples/test_hook_module.cpp b/src/examples/test_hook_module.cpp index d61cd55..c664b56 100644 --- a/src/examples/test_hook_module.cpp +++ b/src/examples/test_hook_module.cpp @@ -128,7 +128,6 @@ public: return environment; } - // This hook locates the file created by environment decorator hook // and deletes it. virtual Try<Nothing> slaveRemoveExecutorHook( @@ -149,6 +148,30 @@ public: return Nothing(); } + + + virtual Result<Labels> slaveTaskStatusLabelDecorator( + const FrameworkID& frameworkId, + const TaskStatus& status) + { + LOG(INFO) << "Executing 'slaveTaskStatusLabelDecorator' hook"; + + Labels labels; + + // Set one known label. + Label* newLabel = labels.add_labels(); + newLabel->set_key("bar"); + newLabel->set_value("qux"); + + // Remove label which was set by test. + foreach (const Label& oldLabel, status.labels().labels()) { + if (oldLabel.key() != "foo") { + labels.add_labels()->CopyFrom(oldLabel); + } + } + + return labels; + } }; http://git-wip-us.apache.org/repos/asf/mesos/blob/a434ecc1/src/hook/manager.cpp ---------------------------------------------------------------------- diff --git a/src/hook/manager.cpp b/src/hook/manager.cpp index 0108534..11e6b0a 100644 --- a/src/hook/manager.cpp +++ b/src/hook/manager.cpp @@ -111,7 +111,7 @@ Labels HookManager::masterLaunchTaskLabelDecorator( TaskInfo taskInfo_ = taskInfo; foreachpair (const string& name, Hook* hook, availableHooks) { - const Result<Labels>& result = + const Result<Labels> result = hook->masterLaunchTaskLabelDecorator( taskInfo_, frameworkInfo, @@ -141,7 +141,7 @@ Labels HookManager::slaveRunTaskLabelDecorator( TaskInfo taskInfo_ = taskInfo; foreachpair (const string& name, Hook* hook, availableHooks) { - const Result<Labels>& result = + const Result<Labels> result = hook->slaveRunTaskLabelDecorator(taskInfo_, frameworkInfo, slaveInfo); // NOTE: If the hook returns None(), the task labels won't be @@ -164,7 +164,7 @@ Environment HookManager::slaveExecutorEnvironmentDecorator( { synchronized (mutex) { foreachpair (const string& name, Hook* hook, availableHooks) { - const Result<Environment>& result = + const Result<Environment> result = hook->slaveExecutorEnvironmentDecorator(executorInfo); // NOTE: If the hook returns None(), the environment won't be @@ -188,7 +188,7 @@ void HookManager::slaveRemoveExecutorHook( const ExecutorInfo& executorInfo) { foreachpair (const string& name, Hook* hook, availableHooks) { - const Try<Nothing>& result = + const Try<Nothing> result = hook->slaveRemoveExecutorHook(frameworkInfo, executorInfo); if (result.isError()) { LOG(WARNING) << "Slave remove executor hook failed for module '" @@ -197,5 +197,28 @@ void HookManager::slaveRemoveExecutorHook( } } + +Labels HookManager::slaveTaskStatusLabelDecorator( + const FrameworkID& frameworkId, + TaskStatus status) +{ + synchronized (mutex) { + foreachpair (const string& name, Hook* hook, availableHooks) { + const Result<Labels> result = + hook->slaveTaskStatusLabelDecorator(frameworkId, status); + + // NOTE: Labels remain unchanged if the hook returns None(). + if (result.isSome()) { + status.mutable_labels()->CopyFrom(result.get()); + } else if (result.isError()) { + LOG(WARNING) << "Slave TaskStatus label decorator hook failed for " + << "module '" << name << "': " << result.error(); + } + } + + return status.labels(); + } +} + } // namespace internal { } // namespace mesos { http://git-wip-us.apache.org/repos/asf/mesos/blob/a434ecc1/src/hook/manager.hpp ---------------------------------------------------------------------- diff --git a/src/hook/manager.hpp b/src/hook/manager.hpp index 47e8eb7..8153ce4 100644 --- a/src/hook/manager.hpp +++ b/src/hook/manager.hpp @@ -56,6 +56,10 @@ public: static void slaveRemoveExecutorHook( const FrameworkInfo& frameworkInfo, const ExecutorInfo& executorInfo); + + static Labels slaveTaskStatusLabelDecorator( + const FrameworkID& frameworkId, + TaskStatus status); }; } // namespace internal { http://git-wip-us.apache.org/repos/asf/mesos/blob/a434ecc1/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 2119b51..dc12c45 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -2708,6 +2708,13 @@ void Slave::statusUpdate(StatusUpdate update, const UPID& pid) return; } + if (HookManager::hooksAvailable()) { + // Set TaskStatus labels from run task label decorator. + update.mutable_status()->mutable_labels()->CopyFrom( + HookManager::slaveTaskStatusLabelDecorator( + update.framework_id(), update.status())); + } + TaskStatus status = update.status(); Executor* executor = framework->getExecutor(status.task_id()); http://git-wip-us.apache.org/repos/asf/mesos/blob/a434ecc1/src/tests/hook_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/hook_tests.cpp b/src/tests/hook_tests.cpp index 308ca9f..6827dec 100644 --- a/src/tests/hook_tests.cpp +++ b/src/tests/hook_tests.cpp @@ -65,6 +65,7 @@ using std::vector; using testing::_; using testing::DoAll; using testing::Return; +using testing::SaveArg; namespace mesos { namespace internal { @@ -422,6 +423,104 @@ TEST_F(HookTest, VerifySlaveRunTaskHook) Shutdown(); // Must shutdown before 'containerizer' gets deallocated. } + +// This test verifies that the slave task status label decorator can +// add and remove labels from a TaskStatus during the status update +// sequence. A TaskStatus with two labels ("foo":"bar" and +// "bar":"baz") is sent from the executor. The labels get modified by +// the slave hook to strip the "foo":"bar" pair and/ add a new +// "baz":"qux" pair. +TEST_F(HookTest, VerifySlaveTaskStatusLabelDecorator) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + MockExecutor exec(DEFAULT_EXECUTOR_ID); + + TestContainerizer containerizer(&exec); + + Try<PID<Slave>> slave = StartSlave(&containerizer); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); + + EXPECT_CALL(sched, registered(&driver, _, _)); + + Future<vector<Offer>> offers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&offers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + driver.start(); + + AWAIT_READY(offers); + ASSERT_EQ(1u, offers.get().size()); + + // Start a task. + TaskInfo task = createTask(offers.get()[0], "", DEFAULT_EXECUTOR_ID); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + ExecutorDriver* execDriver; + EXPECT_CALL(exec, registered(_, _, _, _)) + .WillOnce(SaveArg<0>(&execDriver)); + + Future<TaskInfo> execTask; + EXPECT_CALL(exec, launchTask(_, _)) + .WillOnce(FutureArg<1>(&execTask)); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offers.get()[0].id(), tasks); + + AWAIT_READY(execTask); + + // Now send TASK_RUNNING update with two labels. The first label + // ("foo:bar") will be removed by the task status hook to ensure + // that it can remove labels. The second label will be preserved + // and forwarded to Master (and eventually to the framework). + // The hook also adds a new label with the same key but a different + // value ("bar:quz"). + TaskStatus runningStatus; + runningStatus.mutable_task_id()->MergeFrom(execTask.get().task_id()); + runningStatus.set_state(TASK_RUNNING); + + // Add two labels to the TaskStatus + Labels* labels = runningStatus.mutable_labels(); + + labels->add_labels()->CopyFrom(createLabel("foo", "bar")); + labels->add_labels()->CopyFrom(createLabel("bar", "baz")); + + execDriver->sendStatusUpdate(runningStatus); + + AWAIT_READY(status); + + // The master hook will hang an extra label off. + const Labels& labels_ = status.get().labels(); + + EXPECT_EQ(2, labels_.labels_size()); + + // The test hook will prepend a new "baz":"qux" label. + EXPECT_EQ(labels_.labels(0).key(), "bar"); + EXPECT_EQ(labels_.labels(0).value(), "qux"); + + // And lastly, we only expect the "foo":"bar" pair to be stripped by + // the module. The last pair should be the original "bar":"baz" + // pair set by the test. + EXPECT_EQ(labels_.labels(1).key(), "bar"); + EXPECT_EQ(labels_.labels(1).value(), "baz"); + + driver.stop(); + driver.join(); + + Shutdown(); // Must shutdown before 'containerizer' gets deallocated. +} + } // namespace tests { } // namespace internal { } // namespace mesos {
