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:45 UTC
[3/7] mesos git commit: Updated Isolator::prepare to return list of
required namespaces.
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.