You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by nn...@apache.org on 2015/09/17 03:21:48 UTC
[6/7] mesos git commit: Replaced slaveTaskStatusLabelDecorator hook
with slaveTaskStatusDecorator hook.
Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.
This allows the hook to not only update TaskStatus::labels, but also
TaskStatus::container_status.
Enhanced example hook module and relevant test to reflect the change.
Review: https://reviews.apache.org/r/38368
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ddfcec81
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ddfcec81
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ddfcec81
Branch: refs/heads/master
Commit: ddfcec81820878222cf34b806985eabdec03b4ac
Parents: f7b470e
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Wed Sep 16 17:02:23 2015 -0700
Committer: Niklas Q. Nielsen <ni...@qni.dk>
Committed: Wed Sep 16 18:16:10 2015 -0700
----------------------------------------------------------------------
include/mesos/hook.hpp | 11 ++++++-----
src/examples/test_hook_module.cpp | 21 ++++++++++++++++++---
src/hook/manager.cpp | 22 +++++++++++++++-------
src/hook/manager.hpp | 2 +-
src/slave/slave.cpp | 18 ++++++++++++++----
src/tests/hook_tests.cpp | 29 +++++++++++++++++++++++++++--
6 files changed, 81 insertions(+), 22 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ddfcec81/include/mesos/hook.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/hook.hpp b/include/mesos/hook.hpp
index d90bacc..2353602 100644
--- a/include/mesos/hook.hpp
+++ b/include/mesos/hook.hpp
@@ -102,11 +102,12 @@ 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(
+ // 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
+ // TaskStatus with a set of labels and container_status. These labels and
+ // container status overwrite the existing labels on the TaskStatus. Remaining
+ // fields from the returned TaskStatus are discarded.
+ virtual Result<TaskStatus> slaveTaskStatusDecorator(
const FrameworkID& frameworkId,
const TaskStatus& status)
{
http://git-wip-us.apache.org/repos/asf/mesos/blob/ddfcec81/src/examples/test_hook_module.cpp
----------------------------------------------------------------------
diff --git a/src/examples/test_hook_module.cpp b/src/examples/test_hook_module.cpp
index bc13a8a..0dc74d6 100644
--- a/src/examples/test_hook_module.cpp
+++ b/src/examples/test_hook_module.cpp
@@ -171,11 +171,11 @@ public:
}
- virtual Result<Labels> slaveTaskStatusLabelDecorator(
+ virtual Result<TaskStatus> slaveTaskStatusDecorator(
const FrameworkID& frameworkId,
const TaskStatus& status)
{
- LOG(INFO) << "Executing 'slaveTaskStatusLabelDecorator' hook";
+ LOG(INFO) << "Executing 'slaveTaskStatusDecorator' hook";
Labels labels;
@@ -191,7 +191,22 @@ public:
}
}
- return labels;
+ TaskStatus result;
+ result.mutable_labels()->CopyFrom(labels);
+
+ // Set an IP address, a network isolation group, and a known label
+ // in network info. This data is later validated by the
+ // 'HookTest.VerifySlaveTaskStatusDecorator' test.
+ NetworkInfo* networkInfo =
+ result.mutable_container_status()->add_network_infos();
+ networkInfo->set_ip_address("4.3.2.1");
+ networkInfo->add_groups("public");
+
+ Label* networkInfoLabel = networkInfo->mutable_labels()->add_labels();
+ networkInfoLabel->set_key("net_foo");
+ networkInfoLabel->set_value("net_bar");
+
+ return result;
}
};
http://git-wip-us.apache.org/repos/asf/mesos/blob/ddfcec81/src/hook/manager.cpp
----------------------------------------------------------------------
diff --git a/src/hook/manager.cpp b/src/hook/manager.cpp
index 754c238..691976e 100644
--- a/src/hook/manager.cpp
+++ b/src/hook/manager.cpp
@@ -231,25 +231,33 @@ void HookManager::slaveRemoveExecutorHook(
}
-Labels HookManager::slaveTaskStatusLabelDecorator(
+TaskStatus HookManager::slaveTaskStatusDecorator(
const FrameworkID& frameworkId,
TaskStatus status)
{
synchronized (mutex) {
foreachpair (const string& name, Hook* hook, availableHooks) {
- const Result<Labels> result =
- hook->slaveTaskStatusLabelDecorator(frameworkId, status);
+ const Result<TaskStatus> result =
+ hook->slaveTaskStatusDecorator(frameworkId, status);
- // NOTE: Labels remain unchanged if the hook returns None().
+ // NOTE: Labels/ContainerStatus remain unchanged if the hook returns
+ // None().
if (result.isSome()) {
- status.mutable_labels()->CopyFrom(result.get());
+ if (result.get().has_labels()) {
+ status.mutable_labels()->CopyFrom(result.get().labels());
+ }
+
+ if (result.get().has_container_status()) {
+ status.mutable_container_status()->CopyFrom(
+ result.get().container_status());
+ }
} else if (result.isError()) {
- LOG(WARNING) << "Slave TaskStatus label decorator hook failed for "
+ LOG(WARNING) << "Slave TaskStatus decorator hook failed for "
<< "module '" << name << "': " << result.error();
}
}
- return status.labels();
+ return status;
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/ddfcec81/src/hook/manager.hpp
----------------------------------------------------------------------
diff --git a/src/hook/manager.hpp b/src/hook/manager.hpp
index 30d8321..a517c05 100644
--- a/src/hook/manager.hpp
+++ b/src/hook/manager.hpp
@@ -69,7 +69,7 @@ public:
const FrameworkInfo& frameworkInfo,
const ExecutorInfo& executorInfo);
- static Labels slaveTaskStatusLabelDecorator(
+ static TaskStatus slaveTaskStatusDecorator(
const FrameworkID& frameworkId,
TaskStatus status);
};
http://git-wip-us.apache.org/repos/asf/mesos/blob/ddfcec81/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 43fc737..93353a1 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2759,10 +2759,20 @@ void Slave::statusUpdate(StatusUpdate update, const UPID& pid)
}
if (HookManager::hooksAvailable()) {
- // Set TaskStatus labels from run task label decorator.
- update.mutable_status()->mutable_labels()->CopyFrom(
- HookManager::slaveTaskStatusLabelDecorator(
- update.framework_id(), update.status()));
+ // Even though the hook(s) return a TaskStatus, we only use two fields:
+ // container_status and labels. Remaining fields are discarded.
+ TaskStatus statusFromHooks =
+ HookManager::slaveTaskStatusDecorator(
+ update.framework_id(), update.status());
+ if (statusFromHooks.has_labels()) {
+ update.mutable_status()->mutable_labels()->CopyFrom(
+ statusFromHooks.labels());
+ }
+
+ if (statusFromHooks.has_container_status()) {
+ update.mutable_status()->mutable_container_status()->CopyFrom(
+ statusFromHooks.container_status());
+ }
}
// Fill in the container IP address with the IP from the agent PID, if not
http://git-wip-us.apache.org/repos/asf/mesos/blob/ddfcec81/src/tests/hook_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hook_tests.cpp b/src/tests/hook_tests.cpp
index deb4343..b23a587 100644
--- a/src/tests/hook_tests.cpp
+++ b/src/tests/hook_tests.cpp
@@ -428,7 +428,7 @@ TEST_F(HookTest, VerifySlaveRunTaskHook)
// "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)
+TEST_F(HookTest, VerifySlaveTaskStatusDecorator)
{
Try<PID<Master>> master = StartMaster();
ASSERT_SOME(master);
@@ -495,7 +495,7 @@ TEST_F(HookTest, VerifySlaveTaskStatusLabelDecorator)
AWAIT_READY(status);
- // The master hook will hang an extra label off.
+ // The hook will hang an extra label off.
const Labels& labels_ = status.get().labels();
EXPECT_EQ(2, labels_.labels_size());
@@ -510,6 +510,31 @@ TEST_F(HookTest, VerifySlaveTaskStatusLabelDecorator)
EXPECT_EQ("bar", labels_.labels(1).key());
EXPECT_EQ("baz", labels_.labels(1).value());
+ // Now validate TaskInfo.container_status. We must have received a
+ // container_status with one network_info set by the test hook module.
+ EXPECT_TRUE(status.get().has_container_status());
+ EXPECT_EQ(1, status.get().container_status().network_infos().size());
+
+ const NetworkInfo networkInfo =
+ status.get().container_status().network_infos(0);
+
+ // The hook module sets up '4.3.2.1' as the IP address and 'public' as the
+ // network isolation group.
+ EXPECT_TRUE(networkInfo.has_ip_address());
+ EXPECT_EQ("4.3.2.1", networkInfo.ip_address());
+
+ EXPECT_EQ(1, networkInfo.groups().size());
+ EXPECT_EQ("public", networkInfo.groups(0));
+
+ EXPECT_TRUE(networkInfo.has_labels());
+ EXPECT_EQ(1, networkInfo.labels().labels().size());
+
+ const Label networkInfoLabel = networkInfo.labels().labels(0);
+
+ // Finally, the labels set inside NetworkInfo by the hook module.
+ EXPECT_EQ("net_foo", networkInfoLabel.key());
+ EXPECT_EQ("net_bar", networkInfoLabel.value());
+
driver.stop();
driver.join();