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:43 UTC
[1/7] mesos git commit: Refactored container Launcher to accept
namespaces dynamically.
Repository: mesos
Updated Branches:
refs/heads/master 0e4019fff -> 4b12df29c
Refactored container Launcher to accept namespaces dynamically.
This prepares MesosContainerizer to allow isolators to specify
namespaces for each individual containerizer. Thus, allowing isolators
to decide whether or not to enable isolation for a particular container.
Review: https://reviews.apache.org/r/38363
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e047f7d6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e047f7d6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e047f7d6
Branch: refs/heads/master
Commit: e047f7d69b5297cc787487b6093119a3be517e48
Parents: 0e4019f
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Wed Sep 16 17:00:39 2015 -0700
Committer: Niklas Q. Nielsen <ni...@qni.dk>
Committed: Wed Sep 16 17:00:40 2015 -0700
----------------------------------------------------------------------
src/slave/containerizer/launcher.cpp | 3 +-
src/slave/containerizer/launcher.hpp | 6 ++--
src/slave/containerizer/linux_launcher.cpp | 21 +++++------
src/slave/containerizer/linux_launcher.hpp | 9 ++---
src/slave/containerizer/mesos/containerizer.cpp | 26 ++++++++------
.../containerizer/filesystem_isolator_tests.cpp | 3 +-
src/tests/containerizer/isolator_tests.cpp | 37 +++++++++++---------
src/tests/containerizer/launcher.hpp | 11 +++---
8 files changed, 62 insertions(+), 54 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/launcher.cpp b/src/slave/containerizer/launcher.cpp
index 5267eec..6649d35 100644
--- a/src/slave/containerizer/launcher.cpp
+++ b/src/slave/containerizer/launcher.cpp
@@ -107,7 +107,8 @@ Try<pid_t> PosixLauncher::fork(
const Subprocess::IO& err,
const Option<flags::FlagsBase>& flags,
const Option<map<string, string>>& environment,
- const Option<lambda::function<int()>>& setup)
+ const Option<lambda::function<int()>>& setup,
+ const Option<int>& namespaces)
{
if (pids.contains(containerId)) {
return Error("Process has already been forked for container " +
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/launcher.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/launcher.hpp b/src/slave/containerizer/launcher.hpp
index 8cc832e..8945776 100644
--- a/src/slave/containerizer/launcher.hpp
+++ b/src/slave/containerizer/launcher.hpp
@@ -72,7 +72,8 @@ public:
const process::Subprocess::IO& err,
const Option<flags::FlagsBase>& flags,
const Option<std::map<std::string, std::string>>& environment,
- const Option<lambda::function<int()>>& setup) = 0;
+ const Option<lambda::function<int()>>& setup,
+ const Option<int>& namespaces) = 0;
// Kill all processes in the containerized context.
virtual process::Future<Nothing> destroy(const ContainerID& containerId) = 0;
@@ -102,7 +103,8 @@ public:
const process::Subprocess::IO& err,
const Option<flags::FlagsBase>& flags,
const Option<std::map<std::string, std::string>>& environment,
- const Option<lambda::function<int()>>& setup);
+ const Option<lambda::function<int()>>& setup,
+ const Option<int>& namespaces);
virtual process::Future<Nothing> destroy(const ContainerID& containerId);
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/linux_launcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/linux_launcher.cpp b/src/slave/containerizer/linux_launcher.cpp
index 12acf4d..dde552f 100644
--- a/src/slave/containerizer/linux_launcher.cpp
+++ b/src/slave/containerizer/linux_launcher.cpp
@@ -67,16 +67,12 @@ static ContainerID container(const string& cgroup)
LinuxLauncher::LinuxLauncher(
const Flags& _flags,
- int _namespaces,
const string& _hierarchy)
: flags(_flags),
- namespaces(_namespaces),
hierarchy(_hierarchy) {}
-Try<Launcher*> LinuxLauncher::create(
- const Flags& flags,
- const Option<int>& namespaces)
+Try<Launcher*> LinuxLauncher::create(const Flags& flags)
{
Try<string> hierarchy = cgroups::prepare(
flags.cgroups_hierarchy,
@@ -104,7 +100,6 @@ Try<Launcher*> LinuxLauncher::create(
return new LinuxLauncher(
flags,
- namespaces.isSome() ? namespaces.get() : 0,
hierarchy.get());
}
@@ -178,7 +173,9 @@ static int childMain(void* _func)
// The customized clone function which will be used by 'subprocess()'.
-static pid_t clone(const lambda::function<int()>& func, int namespaces)
+static pid_t clone(
+ const lambda::function<int()>& func,
+ const Option<int>& namespaces)
{
// Stack for the child.
// - unsigned long long used for best alignment.
@@ -186,13 +183,16 @@ static pid_t clone(const lambda::function<int()>& func, int namespaces)
// - 8 MiB appears to be the default for "ulimit -s" on OSX and Linux.
static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)];
+ int flags = namespaces.isSome() ? namespaces.get() : 0;
+ flags |= SIGCHLD; // Specify SIGCHLD as child termination signal.
+
LOG(INFO) << "Cloning child process with flags = "
- << ns::stringify(namespaces);
+ << ns::stringify(flags);
return ::clone(
childMain,
&stack[sizeof(stack)/sizeof(stack[0]) - 1], // stack grows down.
- namespaces | SIGCHLD, // Specify SIGCHLD as child termination signal.
+ flags,
(void*) &func);
}
@@ -246,7 +246,8 @@ Try<pid_t> LinuxLauncher::fork(
const process::Subprocess::IO& err,
const Option<flags::FlagsBase>& flags,
const Option<map<string, string>>& environment,
- const Option<lambda::function<int()>>& setup)
+ const Option<lambda::function<int()>>& setup,
+ const Option<int>& namespaces)
{
// Create a freezer cgroup for this container if necessary.
Try<bool> exists = cgroups::exists(hierarchy, cgroup(containerId));
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/linux_launcher.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/linux_launcher.hpp b/src/slave/containerizer/linux_launcher.hpp
index bf6bf3f..f6112c9 100644
--- a/src/slave/containerizer/linux_launcher.hpp
+++ b/src/slave/containerizer/linux_launcher.hpp
@@ -30,9 +30,7 @@ namespace slave {
class LinuxLauncher : public Launcher
{
public:
- static Try<Launcher*> create(
- const Flags& flags,
- const Option<int>& namespaces);
+ static Try<Launcher*> create(const Flags& flags);
virtual ~LinuxLauncher() {}
@@ -48,19 +46,18 @@ public:
const process::Subprocess::IO& err,
const Option<flags::FlagsBase>& flags,
const Option<std::map<std::string, std::string>>& environment,
- const Option<lambda::function<int()>>& setup);
+ const Option<lambda::function<int()>>& setup,
+ const Option<int>& namespaces);
virtual process::Future<Nothing> destroy(const ContainerID& containerId);
private:
LinuxLauncher(
const Flags& flags,
- int namespaces,
const std::string& hierarchy);
static const std::string subsystem;
const Flags flags;
- const int namespaces;
const std::string hierarchy;
std::string cgroup(const ContainerID& containerId);
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 0023f1d..71f16cc 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -233,17 +233,11 @@ Try<MesosContainerizer*> MesosContainerizer::create(
}
#ifdef __linux__
- int namespaces = 0;
- foreach (const Owned<Isolator>& isolator, isolators) {
- if (isolator->namespaces().get().isSome()) {
- namespaces |= isolator->namespaces().get().get();
- }
- }
-
- // Determine which launcher to use based on the isolation flag.
+ // Always use LinuxLauncher if running as root since we will be picking
+ // namespaces during the fork() call.
Try<Launcher*> launcher =
- (strings::contains(isolation, "cgroups") || namespaces != 0)
- ? LinuxLauncher::create(flags_, namespaces)
+ (geteuid() == 0)
+ ? LinuxLauncher::create(flags_)
: PosixLauncher::create(flags_);
#else
Try<Launcher*> launcher = PosixLauncher::create(flags_);
@@ -836,6 +830,15 @@ Future<bool> MesosContainerizerProcess::_launch(
launchFlags.commands = object;
+ // TODO(karya): Create ContainerPrepareInfo.namespaces and use that instead of
+ // Isolator::namespaces().
+ int namespaces = 0;
+ foreach (const Owned<Isolator>& isolator, isolators) {
+ if (isolator->namespaces().get().isSome()) {
+ namespaces |= isolator->namespaces().get().get();
+ }
+ }
+
// Fork the child using launcher.
vector<string> argv(2);
argv[0] = MESOS_CONTAINERIZER;
@@ -852,7 +855,8 @@ Future<bool> MesosContainerizerProcess::_launch(
: Subprocess::PATH(path::join(directory, "stderr"))),
launchFlags,
environment,
- None());
+ None(),
+ namespaces); // 'namespaces' will be ignored by PosixLauncher.
if (forked.isError()) {
return Failure("Failed to fork executor: " + forked.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/tests/containerizer/filesystem_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/filesystem_isolator_tests.cpp b/src/tests/containerizer/filesystem_isolator_tests.cpp
index 4dbfeac..ca9f423 100644
--- a/src/tests/containerizer/filesystem_isolator_tests.cpp
+++ b/src/tests/containerizer/filesystem_isolator_tests.cpp
@@ -105,8 +105,7 @@ public:
Owned<Isolator> isolator(_isolator.get());
- Try<Launcher*> _launcher =
- LinuxLauncher::create(flags, isolator->namespaces().get().get());
+ Try<Launcher*> _launcher = LinuxLauncher::create(flags);
if (_launcher.isError()) {
return Error("Failed to create LinuxLauncher: " + _launcher.error());
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index d34c82a..1d0d41b 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -198,7 +198,8 @@ TYPED_TEST(CpuIsolatorTest, UserCpuUsage)
Subprocess::FD(STDERR_FILENO),
None(),
None(),
- lambda::bind(&childSetup, pipes));
+ lambda::bind(&childSetup, pipes),
+ None());
ASSERT_SOME(pid);
@@ -308,7 +309,8 @@ TYPED_TEST(CpuIsolatorTest, SystemCpuUsage)
Subprocess::FD(STDERR_FILENO),
None(),
None(),
- lambda::bind(&childSetup, pipes));
+ lambda::bind(&childSetup, pipes),
+ None());
ASSERT_SOME(pid);
@@ -405,6 +407,7 @@ TEST_F(RevocableCpuIsolatorTest, ROOT_CGROUPS_RevocableCpu)
Subprocess::PATH("/dev/null"),
None(),
None(),
+ None(),
None());
ASSERT_SOME(pid);
@@ -451,8 +454,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs)
Try<Isolator*> isolator = CgroupsCpushareIsolatorProcess::create(flags);
CHECK_SOME(isolator);
- Try<Launcher*> launcher =
- LinuxLauncher::create(flags, isolator.get()->namespaces().get());
+ Try<Launcher*> launcher = LinuxLauncher::create(flags);
CHECK_SOME(launcher);
// Set the executor's resources to 0.5 cpu.
@@ -500,7 +502,8 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs)
Subprocess::FD(STDERR_FILENO),
None(),
None(),
- lambda::bind(&childSetup, pipes));
+ lambda::bind(&childSetup, pipes),
+ isolator.get()->namespaces().get());
ASSERT_SOME(pid);
@@ -562,8 +565,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs_Big_Quota)
Try<Isolator*> isolator = CgroupsCpushareIsolatorProcess::create(flags);
CHECK_SOME(isolator);
- Try<Launcher*> launcher =
- LinuxLauncher::create(flags, isolator.get()->namespaces().get());
+ Try<Launcher*> launcher = LinuxLauncher::create(flags);
CHECK_SOME(launcher);
// Set the executor's resources to 100.5 cpu.
@@ -602,7 +604,8 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs_Big_Quota)
Subprocess::FD(STDERR_FILENO),
None(),
None(),
- lambda::bind(&childSetup, pipes));
+ lambda::bind(&childSetup, pipes),
+ isolator.get()->namespaces().get());
ASSERT_SOME(pid);
@@ -646,8 +649,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
Try<Isolator*> isolator = CgroupsCpushareIsolatorProcess::create(flags);
CHECK_SOME(isolator);
- Try<Launcher*> launcher =
- LinuxLauncher::create(flags, isolator.get()->namespaces().get());
+ Try<Launcher*> launcher = LinuxLauncher::create(flags);
CHECK_SOME(launcher);
ExecutorInfo executorInfo;
@@ -692,7 +694,8 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
Subprocess::FD(STDERR_FILENO),
None(),
None(),
- lambda::bind(&childSetup, pipes));
+ lambda::bind(&childSetup, pipes),
+ isolator.get()->namespaces().get());
ASSERT_SOME(pid);
@@ -921,8 +924,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume)
Try<Isolator*> isolator = SharedFilesystemIsolatorProcess::create(flags);
CHECK_SOME(isolator);
- Try<Launcher*> launcher =
- LinuxLauncher::create(flags, isolator.get()->namespaces().get());
+ Try<Launcher*> launcher = LinuxLauncher::create(flags);
CHECK_SOME(launcher);
// Use /var/tmp so we don't mask the work directory (under /tmp).
@@ -975,7 +977,8 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume)
Subprocess::FD(STDERR_FILENO),
None(),
None(),
- None());
+ None(),
+ isolator.get()->namespaces().get());
ASSERT_SOME(pid);
// Set up the reaper to wait on the forked child.
@@ -1027,8 +1030,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume)
Try<Isolator*> isolator = SharedFilesystemIsolatorProcess::create(flags);
CHECK_SOME(isolator);
- Try<Launcher*> launcher =
- LinuxLauncher::create(flags, isolator.get()->namespaces().get());
+ Try<Launcher*> launcher = LinuxLauncher::create(flags);
CHECK_SOME(launcher);
// We'll mount the absolute test work directory as /var/tmp in the
@@ -1080,7 +1082,8 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume)
Subprocess::FD(STDERR_FILENO),
None(),
None(),
- None());
+ None(),
+ isolator.get()->namespaces().get());
ASSERT_SOME(pid);
// Set up the reaper to wait on the forked child.
http://git-wip-us.apache.org/repos/asf/mesos/blob/e047f7d6/src/tests/containerizer/launcher.hpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launcher.hpp b/src/tests/containerizer/launcher.hpp
index 6f020f9..5d34bab 100644
--- a/src/tests/containerizer/launcher.hpp
+++ b/src/tests/containerizer/launcher.hpp
@@ -55,7 +55,7 @@ ACTION_P(InvokeRecover, launcher)
ACTION_P(InvokeFork, launcher)
{
return launcher->real->fork(
- arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8);
+ arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9);
}
@@ -79,9 +79,9 @@ public:
EXPECT_CALL(*this, recover(_))
.WillRepeatedly(DoDefault());
- ON_CALL(*this, fork(_, _, _, _, _, _, _, _, _))
+ ON_CALL(*this, fork(_, _, _, _, _, _, _, _, _, _))
.WillByDefault(InvokeFork(this));
- EXPECT_CALL(*this, fork(_, _, _, _, _, _, _, _, _))
+ EXPECT_CALL(*this, fork(_, _, _, _, _, _, _, _, _, _))
.WillRepeatedly(DoDefault());
ON_CALL(*this, destroy(_))
@@ -97,7 +97,7 @@ public:
process::Future<hashset<ContainerID>>(
const std::list<mesos::slave::ContainerState>& states));
- MOCK_METHOD9(
+ MOCK_METHOD10(
fork,
Try<pid_t>(
const ContainerID& containerId,
@@ -108,7 +108,8 @@ public:
const process::Subprocess::IO& err,
const Option<flags::FlagsBase>& flags,
const Option<std::map<std::string, std::string> >& env,
- const Option<lambda::function<int()> >& setup));
+ const Option<lambda::function<int()> >& setup,
+ const Option<int>& namespaces));
MOCK_METHOD1(
destroy,
[2/7] mesos git commit: Minor refactor for MesosContainerizer.
Posted by nn...@apache.org.
Minor refactor for MesosContainerizer.
This change moves two different pieces of code, that each iterate over
list of ContainerPrepareInfos, close together for readability. It
becomes even more relevant when looking at
https://reviews.apache.org/r/38365/ that iterates over yet another
member from ContainerPrepareInfo.
Review: https://reviews.apache.org/r/38364
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fc541a9a
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fc541a9a
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fc541a9a
Branch: refs/heads/master
Commit: fc541a9a97eb1d86c27452019ff217eed11ed5a3
Parents: e047f7d
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Wed Sep 16 17:00:55 2015 -0700
Committer: Niklas Q. Nielsen <ni...@qni.dk>
Committed: Wed Sep 16 17:00:55 2015 -0700
----------------------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp | 38 ++++++++++----------
1 file changed, 19 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/fc541a9a/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 71f16cc..3f0d8fb 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -786,17 +786,31 @@ Future<bool> MesosContainerizerProcess::_launch(
environment[variable.name()] = variable.value();
}
- // Include any environment variables returned from
- // Isolator::prepare().
+ JSON::Array commandArray;
foreach (const Option<ContainerPrepareInfo>& prepareInfo, prepareInfos) {
- if (prepareInfo.isSome() && prepareInfo.get().has_environment()) {
+ if (!prepareInfo.isSome()) {
+ continue;
+ }
+
+ // Populate the list of additional commands to be run inside the container
+ // context.
+ foreach (const CommandInfo& command, prepareInfo.get().commands()) {
+ commandArray.values.push_back(JSON::Protobuf(command));
+ }
+
+ // Process additional environment variables returned by isolators.
+ if (prepareInfo.get().has_environment()) {
foreach (const Environment::Variable& variable,
- prepareInfo.get().environment().variables()) {
+ prepareInfo.get().environment().variables()) {
environment[variable.name()] = variable.value();
}
}
}
+ // TODO(jieyu): Use JSON::Array once we have generic parse support.
+ JSON::Object commands;
+ commands.values["commands"] = commandArray;
+
// Use a pipe to block the child until it's been isolated.
int pipes[2];
@@ -814,21 +828,7 @@ Future<bool> MesosContainerizerProcess::_launch(
launchFlags.user = user;
launchFlags.pipe_read = pipes[0];
launchFlags.pipe_write = pipes[1];
-
- // Prepare the additional prepareInfo commands.
- // TODO(jieyu): Use JSON::Array once we have generic parse support.
- JSON::Object object;
- JSON::Array array;
- foreach (const Option<ContainerPrepareInfo>& prepareInfo, prepareInfos) {
- if (prepareInfo.isSome()) {
- foreach (const CommandInfo& command, prepareInfo.get().commands()) {
- array.values.push_back(JSON::Protobuf(command));
- }
- }
- }
- object.values["commands"] = array;
-
- launchFlags.commands = object;
+ launchFlags.commands = commands;
// TODO(karya): Create ContainerPrepareInfo.namespaces and use that instead of
// Isolator::namespaces().
[6/7] mesos git commit: Replaced slaveTaskStatusLabelDecorator hook
with slaveTaskStatusDecorator hook.
Posted by nn...@apache.org.
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();
[3/7] mesos git commit: Updated Isolator::prepare to return list of
required namespaces.
Posted by nn...@apache.org.
Updated Isolator::prepare to return list of required namespaces.
This allows the Isolators to decide whether or not to provide resource
isolation on a per-container level.
Review: https://reviews.apache.org/r/38365
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6923bb3e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6923bb3e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6923bb3e
Branch: refs/heads/master
Commit: 6923bb3e8cfbddde9fbabc6ca4edc29d9fc96c06
Parents: fc541a9
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Wed Sep 16 17:01:16 2015 -0700
Committer: Niklas Q. Nielsen <ni...@qni.dk>
Committed: Wed Sep 16 17:01:18 2015 -0700
----------------------------------------------------------------------
include/mesos/slave/isolator.hpp | 9 ----
include/mesos/slave/isolator.proto | 4 ++
src/slave/containerizer/isolator.cpp | 6 ---
src/slave/containerizer/isolator.hpp | 4 --
.../isolators/filesystem/linux.cpp | 7 +--
.../isolators/filesystem/linux.hpp | 2 -
.../isolators/filesystem/shared.cpp | 7 +--
.../isolators/filesystem/shared.hpp | 2 -
.../containerizer/isolators/namespaces/pid.cpp | 7 +--
.../containerizer/isolators/namespaces/pid.hpp | 2 -
.../isolators/network/port_mapping.cpp | 18 +++----
.../isolators/network/port_mapping.hpp | 2 -
src/slave/containerizer/mesos/containerizer.cpp | 14 ++----
src/tests/containerizer/isolator_tests.cpp | 51 ++++++++++++--------
14 files changed, 50 insertions(+), 85 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/include/mesos/slave/isolator.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/slave/isolator.hpp b/include/mesos/slave/isolator.hpp
index 4db23dc..ea14bff 100644
--- a/include/mesos/slave/isolator.hpp
+++ b/include/mesos/slave/isolator.hpp
@@ -44,15 +44,6 @@ class Isolator
public:
virtual ~Isolator() {}
- // Returns the namespaces required by the isolator. The namespaces
- // are created while launching the executor. Isolators may return
- // a None() to indicate that they don't require any namespaces
- // (e.g., Isolators for OS X).
- // TODO(karya): Since namespaces are Linux-only, create a separate
- // LinuxIsolator (and corresponding LinuxIsolatorProcess) class
- // for Linux-specific isolators.
- virtual process::Future<Option<int>> namespaces() { return None(); }
-
// Recover containers from the run states and the orphan containers
// (known to the launcher but not known to the slave) detected by
// the launcher.
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/include/mesos/slave/isolator.proto
----------------------------------------------------------------------
diff --git a/include/mesos/slave/isolator.proto b/include/mesos/slave/isolator.proto
index 12ea6ac..9d38a25 100644
--- a/include/mesos/slave/isolator.proto
+++ b/include/mesos/slave/isolator.proto
@@ -71,4 +71,8 @@ message ContainerPrepareInfo
// The root filesystem for the container.
optional string rootfs = 3;
+
+ // (Linux only) The namespaces required for the container.
+ // The namespaces are created while launching the executor.
+ optional uint32 namespaces = 4 [default = 0];
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolator.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolator.cpp b/src/slave/containerizer/isolator.cpp
index 7973100..5a7bad1 100644
--- a/src/slave/containerizer/isolator.cpp
+++ b/src/slave/containerizer/isolator.cpp
@@ -47,12 +47,6 @@ MesosIsolator::~MesosIsolator()
}
-Future<Option<int>> MesosIsolator::namespaces()
-{
- return dispatch(process.get(), &MesosIsolatorProcess::namespaces);
-}
-
-
Future<Nothing> MesosIsolator::recover(
const list<ContainerState>& state,
const hashset<ContainerID>& orphans)
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolator.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolator.hpp b/src/slave/containerizer/isolator.hpp
index fbb7c8a..da58cbe 100644
--- a/src/slave/containerizer/isolator.hpp
+++ b/src/slave/containerizer/isolator.hpp
@@ -43,8 +43,6 @@ public:
explicit MesosIsolator(process::Owned<MesosIsolatorProcess> process);
virtual ~MesosIsolator();
- virtual process::Future<Option<int>> namespaces();
-
virtual process::Future<Nothing> recover(
const std::list<mesos::slave::ContainerState>& states,
const hashset<ContainerID>& orphans);
@@ -82,8 +80,6 @@ class MesosIsolatorProcess : public process::Process<MesosIsolatorProcess>
public:
virtual ~MesosIsolatorProcess() {}
- virtual process::Future<Option<int>> namespaces() { return None(); }
-
virtual process::Future<Nothing> recover(
const std::list<mesos::slave::ContainerState>& states,
const hashset<ContainerID>& orphans) = 0;
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/filesystem/linux.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/filesystem/linux.cpp b/src/slave/containerizer/isolators/filesystem/linux.cpp
index 297a296..d8a0e07 100644
--- a/src/slave/containerizer/isolators/filesystem/linux.cpp
+++ b/src/slave/containerizer/isolators/filesystem/linux.cpp
@@ -84,12 +84,6 @@ LinuxFilesystemIsolatorProcess::LinuxFilesystemIsolatorProcess(
LinuxFilesystemIsolatorProcess::~LinuxFilesystemIsolatorProcess() {}
-Future<Option<int>> LinuxFilesystemIsolatorProcess::namespaces()
-{
- return CLONE_NEWNS;
-}
-
-
Future<Nothing> LinuxFilesystemIsolatorProcess::recover(
const list<ContainerState>& states,
const hashset<ContainerID>& orphans)
@@ -268,6 +262,7 @@ Future<Option<ContainerPrepareInfo>> LinuxFilesystemIsolatorProcess::__prepare(
const Owned<Info>& info = infos[containerId];
ContainerPrepareInfo prepareInfo;
+ prepareInfo.set_namespaces(CLONE_NEWNS);
if (rootfs.isNone()) {
// It the container does not change its root filesystem, we need
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/filesystem/linux.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/filesystem/linux.hpp b/src/slave/containerizer/isolators/filesystem/linux.hpp
index b0016e5..99f939f 100644
--- a/src/slave/containerizer/isolators/filesystem/linux.hpp
+++ b/src/slave/containerizer/isolators/filesystem/linux.hpp
@@ -49,8 +49,6 @@ public:
virtual ~LinuxFilesystemIsolatorProcess();
- virtual process::Future<Option<int>> namespaces();
-
virtual process::Future<Nothing> recover(
const std::list<mesos::slave::ContainerState>& states,
const hashset<ContainerID>& orphans);
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/filesystem/shared.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/filesystem/shared.cpp b/src/slave/containerizer/isolators/filesystem/shared.cpp
index 4b4520e..73804ca 100644
--- a/src/slave/containerizer/isolators/filesystem/shared.cpp
+++ b/src/slave/containerizer/isolators/filesystem/shared.cpp
@@ -64,12 +64,6 @@ Try<Isolator*> SharedFilesystemIsolatorProcess::create(const Flags& flags)
}
-process::Future<Option<int>> SharedFilesystemIsolatorProcess::namespaces()
-{
- return CLONE_NEWNS;
-}
-
-
Future<Nothing> SharedFilesystemIsolatorProcess::recover(
const list<ContainerState>& states,
const hashset<ContainerID>& orphans)
@@ -108,6 +102,7 @@ Future<Option<ContainerPrepareInfo>> SharedFilesystemIsolatorProcess::prepare(
containerPaths.insert(directory);
ContainerPrepareInfo prepareInfo;
+ prepareInfo.set_namespaces(CLONE_NEWNS);
foreach (const Volume& volume, executorInfo.container().volumes()) {
// Because the filesystem is shared we require the container path
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/filesystem/shared.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/filesystem/shared.hpp b/src/slave/containerizer/isolators/filesystem/shared.hpp
index a21bc79..3a2f7db 100644
--- a/src/slave/containerizer/isolators/filesystem/shared.hpp
+++ b/src/slave/containerizer/isolators/filesystem/shared.hpp
@@ -39,8 +39,6 @@ public:
virtual ~SharedFilesystemIsolatorProcess();
- virtual process::Future<Option<int>> namespaces();
-
virtual process::Future<Nothing> recover(
const std::list<mesos::slave::ContainerState>& states,
const hashset<ContainerID>& orphans);
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/namespaces/pid.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/namespaces/pid.cpp b/src/slave/containerizer/isolators/namespaces/pid.cpp
index 35cb664..a9823e0 100644
--- a/src/slave/containerizer/isolators/namespaces/pid.cpp
+++ b/src/slave/containerizer/isolators/namespaces/pid.cpp
@@ -121,12 +121,6 @@ Result<ino_t> NamespacesPidIsolatorProcess::getNamespace(
}
-process::Future<Option<int>> NamespacesPidIsolatorProcess::namespaces()
-{
- return CLONE_NEWPID | CLONE_NEWNS;
-}
-
-
Future<Nothing> NamespacesPidIsolatorProcess::recover(
const list<ContainerState>& states,
const hashset<ContainerID>& orphans)
@@ -166,6 +160,7 @@ Future<Option<ContainerPrepareInfo>> NamespacesPidIsolatorProcess::prepare(
const Option<string>& user)
{
ContainerPrepareInfo prepareInfo;
+ prepareInfo.set_namespaces(CLONE_NEWPID | CLONE_NEWNS);
// Mask the bind mount root directory in each container so
// containers cannot see the namespace bind mount of other
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/namespaces/pid.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/namespaces/pid.hpp b/src/slave/containerizer/isolators/namespaces/pid.hpp
index b22f5ba..87270d0 100644
--- a/src/slave/containerizer/isolators/namespaces/pid.hpp
+++ b/src/slave/containerizer/isolators/namespaces/pid.hpp
@@ -56,8 +56,6 @@ public:
virtual ~NamespacesPidIsolatorProcess() {}
- virtual process::Future<Option<int>> namespaces();
-
virtual process::Future<Nothing> recover(
const std::list<mesos::slave::ContainerState>& states,
const hashset<ContainerID>& orphans);
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/network/port_mapping.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/network/port_mapping.cpp b/src/slave/containerizer/isolators/network/port_mapping.cpp
index 34ba229..e6bb75e 100644
--- a/src/slave/containerizer/isolators/network/port_mapping.cpp
+++ b/src/slave/containerizer/isolators/network/port_mapping.cpp
@@ -1619,17 +1619,6 @@ Try<Isolator*> PortMappingIsolatorProcess::create(const Flags& flags)
}
-process::Future<Option<int>> PortMappingIsolatorProcess::namespaces()
-{
- // NOTE: the port mapping isolator itself doesn't require mount
- // namespace. However, if mount namespace is enabled because of
- // other isolators, we need to set mount sharing accordingly for
- // PORT_MAPPING_BIND_MOUNT_ROOT to avoid races described in
- // MESOS-1558. So we turn on mount namespace here for consistency.
- return CLONE_NEWNET | CLONE_NEWNS;
-}
-
-
Future<Nothing> PortMappingIsolatorProcess::recover(
const list<ContainerState>& states,
const hashset<ContainerID>& orphans)
@@ -2135,6 +2124,13 @@ Future<Option<ContainerPrepareInfo>> PortMappingIsolatorProcess::prepare(
ContainerPrepareInfo prepareInfo;
prepareInfo.add_commands()->set_value(scripts(infos[containerId]));
+ // NOTE: the port mapping isolator itself doesn't require mount
+ // namespace. However, if mount namespace is enabled because of
+ // other isolators, we need to set mount sharing accordingly for
+ // PORT_MAPPING_BIND_MOUNT_ROOT to avoid races described in
+ // MESOS-1558. So we turn on mount namespace here for consistency.
+ prepareInfo.set_namespaces(CLONE_NEWNET | CLONE_NEWNS);
+
return prepareInfo;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/isolators/network/port_mapping.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/isolators/network/port_mapping.hpp b/src/slave/containerizer/isolators/network/port_mapping.hpp
index 4bca0b8..ae53c1b 100644
--- a/src/slave/containerizer/isolators/network/port_mapping.hpp
+++ b/src/slave/containerizer/isolators/network/port_mapping.hpp
@@ -152,8 +152,6 @@ public:
virtual ~PortMappingIsolatorProcess() {}
- virtual process::Future<Option<int>> namespaces();
-
virtual process::Future<Nothing> recover(
const std::list<mesos::slave::ContainerState>& states,
const hashset<ContainerID>& orphans);
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 3f0d8fb..e8a89bf 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -787,6 +787,7 @@ Future<bool> MesosContainerizerProcess::_launch(
}
JSON::Array commandArray;
+ int namespaces = 0;
foreach (const Option<ContainerPrepareInfo>& prepareInfo, prepareInfos) {
if (!prepareInfo.isSome()) {
continue;
@@ -805,6 +806,10 @@ Future<bool> MesosContainerizerProcess::_launch(
environment[variable.name()] = variable.value();
}
}
+
+ if (prepareInfo.get().has_namespaces()) {
+ namespaces |= prepareInfo.get().namespaces();
+ }
}
// TODO(jieyu): Use JSON::Array once we have generic parse support.
@@ -830,15 +835,6 @@ Future<bool> MesosContainerizerProcess::_launch(
launchFlags.pipe_write = pipes[1];
launchFlags.commands = commands;
- // TODO(karya): Create ContainerPrepareInfo.namespaces and use that instead of
- // Isolator::namespaces().
- int namespaces = 0;
- foreach (const Owned<Isolator>& isolator, isolators) {
- if (isolator->namespaces().get().isSome()) {
- namespaces |= isolator->namespaces().get().get();
- }
- }
-
// Fork the child using launcher.
vector<string> argv(2);
argv[0] = MESOS_CONTAINERIZER;
http://git-wip-us.apache.org/repos/asf/mesos/blob/6923bb3e/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index 1d0d41b..a25ae97 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -470,11 +470,14 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs)
Try<string> dir = os::mkdtemp(path::join(os::getcwd(), "XXXXXX"));
ASSERT_SOME(dir);
- AWAIT_READY(isolator.get()->prepare(
- containerId,
- executorInfo,
- dir.get(),
- None()));
+ Future<Option<ContainerPrepareInfo>> prepare =
+ isolator.get()->prepare(
+ containerId,
+ executorInfo,
+ dir.get(),
+ None());
+
+ AWAIT_READY(prepare);
// Generate random numbers to max out a single core. We'll run this for 0.5
// seconds of wall time so it should consume approximately 250 ms of total
@@ -503,7 +506,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs)
None(),
None(),
lambda::bind(&childSetup, pipes),
- isolator.get()->namespaces().get());
+ prepare.get().isSome() ? prepare.get().get().namespaces() : 0);
ASSERT_SOME(pid);
@@ -581,11 +584,14 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs_Big_Quota)
Try<string> dir = os::mkdtemp(path::join(os::getcwd(), "XXXXXX"));
ASSERT_SOME(dir);
- AWAIT_READY(isolator.get()->prepare(
- containerId,
- executorInfo,
- dir.get(),
- None()));
+ Future<Option<ContainerPrepareInfo>> prepare =
+ isolator.get()->prepare(
+ containerId,
+ executorInfo,
+ dir.get(),
+ None());
+
+ AWAIT_READY(prepare);
int pipes[2];
ASSERT_NE(-1, ::pipe(pipes));
@@ -605,7 +611,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Cfs_Big_Quota)
None(),
None(),
lambda::bind(&childSetup, pipes),
- isolator.get()->namespaces().get());
+ prepare.get().isSome() ? prepare.get().get().namespaces() : 0);
ASSERT_SOME(pid);
@@ -664,11 +670,14 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
Try<string> dir = os::mkdtemp(path::join(os::getcwd(), "XXXXXX"));
ASSERT_SOME(dir);
- AWAIT_READY(isolator.get()->prepare(
- containerId,
- executorInfo,
- dir.get(),
- None()));
+ Future<Option<ContainerPrepareInfo>> prepare =
+ isolator.get()->prepare(
+ containerId,
+ executorInfo,
+ dir.get(),
+ None());
+
+ AWAIT_READY(prepare);
// Right after the creation of the cgroup, which happens in
// 'prepare', we check that it is empty.
@@ -695,7 +704,7 @@ TEST_F(LimitedCpuIsolatorTest, ROOT_CGROUPS_Pids_and_Tids)
None(),
None(),
lambda::bind(&childSetup, pipes),
- isolator.get()->namespaces().get());
+ prepare.get().isSome() ? prepare.get().get().namespaces() : 0);
ASSERT_SOME(pid);
@@ -955,6 +964,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume)
AWAIT_READY(prepare);
ASSERT_SOME(prepare.get());
ASSERT_EQ(1, prepare.get().get().commands().size());
+ EXPECT_TRUE(prepare.get().get().has_namespaces());
// The test will touch a file in container path.
const string file = path::join(containerPath, UUID::random().toString());
@@ -978,7 +988,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_RelativeVolume)
None(),
None(),
None(),
- isolator.get()->namespaces().get());
+ prepare.get().get().namespaces());
ASSERT_SOME(pid);
// Set up the reaper to wait on the forked child.
@@ -1059,6 +1069,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume)
AWAIT_READY(prepare);
ASSERT_SOME(prepare.get());
ASSERT_EQ(1, prepare.get().get().commands().size());
+ EXPECT_TRUE(prepare.get().get().has_namespaces());
// Test the volume mounting by touching a file in the container's
// /tmp, which should then be in flags.work_dir.
@@ -1083,7 +1094,7 @@ TEST_F(SharedFilesystemIsolatorTest, DISABLED_ROOT_AbsoluteVolume)
None(),
None(),
None(),
- isolator.get()->namespaces().get());
+ prepare.get().get().namespaces());
ASSERT_SOME(pid);
// Set up the reaper to wait on the forked child.
[5/7] mesos git commit: Added NetworkInfo message to ContainerInfo
and TaskStatus.
Posted by nn...@apache.org.
Added NetworkInfo message to ContainerInfo and TaskStatus.
This allows the frameworks to specify an intent to enable
ip-per-container. The IP information is supplied back to the framework
as well as state.json endpoints by including NetworkInfo inside
TaskStatus.
Review: https://reviews.apache.org/r/38367
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f7b470e4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f7b470e4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f7b470e4
Branch: refs/heads/master
Commit: f7b470e46a84ddc6d9702c1e76d97073cd6aa48a
Parents: fd0a431
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Wed Sep 16 17:02:06 2015 -0700
Committer: Niklas Q. Nielsen <ni...@qni.dk>
Committed: Wed Sep 16 18:16:08 2015 -0700
----------------------------------------------------------------------
include/mesos/mesos.proto | 64 ++++++++++++++++++++++++++++++
src/common/http.cpp | 45 +++++++++++++++++++++
src/common/http.hpp | 2 +
src/slave/slave.cpp | 10 +++++
src/tests/master_tests.cpp | 88 +++++++++++++++++++++++++++++++++++++++++
src/tests/slave_tests.cpp | 88 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 297 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f7b470e4/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index b1deed4..899d52f 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1159,6 +1159,10 @@ message TaskStatus {
// labels should be used to tag TaskStatus message with light-weight
// meta-data.
optional Labels labels = 12;
+
+ // Container related information that is resolved dynamically such as
+ // network address.
+ optional ContainerStatus container_status = 13;
}
@@ -1352,6 +1356,52 @@ message Volume {
/**
+ * Describes a network request by framework as well as network resolution
+ * provided by the the executor or Agent.
+ *
+ * A framework may request the network isolator on the Agent to assign an IP
+ * address to the container being launched. Alternatively, it can provide a
+ * specific IP address to be assigned to the container. The NetworkInfo message
+ * is not interpreted by the Master or Agent and is intended to be use by Agent
+ * modules implementing network isolation. If the modules are missing, the
+ * message is simply ignored. In future, the task launch will fail if there is
+ * no module providing the network isolation capabilities (MESOS-3390).
+ *
+ * An executor, Agent, or an Agent module may append NetworkInfos inside
+ * TaskStatus::container_status to provide information such as the container IP
+ * address and isolation groups.
+ */
+message NetworkInfo {
+ enum Protocol {
+ IPv4 = 1;
+ IPv6 = 2;
+ }
+
+ // Specify IP address requirement. Set protocol to the desired value to
+ // request the network isolator on the Agent to assign an IP address to the
+ // container being launched. If a specific IP address is specified in
+ // ip_address, this field should not be set.
+ optional Protocol protocol = 1;
+
+ // Statically assigned IP provided by the Framework. This IP will be assigned
+ // to the container by the network isolator module on the Agent. This field
+ // should not be used with the protocol field above.
+ // NOTE: It is up to the networking 'provider' (IPAM/Isolator) to interpret
+ // this either as a hint of as a requirement for assigning the IP.
+ optional string ip_address = 2;
+
+ // A group is the name given to a set of logically-related IPs that are
+ // allowed to communicate within themselves. For example, one might want
+ // to create separate groups for isolating dev, testing, qa and prod
+ // deployment environments.
+ repeated string groups = 3;
+
+ // To tag certain metadata to be used by Isolator/IPAM, e.g., rack, etc.
+ optional Labels labels = 4;
+};
+
+
+/**
* Describes a container configuration and allows extensible
* configurations for different container implementations.
*/
@@ -1410,6 +1460,20 @@ message ContainerInfo {
// the type.
optional DockerInfo docker = 3;
optional MesosInfo mesos = 5;
+
+ // A list of network requests. A framework can request multiple IP addresses
+ // for the container.
+ repeated NetworkInfo network_infos = 7;
+}
+
+
+/**
+ * Container related information that is resolved during container setup. The
+ * information is sent back to the framework as part of the TaskStatus message.
+ */
+message ContainerStatus {
+ // This field can be reliably used to identify the container IP address.
+ repeated NetworkInfo network_infos = 1;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/f7b470e4/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index 85fb932..aaef10b 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -150,6 +150,48 @@ JSON::Array model(const Labels& labels)
}
+JSON::Object model(const NetworkInfo& info)
+{
+ JSON::Object object;
+
+ if (info.has_ip_address()) {
+ object.values["ip_address"] = info.ip_address();
+ }
+
+ if (info.groups().size() > 0) {
+ JSON::Array array;
+ array.values.reserve(info.groups().size()); // MESOS-2353.
+ foreach (const string& group, info.groups()) {
+ array.values.push_back(group);
+ }
+ object.values["groups"] = std::move(array);
+ }
+
+ if (info.has_labels()) {
+ object.values["labels"] = std::move(model(info.labels()));
+ }
+
+ return object;
+}
+
+
+JSON::Object model(const ContainerStatus& status)
+{
+ JSON::Object object;
+
+ if (status.network_infos().size() > 0) {
+ JSON::Array array;
+ array.values.reserve(status.network_infos().size()); // MESOS-2353.
+ foreach (const NetworkInfo& info, status.network_infos()) {
+ array.values.push_back(model(info));
+ }
+ object.values["network_infos"] = std::move(array);
+ }
+
+ return object;
+}
+
+
// Returns a JSON object modeled on a TaskStatus.
JSON::Object model(const TaskStatus& status)
{
@@ -159,7 +201,10 @@ JSON::Object model(const TaskStatus& status)
if (status.has_labels()) {
object.values["labels"] = std::move(model(status.labels()));
+ }
+ if (status.has_container_status()) {
+ object.values["container_status"] = model(status.container_status());
}
return object;
http://git-wip-us.apache.org/repos/asf/mesos/blob/f7b470e4/src/common/http.hpp
----------------------------------------------------------------------
diff --git a/src/common/http.hpp b/src/common/http.hpp
index 1e61888..058baa6 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -80,6 +80,8 @@ JSON::Object model(const Attributes& attributes);
JSON::Object model(const CommandInfo& command);
JSON::Object model(const ExecutorInfo& executorInfo);
JSON::Array model(const Labels& labels);
+JSON::Object model(const NetworkInfo& info);
+JSON::Object model(const ContainerStatus& status);
// These are the two identical ways to model a task, depending on
// whether you have a 'Task' or a 'TaskInfo' available.
http://git-wip-us.apache.org/repos/asf/mesos/blob/f7b470e4/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 021786b..43fc737 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -2765,6 +2765,16 @@ void Slave::statusUpdate(StatusUpdate update, const UPID& pid)
update.framework_id(), update.status()));
}
+ // Fill in the container IP address with the IP from the agent PID, if not
+ // already filled in.
+ // TODO(karya): Fill in the IP address by looking up the executor PID.
+ ContainerStatus* containerStatus =
+ update.mutable_status()->mutable_container_status();
+ if (containerStatus->network_infos().size() == 0) {
+ NetworkInfo* networkInfo = containerStatus->add_network_infos();
+ networkInfo->set_ip_address(stringify(self().address.ip));
+ }
+
TaskStatus status = update.status();
Executor* executor = framework->getExecutor(status.task_id());
http://git-wip-us.apache.org/repos/asf/mesos/blob/f7b470e4/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index dd65fcc..06d74c3 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -3244,6 +3244,94 @@ TEST_F(MasterTest, TaskStatusLabels)
}
+// This test verifies that TaskStatus::container_status is exposed over the
+// master state endpoint.
+TEST_F(MasterTest, TaskStatusContainerStatus)
+{
+ Try<PID<Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ MockExecutor exec(DEFAULT_EXECUTOR_ID);
+
+ Try<PID<Slave>> slave = StartSlave(&exec);
+ ASSERT_SOME(slave);
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _))
+ .Times(1);
+
+ Future<vector<Offer>> offers;
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ driver.start();
+
+ AWAIT_READY(offers);
+ EXPECT_NE(0u, offers.get().size());
+
+ TaskInfo task = createTask(offers.get()[0], "sleep 100", DEFAULT_EXECUTOR_ID);
+
+ ExecutorDriver* execDriver;
+ EXPECT_CALL(exec, registered(_, _, _, _))
+ .WillOnce(SaveArg<0>(&execDriver));
+
+ EXPECT_CALL(exec, launchTask(_, _))
+ .WillOnce(SendStatusUpdateFromTask(TASK_RUNNING));
+
+ Future<TaskStatus> status;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&status));
+
+ driver.launchTasks(offers.get()[0].id(), {task});
+
+ AWAIT_READY(status);
+
+ const string slaveIPAddress = stringify(slave.get().address.ip);
+
+ // Validate that the Slave has passed in its IP address in
+ // TaskStatus.container_status.network_infos[0].ip_address.
+ EXPECT_TRUE(status.get().has_container_status());
+ EXPECT_EQ(1, status.get().container_status().network_infos().size());
+ EXPECT_TRUE(
+ status.get().container_status().network_infos(0).has_ip_address());
+ EXPECT_EQ(
+ slaveIPAddress,
+ status.get().container_status().network_infos(0).ip_address());
+
+ // Now do the same validation with state endpoint.
+ Future<process::http::Response> response =
+ process::http::get(master.get(), "state.json");
+ AWAIT_READY(response);
+
+ EXPECT_SOME_EQ(
+ "application/json",
+ response.get().headers.get("Content-Type"));
+
+ Try<JSON::Object> parse = JSON::parse<JSON::Object>(response.get().body);
+ ASSERT_SOME(parse);
+
+ // Validate that the IP address passed in by the Slave is available at the
+ // state endpoint.
+ ASSERT_SOME_EQ(
+ slaveIPAddress,
+ parse.get().find<JSON::String>(
+ "frameworks[0].tasks[0].statuses[0]"
+ ".container_status.network_infos[0].ip_address"));
+
+ EXPECT_CALL(exec, shutdown(_))
+ .Times(AtMost(1));
+
+ driver.stop();
+ driver.join();
+
+ Shutdown(); // Must shutdown before 'containerizer' gets deallocated.
+}
+
+
// This tests the 'active' field in slave entries from the master's
// state endpoint. We first verify an active slave, deactivate it
// and verify that the 'active' field is false.
http://git-wip-us.apache.org/repos/asf/mesos/blob/f7b470e4/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 4a1a586..d158ccc 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -2221,6 +2221,94 @@ TEST_F(SlaveTest, TaskStatusLabels)
}
+// This test verifies that TaskStatus::container_status an is exposed over
+// the slave state endpoint.
+TEST_F(SlaveTest, TaskStatusContainerStatus)
+{
+ Try<PID<Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ MockExecutor exec(DEFAULT_EXECUTOR_ID);
+
+ Try<PID<Slave>> slave = StartSlave(&exec);
+ ASSERT_SOME(slave);
+
+ MockScheduler sched;
+ MesosSchedulerDriver driver(
+ &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL);
+
+ EXPECT_CALL(sched, registered(&driver, _, _))
+ .Times(1);
+
+ Future<vector<Offer>> offers;
+ EXPECT_CALL(sched, resourceOffers(&driver, _))
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
+
+ driver.start();
+
+ AWAIT_READY(offers);
+ EXPECT_NE(0u, offers.get().size());
+
+ TaskInfo task = createTask(offers.get()[0], "sleep 100", DEFAULT_EXECUTOR_ID);
+
+ ExecutorDriver* execDriver;
+ EXPECT_CALL(exec, registered(_, _, _, _))
+ .WillOnce(SaveArg<0>(&execDriver));
+
+ EXPECT_CALL(exec, launchTask(_, _))
+ .WillOnce(SendStatusUpdateFromTask(TASK_RUNNING));
+
+ Future<TaskStatus> status;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&status));
+
+ driver.launchTasks(offers.get()[0].id(), {task});
+
+ AWAIT_READY(status);
+
+ const string slaveIPAddress = stringify(slave.get().address.ip);
+
+ // Validate that the Slave has passed in its IP address in
+ // TaskStatus.container_status.network_infos[0].ip_address.
+ EXPECT_TRUE(status.get().has_container_status());
+ EXPECT_EQ(1, status.get().container_status().network_infos().size());
+ EXPECT_TRUE(
+ status.get().container_status().network_infos(0).has_ip_address());
+ EXPECT_EQ(
+ slaveIPAddress,
+ status.get().container_status().network_infos(0).ip_address());
+
+ // Now do the same validation with state endpoint.
+ Future<process::http::Response> response =
+ process::http::get(slave.get(), "state.json");
+ AWAIT_READY(response);
+
+ EXPECT_SOME_EQ(
+ "application/json",
+ response.get().headers.get("Content-Type"));
+
+ Try<JSON::Object> parse = JSON::parse<JSON::Object>(response.get().body);
+ ASSERT_SOME(parse);
+
+ // Validate that the IP address passed in by the Slave is available at the
+ // state endpoint.
+ ASSERT_SOME_EQ(
+ slaveIPAddress,
+ parse.get().find<JSON::String>(
+ "frameworks[0].executors[0].tasks[0].statuses[0]"
+ ".container_status.network_infos[0].ip_address"));
+
+ EXPECT_CALL(exec, shutdown(_))
+ .Times(AtMost(1));
+
+ driver.stop();
+ driver.join();
+
+ Shutdown(); // Must shutdown before 'containerizer' gets deallocated.
+}
+
+
// Test that we can set the executors environment variables and it
// won't inhert the slaves.
TEST_F(SlaveTest, ExecutorEnvironmentVariables)
[7/7] mesos git commit: Updated docker executor to set container IP
in TaskStatus::NetworkInfo.
Posted by nn...@apache.org.
Updated docker executor to set container IP in TaskStatus::NetworkInfo.
Review: https://reviews.apache.org/r/38370
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4b12df29
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4b12df29
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4b12df29
Branch: refs/heads/master
Commit: 4b12df29c93756bb893ef42b8a59da280a05a9fd
Parents: ddfcec8
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Wed Sep 16 17:02:53 2015 -0700
Committer: Niklas Q. Nielsen <ni...@qni.dk>
Committed: Wed Sep 16 18:16:10 2015 -0700
----------------------------------------------------------------------
src/docker/executor.cpp | 5 +++++
src/tests/containerizer/docker_containerizer_tests.cpp | 7 +++++++
2 files changed, 12 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4b12df29/src/docker/executor.cpp
----------------------------------------------------------------------
diff --git a/src/docker/executor.cpp b/src/docker/executor.cpp
index 6809e4a..1e49013 100644
--- a/src/docker/executor.cpp
+++ b/src/docker/executor.cpp
@@ -167,9 +167,14 @@ public:
status.set_state(TASK_RUNNING);
status.set_data(container.output);
if (container.ipAddress.isSome()) {
+ // TODO(karya): Deprecated -- Remove after 0.25.0 has shipped.
Label* label = status.mutable_labels()->add_labels();
label->set_key("Docker.NetworkSettings.IPAddress");
label->set_value(container.ipAddress.get());
+
+ NetworkInfo* networkInfo =
+ status.mutable_container_status()->add_network_infos();
+ networkInfo->set_ip_address(container.ipAddress.get());
}
driver->sendStatusUpdate(status);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/4b12df29/src/tests/containerizer/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/docker_containerizer_tests.cpp b/src/tests/containerizer/docker_containerizer_tests.cpp
index a628922..8771ef6 100644
--- a/src/tests/containerizer/docker_containerizer_tests.cpp
+++ b/src/tests/containerizer/docker_containerizer_tests.cpp
@@ -472,11 +472,18 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Launch)
// Now verify that the Docker.NetworkSettings.IPAddress label is
// present.
+ // TODO(karya): Deprecated -- Remove after 0.25.0 has shipped.
ASSERT_TRUE(statusRunning.get().has_labels());
EXPECT_EQ(1, statusRunning.get().labels().labels().size());
EXPECT_EQ("Docker.NetworkSettings.IPAddress",
statusRunning.get().labels().labels(0).key());
+ // Now verify that the TaskStatus contains the container IP address.
+ ASSERT_TRUE(statusRunning.get().has_container_status());
+ EXPECT_EQ(1, statusRunning.get().container_status().network_infos().size());
+ EXPECT_TRUE(
+ statusRunning.get().container_status().network_infos(0).has_ip_address());
+
ASSERT_TRUE(exists(docker, slaveId, containerId.get()));
Future<containerizer::Termination> termination =
[4/7] mesos git commit: Added helper to model Labels message for
state.json.
Posted by nn...@apache.org.
Added helper to model Labels message for state.json.
Also updated Task modelling to show labels only if Task.has_labels() is
true. This way, the "labels" field won't shown if there are no labels.
This makes it consistent with the modelling of rest of the "optional"
fields.
Review: https://reviews.apache.org/r/38366
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fd0a431c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fd0a431c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fd0a431c
Branch: refs/heads/master
Commit: fd0a431c340d66f96f71715ace0d10d9c1b17b49
Parents: 6923bb3
Author: Kapil Arya <ka...@mesosphere.io>
Authored: Wed Sep 16 17:01:54 2015 -0700
Committer: Niklas Q. Nielsen <ni...@qni.dk>
Committed: Wed Sep 16 17:01:55 2015 -0700
----------------------------------------------------------------------
src/common/http.cpp | 42 ++++++++++++++----------------------
src/common/http.hpp | 1 +
src/common/protobuf_utils.cpp | 4 +++-
src/master/master.cpp | 4 +++-
src/tests/common/http_tests.cpp | 1 -
5 files changed, 23 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/fd0a431c/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index 9c0d31e..85fb932 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -139,6 +139,17 @@ JSON::Object model(const Attributes& attributes)
}
+JSON::Array model(const Labels& labels)
+{
+ JSON::Array array;
+ array.values.reserve(labels.labels().size()); // MESOS-2353.
+ foreach (const Label& label, labels.labels()) {
+ array.values.push_back(JSON::Protobuf(label));
+ }
+ return array;
+}
+
+
// Returns a JSON object modeled on a TaskStatus.
JSON::Object model(const TaskStatus& status)
{
@@ -147,13 +158,8 @@ JSON::Object model(const TaskStatus& status)
object.values["timestamp"] = status.timestamp();
if (status.has_labels()) {
- JSON::Array array;
- array.values.reserve(status.labels().labels().size()); // MESOS-2353.
+ object.values["labels"] = std::move(model(status.labels()));
- foreach (const Label& label, status.labels().labels()) {
- array.values.push_back(JSON::Protobuf(label));
- }
- object.values["labels"] = std::move(array);
}
return object;
@@ -188,16 +194,8 @@ JSON::Object model(const Task& task)
object.values["statuses"] = std::move(array);
}
- {
- JSON::Array array;
- if (task.has_labels()) {
- array.values.reserve(task.labels().labels().size()); // MESOS-2353.
-
- foreach (const Label& label, task.labels().labels()) {
- array.values.push_back(JSON::Protobuf(label));
- }
- }
- object.values["labels"] = std::move(array);
+ if (task.has_labels()) {
+ object.values["labels"] = std::move(model(task.labels()));
}
if (task.has_discovery()) {
@@ -299,16 +297,8 @@ JSON::Object model(
object.values["statuses"] = std::move(array);
}
- {
- JSON::Array array;
- if (task.has_labels()) {
- array.values.reserve(task.labels().labels().size()); // MESOS-2353.
-
- foreach (const Label& label, task.labels().labels()) {
- array.values.push_back(JSON::Protobuf(label));
- }
- }
- object.values["labels"] = std::move(array);
+ if (task.has_labels()) {
+ object.values["labels"] = std::move(model(task.labels()));
}
if (task.has_discovery()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/fd0a431c/src/common/http.hpp
----------------------------------------------------------------------
diff --git a/src/common/http.hpp b/src/common/http.hpp
index 61ad531..1e61888 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -79,6 +79,7 @@ JSON::Object model(const hashmap<std::string, Resources>& roleResources);
JSON::Object model(const Attributes& attributes);
JSON::Object model(const CommandInfo& command);
JSON::Object model(const ExecutorInfo& executorInfo);
+JSON::Array model(const Labels& labels);
// These are the two identical ways to model a task, depending on
// whether you have a 'Task' or a 'TaskInfo' available.
http://git-wip-us.apache.org/repos/asf/mesos/blob/fd0a431c/src/common/protobuf_utils.cpp
----------------------------------------------------------------------
diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp
index 0861270..c8e6211 100644
--- a/src/common/protobuf_utils.cpp
+++ b/src/common/protobuf_utils.cpp
@@ -137,7 +137,9 @@ Task createTask(
t.mutable_executor_id()->CopyFrom(task.executor().executor_id());
}
- t.mutable_labels()->MergeFrom(task.labels());
+ if (task.has_labels()) {
+ t.mutable_labels()->CopyFrom(task.labels());
+ }
if (task.has_discovery()) {
t.mutable_discovery()->MergeFrom(task.discovery());
http://git-wip-us.apache.org/repos/asf/mesos/blob/fd0a431c/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index f26271c..1c4e7af 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -2743,7 +2743,9 @@ Resources Master::addTask(
t->mutable_executor_id()->MergeFrom(executorId.get());
}
- t->mutable_labels()->MergeFrom(task.labels());
+ if (task.has_labels()) {
+ t->mutable_labels()->MergeFrom(task.labels());
+ }
if (task.has_discovery()) {
t->mutable_discovery()->MergeFrom(task.discovery());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/fd0a431c/src/tests/common/http_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/common/http_tests.cpp b/src/tests/common/http_tests.cpp
index bf8712b..8a01ffc 100644
--- a/src/tests/common/http_tests.cpp
+++ b/src/tests/common/http_tests.cpp
@@ -88,7 +88,6 @@ TEST(HTTPTest, ModelTask)
" \"executor_id\":\"\","
" \"framework_id\":\"f\","
" \"id\":\"t\","
- " \"labels\": [],"
" \"name\":\"task\","
" \"resources\":"
" {"