You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jacob Janco <jj...@gmail.com> on 2019/07/03 23:30:39 UTC

Re: Review Request 70757: Added a NNP isolator.

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

(Updated July 3, 2019, 11:30 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Summary (updated)
-----------------

Added a NNP isolator.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 043244841a73fa3f5f7119bc38f6d3a04be8990b 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp b29ec556399a40aa662987a11a2b64e6a16de889 
  src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/2/

Changes: https://reviews.apache.org/r/70757/diff/1-2/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/#review216461
-----------------------------------------------------------



Bad review!

Reviews applied: [70757]

Error:
2019-07-10 00:27:31 URL:https://reviews.apache.org/r/70757/diff/raw/ [12172/12172] -> "70757.patch" [1]
error: patch failed: src/slave/containerizer/mesos/launch.cpp:57
error: src/slave/containerizer/mesos/launch.cpp: patch does not apply
error: src/tests/containerizer/linux_nnp_isolator_tests.cpp: does not exist in index

- Mesos Reviewbot


On July 9, 2019, 6:15 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated July 9, 2019, 6:15 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp 043244841a73fa3f5f7119bc38f6d3a04be8990b 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp b29ec556399a40aa662987a11a2b64e6a16de889 
>   src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Июль 12, 2019, 5:46 п.п., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/isolators/linux/nnp.cpp
> > Lines 71 (patched)
> > <https://reviews.apache.org/r/70757/diff/7/?file=2154555#file2154555line71>
> >
> >     What happens if a framework explicitly set `no_new_privileges` flag to `false` in the `ContainerLaunchInfo`? Does the isolator handle such case?
> 
> James Peach wrote:
>     In that case, the containerizer would do nothing (i.e. default NNP status). This would have the same end result, but I agree that it's worth being explicit here.

At this point, the NNP isolator does not support overriding of a NNP bit by a framework?

Here is an example of how `linux/seccomp` isolator handles `seccomp` flag provided by a framework: https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/linux/seccomp.cpp#L98-L103


- Andrei


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


On Июль 11, 2019, 11:14 п.п., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated Июль 11, 2019, 11:14 п.п.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added docs for the NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.

> On July 12, 2019, 5:46 p.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/isolators/linux/nnp.cpp
> > Lines 71 (patched)
> > <https://reviews.apache.org/r/70757/diff/7/?file=2154555#file2154555line71>
> >
> >     What happens if a framework explicitly set `no_new_privileges` flag to `false` in the `ContainerLaunchInfo`? Does the isolator handle such case?
> 
> James Peach wrote:
>     In that case, the containerizer would do nothing (i.e. default NNP status). This would have the same end result, but I agree that it's worth being explicit here.
> 
> Andrei Budnik wrote:
>     At this point, the NNP isolator does not support overriding of a NNP bit by a framework?
>     
>     Here is an example of how `linux/seccomp` isolator handles `seccomp` flag provided by a framework: https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/linux/seccomp.cpp#L98-L103

Dropping this issue from chat on slack: 
jpeach: 
We need the operator to take some action to enable  NNP, we can't just turn it on since that might break things. The way  I had been thinking about this was that the action would be enabling the NNP isolator. However, when we make it configurable  by frameworks, that action is no longer definitive (i.e. there's no way to be explicit about what you want the default to be for frameworks that don't set the field). This is why I was suggesting that the seccomp isolator deal with  the configurable  part, since NNP is usually associated with seccomp as well.


jpeach  [19 hours ago]
However, if we follow this reasoning, it takes us back to our original aim for the NNP isolator, which was to unconditionally set the NNP flag. Taking this approach, we can add the configurable parts later.


- Jacob


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


On July 17, 2019, 7:19 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated July 17, 2019, 7:19 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added docs for the NNP isolator.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
>   docs/isolators/linux-nnp.md PRE-CREATION 
>   docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/9/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/#review216562
-----------------------------------------------------------




src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 71 (patched)
<https://reviews.apache.org/r/70757/#comment303774>

    What happens if a framework explicitly set `no_new_privileges` flag to `false` in the `ContainerLaunchInfo`? Does the isolator handle such case?


- Andrei Budnik


On Июль 11, 2019, 11:14 п.п., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated Июль 11, 2019, 11:14 п.п.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/#review216755
-----------------------------------------------------------


Fix it, then Ship it!




Just some minor formatting style nits.


src/slave/containerizer/mesos/isolators/linux/nnp.hpp
Lines 21 (patched)
<https://reviews.apache.org/r/70757/#comment303994>

    These should be the other way round and newline separated:
    
    ```
    #include "slave/flags.hpp"
    
    #include "slave/containerizer/mesos/isolator.hpp"
    
    ```



src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 18 (patched)
<https://reviews.apache.org/r/70757/#comment303995>

    You don't need `flags.hpp` again because it is in the isolator header.



src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 22 (patched)
<https://reviews.apache.org/r/70757/#comment303996>

    This should come before "common/kernel_version.hpp"
    
    https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#order-of-includes



src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 48 (patched)
<https://reviews.apache.org/r/70757/#comment303992>

    Nit: Newline here (I can add it when I commit).



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 19 (patched)
<https://reviews.apache.org/r/70757/#comment303997>

    Need to fix order of includes (see c++ style guide).



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 57 (patched)
<https://reviews.apache.org/r/70757/#comment303993>

    We can clean up the includes and using statements:
    ```
    $ git diff
    diff --git a/src/tests/containerizer/linux_nnp_isolator_tests.cpp b/src/tests/containerizer/linux_nnp_isolator_tests.cpp
    index 3afd6c08b..8045acfec 100644
    --- a/src/tests/containerizer/linux_nnp_isolator_tests.cpp
    +++ b/src/tests/containerizer/linux_nnp_isolator_tests.cpp
    @@ -14,31 +14,26 @@
     // See the License for the specific language governing permissions and
     // limitations under the License.
    
    -#include <mesos/mesos.hpp>
    +#include <map>
    +#include <string>
    
    -#include "common/kernel_version.hpp"
    +#include <mesos/mesos.hpp>
    
    -#include <process/owned.hpp>
     #include <process/gtest.hpp>
    +#include <process/owned.hpp>
    
    -#include <stout/error.hpp>
    -#include <stout/gtest.hpp>
    -#include <stout/os.hpp>
    -#include <stout/path.hpp>
     #include <stout/try.hpp>
    
    +#include "common/kernel_version.hpp"
    +
     #include "slave/containerizer/mesos/containerizer.hpp"
    -#include "slave/containerizer/mesos/isolators/linux/nnp.hpp"
    
    -#include "tests/cluster.hpp"
    -#include "tests/environment.hpp"
     #include "tests/mesos.hpp"
    
     #include "tests/containerizer/docker_archive.hpp"
    
     using process::Future;
     using process::Owned;
    -using process::PID;
    
     using std::map;
     using std::string;
    @@ -47,14 +42,9 @@ using mesos::internal::master::Master;
    
     using mesos::internal::slave::Containerizer;
     using mesos::internal::slave::Fetcher;
    -using mesos::internal::slave::LinuxNNPIsolatorProcess;
     using mesos::internal::slave::MesosContainerizer;
    -using mesos::internal::slave::Slave;
    -
    -using mesos::master::detector::MasterDetector;
    
     using mesos::slave::ContainerTermination;
    -using mesos::slave::Isolator;
    
     namespace mesos {
     namespace internal {
    ```


- James Peach


On July 18, 2019, 8:36 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated July 18, 2019, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/15/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 19, 2019, 7:53 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/16/

Changes: https://reviews.apache.org/r/70757/diff/15-16/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 18, 2019, 8:36 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/15/

Changes: https://reviews.apache.org/r/70757/diff/14-15/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 17, 2019, 8:38 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
-------

Broke this off into 3 patches, nnp docs -> this core patch -> kernel version change


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/14/

Changes: https://reviews.apache.org/r/70757/diff/13-14/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 17, 2019, 7:49 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
-------

Opening docs review in a separate patch, for some reason that is dropping the previous commits when posting.


Summary (updated)
-----------------

Added a NNP isolator.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description (updated)
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/13/

Changes: https://reviews.apache.org/r/70757/diff/12-13/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added docs for the NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 17, 2019, 7:34 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
-------

Fixed branch.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

Added docs for the NNP isolator.


Diffs (updated)
-----

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/12/

Changes: https://reviews.apache.org/r/70757/diff/11-12/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added docs for the NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 17, 2019, 7:31 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

Added docs for the NNP isolator.


Diffs (updated)
-----

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/11/

Changes: https://reviews.apache.org/r/70757/diff/10-11/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added docs for the NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 17, 2019, 7:27 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

Added docs for the NNP isolator.


Diffs (updated)
-----

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/10/

Changes: https://reviews.apache.org/r/70757/diff/9-10/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added docs for the NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 17, 2019, 7:19 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Changes
-------

Added docs and fixed patch series I think.


Summary (updated)
-----------------

Added docs for the NNP isolator.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description (updated)
-------

Added docs for the NNP isolator.


Diffs (updated)
-----

  CHANGELOG 164465a71c660ab9f01fb18d43076afc4b892ad5 
  docs/isolators/linux-nnp.md PRE-CREATION 
  docs/mesos-containerizer.md e79976111ec8e9cc8e8d44b5f1b8d6e2c7e072d6 


Diff: https://reviews.apache.org/r/70757/diff/9/

Changes: https://reviews.apache.org/r/70757/diff/8-9/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 16, 2019, 1:57 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/8/

Changes: https://reviews.apache.org/r/70757/diff/7-8/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/#review216600
-----------------------------------------------------------



Bad review!

Reviews applied: [70757]

Error:
2019-07-15 00:35:30 URL:https://reviews.apache.org/r/70757/diff/raw/ [12172/12172] -> "70757.patch" [1]
error: patch failed: src/slave/containerizer/mesos/launch.cpp:57
error: src/slave/containerizer/mesos/launch.cpp: patch does not apply
error: src/tests/containerizer/linux_nnp_isolator_tests.cpp: does not exist in index

- Mesos Reviewbot


On July 11, 2019, 4:14 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated July 11, 2019, 4:14 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Июль 15, 2019, 12:08 д.п., James Peach wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 1162 (patched)
> > <https://reviews.apache.org/r/70757/diff/7/?file=2154556#file2154556line1163>
> >
> >     As per Andrei, if the message field is present, toggle the NNP flag depending on its value. e.g.
> >     ```
> >     int value = launchInfo.no_new_privileges() ? 1 : 0;
> >     
> >     prctl(PR_SET_NO_NEW_PRIVS, value, 0, 0, 0)
> >     ```

`man prctl` states:
```
Once set, this bit cannot be unset.
```
An attempt to unset this bit returns an error, if the bit was previously set.


I meant that a framework may set this flag to `false`, meaning that the NNP isolator is supposed to disable this flag (in the `prepare` method) - that would allow a framework to not set a NNP flag for a particular container even if the NNP isolator enables it by default for all containers. Imagine a situation when a container can't be launched because of a NNP flag set. It'd be very desirable for a framework to support overriding of `nnp` flag similarly to how other isolators (including `linux/seccomp`) handle their flags provided by a framework.


- Andrei


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


On Июль 11, 2019, 11:14 п.п., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated Июль 11, 2019, 11:14 п.п.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/#review216599
-----------------------------------------------------------



This looks like it is in good shape, just some final nits. Looking forward to a later patch to add documentation and changelog updates :)

Please rebase and check the commit series. The review desn't apply because the nnp isolator test are a patch rather than a new file:
```
$ ./support/apply-reviews.py -r 70757
2019-07-14 17:03:13 URL:https://reviews.apache.org/r/70757/diff/raw/ [12172/12172] -> "70757.patch" [1]
error: patch failed: src/slave/containerizer/mesos/launch.cpp:57
error: src/slave/containerizer/mesos/launch.cpp: patch does not apply
error: src/tests/containerizer/linux_nnp_isolator_tests.cpp: does not exist in index
```

I think that this is also why whic review ends up depending on itself when you update it.


src/slave/containerizer/mesos/isolators/linux/nnp.hpp
Lines 46 (patched)
<https://reviews.apache.org/r/70757/#comment303799>

    We don't need to store a copy of the flags, which also means you can remove the constructor.



src/slave/containerizer/mesos/isolators/linux/nnp.cpp
Lines 22 (patched)
<https://reviews.apache.org/r/70757/#comment303800>

    Just 1 empty line here.



src/slave/containerizer/mesos/launch.cpp
Lines 1162 (patched)
<https://reviews.apache.org/r/70757/#comment303801>

    As per Andrei, if the message field is present, toggle the NNP flag depending on its value. e.g.
    ```
    int value = launchInfo.no_new_privileges() ? 1 : 0;
    
    prctl(PR_SET_NO_NEW_PRIVS, value, 0, 0, 0)
    ```



src/slave/containerizer/mesos/launch.cpp
Lines 1163 (patched)
<https://reviews.apache.org/r/70757/#comment303798>

    We should include `os::strerror(errno)` here:
    ```
    cerr << "foo foo foo: " << os::strerror(errno) << endl;
    ```


- James Peach


On July 11, 2019, 11:14 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated July 11, 2019, 11:14 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
>   src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 11, 2019, 11:14 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp c9a369bcdbfc1676912430c3e84fa567bbd7f766 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b 
  src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/7/

Changes: https://reviews.apache.org/r/70757/diff/6-7/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 9, 2019, 6:15 p.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 043244841a73fa3f5f7119bc38f6d3a04be8990b 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp b29ec556399a40aa662987a11a2b64e6a16de889 
  src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/6/

Changes: https://reviews.apache.org/r/70757/diff/5-6/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 9, 2019, 12:55 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 043244841a73fa3f5f7119bc38f6d3a04be8990b 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp b29ec556399a40aa662987a11a2b64e6a16de889 
  src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/5/

Changes: https://reviews.apache.org/r/70757/diff/4-5/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/#review216428
-----------------------------------------------------------




src/slave/containerizer/mesos/launch.cpp
Lines 1175 (patched)
<https://reviews.apache.org/r/70757/#comment303661>

    This check should move to the isolator creation factory function. It's typical for isolators to enforce prerequisites there.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 44 (patched)
<https://reviews.apache.org/r/70757/#comment303663>

    `cout` isn't used.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 85 (patched)
<https://reviews.apache.org/r/70757/#comment303656>

    Add a newline.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 94 (patched)
<https://reviews.apache.org/r/70757/#comment303664>

    You can just use `ASSERT_SOME` on the `version` here.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 99 (patched)
<https://reviews.apache.org/r/70757/#comment303662>

    Here, and in the isolator check, I think that we should phrase this as:
    ```
    kernel version >= xx.yy.zz
    ```
    
    I think that the `+` could be misread as part of the version name.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 100 (patched)
<https://reviews.apache.org/r/70757/#comment303665>

    Maybe `LOG(INFO)`, and make the message say that you are skipping the test.



src/tests/containerizer/linux_nnp_isolator_tests.cpp
Lines 129 (patched)
<https://reviews.apache.org/r/70757/#comment303666>

    I think this would be more readable with a raw string, i.e:
    
    ```
    R"~(
    ...
    )~"
    ```
    
    There's a few examples of tests that use this for scripts.


- James Peach


On July 4, 2019, 12:26 a.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70757/
> -----------------------------------------------------------
> 
> (Updated July 4, 2019, 12:26 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.
> 
> 
> Bugs: MESOS-9770
>     https://issues.apache.org/jira/browse/MESOS-9770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds the isolation capability of flipping the
> NO_NEW_PRIVILEGES bit for process control.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
>   src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
>   src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
>   src/slave/containerizer/mesos/containerizer.cpp 043244841a73fa3f5f7119bc38f6d3a04be8990b 
>   src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp b29ec556399a40aa662987a11a2b64e6a16de889 
>   src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
>   src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70757/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 4, 2019, 12:26 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 043244841a73fa3f5f7119bc38f6d3a04be8990b 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp b29ec556399a40aa662987a11a2b64e6a16de889 
  src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/4/

Changes: https://reviews.apache.org/r/70757/diff/3-4/


Testing
-------


Thanks,

Jacob Janco


Re: Review Request 70757: Added a NNP isolator.

Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70757/
-----------------------------------------------------------

(Updated July 4, 2019, 12:18 a.m.)


Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James Peach.


Bugs: MESOS-9770
    https://issues.apache.org/jira/browse/MESOS-9770


Repository: mesos


Description
-------

This patch adds the isolation capability of flipping the
NO_NEW_PRIVILEGES bit for process control.


Diffs (updated)
-----

  include/mesos/slave/containerizer.proto 2d04f3c182a4274dda527a3da56c894c3c892a12 
  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/containerizer.cpp 043244841a73fa3f5f7119bc38f6d3a04be8990b 
  src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp b29ec556399a40aa662987a11a2b64e6a16de889 
  src/tests/CMakeLists.txt 3ea8b4400ff3b00470aa147cb8f39f62802727e3 
  src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70757/diff/3/

Changes: https://reviews.apache.org/r/70757/diff/2-3/


Testing
-------


Thanks,

Jacob Janco