mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.
Date Fri, 26 Sep 2014 01:17:54 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25865/#review54637
-----------------------------------------------------------



src/slave/containerizer/isolators/namespaces/pid.hpp
<https://reviews.apache.org/r/25865/#comment94869>

    s/NamespacesPid/PidNamespace/ ?



src/slave/containerizer/isolators/namespaces/pid.hpp
<https://reviews.apache.org/r/25865/#comment94870>

    kill new line.



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94871>

    Comment?



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94873>

    As mentioned in the previous review, instead of requiring users/operators to know this
dependency, we should just automatically use fileystem/shared isoator when using pid or network
isolation.



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94877>

    Who is calling this method?



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94882>

    // Cleanup orphans. ?



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94880>

    if you use hashset, you can just do !containerers.contain().



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94883>

    Why not just call cleanup() here?



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94881>

    Can you also say why in the comment? Presumably because you dont want containers to see
other containers runninng in the system?



src/slave/containerizer/isolators/namespaces/pid.cpp
<https://reviews.apache.org/r/25865/#comment94884>

    Add a comment that you are doing this for the ability to cleanup orphans during recovery?
    
    Also, what is the need for manual cleanup or orphans?



src/slave/containerizer/linux_launcher.cpp
<https://reviews.apache.org/r/25865/#comment94867>

    why is this pulled out?



src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25865/#comment94885>

    s/NamespacesPidIsolatorTest/PidNamespaceIsolatorTest/



src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/25865/#comment94886>

    you are writing to files, not stdout and stderr right?


- Vinod Kone


On Sept. 23, 2014, 11:39 p.m., Ian Downes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25865/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2014, 11:39 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 9b973e5503e30180045e270220987ba647da8038 
>   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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message