You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/10/02 20:23:38 UTC
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/
-----------------------------------------------------------
(Updated Oct. 2, 2014, 11:23 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Addressed comments.
Repository: mesos-git
Description
-------
Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
Diffs (updated)
-----
src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b
src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
Diff: https://reviews.apache.org/r/25865/diff/
Testing
-------
Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
Thanks,
Ian Downes
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote:
> > Vinod and Jie asked me to take a look at this review.
> >
> > It looks like there are dependent changes that are not linked in? (filesystem islotor, ContainerInfo::MESOS, os::namespaces os::getns).
> >
> > My main comment is that it seems really unfortuante that the isolator boundary is broken here between the filesystem isolator and the pid namespace isolator. I'm sure there will be folks out there that want pid namespace isolation without having to use the shared filesystem isolator.
This has been cleaned up some by removing the /sys mount (not needed by the port_mapping isolator) and moving the /proc mount to the pid isolator.
> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote:
> > src/slave/containerizer/isolators/filesystem/shared.cpp, lines 85-89
> > <https://reviews.apache.org/r/25865/diff/3/?file=711226#file711226line85>
> >
> > A few questions:
> >
> > (1) Why are you saying lazily here? What aspect of this is done lazily?
> >
> > (2) So.. with pid namespaces you have to manually remount /proc to hide the changes? Can you make the comment more explicit about how important it is to do this?
> >
> > (3) Why is /sys remounted? An explanation in the comment would be great for when us dummies are reading this code later and scratching our heads. :)
> >
> > (4) Curious, is the remounting only specific to pid namespacing, as opposed to other namespaces?
(1) commented
(2) Yes. Commented.
(3) /sys mount removed.
(4) Mostly yes. It turns out the remounting /sys is not required for a network namespace but it definitely is required for a pid namespace so that /proc lists the pids of the namespace, not the parent namespace.
> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote:
> > src/slave/containerizer/isolators/namespaces/pid.cpp, line 50
> > <https://reviews.apache.org/r/25865/diff/3/?file=711228#file711228line50>
> >
> > If it's a "pid namespace isolator", why not call this class 'PidNamespaceIsolator'?
See Vinod's comment. I agree it reads better but this is inline with existing Cgroups{Cpu,Mem}Isolator. Happy to change though...
> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote:
> > src/slave/containerizer/isolators/namespaces/pid.cpp, lines 112-119
> > <https://reviews.apache.org/r/25865/diff/3/?file=711228#file711228line112>
> >
> > Why is state.id an option in the first place?
Historical reasons? It was kept as an Option when we moved from UUID to ContainerID.
> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 362
> > <https://reviews.apache.org/r/25865/diff/3/?file=711229#file711229line362>
> >
> > This line is unused...?
It's used in the os::getns call below. I capture it so I can erase from the map in one place before any return code paths.
> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote:
> > src/tests/isolator_tests.cpp, lines 976-978
> > <https://reviews.apache.org/r/25865/diff/3/?file=711231#file711231line976>
> >
> > What is the difference between os::getns() here and NamespacesPidIsolatorProcess::getNamespace?
> >
> > Where is the review that created os::getns, could we use a better name for that method?
> >
> > Why isn't is linux::namespaces and linux::getns, are namespaces applicable to other systems?
Comment on diffence added to pid.hpp. os::getns() returns any namespace of a pid, getNamespace() returns the pid namespace of *container*.
No, linux specific. os::getns() follows from the existing os::setns() which I agree should be namespaced as linux::
> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote:
> > src/tests/isolator_tests.cpp, line 978
> > <https://reviews.apache.org/r/25865/diff/3/?file=711231#file711231line978>
> >
> > The value itself doesn't matter?
No, it's None if it's in the caller's namespace (which would be a test failure), otherwise it is in its own namespace.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review55685
-----------------------------------------------------------
On Oct. 2, 2014, 11:23 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 11:23 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
>
>
> Diffs
> -----
>
> src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b
> src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/25865/diff/
>
>
> Testing
> -------
>
> Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ben Mahler <be...@gmail.com>.
> On Oct. 7, 2014, 9:23 p.m., Ben Mahler wrote:
> > src/slave/containerizer/linux_launcher.cpp, line 362
> > <https://reviews.apache.org/r/25865/diff/3/?file=711229#file711229line362>
> >
> > This line is unused...?
>
> Ian Downes wrote:
> It's used in the os::getns call below. I capture it so I can erase from the map in one place before any return code paths.
I'm confused, there's no os::getns scope in this method...?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review55685
-----------------------------------------------------------
On Oct. 2, 2014, 6:23 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:23 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
>
>
> Diffs
> -----
>
> src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b
> src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/25865/diff/
>
>
> Testing
> -------
>
> Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review55685
-----------------------------------------------------------
Vinod and Jie asked me to take a look at this review.
It looks like there are dependent changes that are not linked in? (filesystem islotor, ContainerInfo::MESOS, os::namespaces os::getns).
My main comment is that it seems really unfortuante that the isolator boundary is broken here between the filesystem isolator and the pid namespace isolator. I'm sure there will be folks out there that want pid namespace isolation without having to use the shared filesystem isolator.
src/Makefile.am
<https://reviews.apache.org/r/25865/#comment96053>
Can you line up the \'s here?
src/slave/containerizer/isolators/filesystem/shared.cpp
<https://reviews.apache.org/r/25865/#comment96057>
Why is the filesystem isolator doing the pid namespace related isolation? If it's necessary, please add a comment as to why we had to break the isolation boundary here!! Seems really unfortunate that only using the pid namespace isolator (without fs isolation) does not get you the /proc isolation.
Is there any way for this to be the responsibility of the pid namespace isolator instead?
src/slave/containerizer/isolators/filesystem/shared.cpp
<https://reviews.apache.org/r/25865/#comment96056>
A few questions:
(1) Why are you saying lazily here? What aspect of this is done lazily?
(2) So.. with pid namespaces you have to manually remount /proc to hide the changes? Can you make the comment more explicit about how important it is to do this?
(3) Why is /sys remounted? An explanation in the comment would be great for when us dummies are reading this code later and scratching our heads. :)
(4) Curious, is the remounting only specific to pid namespacing, as opposed to other namespaces?
src/slave/containerizer/isolators/namespaces/pid.hpp
<https://reviews.apache.org/r/25865/#comment96058>
Typically, we include what we use, rather than relying on transitive includes.
src/slave/containerizer/isolators/namespaces/pid.hpp
<https://reviews.apache.org/r/25865/#comment96060>
Why does this need to be exposed, who will rely on it?
src/slave/containerizer/isolators/namespaces/pid.hpp
<https://reviews.apache.org/r/25865/#comment96061>
It looks like these can be moved into the .cpp file, instead of being static class methods?
Also, what do they do? ;)
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96063>
Try to include what you're using, instead of relying on transitive includes. The latter are really difficult to manage and reason about.
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96086>
If it's a "pid namespace isolator", why not call this class 'PidNamespaceIsolator'?
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96088>
This is unfortunate, because I'm sure many will want pid namespace isolation **without** having to use the shared filesystem isolator. Why can't the mounting be done in this isolator?
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96090>
Bad indentation?
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96081>
s/buf/s/
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96084>
Why is state.id an option in the first place?
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96083>
Do you want to include the BIND_MOUNT_ROOT in this message to aid debugging?
os::ls seems to include it in some cases.. unfortunately =/
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96069>
Whoops!
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96068>
I'm still confused after reading this comment. :)
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment96065>
When are we going to encounter the case where the mount is not removed yet?
Can you provie more comments here, as to why we don't force or expire?
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/25865/#comment96054>
No need to say "Failed to destroy" within the Failure returned by `destroy`, that is the callers responsibility:
```
Future<Nothing> destroy = launcher->destroy(c);
...
if (destroy.isFailed()) {
LOG(ERROR) << "Failed to destroy container " << c
<< ": " << destroy.failure();
}
```
This type of message leads us to have redundant log messages. In general we would just include a reason, and since the caller knows (1) what was called, and (2) what arguments were provided, it's up to the caller to log the relevant information:
```
return Failure("Unknown container");
```
src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/25865/#comment96055>
This line is unused...?
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25865/#comment96094>
s/>/> /
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25865/#comment96093>
What is the difference between os::getns() here and NamespacesPidIsolatorProcess::getNamespace?
Where is the review that created os::getns, could we use a better name for that method?
Why isn't is linux::namespaces and linux::getns, are namespaces applicable to other systems?
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25865/#comment96095>
The value itself doesn't matter?
- Ben Mahler
On Oct. 2, 2014, 6:23 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:23 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
>
>
> Diffs
> -----
>
> src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b
> src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/25865/diff/
>
>
> Testing
> -------
>
> Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
> On Oct. 2, 2014, 12:12 p.m., Timothy St. Clair wrote:
> > src/slave/containerizer/isolators/filesystem/shared.cpp, line 90
> > <https://reviews.apache.org/r/25865/diff/3/?file=711226#file711226line90>
> >
> > Why not use api's and MS_REMOUNT?
This is run in the forked child process. This code has been removed but remounting /proc is not sufficient to correctly reflect the different pid namespace: "-o remount" or MS_REMOUNT only updates the options and data for an existing mount.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review55231
-----------------------------------------------------------
On Oct. 2, 2014, 11:23 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 11:23 a.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
>
>
> Diffs
> -----
>
> src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b
> src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/25865/diff/
>
>
> Testing
> -------
>
> Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by "Timothy St. Clair" <ts...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review55231
-----------------------------------------------------------
src/slave/containerizer/isolators/filesystem/shared.cpp
<https://reviews.apache.org/r/25865/#comment95626>
Why not use api's and MS_REMOUNT?
- Timothy St. Clair
On Oct. 2, 2014, 6:23 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 6:23 p.m.)
>
>
> Review request for mesos, Jie Yu and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
>
>
> Diffs
> -----
>
> src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b
> src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
> src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/25865/diff/
>
>
> Testing
> -------
>
> Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review58450
-----------------------------------------------------------
Ship it!
src/slave/containerizer/isolators/namespaces/pid.hpp
<https://reviews.apache.org/r/25865/#comment99467>
s/.././
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment99469>
Can you wrap at 70 everywhere for comments?
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment99471>
Maybe a bit of an explanation of the masking technique used?
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment99479>
I'm curious if we can name bindSource and bindTarget to reflect what these are returning without referring to the way the caller uses them:
source -> namespaceProcfile?
target -> namespaceHandle?
src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment99478>
Maybe add a string about stat failing inside the error message?
- Ben Mahler
On Oct. 24, 2014, 10:04 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2014, 10:04 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
>
>
> Diffs
> -----
>
> src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09
> src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
> src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
> src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
> src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36
> src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291
>
> Diff: https://reviews.apache.org/r/25865/diff/
>
>
> Testing
> -------
>
> Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/
-----------------------------------------------------------
(Updated Oct. 24, 2014, 3:04 p.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
Diffs (updated)
-----
src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09
src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36
src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291
Diff: https://reviews.apache.org/r/25865/diff/
Testing
-------
Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
Thanks,
Ian Downes
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 6:28 p.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
Diffs
-----
src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09
src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36
src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291
Diff: https://reviews.apache.org/r/25865/diff/
Testing
-------
Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
Thanks,
Ian Downes
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 10:49 a.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
Diffs
-----
src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09
src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36
src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291
Diff: https://reviews.apache.org/r/25865/diff/
Testing
-------
Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
Thanks,
Ian Downes
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 10:48 a.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
Diffs
-----
src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09
src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36
src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291
Diff: https://reviews.apache.org/r/25865/diff/
Testing
-------
Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
Thanks,
Ian Downes
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 10:48 a.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Repository: mesos-git
Description
-------
Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
Diffs (updated)
-----
src/Makefile.am 2617f77b757cb7414889520c88b1bc203dedef09
src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
src/slave/containerizer/mesos/containerizer.cpp 9f745d897119a814bd9f8e1b6a0ce5eaef60ed36
src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291
Diff: https://reviews.apache.org/r/25865/diff/
Testing
-------
Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
Thanks,
Ian Downes
Re: Review Request 25865: Pid namespace isolator for the
MesosContainerizer.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/
-----------------------------------------------------------
(Updated Oct. 14, 2014, 1:33 p.m.)
Review request for mesos, Jie Yu and Vinod Kone.
Changes
-------
Added comments. Moved some code into cpp. Removed some unnecessary isolator dependencies.
Repository: mesos-git
Description
-------
Add namespaces/pid to --isolation slave flag. Places executor into a pid namespace so it and all descendants will be contained in the namespace. Requires the filesystem/shared isolator so /proc and /sys are remounted to reflect the different namespace.
Diffs (updated)
-----
src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4
src/slave/containerizer/isolators/namespaces/pid.hpp PRE-CREATION
src/slave/containerizer/isolators/namespaces/pid.cpp PRE-CREATION
src/slave/containerizer/linux_launcher.cpp f7bc894830a7ca3f55465dacc7b653cdc2d7758b
src/slave/containerizer/mesos/containerizer.cpp 9d083294caa5c5a47ba3ceaa1b57346144cb795c
src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
Diff: https://reviews.apache.org/r/25865/diff/
Testing
-------
Added test that command in pid namespaced container is in a different namespace and that the command is 'init' (verifies remount of /proc).
Thanks,
Ian Downes