This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 053adba55f9cdac9026df1552017b9981836667f
Author: Radhika Jandhyala <radhi...@microsoft.com>
AuthorDate: Wed Jun 13 15:57:31 2018 -0700
Whitelist inheritable file descriptors in the containerizer.
This commit whitelists the read and write ends of the control pipe
that are intended to be inherited by the containerizer. This is
necessary because this pipe is passed to the child process
implicitly (through environment variables), so previously the
libprocess and stout APIs had no knowledge that these file descriptors
needed to be inheritable.
Adding the whitelist as yet another parameter caused us to exceed the
mock methods of Google Mock, so we had to squash three other
parameters into one, name the `containerIO` in/out/err fields are
instead passed as one, unwrapped later.
Also, a new `forkImpl` method had to be added to the tests because it
is not possible to mock a function with default arguments in Google
Mock, but this can be worked around by mocking in an implementation.
Review: https://reviews.apache.org/r/67394/
(cherry picked from commit f8c8c35af920518ecb6c56d873dd44160390c5c7)
---
src/slave/containerizer/mesos/containerizer.cpp | 9 +++---
src/slave/containerizer/mesos/launcher.cpp | 16 +++++-----
src/slave/containerizer/mesos/launcher.hpp | 15 +++++-----
src/slave/containerizer/mesos/linux_launcher.cpp | 34 ++++++++++------------
src/slave/containerizer/mesos/linux_launcher.hpp | 7 ++---
src/tests/containerizer/launcher.cpp | 6 ++--
src/tests/containerizer/launcher.hpp | 9 +++---
.../containerizer/mesos_containerizer_tests.cpp | 7 ++---
8 files changed, 48 insertions(+), 55 deletions(-)
diff --git a/src/slave/containerizer/mesos/containerizer.cpp
b/src/slave/containerizer/mesos/containerizer.cpp
index 5f29fe1..985c88b 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1666,6 +1666,8 @@ Future<bool> MesosContainerizerProcess::_launch(
launchFlags.pipe_read = pipes[0];
launchFlags.pipe_write = pipes[1];
+ const vector<int_fd> whitelistFds{pipes[0], pipes[1]};
+
#ifndef __WINDOWS__
// Set the `runtime_directory` launcher flag so that the launch
// helper knows where to checkpoint the status of the container
@@ -1751,15 +1753,14 @@ Future<bool> MesosContainerizerProcess::_launch(
containerId,
argv[0],
argv,
- containerIO->in,
- containerIO->out,
- containerIO->err,
+ containerIO.get(),
nullptr,
launchEnvironment,
// 'enterNamespaces' will be ignored by SubprocessLauncher.
_enterNamespaces,
// 'cloneNamespaces' will be ignored by SubprocessLauncher.
- _cloneNamespaces);
+ _cloneNamespaces,
+ whitelistFds);
if (forked.isError()) {
return Failure("Failed to fork: " + forked.error());
diff --git a/src/slave/containerizer/mesos/launcher.cpp
b/src/slave/containerizer/mesos/launcher.cpp
index ec31fa2..5fc5c30 100644
--- a/src/slave/containerizer/mesos/launcher.cpp
+++ b/src/slave/containerizer/mesos/launcher.cpp
@@ -85,13 +85,12 @@ Try<pid_t> SubprocessLauncher::fork(
const ContainerID& containerId,
const string& path,
const vector<string>& argv,
- const Subprocess::IO& in,
- const Subprocess::IO& out,
- const Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<map<string, string>>& environment,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces)
+ const Option<int>& cloneNamespaces,
+ const vector<int_fd>& whitelistFds)
{
if (enterNamespaces.isSome() && enterNamespaces.get() != 0) {
return Error("Subprocess launcher does not support entering namespaces");
@@ -124,14 +123,15 @@ Try<pid_t> SubprocessLauncher::fork(
Try<Subprocess> child = subprocess(
path,
argv,
- in,
- out,
- err,
+ containerIO.in,
+ containerIO.out,
+ containerIO.err,
flags,
environment,
None(),
parentHooks,
- {Subprocess::ChildHook::SETSID()});
+ {Subprocess::ChildHook::SETSID()},
+ whitelistFds);
if (child.isError()) {
return Error("Failed to fork a child process: " + child.error());
diff --git a/src/slave/containerizer/mesos/launcher.hpp
b/src/slave/containerizer/mesos/launcher.hpp
index f69d934..90a50b1 100644
--- a/src/slave/containerizer/mesos/launcher.hpp
+++ b/src/slave/containerizer/mesos/launcher.hpp
@@ -38,6 +38,7 @@
#include <stout/try.hpp>
#include "slave/flags.hpp"
+#include "slave/containerizer/containerizer.hpp"
namespace mesos {
namespace internal {
@@ -64,13 +65,12 @@ public:
const ContainerID& containerId,
const std::string& path,
const std::vector<std::string>& argv,
- const process::Subprocess::IO& in,
- const process::Subprocess::IO& out,
- const process::Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& environment,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces) = 0;
+ const Option<int>& cloneNamespaces,
+ const std::vector<int_fd>& whitelistFds) = 0;
// Kill all processes in the containerized context.
virtual process::Future<Nothing> destroy(const ContainerID& containerId) = 0;
@@ -102,13 +102,12 @@ public:
const ContainerID& containerId,
const std::string& path,
const std::vector<std::string>& argv,
- const process::Subprocess::IO& in,
- const process::Subprocess::IO& out,
- const process::Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& environment,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces);
+ const Option<int>& cloneNamespaces,
+ const std::vector<int_fd>& whitelistFds);
virtual process::Future<Nothing> destroy(const ContainerID& containerId);
diff --git a/src/slave/containerizer/mesos/linux_launcher.cpp
b/src/slave/containerizer/mesos/linux_launcher.cpp
index 1cea04e..27997f7 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -74,13 +74,12 @@ public:
const ContainerID& containerId,
const string& path,
const vector<string>& argv,
- const process::Subprocess::IO& in,
- const process::Subprocess::IO& out,
- const process::Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<map<string, string>>& environment,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces);
+ const Option<int>& cloneNamespaces,
+ const vector<int_fd>& whitelistFds);
virtual process::Future<Nothing> destroy(const ContainerID& containerId);
@@ -206,13 +205,12 @@ Try<pid_t> LinuxLauncher::fork(
const ContainerID& containerId,
const string& path,
const vector<string>& argv,
- const process::Subprocess::IO& in,
- const process::Subprocess::IO& out,
- const process::Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<map<string, string>>& environment,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces)
+ const Option<int>& cloneNamespaces,
+ const vector<int_fd>& whitelistFds)
{
return dispatch(
process.get(),
@@ -220,13 +218,12 @@ Try<pid_t> LinuxLauncher::fork(
containerId,
path,
argv,
- in,
- out,
- err,
+ containerIO,
flags,
environment,
enterNamespaces,
- cloneNamespaces).get();
+ cloneNamespaces,
+ whitelistFds).get();
}
@@ -383,13 +380,12 @@ Try<pid_t> LinuxLauncherProcess::fork(
const ContainerID& containerId,
const string& path,
const vector<string>& argv,
- const process::Subprocess::IO& in,
- const process::Subprocess::IO& out,
- const process::Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<map<string, string>>& environment,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces)
+ const Option<int>& cloneNamespaces,
+ const vector<int_fd>& whitelistFds)
{
// Make sure this container (nested or not) is unique.
if (containers.contains(containerId)) {
@@ -461,9 +457,9 @@ Try<pid_t> LinuxLauncherProcess::fork(
Try<Subprocess> child = subprocess(
path,
argv,
- in,
- out,
- err,
+ containerIO.in,
+ containerIO.out,
+ containerIO.err,
flags,
environment,
[target, enterFlags, cloneFlags](const lambda::function<int()>& child) {
diff --git a/src/slave/containerizer/mesos/linux_launcher.hpp
b/src/slave/containerizer/mesos/linux_launcher.hpp
index e152523..7a8f6f0 100644
--- a/src/slave/containerizer/mesos/linux_launcher.hpp
+++ b/src/slave/containerizer/mesos/linux_launcher.hpp
@@ -46,13 +46,12 @@ public:
const ContainerID& containerId,
const std::string& path,
const std::vector<std::string>& argv,
- const process::Subprocess::IO& in,
- const process::Subprocess::IO& out,
- const process::Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& environment,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces);
+ const Option<int>& cloneNamespaces,
+ const std::vector<int_fd>& whitelistFds);
virtual process::Future<Nothing> destroy(const ContainerID& containerId);
diff --git a/src/tests/containerizer/launcher.cpp
b/src/tests/containerizer/launcher.cpp
index a92d989..51ae4f9 100644
--- a/src/tests/containerizer/launcher.cpp
+++ b/src/tests/containerizer/launcher.cpp
@@ -30,7 +30,7 @@ ACTION_P(InvokeRecover, launcher)
ACTION_P(InvokeFork, launcher)
{
return launcher->real->fork(
- arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9);
+ arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8);
}
@@ -51,9 +51,9 @@ TestLauncher::TestLauncher(const
process::Owned<slave::Launcher>& _real)
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(_))
diff --git a/src/tests/containerizer/launcher.hpp
b/src/tests/containerizer/launcher.hpp
index a8e436f..6057286 100644
--- a/src/tests/containerizer/launcher.hpp
+++ b/src/tests/containerizer/launcher.hpp
@@ -56,19 +56,18 @@ public:
process::Future<hashset<ContainerID>>(
const std::list<mesos::slave::ContainerState>& states));
- MOCK_METHOD10(
+ MOCK_METHOD9(
fork,
Try<pid_t>(
const ContainerID& containerId,
const std::string& path,
const std::vector<std::string>& argv,
- const process::Subprocess::IO& in,
- const process::Subprocess::IO& out,
- const process::Subprocess::IO& err,
+ const mesos::slave::ContainerIO& containerIO,
const flags::FlagsBase* flags,
const Option<std::map<std::string, std::string>>& env,
const Option<int>& enterNamespaces,
- const Option<int>& cloneNamespaces));
+ const Option<int>& cloneNamespaces,
+ const std::vector<int_fd>& whitelistFds));
MOCK_METHOD1(
destroy,
diff --git a/src/tests/containerizer/mesos_containerizer_tests.cpp
b/src/tests/containerizer/mesos_containerizer_tests.cpp
index 1fc56c4..59637c9 100644
--- a/src/tests/containerizer/mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/mesos_containerizer_tests.cpp
@@ -1284,13 +1284,12 @@ TEST_F(MesosLauncherStatusTest, ExecutorPIDTest)
containerId,
path::join(flags.launcher_dir, MESOS_CONTAINERIZER),
vector<string>(),
- Subprocess::FD(STDIN_FILENO),
- Subprocess::FD(STDOUT_FILENO),
- Subprocess::FD(STDERR_FILENO),
+ mesos::slave::ContainerIO(),
nullptr,
None(),
None(),
- None());
+ None(),
+ vector<int_fd>());
ASSERT_SOME(forked);
