You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2018/10/10 23:54:10 UTC
[mesos] branch 1.4.x updated (10fdcec -> 0f63358)
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a change to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 10fdcec Fixed failed test `CGROUPS_ROOT_PidNamespace*`.
new 5f2233b Whitelist inheritable file descriptors in libprocess.
new 053adba Whitelist inheritable file descriptors in the containerizer.
new a64cf87 Fixed break of `port_mapping_tests.cpp`.
new d23b711 Changed default executor tests to not use pipes for synchronization.
new 2e9293a Avoided leaking file descriptors in Mesos containerizer.
new 011be7b Updated `os::pipe()` to always return O_CLOEXEC descriptors.
new 33dbeeb Made the containerizer launch be explicit about O_CLOEXEC.
new b871d1a Fixed Windows build break.
new 1a7b092 Updated the ::pipe() system calls to pipe2 in posix subprocess.
new 0dcfcf6 Updated the ::pipe() system calls to pipe2 in lib_logrotate.
new 48f780f Fixed the lib_logrotate inappropriate UNSET_CLOEXEC via ChildHook.
new c838aa2 Made overlay backend destroy more robust.
new dadb7cd Made aufs backend destroy more robust.
new f28b099 Made bind backend destroy more robust.
new e30004c Made copy backend destroy more robust.
new 745fe90 Added MESOS-8917 to 1.4.3 CHANGELOG.
new a0ed3d6 Added MESOS-8128 to 1.4.3 CHANGELOG.
new e5fcae0 Added MESOS-9151 to 1.4.3 CHANGELOG.
new 0f63358 Added MESOS-9196 to 1.4.3 CHANGELOG.
The 19 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
3rdparty/libprocess/include/process/subprocess.hpp | 15 +-
3rdparty/libprocess/src/subprocess.cpp | 6 +-
3rdparty/libprocess/src/subprocess_posix.cpp | 22 +--
3rdparty/libprocess/src/subprocess_windows.hpp | 3 +-
3rdparty/stout/include/stout/os/posix/pipe.hpp | 52 ++++++-
CHANGELOG | 4 +
src/slave/container_loggers/lib_logrotate.cpp | 44 ++----
src/slave/containerizer/mesos/containerizer.cpp | 18 ++-
src/slave/containerizer/mesos/launch.cpp | 59 +++++++-
src/slave/containerizer/mesos/launcher.cpp | 27 ++--
src/slave/containerizer/mesos/launcher.hpp | 15 +-
src/slave/containerizer/mesos/linux_launcher.cpp | 45 +++---
src/slave/containerizer/mesos/linux_launcher.hpp | 7 +-
.../mesos/provisioner/backends/aufs.cpp | 20 ++-
.../mesos/provisioner/backends/bind.cpp | 7 +-
.../mesos/provisioner/backends/copy.cpp | 12 +-
.../mesos/provisioner/backends/overlay.cpp | 20 ++-
src/tests/check_tests.cpp | 41 ++----
src/tests/containerizer/launcher.cpp | 6 +-
src/tests/containerizer/launcher.hpp | 9 +-
.../containerizer/mesos_containerizer_tests.cpp | 7 +-
.../nested_mesos_containerizer_tests.cpp | 156 ++++++---------------
src/tests/containerizer/port_mapping_tests.cpp | 10 +-
23 files changed, 335 insertions(+), 270 deletions(-)
[mesos] 16/19: Added MESOS-8917 to 1.4.3 CHANGELOG.
Posted by gi...@apache.org.
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 745fe90b505eb8d717a99fc70e8a69b7389858bf
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Tue Sep 18 14:48:37 2018 -0700
Added MESOS-8917 to 1.4.3 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index 7e32ac2..f2fcecf 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,7 @@ Release Notes - Mesos - Version 1.4.3 (WIP)
** Bug
* [MESOS-8568] - Command checks should always call `WAIT_NESTED_CONTAINER` before `REMOVE_NESTED_CONTAINER`
* [MESOS-8620] - Containers stuck in FETCHING possibly due to unresponsive server.
+ * [MESOS-8917] - Agent leaking file descriptors into forked processes.
* [MESOS-8921] - Autotools don't work with newer OpenJDK versions
* [MESOS-9144] - Master authentication handling leads to request amplification.
* [MESOS-9145] - Master has a fragile burned-in 5s authentication timeout.
[mesos] 14/19: Made bind backend destroy more robust.
Posted by gi...@apache.org.
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 f28b0994b96bc7cc15ac8ede35229443ff6fb5b8
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Fri Aug 31 22:29:45 2018 -0700
Made bind backend destroy more robust.
Use MNT_DETACH for `unmount` so that if there are still processes
holding files or directories in the rootfs (e.g., regular filesystem
indexing), the unmount will still be successful. The kernel will cleanup
the mount when the number of references reach zero. See more details in
MESOS-9196.
Review: https://reviews.apache.org/r/68596/
(cherry picked from commit 576a455149cfe5721a3447a100548dae69b6d49a)
---
src/slave/containerizer/mesos/provisioner/backends/bind.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/slave/containerizer/mesos/provisioner/backends/bind.cpp b/src/slave/containerizer/mesos/provisioner/backends/bind.cpp
index d6cdc93..9c312c1 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/bind.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/bind.cpp
@@ -195,8 +195,11 @@ Future<bool> BindBackendProcess::destroy(const string& rootfs)
// to check `strings::startsWith(entry.target, rootfs)` here to
// unmount all nested mounts.
if (entry.target == rootfs) {
- // NOTE: This would fail if the rootfs is still in use.
- Try<Nothing> unmount = fs::unmount(entry.target);
+ // NOTE: Use MNT_DETACH here so that if there are still
+ // processes holding files or directories in the rootfs, the
+ // unmount will still be successful. The kernel will cleanup the
+ // mount when the number of references reach zero.
+ Try<Nothing> unmount = fs::unmount(entry.target, MNT_DETACH);
if (unmount.isError()) {
return Failure(
"Failed to destroy bind-mounted rootfs '" + rootfs + "': " +
[mesos] 08/19: Fixed Windows build break.
Posted by gi...@apache.org.
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 b871d1a74eef827de6d4920d581e72b6b7bba6bf
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
AuthorDate: Fri Aug 17 17:10:44 2018 -0700
Fixed Windows build break.
The `SETSID()` and `UNSET_CLOEXEC()` child hooks do not exist on
Windows (in fact, no child hooks exist on Windows).
(cherry picked from commit 0ac5af21d7d2408b91e877b163c8c70b6ef81438)
---
src/slave/containerizer/mesos/launcher.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/slave/containerizer/mesos/launcher.cpp b/src/slave/containerizer/mesos/launcher.cpp
index 1cdd043..aa594ec 100644
--- a/src/slave/containerizer/mesos/launcher.cpp
+++ b/src/slave/containerizer/mesos/launcher.cpp
@@ -122,12 +122,14 @@ Try<pid_t> SubprocessLauncher::fork(
vector<Subprocess::ChildHook> childHooks;
+#ifndef __WINDOWS__
childHooks.push_back(Subprocess::ChildHook::SETSID());
// TODO(jpeach) libprocess should take care of this, see MESOS-9164.
foreach (int_fd fd, whitelistFds) {
childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
}
+#endif // __WINDOWS__
Try<Subprocess> child = subprocess(
path,
[mesos] 13/19: Made aufs backend destroy more robust.
Posted by gi...@apache.org.
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 dadb7cd9a1af9e0b80cdbef8ee6dbcef9c7fec9a
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Fri Aug 31 22:29:41 2018 -0700
Made aufs backend destroy more robust.
Use MNT_DETACH for `unmount` so that if there are still processes
holding files or directories in the rootfs (e.g., regular filesystem
indexing), the unmount will still be successful. The kernel will cleanup
the mount when the number of references reach zero. Also, do not return
hard failure if `rmdir` failed. It's also possible that `rmdir` returns
EBUSY. See more details in MESOS-9196.
Review: https://reviews.apache.org/r/68595/
(cherry picked from commit 8715f135376719613276130f3e3af3b244ff06ee)
---
.../mesos/provisioner/backends/aufs.cpp | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp b/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp
index 53ccbd1..2f082ac 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/aufs.cpp
@@ -251,8 +251,11 @@ Future<bool> AufsBackendProcess::destroy(
foreach (const fs::MountInfoTable::Entry& entry, mountTable->entries) {
if (entry.target == rootfs) {
- // NOTE: This would fail if the rootfs is still in use.
- Try<Nothing> unmount = fs::unmount(entry.target);
+ // NOTE: Use MNT_DETACH here so that if there are still
+ // processes holding files or directories in the rootfs, the
+ // unmount will still be successful. The kernel will cleanup the
+ // mount when the number of references reach zero.
+ Try<Nothing> unmount = fs::unmount(entry.target, MNT_DETACH);
if (unmount.isError()) {
return Failure(
"Failed to destroy aufs-mounted rootfs '" + rootfs + "': " +
@@ -261,9 +264,16 @@ Future<bool> AufsBackendProcess::destroy(
Try<Nothing> rmdir = os::rmdir(rootfs);
if (rmdir.isError()) {
- return Failure(
- "Failed to remove rootfs mount point '" + rootfs + "': " +
- rmdir.error());
+ // NOTE: Due to the use of MNT_DETACH above, it's possible
+ // that `rmdir` will fail with EBUSY if some other mounts in
+ // other mount namespaces are still on this mount point on
+ // some old kernel (https://lwn.net/Articles/570338/). No need
+ // to return a hard failure here because the directory will be
+ // removed later and re-attempted on agent recovery.
+ //
+ // TODO(jieyu): Consider only ignore EBUSY error.
+ LOG(ERROR) << "Failed to remove rootfs mount point "
+ << "'" << rootfs << "': " << rmdir.error();
}
// Clean up tempDir used for image layer links.
[mesos] 17/19: Added MESOS-8128 to 1.4.3 CHANGELOG.
Posted by gi...@apache.org.
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 a0ed3d601357742c9df58b9895e13861b37359ce
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Tue Sep 18 14:50:23 2018 -0700
Added MESOS-8128 to 1.4.3 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index f2fcecf..552f169 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,7 @@ Release Notes - Mesos - Version 1.4.3 (WIP)
* This is a bug fix release.
** Bug
+ * [MESOS-8128] - Make os::pipe file descriptors O_CLOEXEC.
* [MESOS-8568] - Command checks should always call `WAIT_NESTED_CONTAINER` before `REMOVE_NESTED_CONTAINER`
* [MESOS-8620] - Containers stuck in FETCHING possibly due to unresponsive server.
* [MESOS-8917] - Agent leaking file descriptors into forked processes.
[mesos] 05/19: Avoided leaking file descriptors in Mesos
containerizer.
Posted by gi...@apache.org.
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 2e9293a727e78bcff7a0e58fd297aa5f329e8e00
Author: Benjamin Bannier <be...@mesosphere.io>
AuthorDate: Mon Jun 25 20:08:48 2018 +0200
Avoided leaking file descriptors in Mesos containerizer.
This patch explicitly closes not required file descriptors when
forking a Mesos containerizer instance. We currently only pass on
stdin, stout, and sterr.
While it would in principle be possible to open all file descriptors
with `O_CLOEXEC`, this is not practical as
* `O_CLOEXEC` is not supported on BSDs (not Windows either, but file
descriptor leaks are prevented there in a different way), and
* we might call thirdparty code outside of our control which does not
use e.g., `O_CLOEXEC` either (the case e.g., for leveldb).
Closing all file descriptors after the fork avoids leaks even in above
scenarios.
Review: https://reviews.apache.org/r/67137/
(cherry picked from commit 3f60c9857f8f68c3d2330ff3d03c8aa84b630db7)
---
src/slave/containerizer/mesos/launch.cpp | 59 +++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 0affcf5..2749b5d 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -29,8 +29,10 @@
#include <stout/foreach.hpp>
#include <stout/os.hpp>
-#include <stout/protobuf.hpp>
#include <stout/path.hpp>
+#include <stout/protobuf.hpp>
+#include <stout/stringify.hpp>
+#include <stout/try.hpp>
#include <stout/unreachable.hpp>
#include <mesos/mesos.hpp>
@@ -57,6 +59,7 @@
using std::cerr;
using std::cout;
using std::endl;
+using std::list;
using std::set;
using std::string;
using std::vector;
@@ -230,6 +233,41 @@ static void exitWithStatus(int status)
}
+// On Windows all new processes create by Mesos go through the
+// `create_process` wrapper which with the completion of MESOS-8926
+// will prevent inadvertent leaks making this code unnecessary there.
+//
+// TODO(bbannier): Consider moving this to stout as e.g., `os::lsof`.
+#ifndef __WINDOWS__
+static Try<hashset<int_fd>> getOpenFileDescriptors()
+{
+ Try<list<string>> fds =
+#if defined(__linux__)
+ os::ls("/proc/self/fd");
+#elif defined(__APPLE__)
+ os::ls("/dev/fd");
+#endif
+
+ if (fds.isError()) {
+ return Error(fds.error());
+ }
+
+ hashset<int_fd> result;
+ foreach (const string& fd, fds.get()) {
+ Try<int_fd> fd_ = numify<int_fd>(fd);
+
+ if (fd_.isError()) {
+ return Error("Could not interpret file descriptor: " + fd_.error());
+ }
+
+ result.insert(fd_.get());
+ }
+
+ return result;
+}
+#endif // __WINDOWS__
+
+
int MesosContainerizerLaunch::execute()
{
if (flags.help) {
@@ -746,6 +784,18 @@ int MesosContainerizerLaunch::execute()
}
#ifndef __WINDOWS__
+ // Construct a set of file descriptors to close before `exec`'ing.
+ Try<hashset<int_fd>> fds = getOpenFileDescriptors();
+ CHECK_SOME(fds);
+
+ std::set<int_fd> whitelistedFds = {
+ STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO};
+
+ // Exclude required file descriptors from closing.
+ foreach (int_fd fd, whitelistedFds) {
+ fds->erase(fd);
+ }
+
// If we have `containerStatusFd` set, then we need to fork-exec the
// command we are launching and checkpoint its status on exit. We
// use fork-exec directly (as opposed to `process::subprocess()`) to
@@ -816,6 +866,13 @@ int MesosContainerizerLaunch::execute()
os::close(containerStatusFd.get());
::_exit(EXIT_SUCCESS);
}
+
+ // Avoid leaking not required file descriptors into the forked process.
+ foreach (int_fd fd, fds.get()) {
+ // We use the unwrapped `::close` as opposed to `os::close`
+ // since the former is guaranteed to be async signal safe.
+ ::close(fd);
+ }
}
#endif // __WINDOWS__
[mesos] 02/19: Whitelist inheritable file descriptors in the
containerizer.
Posted by gi...@apache.org.
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 <ra...@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);
[mesos] 01/19: Whitelist inheritable file descriptors in libprocess.
Posted by gi...@apache.org.
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 5f2233bb70e464491741162c7ff1893abe3d6a69
Author: Radhika Jandhyala <ra...@microsoft.com>
AuthorDate: Wed Jun 13 15:56:20 2018 -0700
Whitelist inheritable file descriptors in libprocess.
This commit plumbs the list of whitelisted file descriptors through
the libprocess APIs.
Review: https://reviews.apache.org/r/67287/
(cherry picked from commit 281cf5dd7239dce5103ee34b64df2b785672271f)
---
3rdparty/libprocess/include/process/subprocess.hpp | 15 ++++++++++-----
3rdparty/libprocess/src/subprocess.cpp | 6 ++++--
3rdparty/libprocess/src/subprocess_windows.hpp | 3 ++-
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/3rdparty/libprocess/include/process/subprocess.hpp b/3rdparty/libprocess/include/process/subprocess.hpp
index 6a12623..135bf24 100644
--- a/3rdparty/libprocess/include/process/subprocess.hpp
+++ b/3rdparty/libprocess/include/process/subprocess.hpp
@@ -125,7 +125,8 @@ public:
const Option<lambda::function<
pid_t(const lambda::function<int()>&)>>& clone,
const std::vector<Subprocess::ParentHook>& parent_hooks,
- const std::vector<Subprocess::ChildHook>& child_hooks);
+ const std::vector<Subprocess::ChildHook>& child_hooks,
+ const std::vector<int_fd>& whitelist_fds);
IO(const lambda::function<Try<InputFileDescriptors>()>& _input,
const lambda::function<Try<OutputFileDescriptors>()>& _output)
@@ -305,7 +306,8 @@ private:
const Option<lambda::function<
pid_t(const lambda::function<int()>&)>>& clone,
const std::vector<Subprocess::ParentHook>& parent_hooks,
- const std::vector<Subprocess::ChildHook>& child_hooks);
+ const std::vector<Subprocess::ChildHook>& child_hooks,
+ const std::vector<int_fd>& whitelist_fds);
struct Data
{
@@ -377,7 +379,8 @@ Try<Subprocess> subprocess(
const Option<lambda::function<
pid_t(const lambda::function<int()>&)>>& clone = None(),
const std::vector<Subprocess::ParentHook>& parent_hooks = {},
- const std::vector<Subprocess::ChildHook>& child_hooks = {});
+ const std::vector<Subprocess::ChildHook>& child_hooks = {},
+ const std::vector<int_fd>& whitelist_fds = {});
/**
@@ -413,7 +416,8 @@ inline Try<Subprocess> subprocess(
const Option<lambda::function<
pid_t(const lambda::function<int()>&)>>& clone = None(),
const std::vector<Subprocess::ParentHook>& parent_hooks = {},
- const std::vector<Subprocess::ChildHook>& child_hooks = {})
+ const std::vector<Subprocess::ChildHook>& child_hooks = {},
+ const std::vector<int_fd>& whitelist_fds = {})
{
std::vector<std::string> argv = {os::Shell::arg0, os::Shell::arg1, command};
@@ -427,7 +431,8 @@ inline Try<Subprocess> subprocess(
environment,
clone,
parent_hooks,
- child_hooks);
+ child_hooks,
+ whitelist_fds);
}
} // namespace process {
diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp
index 785e2e1..fffb640 100644
--- a/3rdparty/libprocess/src/subprocess.cpp
+++ b/3rdparty/libprocess/src/subprocess.cpp
@@ -324,7 +324,8 @@ Try<Subprocess> subprocess(
const Option<lambda::function<
pid_t(const lambda::function<int()>&)>>& _clone,
const vector<Subprocess::ParentHook>& parent_hooks,
- const vector<Subprocess::ChildHook>& child_hooks)
+ const vector<Subprocess::ChildHook>& child_hooks,
+ const vector<int_fd>& whitelist_fds)
{
// TODO(hausdorff): We should error out on Windows here if we are passing
// parameters that aren't used.
@@ -423,7 +424,8 @@ Try<Subprocess> subprocess(
parent_hooks,
stdinfds,
stdoutfds,
- stderrfds);
+ stderrfds,
+ whitelist_fds);
if (process_data.isError()) {
process::internal::close(stdinfds, stdoutfds, stderrfds);
diff --git a/3rdparty/libprocess/src/subprocess_windows.hpp b/3rdparty/libprocess/src/subprocess_windows.hpp
index 0183bb4..fab5733 100644
--- a/3rdparty/libprocess/src/subprocess_windows.hpp
+++ b/3rdparty/libprocess/src/subprocess_windows.hpp
@@ -51,7 +51,8 @@ inline Try<::internal::windows::ProcessData> createChildProcess(
const std::vector<Subprocess::ParentHook>& parent_hooks,
const InputFileDescriptors stdinfds,
const OutputFileDescriptors stdoutfds,
- const OutputFileDescriptors stderrfds)
+ const OutputFileDescriptors stderrfds,
+ const std::vector<int_fd>& whitelist_fds = {})
{
Try<::internal::windows::ProcessData> process_data =
::internal::windows::create_process(
[mesos] 12/19: Made overlay backend destroy more robust.
Posted by gi...@apache.org.
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 c838aa2a411aa98279181f7229e38516b83d6ada
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Fri Aug 31 22:29:36 2018 -0700
Made overlay backend destroy more robust.
Use MNT_DETACH for `unmount` so that if there are still processes
holding files or directories in the rootfs (e.g., regular filesystem
indexing), the unmount will still be successful. The kernel will cleanup
the mount when the number of references reach zero. Also, do not return
hard failure if `rmdir` failed. It's also possible that `rmdir` returns
EBUSY. See more details in MESOS-9196.
Review: https://reviews.apache.org/r/68594/
(cherry picked from commit 33710c0af5662c53ee8b309b4474cfa005eac2c6)
---
.../mesos/provisioner/backends/overlay.cpp | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp b/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
index 3149220..754ef1b 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/overlay.cpp
@@ -250,8 +250,11 @@ Future<bool> OverlayBackendProcess::destroy(
foreach (const fs::MountInfoTable::Entry& entry, mountTable->entries) {
if (entry.target == rootfs) {
- // NOTE: This would fail if the rootfs is still in use.
- Try<Nothing> unmount = fs::unmount(entry.target);
+ // NOTE: Use MNT_DETACH here so that if there are still
+ // processes holding files or directories in the rootfs, the
+ // unmount will still be successful. The kernel will cleanup the
+ // mount when the number of references reach zero.
+ Try<Nothing> unmount = fs::unmount(entry.target, MNT_DETACH);
if (unmount.isError()) {
return Failure(
"Failed to destroy overlay-mounted rootfs '" + rootfs + "': " +
@@ -260,9 +263,16 @@ Future<bool> OverlayBackendProcess::destroy(
Try<Nothing> rmdir = os::rmdir(rootfs);
if (rmdir.isError()) {
- return Failure(
- "Failed to remove rootfs mount point '" + rootfs + "': " +
- rmdir.error());
+ // NOTE: Due to the use of MNT_DETACH above, it's possible
+ // that `rmdir` will fail with EBUSY if some other mounts in
+ // other mount namespaces are still on this mount point on
+ // some old kernel (https://lwn.net/Articles/570338/). No need
+ // to return a hard failure here because the directory will be
+ // removed later and re-attempted on agent recovery.
+ //
+ // TODO(jieyu): Consider only ignore EBUSY error.
+ LOG(ERROR) << "Failed to remove rootfs mount point "
+ << "'" << rootfs << "': " << rmdir.error();
}
// Clean up tempDir used for image layer links.
[mesos] 11/19: Fixed the lib_logrotate inappropriate UNSET_CLOEXEC
via ChildHook.
Posted by gi...@apache.org.
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 48f780f94a0e2d2ff963faac130f4ea608f14a53
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Tue Aug 21 14:35:01 2018 -0700
Fixed the lib_logrotate inappropriate UNSET_CLOEXEC via ChildHook.
Previously, we call os::unsetCloexec() on write fd for both outfds and
errfds. Basically, this is not needed because all stdout and stderr
fds will be closed at childMain() method. As a result, no need to do
unsetCloexec() either from the parent process side or UNSET_CLOEXEC
via the ChildHook.
Review: https://reviews.apache.org/r/68458
(cherry picked from commit b9d19cd4e03f70014063341c64f96b5860db8b0f)
---
src/slave/container_loggers/lib_logrotate.cpp | 24 ++----------------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/src/slave/container_loggers/lib_logrotate.cpp b/src/slave/container_loggers/lib_logrotate.cpp
index 1bc8b61..8d003e2 100644
--- a/src/slave/container_loggers/lib_logrotate.cpp
+++ b/src/slave/container_loggers/lib_logrotate.cpp
@@ -162,8 +162,6 @@ public:
outfds.read = pipefd->at(0);
outfds.write = pipefd->at(1);
- const std::vector<int_fd> whitelistOutFds{pipefd->at(0), pipefd->at(1)};
-
// Spawn a process to handle stdout.
mesos::internal::logger::rotate::Flags outFlags;
outFlags.max_size = overriddenFlags.max_stdout_size;
@@ -183,12 +181,6 @@ public:
}
#endif // __linux__
- // TODO(gilbert): libprocess should take care of this, see MESOS-9164.
- std::vector<Subprocess::ChildHook> childHooks;
- foreach (int_fd fd, whitelistOutFds) {
- childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
- }
-
Try<Subprocess> outProcess = subprocess(
path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
{mesos::internal::logger::rotate::NAME},
@@ -198,9 +190,7 @@ public:
&outFlags,
environment,
None(),
- parentHooks,
- childHooks,
- whitelistOutFds);
+ parentHooks);
if (outProcess.isError()) {
os::close(outfds.write.get());
@@ -220,8 +210,6 @@ public:
errfds.read = pipefd->at(0);
errfds.write = pipefd->at(1);
- const std::vector<int_fd> whitelistErrFds{pipefd->at(0), pipefd->at(1)};
-
// Spawn a process to handle stderr.
mesos::internal::logger::rotate::Flags errFlags;
errFlags.max_size = overriddenFlags.max_stderr_size;
@@ -230,12 +218,6 @@ public:
errFlags.logrotate_path = flags.logrotate_path;
errFlags.user = user;
- // TODO(gilbert): libprocess should take care of this, see MESOS-9164.
- childHooks.clear();
- foreach (int_fd fd, whitelistErrFds) {
- childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
- }
-
Try<Subprocess> errProcess = subprocess(
path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
{mesos::internal::logger::rotate::NAME},
@@ -245,9 +227,7 @@ public:
&errFlags,
environment,
None(),
- parentHooks,
- childHooks,
- whitelistErrFds);
+ parentHooks);
if (errProcess.isError()) {
os::close(outfds.write.get());
[mesos] 04/19: Changed default executor tests to not use pipes for
synchronization.
Posted by gi...@apache.org.
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 d23b711f7511b58010d2b0f3a43e67e46e054737
Author: Benjamin Bannier <be...@mesosphere.io>
AuthorDate: Mon Jun 25 20:08:43 2018 +0200
Changed default executor tests to not use pipes for synchronization.
Some tests of nested container functionality used pipes passed to
launched tasks to detect whether a task has actually started executing
the workload (`TASK_RUNNING` updates might be sent before the task
workload is actually started).
Once we avoid leaking unspecified file descriptors into forked
processes, this test setup will be broken. In this patch we replace
the use of pipes for synchronization with HTTP requests to an actor
running in the tests, or wait on other observable side effects.
Review: https://reviews.apache.org/r/67398/
(cherry picked from commit a666047c9324a0b24b26fa8d89b3fdb73537f44f)
---
src/tests/check_tests.cpp | 41 ++----
.../nested_mesos_containerizer_tests.cpp | 156 ++++++---------------
2 files changed, 54 insertions(+), 143 deletions(-)
diff --git a/src/tests/check_tests.cpp b/src/tests/check_tests.cpp
index 0810851..bb0b3e8 100644
--- a/src/tests/check_tests.cpp
+++ b/src/tests/check_tests.cpp
@@ -1889,47 +1889,22 @@ TEST_F_TEMP_DISABLED_ON_WINDOWS(
.WillOnce(FutureArg<1>(&updateCheckResult))
.WillRepeatedly(Return()); // Ignore subsequent updates.
- // Default executor delegates launching both the task and its check to the
- // agent. To avoid a race, we explicitly synchronize.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
-
const string filename = "nested_inherits_work_dir";
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- v1::CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "touch " + filename + ";echo running >&" +
- stringify(pipes[1]) + ";sleep 1000");
-
- v1::TaskInfo taskInfo = v1::createTask(agentId, resources, command);
+ v1::TaskInfo taskInfo = v1::createTask(
+ agentId,
+ resources,
+ v1::createCommandInfo(
+ strings::format("touch %s; sleep 1000", filename).get()));
v1::CheckInfo* checkInfo = taskInfo.mutable_check();
checkInfo->set_type(v1::CheckInfo::COMMAND);
checkInfo->set_delay_seconds(0);
checkInfo->set_interval_seconds(0);
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'read', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- v1::CommandInfo* checkCommand =
- checkInfo->mutable_command()->mutable_command();
- checkCommand->set_shell(false);
- checkCommand->set_value("/bin/bash");
- checkCommand->add_arguments("bash");
- checkCommand->add_arguments("-c");
- checkCommand->add_arguments(
- "read INPUT <&" + stringify(pipes[0]) + ";ls " + filename);
+ // Wait in a busy loop until the file has been created.
+ checkInfo->mutable_command()->mutable_command()->CopyFrom(
+ v1::createCommandInfo("while [ -f " + filename + "]; do :; done"));
v1::TaskGroupInfo taskGroup;
taskGroup.add_tasks()->CopyFrom(taskInfo);
diff --git a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
index f8b4423..c05c916 100644
--- a/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
+++ b/src/tests/containerizer/nested_mesos_containerizer_tests.cpp
@@ -14,6 +14,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include <sys/stat.h>
#include <sys/wait.h>
#include <map>
@@ -449,23 +450,11 @@ TEST_F(NestedMesosContainerizerTest,
ContainerID containerId;
containerId.set_value(UUID::random().toString());
- // Use a pipe to pass parent's MESOS_SANDBOX value to a child container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- const std::array<int_fd, 2>& pipes = pipes_.get();
-
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "echo $MESOS_SANDBOX >&" + stringify(pipes[1]) + ";" + "sleep 1000");
+ CommandInfo command =
+ createCommandInfo("echo ${MESOS_SANDBOX} > " + pipe + " ; sleep 1000");
ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
@@ -483,30 +472,16 @@ TEST_F(NestedMesosContainerizerTest,
AWAIT_ASSERT_TRUE(launch);
- // Wait for the parent container to start running its task
- // before launching a debug container inside it.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[1]);
-
- // Launch a nested debug container that compares MESOS_SANDBOX
+ // Launch a nested debug container that compares `MESOS_SANDBOX`
// it sees with the one its parent sees.
{
ContainerID nestedContainerId;
nestedContainerId.mutable_parent()->CopyFrom(containerId);
nestedContainerId.set_value(UUID::random().toString());
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'read', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo nestedCommand;
- nestedCommand.set_shell(false);
- nestedCommand.set_value("/bin/bash");
- nestedCommand.add_arguments("bash");
- nestedCommand.add_arguments("-c");
- nestedCommand.add_arguments(
- "read PARENT_SANDBOX <&" + stringify(pipes[0]) + ";"
- "[ ${PARENT_SANDBOX} == ${MESOS_SANDBOX} ] && exit 0 || exit 1;");
+ CommandInfo nestedCommand = createCommandInfo(
+ "read PARENT_SANDBOX < " + pipe + ";"
+ "[ ${PARENT_SANDBOX} = ${MESOS_SANDBOX} ] && exit 0 || exit 1;");
Future<bool> launchNested = containerizer->launch(
nestedContainerId,
@@ -526,8 +501,6 @@ TEST_F(NestedMesosContainerizerTest,
ASSERT_SOME(waitNested.get());
ASSERT_TRUE(waitNested.get()->has_status());
EXPECT_WEXITSTATUS_EQ(0, waitNested.get()->status());
-
- close(pipes[0]);
}
// Destroy the containerizer with all associated containers.
@@ -571,27 +544,15 @@ TEST_F(NestedMesosContainerizerTest,
containerId.set_value(UUID::random().toString());
// Use a pipe to synchronize with the top-level container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
const string filename = "nested_inherits_work_dir";
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "touch " + filename + ";echo running >&" +
- stringify(pipes[1]) + ";sleep 1000");
-
- ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
+ ExecutorInfo executor = createExecutorInfo(
+ "executor",
+ "touch " + filename + "; echo running > " + pipe + "; sleep 1000",
+ "cpus:1");
Try<string> directory = environment->mkdtemp();
ASSERT_SOME(directory);
@@ -614,9 +575,9 @@ TEST_F(NestedMesosContainerizerTest,
// Wait for the parent container to start running its task
// before launching a debug container inside it.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[1]);
- close(pipes[0]);
+ Result<string> read = os::read(pipe);
+ ASSERT_SOME(read);
+ ASSERT_EQ("running\n", read.get());
Future<ContainerStatus> status = containerizer->status(containerId);
AWAIT_READY(status);
@@ -1016,19 +977,17 @@ TEST_F(NestedMesosContainerizerTest,
ASSERT_EQ(1u, offers->size());
// Use a pipe to synchronize with the top-level container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- // Launch a command task within the `alpine` docker image and
- // synchronize its launch with the launch of a debug container below.
+ // Launch a command task within the `alpine` docker image.
TaskInfo task = createTask(
offers->front().slave_id(),
offers->front().resources(),
- "echo running >&" + stringify(pipes[1]) + ";" + "sleep 1000");
+ "echo running > /tmp/pipe; sleep 1000");
- task.mutable_container()->CopyFrom(createContainerInfo("alpine"));
+ task.mutable_container()->CopyFrom(createContainerInfo(
+ "alpine", {createVolumeFromHostPath("/tmp", sandbox.get(), Volume::RW)}));
Future<TaskStatus> statusRunning;
EXPECT_CALL(sched, statusUpdate(_, _))
@@ -1041,12 +1000,11 @@ TEST_F(NestedMesosContainerizerTest,
AWAIT_READY_FOR(statusRunning, Seconds(120));
ASSERT_EQ(TASK_RUNNING, statusRunning->state());
- close(pipes[1]);
-
// Wait for the parent container to start running its task
// before launching a debug container inside it.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[0]);
+ Result<string> read = os::read(pipe);
+ ASSERT_SOME(read);
+ ASSERT_EQ("running\n", read.get());
ASSERT_TRUE(statusRunning->has_slave_id());
ASSERT_TRUE(statusRunning->has_container_status());
@@ -1385,23 +1343,14 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentExit)
ContainerID containerId;
containerId.set_value(UUID::random().toString());
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- const std::array<int_fd, 2>& pipes = pipes_.get();
-
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'read', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments("read key <&" + stringify(pipes[0]));
-
- ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
+ // We launch a blocking `read` after which we return with a non-success code.
+ ExecutorInfo executor = createExecutorInfo(
+ "executor",
+ createCommandInfo("read < " + pipe + " && exit 1"),
+ "cpus:1");
Try<string> directory = environment->mkdtemp();
ASSERT_SOME(directory);
@@ -1412,8 +1361,6 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentExit)
map<string, string>(),
None());
- close(pipes[0]); // We're never going to read.
-
AWAIT_ASSERT_TRUE(launch);
// Now launch nested container.
@@ -1434,7 +1381,8 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentExit)
Future<Option<ContainerTermination>> nestedWait = containerizer->wait(
nestedContainerId);
- close(pipes[1]); // Force the 'read key' to exit!
+ // Write to the fifo to unblock the `read` in the parent container.
+ os::write(pipe, "");
AWAIT_READY(wait);
ASSERT_SOME(wait.get());
@@ -1479,25 +1427,14 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
ContainerID containerId;
containerId.set_value(UUID::random().toString());
- // Use a pipe to synchronize with the top-level container.
- Try<std::array<int_fd, 2>> pipes_ = os::pipe();
- ASSERT_SOME(pipes_);
-
- const std::array<int_fd, 2>& pipes = pipes_.get();
-
- // NOTE: We use a non-shell command here to use 'bash -c' to execute
- // the 'echo', which deals with the file descriptor, because of a bug
- // in ubuntu dash. Multi-digit file descriptor is not supported in
- // ubuntu dash, which executes the shell command.
- CommandInfo command;
- command.set_shell(false);
- command.set_value("/bin/bash");
- command.add_arguments("bash");
- command.add_arguments("-c");
- command.add_arguments(
- "echo running >&" + stringify(pipes[1]) + ";" + "sleep 1000");
+ // Use a fifo to synchronize with the top-level container.
+ string pipe = path::join(sandbox.get(), "pipe");
+ ASSERT_EQ(0, ::mkfifo(pipe.c_str(), 0700));
- ExecutorInfo executor = createExecutorInfo("executor", command, "cpus:1");
+ ExecutorInfo executor = createExecutorInfo(
+ "executor",
+ createCommandInfo("echo running > " + pipe + "; sleep 1000"),
+ "cpus:1");
Try<string> directory = environment->mkdtemp();
ASSERT_SOME(directory);
@@ -1510,8 +1447,6 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
AWAIT_ASSERT_TRUE(launch);
- close(pipes[1]);
-
// Now launch nested container.
ContainerID nestedContainerId;
nestedContainerId.mutable_parent()->CopyFrom(containerId);
@@ -1536,8 +1471,9 @@ TEST_F(NestedMesosContainerizerTest, ROOT_CGROUPS_ParentSigterm)
// Wait for the parent container to start running its executor
// process before sending it a signal.
- AWAIT_READY(process::io::poll(pipes[0], process::io::READ));
- close(pipes[0]);
+ Result<string> read = os::read(pipe);
+ ASSERT_SOME(read);
+ ASSERT_EQ("running\n", read.get());
ASSERT_EQ(0u, os::kill(status->executor_pid(), SIGTERM));
[mesos] 18/19: Added MESOS-9151 to 1.4.3 CHANGELOG.
Posted by gi...@apache.org.
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 e5fcae09d7c54f72f3ad6bdb80fde846c0a0fadf
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Tue Sep 18 14:53:31 2018 -0700
Added MESOS-9151 to 1.4.3 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index 552f169..b242043 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -12,6 +12,7 @@ Release Notes - Mesos - Version 1.4.3 (WIP)
* [MESOS-9145] - Master has a fragile burned-in 5s authentication timeout.
* [MESOS-9146] - Agent has a fragile burn-in 5s authentication timeout.
* [MESOS-9147] - Agent and scheduler driver authentication retry backoff time could overflow.
+ * [MESOS-9151] - Container stuck at ISOLATING due to FD leak.
* [MESOS-9170] - Zookeeper doesn't compile with newer gcc due to format error.
[mesos] 03/19: Fixed break of `port_mapping_tests.cpp`.
Posted by gi...@apache.org.
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 a64cf878163d9cc128b4e0b898267e07e69c720d
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
AuthorDate: Thu Jun 14 09:45:34 2018 -0700
Fixed break of `port_mapping_tests.cpp`.
The last change applied to the file descriptor whitelist chain removed
the default value from the new argument, and this function was not
updated. The other change made was that the three FD arguments were
turned into one `ContainerIO`. This was missed because this entire
isolator and its associated tests are not in the CMake build.
Review: https://reviews.apache.org/r/67600
(cherry picked from commit 85adb1e8711325acb169239a2d9160e8f18f9579)
---
src/tests/containerizer/port_mapping_tests.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp
index 84b39b1..06dcce9 100644
--- a/src/tests/containerizer/port_mapping_tests.cpp
+++ b/src/tests/containerizer/port_mapping_tests.cpp
@@ -35,8 +35,9 @@
#include <stout/stopwatch.hpp>
#include <stout/os/constants.hpp>
-#include <stout/os/stat.hpp>
#include <stout/os/exists.hpp>
+#include <stout/os/int_fd.hpp>
+#include <stout/os/stat.hpp>
#include "linux/fs.hpp"
#include "linux/ns.hpp"
@@ -346,13 +347,12 @@ protected:
containerId,
path::join(flags.launcher_dir, MESOS_CONTAINERIZER),
argv,
- Subprocess::FD(STDIN_FILENO),
- Subprocess::FD(STDOUT_FILENO),
- Subprocess::FD(STDERR_FILENO),
+ mesos::slave::ContainerIO(),
&launchFlags,
None(),
None(),
- CLONE_NEWNET | CLONE_NEWNS);
+ CLONE_NEWNET | CLONE_NEWNS,
+ vector<int_fd>());
return pid;
}
[mesos] 07/19: Made the containerizer launch be explicit about
O_CLOEXEC.
Posted by gi...@apache.org.
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 33dbeeb248c9ea6d525233db41cff6b4f98517f9
Author: James Peach <jp...@apache.org>
AuthorDate: Fri Aug 17 11:45:56 2018 -0700
Made the containerizer launch be explicit about O_CLOEXEC.
Since the containerizer launch depends on the inherited pipe to signal
the forked child, be explicit about manipulating O_CLOEXEC on the pipe
file descriptors. Make sure to close the pipe on the error paths.
Review: https://reviews.apache.org/r/63280/
(cherry picked from commit 7569ffd05903be9b5284100f67eeffb35fcc7703)
---
src/slave/containerizer/mesos/containerizer.cpp | 9 +++++++++
src/slave/containerizer/mesos/launcher.cpp | 11 ++++++++++-
src/slave/containerizer/mesos/linux_launcher.cpp | 11 ++++++++++-
3 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 985c88b..73334ff 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1763,6 +1763,9 @@ Future<bool> MesosContainerizerProcess::_launch(
whitelistFds);
if (forked.isError()) {
+ os::close(pipes[0]);
+ os::close(pipes[1]);
+
return Failure("Failed to fork: " + forked.error());
}
@@ -1781,6 +1784,9 @@ Future<bool> MesosContainerizerProcess::_launch(
LOG(ERROR) << "Failed to checkpoint container's forked pid to '"
<< pidCheckpointPath.get() << "': " << checkpointed.error();
+ os::close(pipes[0]);
+ os::close(pipes[1]);
+
return Failure("Could not checkpoint container's pid");
}
}
@@ -1804,6 +1810,9 @@ Future<bool> MesosContainerizerProcess::_launch(
checkpointed = slave::state::checkpoint(pidPath, stringify(pid));
if (checkpointed.isError()) {
+ os::close(pipes[0]);
+ os::close(pipes[1]);
+
return Failure("Failed to checkpoint the container pid to"
" '" + pidPath + "': " + checkpointed.error());
}
diff --git a/src/slave/containerizer/mesos/launcher.cpp b/src/slave/containerizer/mesos/launcher.cpp
index 5fc5c30..1cdd043 100644
--- a/src/slave/containerizer/mesos/launcher.cpp
+++ b/src/slave/containerizer/mesos/launcher.cpp
@@ -120,6 +120,15 @@ Try<pid_t> SubprocessLauncher::fork(
parentHooks.emplace_back(Subprocess::ParentHook::CREATE_JOB());
#endif // __linux__
+ vector<Subprocess::ChildHook> childHooks;
+
+ childHooks.push_back(Subprocess::ChildHook::SETSID());
+
+ // TODO(jpeach) libprocess should take care of this, see MESOS-9164.
+ foreach (int_fd fd, whitelistFds) {
+ childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+ }
+
Try<Subprocess> child = subprocess(
path,
argv,
@@ -130,7 +139,7 @@ Try<pid_t> SubprocessLauncher::fork(
environment,
None(),
parentHooks,
- {Subprocess::ChildHook::SETSID()},
+ childHooks,
whitelistFds);
if (child.isError()) {
diff --git a/src/slave/containerizer/mesos/linux_launcher.cpp b/src/slave/containerizer/mesos/linux_launcher.cpp
index 27997f7..55602f6 100644
--- a/src/slave/containerizer/mesos/linux_launcher.cpp
+++ b/src/slave/containerizer/mesos/linux_launcher.cpp
@@ -454,6 +454,15 @@ Try<pid_t> LinuxLauncherProcess::fork(
child);
}));
+ vector<Subprocess::ChildHook> childHooks;
+
+ childHooks.push_back(Subprocess::ChildHook::SETSID());
+
+ // TODO(jpeach) libprocess should take care of this, see MESOS-9164.
+ foreach (int_fd fd, whitelistFds) {
+ childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+ }
+
Try<Subprocess> child = subprocess(
path,
argv,
@@ -480,7 +489,7 @@ Try<pid_t> LinuxLauncherProcess::fork(
}
},
parentHooks,
- {Subprocess::ChildHook::SETSID()});
+ childHooks);
if (child.isError()) {
return Error("Failed to clone child process: " + child.error());
[mesos] 15/19: Made copy backend destroy more robust.
Posted by gi...@apache.org.
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 e30004c49da5c4d16ada95b550bd941b2a04ed46
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Fri Aug 31 22:29:48 2018 -0700
Made copy backend destroy more robust.
Do not return hard failure if `rm -rf` failed. It's also possible that
it fails if some other processes are accessing the files there. The
cleanup will be re-attempted during provisioner destroy and agent
recovery.
Review: https://reviews.apache.org/r/68597/
(cherry picked from commit 3e4e78009d94c15409735ecf4bc2941ec7523d6b)
---
src/slave/containerizer/mesos/provisioner/backends/copy.cpp | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
index 69faa03..1479bc9 100644
--- a/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
+++ b/src/slave/containerizer/mesos/provisioner/backends/copy.cpp
@@ -327,9 +327,15 @@ Future<bool> CopyBackendProcess::destroy(const string& rootfs)
.then([](const Option<int>& status) -> Future<bool> {
if (status.isNone()) {
return Failure("Failed to reap subprocess to destroy rootfs");
- } else if (status.get() != 0) {
- return Failure("Failed to destroy rootfs, exit status: " +
- WSTRINGIFY(status.get()));
+ }
+
+ if (status.get() != 0) {
+ // It's possible that `rm -rf` will fail if some other
+ // programs are accessing the files. No need to return a hard
+ // failure here because the directory will be removed later
+ // and re-attempted on agent recovery.
+ LOG(ERROR) << "Failed to destroy rootfs, exit status: "
+ << WSTRINGIFY(status.get());
}
return true;
[mesos] 10/19: Updated the ::pipe() system calls to pipe2 in
lib_logrotate.
Posted by gi...@apache.org.
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 0dcfcf66a8b439231c41f2ebced17459725e78ec
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Thu Aug 16 11:05:41 2018 -0700
Updated the ::pipe() system calls to pipe2 in lib_logrotate.
Review: https://reviews.apache.org/r/68397
(cherry picked from commit 3f49cf4351d200e4ba0ac7aa2cb69791b2786a23)
---
src/slave/container_loggers/lib_logrotate.cpp | 66 ++++++++++++++-------------
1 file changed, 34 insertions(+), 32 deletions(-)
diff --git a/src/slave/container_loggers/lib_logrotate.cpp b/src/slave/container_loggers/lib_logrotate.cpp
index bc13e6a..1bc8b61 100644
--- a/src/slave/container_loggers/lib_logrotate.cpp
+++ b/src/slave/container_loggers/lib_logrotate.cpp
@@ -43,6 +43,7 @@
#include <stout/os/environment.hpp>
#include <stout/os/fcntl.hpp>
#include <stout/os/killtree.hpp>
+#include <stout/os/pipe.hpp>
#ifdef __linux__
#include "linux/systemd.hpp"
@@ -55,6 +56,7 @@
using namespace mesos;
using namespace process;
+using std::array;
using std::map;
using std::string;
@@ -151,24 +153,16 @@ public:
// of the pipe and will be solely responsible for closing that end.
// The ownership of the write-end will be passed to the caller
// of this function.
- int pipefd[2];
- if (::pipe(pipefd) == -1) {
- return Failure(ErrnoError("Failed to create pipe").message);
+ Try<array<int, 2>> pipefd = os::pipe();
+ if (pipefd.isError()) {
+ return Failure("Failed to create pipe: " + pipefd.error());
}
Subprocess::IO::InputFileDescriptors outfds;
- outfds.read = pipefd[0];
- outfds.write = pipefd[1];
-
- // NOTE: We need to `cloexec` this FD so that it will be closed when
- // the child subprocess is spawned and so that the FD will not be
- // inherited by the second child for stderr.
- Try<Nothing> cloexec = os::cloexec(outfds.write.get());
- if (cloexec.isError()) {
- os::close(outfds.read);
- os::close(outfds.write.get());
- return Failure("Failed to cloexec: " + cloexec.error());
- }
+ outfds.read = pipefd->at(0);
+ outfds.write = pipefd->at(1);
+
+ const std::vector<int_fd> whitelistOutFds{pipefd->at(0), pipefd->at(1)};
// Spawn a process to handle stdout.
mesos::internal::logger::rotate::Flags outFlags;
@@ -189,6 +183,12 @@ public:
}
#endif // __linux__
+ // TODO(gilbert): libprocess should take care of this, see MESOS-9164.
+ std::vector<Subprocess::ChildHook> childHooks;
+ foreach (int_fd fd, whitelistOutFds) {
+ childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+ }
+
Try<Subprocess> outProcess = subprocess(
path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
{mesos::internal::logger::rotate::NAME},
@@ -198,7 +198,9 @@ public:
&outFlags,
environment,
None(),
- parentHooks);
+ parentHooks,
+ childHooks,
+ whitelistOutFds);
if (outProcess.isError()) {
os::close(outfds.write.get());
@@ -207,26 +209,18 @@ public:
// NOTE: We manually construct a pipe here to properly express
// ownership of the FDs. See the NOTE above.
- if (::pipe(pipefd) == -1) {
+ pipefd = os::pipe();
+ if (pipefd.isError()) {
os::close(outfds.write.get());
- os::killtree(outProcess.get().pid(), SIGKILL);
- return Failure(ErrnoError("Failed to create pipe").message);
+ os::killtree(outProcess->pid(), SIGKILL);
+ return Failure("Failed to create pipe: " + pipefd.error());
}
Subprocess::IO::InputFileDescriptors errfds;
- errfds.read = pipefd[0];
- errfds.write = pipefd[1];
+ errfds.read = pipefd->at(0);
+ errfds.write = pipefd->at(1);
- // NOTE: We need to `cloexec` this FD so that it will be closed when
- // the child subprocess is spawned.
- cloexec = os::cloexec(errfds.write.get());
- if (cloexec.isError()) {
- os::close(outfds.write.get());
- os::close(errfds.read);
- os::close(errfds.write.get());
- os::killtree(outProcess.get().pid(), SIGKILL);
- return Failure("Failed to cloexec: " + cloexec.error());
- }
+ const std::vector<int_fd> whitelistErrFds{pipefd->at(0), pipefd->at(1)};
// Spawn a process to handle stderr.
mesos::internal::logger::rotate::Flags errFlags;
@@ -236,6 +230,12 @@ public:
errFlags.logrotate_path = flags.logrotate_path;
errFlags.user = user;
+ // TODO(gilbert): libprocess should take care of this, see MESOS-9164.
+ childHooks.clear();
+ foreach (int_fd fd, whitelistErrFds) {
+ childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+ }
+
Try<Subprocess> errProcess = subprocess(
path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
{mesos::internal::logger::rotate::NAME},
@@ -245,7 +245,9 @@ public:
&errFlags,
environment,
None(),
- parentHooks);
+ parentHooks,
+ childHooks,
+ whitelistErrFds);
if (errProcess.isError()) {
os::close(outfds.write.get());
[mesos] 09/19: Updated the ::pipe() system calls to pipe2 in posix
subprocess.
Posted by gi...@apache.org.
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 1a7b092989c9b7abee957b377e2583a4f74df39f
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Thu Aug 16 11:03:52 2018 -0700
Updated the ::pipe() system calls to pipe2 in posix subprocess.
Review: https://reviews.apache.org/r/68396
(cherry picked from commit 7a7a879310a5c01b69c8a56b3c1d85555888ec3e)
---
3rdparty/libprocess/src/subprocess_posix.cpp | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/3rdparty/libprocess/src/subprocess_posix.cpp b/3rdparty/libprocess/src/subprocess_posix.cpp
index 01e3272..caba704 100644
--- a/3rdparty/libprocess/src/subprocess_posix.cpp
+++ b/3rdparty/libprocess/src/subprocess_posix.cpp
@@ -28,10 +28,12 @@
#include <stout/foreach.hpp>
#include <stout/option.hpp>
#include <stout/os.hpp>
+#include <stout/os/pipe.hpp>
#include <stout/os/strerror.hpp>
#include <stout/strings.hpp>
#include <stout/try.hpp>
+using std::array;
using std::map;
using std::string;
using std::vector;
@@ -46,25 +48,25 @@ Subprocess::IO Subprocess::PIPE()
{
return Subprocess::IO(
[]() -> Try<InputFileDescriptors> {
- int pipefd[2];
- if (::pipe(pipefd) == -1) {
- return ErrnoError("Failed to create pipe");
+ Try<array<int, 2>> pipefd = os::pipe();
+ if (pipefd.isError()) {
+ return Error(pipefd.error());
}
InputFileDescriptors fds;
- fds.read = pipefd[0];
- fds.write = pipefd[1];
+ fds.read = pipefd->at(0);
+ fds.write = pipefd->at(1);
return fds;
},
[]() -> Try<OutputFileDescriptors> {
- int pipefd[2];
- if (::pipe(pipefd) == -1) {
- return ErrnoError("Failed to create pipe");
+ Try<array<int, 2>> pipefd = os::pipe();
+ if (pipefd.isError()) {
+ return Error(pipefd.error());
}
OutputFileDescriptors fds;
- fds.read = pipefd[0];
- fds.write = pipefd[1];
+ fds.read = pipefd->at(0);
+ fds.write = pipefd->at(1);
return fds;
});
}
[mesos] 19/19: Added MESOS-9196 to 1.4.3 CHANGELOG.
Posted by gi...@apache.org.
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 0f633584df9b85bdce4a670787b9fc2a59e99664
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Tue Sep 18 14:54:38 2018 -0700
Added MESOS-9196 to 1.4.3 CHANGELOG.
---
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG b/CHANGELOG
index b242043..f87df67 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@ Release Notes - Mesos - Version 1.4.3 (WIP)
* [MESOS-9147] - Agent and scheduler driver authentication retry backoff time could overflow.
* [MESOS-9151] - Container stuck at ISOLATING due to FD leak.
* [MESOS-9170] - Zookeeper doesn't compile with newer gcc due to format error.
+ * [MESOS-9196] - Removing rootfs mounts may fail with EBUSY.
Release Notes - Mesos - Version 1.4.2
[mesos] 06/19: Updated `os::pipe()` to always return O_CLOEXEC
descriptors.
Posted by gi...@apache.org.
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 011be7bfb78cf88902c74c54802d101d7958b800
Author: James Peach <jp...@apache.org>
AuthorDate: Fri Aug 17 11:45:52 2018 -0700
Updated `os::pipe()` to always return O_CLOEXEC descriptors.
Updated `os::pipe()` to always return O_CLOEXEC descriptors,
atomically if we are on Linux or FreeBSD and the `pipe2(2)`
system call is available.
Review: https://reviews.apache.org/r/63270/
(cherry picked from commit 2388ca4bd3be3ed5da266e74b518dd284de1be94)
---
3rdparty/stout/include/stout/os/posix/pipe.hpp | 52 +++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/3rdparty/stout/include/stout/os/posix/pipe.hpp b/3rdparty/stout/include/stout/os/posix/pipe.hpp
index ac76224..9838d7b 100644
--- a/3rdparty/stout/include/stout/os/posix/pipe.hpp
+++ b/3rdparty/stout/include/stout/os/posix/pipe.hpp
@@ -15,20 +15,70 @@
#include <unistd.h>
+#include <sys/syscall.h>
+
#include <array>
#include <stout/error.hpp>
#include <stout/try.hpp>
+#include <stout/os/posix/fcntl.hpp>
+
namespace os {
-// Create pipes for interprocess communication.
+// Create pipes for interprocess communication. The pipe file descriptors
+// will be marked O_CLOEXEC (atomically if the platform supports it). To
+// pass the pipe to a child process, the caller should clear the CLOEXEC
+// flag after fork(2) but before exec(2).
inline Try<std::array<int, 2>> pipe()
{
std::array<int, 2> result;
+
+ // The pipe2() function appeared in FreeBSD 10.0.
+#if defined(_FreeBSD__) && __FreeBSD_version >= 1000000
+
+ if (::pipe2(result.data(), O_CLOEXEC) < 0) {
+ return ErrnoError();
+ }
+
+#else
+
+ // pipe2() appeared in Linux 2.6.27 and glibc 2.9.
+#if defined(__linux__) && defined(SYS_pipe2)
+ if (::syscall(SYS_pipe2, result.data(), O_CLOEXEC) == 0) {
+ return result;
+ }
+
+ // Fall back if the kernel doesn't support pipe2().
+ if (errno != ENOSYS) {
+ return ErrnoError();
+ }
+#endif
+
if (::pipe(result.data()) < 0) {
return ErrnoError();
}
+
+ Try<Nothing> cloexec = Nothing();
+
+ cloexec = os::cloexec(result[0]);
+ if (cloexec.isError()) {
+ Error error = Error("Failed to cloexec pipe: " + cloexec.error());
+ ::close(result[0]);
+ ::close(result[1]);
+ return error;
+ }
+
+ cloexec = os::cloexec(result[1]);
+ if (cloexec.isError()) {
+ Error error = Error("Failed to cloexec pipe: " + cloexec.error());
+ ::close(result[0]);
+ ::close(result[1]);
+ return error;
+ }
+
+#endif
+
return result;
}