subject:"Review Request 50271\: Created an isolator for Linux capabilities."

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-10-07 Thread Jie Yu


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


Ship it!




I'll do some adjustment on the test when committing. Take a look at the final 
patch.

- Jie Yu


On Oct. 6, 2016, 2:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Oct. 6, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 86d1859854b44f237ac2ca1ef74982b543c08d25 
>   src/CMakeLists.txt ba49d81335fd47a8ced334e282eadb00657bf2c2 
>   src/Makefile.am 184afb59fc42de00db85d95fff3fbc4992be7f3b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
>   src/slave/containerizer/mesos/launch.cpp 
> 7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
>   src/tests/containerizer/capabilities_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> `make` (OS X, clang-4.0 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-10-06 Thread Benjamin Bannier


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

(Updated Oct. 6, 2016, 4:28 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
86d1859854b44f237ac2ca1ef74982b543c08d25 
  src/CMakeLists.txt ba49d81335fd47a8ced334e282eadb00657bf2c2 
  src/Makefile.am 184afb59fc42de00db85d95fff3fbc4992be7f3b 
  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/launch.cpp 
7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
  src/tests/containerizer/capabilities_isolator_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)
`make` (OS X, clang-4.0 w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-10-04 Thread Benjamin Bannier



> On Oct. 2, 2016, 2:22 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, lines 665-682
> > 
> >
> > I am wondering if we can parameterize this as well?

Yes we can :D

Updated the RR to this effect.


- Benjamin


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


On Oct. 4, 2016, 5:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Oct. 4, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 86d1859854b44f237ac2ca1ef74982b543c08d25 
>   src/CMakeLists.txt ba49d81335fd47a8ced334e282eadb00657bf2c2 
>   src/Makefile.am 184afb59fc42de00db85d95fff3fbc4992be7f3b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
>   src/slave/containerizer/mesos/launch.cpp 
> 7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
>   src/tests/containerizer/capabilities_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> `make` (OS X, clang-4.0 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-10-04 Thread Benjamin Bannier


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

(Updated Oct. 4, 2016, 5:03 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Parametrize test for task with image as well.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
86d1859854b44f237ac2ca1ef74982b543c08d25 
  src/CMakeLists.txt ba49d81335fd47a8ced334e282eadb00657bf2c2 
  src/Makefile.am 184afb59fc42de00db85d95fff3fbc4992be7f3b 
  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/launch.cpp 
7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
  src/tests/containerizer/capabilities_isolator_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)
`make` (OS X, clang-4.0 w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-10-04 Thread Benjamin Bannier



> On Oct. 2, 2016, 2:22 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 378
> > 
> >
> > I suggest we take `Option` here so that the caller 
> > can do:
> > ```
> > TestConfig(
> > {NET_RAW, SYS_ADMIN},
> > {...},
> > ...
> > )
> > ```

I have changed this to now take `Option` since that's what 
`capabilities::convert` accepts.


> On Oct. 2, 2016, 2:22 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 381
> > 
> >
> > I would simply this to be a bool: if the task should finish normally or 
> > not.
> > 
> > In the test, I would watch for terminal status update and see if it's 
> > TASK_FINISHED or not.

With the test in its current form this cannot be a `bool` since a task might 
fail in different ways (failure to start, or return with a failure when run). 
Depending on the number of task status updates we expect we do `EXPECT` 
differently.

In its current form we can be both explicit about when we expect a task to 
fail, and also avoid introducing an extra parameter storing the number of task 
status updates we expect. Can we leave it like is?


> On Oct. 2, 2016, 2:22 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, lines 573-578
> > 
> >
> > This can be
> > ```
> > TestConfig({}, None(), false)
> > ```

See above.


> On Oct. 2, 2016, 2:22 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, lines 657-659
> > 
> >
> > Should we fix those?

In fact I already did (https://reviews.apache.org/r/51931/), but failed to 
remove the `FIXME`.


> On Oct. 2, 2016, 2:22 a.m., Jie Yu wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 380
> > 
> >
> > I would remove this and leave a TODO.

We currently have a test for the base case relying on this (the first one in 
`NoCapabilitiesConfigured`). Is your suggestion to remove that test case?


- Benjamin


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


On Oct. 4, 2016, 3:08 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Oct. 4, 2016, 3:08 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 86d1859854b44f237ac2ca1ef74982b543c08d25 
>   src/CMakeLists.txt ba49d81335fd47a8ced334e282eadb00657bf2c2 
>   src/Makefile.am 184afb59fc42de00db85d95fff3fbc4992be7f3b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
>   src/slave/containerizer/mesos/launch.cpp 
> 7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
>   src/tests/containerizer/capabilities_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> `make` (OS X, clang-4.0 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-10-04 Thread Benjamin Bannier


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

(Updated Oct. 4, 2016, 3:08 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Addressed on round of comments from Jie.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
86d1859854b44f237ac2ca1ef74982b543c08d25 
  src/CMakeLists.txt ba49d81335fd47a8ced334e282eadb00657bf2c2 
  src/Makefile.am 184afb59fc42de00db85d95fff3fbc4992be7f3b 
  src/slave/containerizer/mesos/containerizer.cpp 
e6bd9f7a8284d220be157a3db2da094e6b1b6d33 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/launch.cpp 
7dd10d5030260fbfdf7396a7c05a52b7c3d983e8 
  src/tests/containerizer/capabilities_isolator_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)
`make` (OS X, clang-4.0 w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-29 Thread Benjamin Bannier


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

(Updated Sept. 29, 2016, 6:20 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
86d1859854b44f237ac2ca1ef74982b543c08d25 
  src/CMakeLists.txt 35eb63fe9a8e47d97512e9904bf5a714c63722a7 
  src/Makefile.am f093000e0282a8d5ac17e7ba33711690ccdfe68a 
  src/slave/containerizer/mesos/containerizer.cpp 
31064aefa6053eb65fbb2855929118e798b131ad 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
a9b6ee2f0da2c1c84c6a0642adbce20a9f0218dc 
  src/slave/containerizer/mesos/launch.cpp 
7cc0c3f46fe1a5883b7ccd474595b8be412b355c 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/tests/containerizer/isolator_tests.cpp 
9458e7ef205c1e7ed496b53cd28ffc23e1dc1401 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)
`make` (OS X, clang-4.0 w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-22 Thread Benjamin Bannier


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

(Updated Sept. 22, 2016, 11:48 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Addressed Jie's comments,

* only build capabilities isolator under Linux,
* do not yet reject tasks with empty requested capability set when no agent 
capability isolation active.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 
  src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
  src/Makefile.am bfdb66a6969a35660d545210c1c6951926117ef3 
  src/slave/containerizer/mesos/containerizer.cpp 
144b0db501d40d4e0bba12672723616bedd76e7e 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
859990cb85e9e8c06400397256cfc512f0811800 
  src/slave/containerizer/mesos/launch.cpp 
48ec3707d772ec68e34acfc5adb47e25336ae8d3 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/tests/containerizer/isolator_tests.cpp 
b4d25e57df7f0e157769c9ae4f7847657c505e78 

Diff: https://reviews.apache.org/r/50271/diff/


Testing (updated)
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)
`make` (OS X, clang-4.0 w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-22 Thread Jie Yu



> On Sept. 20, 2016, 2:38 a.m., Jie Yu wrote:
> > src/Makefile.am, line 834
> > 
> >
> > This should belong linux files below?
> 
> Benjamin Bannier wrote:
> This isolator has no hard dependency on Linux so we can build it on all 
> platforms. I think it would be great to have as much code as possible not 
> behind `ifdef`s. What do you think?

I don't see any way to impl that on mac. All the capability constants we 
defined in header is linux specific. I still suggest we make it linux specific.


- Jie


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


On Sept. 20, 2016, 11:40 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 20, 2016, 11:40 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> 859990cb85e9e8c06400397256cfc512f0811800 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-22 Thread Jie Yu



> On Sept. 20, 2016, 2:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 1242-1251
> > 
> >
> > I understand why you want to do this check, but we usually do not do 
> > such checks in containerizer. We rely on operator to properly configure the 
> > agent.
> > 
> > The correct solution I believe is to advertise agent isolation 
> > capabilities to the master and let master reject those tasks.
> > 
> > I would simply remove this check here for now.
> 
> Benjamin Bannier wrote:
> Like you probably know I added this to satisfy a requirement from the 
> design doc where we specified for the case when `--isolation=capabilities` is 
> absent:
> 
> a) ContainerInfo::CapabilityInfo not provided:
> All capabilities for the user. (no isolation)
> b) ContainerInfo::CapabilityInfo provided, ContainerInfo::CapabilityInfo 
> is empty:
> Not supported. Agent will fail the task with appropriate reasoning.
> c) ContainerInfo::CapabilityInfo is NOT empty:
> Not supported. Agent will fail the task with appropriate reasoning.   
>  
> 
> Do you mean the design doc is wrong when it says this work should be 
> performed in the agent (instead the master should be fail the task)? In that 
> case I wonder if we wouldn't still need to check in the agent, e.g., to allow 
> restarting agents with changed capabilities.

yeah, I feel this is the wrong way to solve the issue. If we need to do this, 
we need to do a lot for other isolators as well (e.g., gpu).

For instance, maybe we should still invoke those some validation method even if 
the isolator is not enabled. In that way, the checks can be distributed to each 
isolator, instead of accumulating in containerizer. I just feel that adding 
isolator specific checks in the containerizer like that is not going to scale.

For now, i'll just punt on that and clearly document that in the doc. Sounds 
good?


- Jie


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


On Sept. 20, 2016, 11:40 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 20, 2016, 11:40 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> 859990cb85e9e8c06400397256cfc512f0811800 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-20 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [51784, 51930, 51931, 52081, 50271]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 20, 2016, 11:40 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 20, 2016, 11:40 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.hpp 
> 859990cb85e9e8c06400397256cfc512f0811800 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-20 Thread Benjamin Bannier


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

(Updated Sept. 20, 2016, 1:40 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 
  src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
  src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
  src/slave/containerizer/mesos/containerizer.cpp 
e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
859990cb85e9e8c06400397256cfc512f0811800 
  src/slave/containerizer/mesos/launch.cpp 
fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/tests/containerizer/isolator_tests.cpp 
93ce75180520d382f63ce7323be844fe43c6594e 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-20 Thread Benjamin Bannier

> On Sept. 20, 2016, 4:38 a.m., Jie Yu wrote:
> > src/Makefile.am, line 834
> > 
> >
> > This should belong linux files below?

This isolator has no hard dependency on Linux so we can build it on all 
platforms. I think it would be great to have as much code as possible not 
behind `ifdef`s. What do you think?

> On Sept. 20, 2016, 4:38 a.m., Jie Yu wrote:
> > src/Makefile.am, line 959
> > 
> >
> > Ditto.

See above.

> On Sept. 20, 2016, 4:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 99
> > 
> >
> > Not yours, but why this is not wrapped with ifdef linux?

This isolator builds fine under e.g., macos, so we can build it everywhere. I 
believe this is A Good Thing.

> On Sept. 20, 2016, 4:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 73
> > 
> >
> > Hum, the headers here is crazy (too many ifdef linux here). The order 
> > of this is not correct.
> > 
> > I suggest we group all linux related headers together.

I moved this to l.59 outside of any conditionally included code.

I also added https://reviews.apache.org/r/52081 up the chain to reorganize the 
includes here, could you shepherd that one?

> On Sept. 20, 2016, 4:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 1242-1251
> > 
> >
> > I understand why you want to do this check, but we usually do not do 
> > such checks in containerizer. We rely on operator to properly configure the 
> > agent.
> > 
> > The correct solution I believe is to advertise agent isolation 
> > capabilities to the master and let master reject those tasks.
> > 
> > I would simply remove this check here for now.

Like you probably know I added this to satisfy a requirement from the design 
doc where we specified for the case when `--isolation=capabilities` is absent:

a) ContainerInfo::CapabilityInfo not provided:
All capabilities for the user. (no isolation)
b) ContainerInfo::CapabilityInfo provided, ContainerInfo::CapabilityInfo is 
empty:
Not supported. Agent will fail the task with appropriate reasoning.
c) ContainerInfo::CapabilityInfo is NOT empty:
Not supported. Agent will fail the task with appropriate reasoning.

Do you mean the design doc is wrong when it says this work should be performed 
in the agent (instead the master should be fail the task)? In that case I 
wonder if we wouldn't still need to check in the agent, e.g., to allow 
restarting agents with changed capabilities.

> On Sept. 20, 2016, 4:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp, lines 
> > 103-104
> > 
> >
> > Hum, if i specify `--allowed_capabilities` and the framework does not 
> > specify capabilities for TaskInfo.container. Will this work?
> > 
> > I think you need to consider three cases here:
> > 1) For command task that has rootfs. In that case, we need to make sure 
> > when we launch the executor, we don't specify capabilities (keep it None() 
> > in launchInfo, indicating that we don't want to limit capabilities). This 
> > is because the executor will perform priviledged operations like pivot_root 
> > and setuid, we want to make sure it is not restricted.
> > 2) For command task that has no rootfs. In that case, simply set 
> > launchInfo.capabilities, meaning that when launching the executor, we limit 
> > the capabilities of it. So the task will inherit the capabilities when the 
> > executor fork-exec the task.
> > 3) For custom executor case, simply set launchInfo.capabilities.
> > 
> > To check if it is a command task is by checking 
> > containerConfig.has_task_info().

You are right, there are two issues here (improper handling of the custom 
executor case, and incorrect setup for command task with rootfs of in case only 
agent `--allowed_capabilities` are give.

I simplified and fixed the logic here like you suggested above.

> On Sept. 20, 2016, 4:38 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp, line 94
> > 
> >
> > I don't follow the if/else here. Should we simply calculate "what will 
> > be the capabilities for the task/executor, depending on what is inside 
> > containerConfig.container_info() and flags.allowed_capabilities" first, and 
> > then, depending on if the container is a command task (w or w/ rootfs) or 
> > custom executor, setting launchInfo

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-20 Thread Benjamin Bannier


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

(Updated Sept. 20, 2016, 1:12 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Addressed one round of comments from Jie.


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


Repository: mesos


Description (updated)
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 
  src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
  src/Makefile.am 478fb5ae01dbdf1a735680edf7c5f30867726e07 
  src/slave/containerizer/mesos/containerizer.cpp 
e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
859990cb85e9e8c06400397256cfc512f0811800 
  src/slave/containerizer/mesos/launch.cpp 
fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
  src/slave/flags.hpp 3952d04f6a00ac1dca1adf2bea7cc6e415620ce5 
  src/tests/containerizer/isolator_tests.cpp 
93ce75180520d382f63ce7323be844fe43c6594e 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-19 Thread Jie Yu

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

Haven't looked at the tests yet.

src/Makefile.am (line 833)

This should belong linux files below?

src/Makefile.am (line 958)

Ditto.

src/slave/containerizer/mesos/containerizer.cpp (line 73)

Hum, the headers here is crazy (too many ifdef linux here). The order of 
this is not correct.

I suggest we group all linux related headers together.

src/slave/containerizer/mesos/containerizer.cpp (line 99)

Not yours, but why this is not wrapped with ifdef linux?

src/slave/containerizer/mesos/containerizer.cpp (lines 1242 - 1251)

I understand why you want to do this check, but we usually do not do such 
checks in containerizer. We rely on operator to properly configure the agent.

The correct solution I believe is to advertise agent isolation capabilities 
to the master and let master reject those tasks.

I would simply remove this check here for now.

src/slave/containerizer/mesos/isolators/linux/capabilities.hpp (line 17)

LINUX_CAPABILITIES

src/slave/containerizer/mesos/isolators/linux/capabilities.cpp (line 40)

Given that we don't need to do the check in the agent, please just inline 
this method.

src/slave/containerizer/mesos/isolators/linux/capabilities.cpp (line 94)

I don't follow the if/else here. Should we simply calculate "what will be 
the capabilities for the task/executor, depending on what is inside 
containerConfig.container_info() and flags.allowed_capabilities" first, and 
then, depending on if the container is a command task (w or w/ rootfs) or 
custom executor, setting launchInfo accordingly?

src/slave/containerizer/mesos/isolators/linux/capabilities.cpp (lines 103 - 104)

Hum, if i specify `--allowed_capabilities` and the framework does not 
specify capabilities for TaskInfo.container. Will this work?

I think you need to consider three cases here:
1) For command task that has rootfs. In that case, we need to make sure 
when we launch the executor, we don't specify capabilities (keep it None() in 
launchInfo, indicating that we don't want to limit capabilities). This is 
because the executor will perform priviledged operations like pivot_root and 
setuid, we want to make sure it is not restricted.
2) For command task that has no rootfs. In that case, simply set 
launchInfo.capabilities, meaning that when launching the executor, we limit the 
capabilities of it. So the task will inherit the capabilities when the executor 
fork-exec the task.
3) For custom executor case, simply set launchInfo.capabilities.

To check if it is a command task is by checking 
containerConfig.has_task_info().

- Jie Yu

On Sept. 19, 2016, 2:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 19, 2016, 2:21 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes
> net capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed from the isolator via a new
> capability field in `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 6fb095f58cf943c5597175df695046cfa21b68fd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-19 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [51784, 51930, 51931, 50271]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 19, 2016, 2:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 19, 2016, 2:21 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes
> net capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed from the isolator via a new
> capability field in `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 6fb095f58cf943c5597175df695046cfa21b68fd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-19 Thread Benjamin Bannier


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

(Updated Sept. 19, 2016, 4:21 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Allow to isolate tasks with image, rebased.


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


Repository: mesos


Description (updated)
---

This isolator evaluates agent allowed capabilities and passes
net capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed from the isolator via a new
capability field in `ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 
  src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
  src/Makefile.am 6fb095f58cf943c5597175df695046cfa21b68fd 
  src/slave/containerizer/mesos/containerizer.cpp 
e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.cpp 
fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
  src/tests/containerizer/isolator_tests.cpp 
93ce75180520d382f63ce7323be844fe43c6594e 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-19 Thread Benjamin Bannier



> On Sept. 10, 2016, 8:23 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp, lines 47-49
> > 
> >
> > We need to have special case for command tasks (i.e., 
> > containerConfig.has_task_info()).
> > 
> > For command tasks, we need to make sure the executor is running under 
> > root, having all capabilities. I.e., `launchInfo.capability` should not be 
> > set.
> > 
> > Instead, we need to add a new flag to command executor 
> > (`--capabilities`) and command executor will pass that to 
> > `mesos-containerizer launch` helper.
> 
> Jie Yu wrote:
> You need this patch:
> https://reviews.apache.org/r/51784

Adjusted, and new dependent rr linked.


- Benjamin


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


On Sept. 19, 2016, 4:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 19, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes
> net capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed from the isolator via a new
> capability field in `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 42c52b60cc850901f2eff1545cf7900f4a65ca81 
>   src/Makefile.am 6fb095f58cf943c5597175df695046cfa21b68fd 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e54169ba00f6e0cdd7043075b4cdd12d714c99e3 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/launch.cpp 
> fc51e04ec1572679e6a48ff4f0fa31ef2dfd6ec3 
>   src/tests/containerizer/isolator_tests.cpp 
> 93ce75180520d382f63ce7323be844fe43c6594e 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-13 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [51654, 50271]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 13, 2016, 11:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 13, 2016, 11:52 a.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 2dd7913a1477f3c3560be4e2c1450b93fb3afc78 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1a120f191e4ff0e2b31dd0a9a6bced784a56612c 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-13 Thread Benjamin Bannier


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

(Updated Sept. 13, 2016, 1:52 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Added more tests and fixed uncovered corner cases, rebased.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
20db010ea158a813034b41ce9cddac7d8317 
  src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
  src/Makefile.am 2dd7913a1477f3c3560be4e2c1450b93fb3afc78 
  src/slave/containerizer/mesos/containerizer.cpp 
1a120f191e4ff0e2b31dd0a9a6bced784a56612c 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
f8056ca08029feed5f164d4f94e24d521183bdfc 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-13 Thread Benjamin Bannier



> On Sept. 8, 2016, 11:12 a.m., Jay Guo wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 1802
> > 
> >
> > I think we need more comprehensive tests for all the cases listed in 
> > the matrix in design doc. For instance, a test case for forbidden 
> > operations.
> 
> Benjamin Bannier wrote:
> Agreed, I'll add a parameterized test capturing that matrix.

Marking this one as fixed since I added a number of tests (almost everything 
modulo tests for agents with `uid != 0`).


- Benjamin


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


On Sept. 13, 2016, 1:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 13, 2016, 1:52 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 2dd7913a1477f3c3560be4e2c1450b93fb3afc78 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1a120f191e4ff0e2b31dd0a9a6bced784a56612c 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-13 Thread Benjamin Bannier



> On Sept. 10, 2016, 8:23 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp, lines 57-68
> > 
> >
> > Do you know that `Set` supports union and intersection. I think here we 
> > can just leverage Set intersection.

I changed this code to use `set` intersection from `stout/set.hpp`.


- Benjamin


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


On Sept. 13, 2016, 1:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 13, 2016, 1:52 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 2dd7913a1477f3c3560be4e2c1450b93fb3afc78 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1a120f191e4ff0e2b31dd0a9a6bced784a56612c 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-13 Thread Benjamin Bannier



> On Sept. 10, 2016, 2:34 a.m., Jie Yu wrote:
> > Can you move tests into a separate patch?

If you have no strong objections I would really like to keep them as part of 
this patch.


- Benjamin


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


On Sept. 13, 2016, 1:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 13, 2016, 1:52 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 20db010ea158a813034b41ce9cddac7d8317 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 2dd7913a1477f3c3560be4e2c1450b93fb3afc78 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1a120f191e4ff0e2b31dd0a9a6bced784a56612c 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-10 Thread Jie Yu



> On Sept. 10, 2016, 6:23 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp, lines 47-49
> > 
> >
> > We need to have special case for command tasks (i.e., 
> > containerConfig.has_task_info()).
> > 
> > For command tasks, we need to make sure the executor is running under 
> > root, having all capabilities. I.e., `launchInfo.capability` should not be 
> > set.
> > 
> > Instead, we need to add a new flag to command executor 
> > (`--capabilities`) and command executor will pass that to 
> > `mesos-containerizer launch` helper.

You need this patch:
https://reviews.apache.org/r/51784


- Jie


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


On Sept. 9, 2016, 3:26 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 9, 2016, 3:26 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-09 Thread Jie Yu



> On Sept. 8, 2016, 9:12 a.m., Jay Guo wrote:
> > Could list test steps to verify functionalities? I tried following steps:
> > // start master
> > $ mesos-master --ip= --work_dir=
> > // start agent
> > $ sudo mesos-agent --master=: 
> > --work_dir= --isolation=linux/capabilities 
> > --allowed_capabilities='{"capabilities":["CHOWN"]}'
> > // mesos-execute
> > $ mesos-execute --master=: --name="test" 
> > --command="touch foo && chown nobody foo"
> > 
> > I'm expecting that I could `chown` with `CAP_CHOWN` however I got 
> > `operation not permitted`. I could be misunderstanding this completely, 
> > please advise
> 
> Benjamin Bannier wrote:
> Your confusion comes from the fact that you assume this patch would allow 
> to assign any capabilities to a container. The command you gave would also 
> not work under plain Linux had you given an unpreviledged user `CAP_CHOWN` 
> since `chown` neither makes direct use of capabilities by itself (it does not 
> link against `libcap*`), nor has file capabilities. There is nothing in the 
> current patch which would allow it to suddenly gain capability features. To 
> achieve the effect you expected you ccould e.g., add `cap_chown` to `chown`'s 
> file-based capabilities,
> 
> $ sudo /sbin/setcap cap_chown+ep `which chown`
> 
> With this addition and above agent invocation the task would succeed, 
> while if you had specified an emtpy capability set 
> (`--allowed_capabilities='{}'`) it would have failed, just like one would 
> expect.

Jay, to achieve what you said, we need ambient capabilities support, which is 
in kernel 4.3 and later.
https://lwn.net/Articles/636533/

This phase, we only support the following:
Restrict the capabilities of a container so that it cannot do certain things 
even if it uses a setuid executable, or it is running under root.

The next phase, we might leverage ambient capabilities to support what you want 
to do.


- Jie


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


On Sept. 9, 2016, 3:26 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 9, 2016, 3:26 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-09 Thread Jie Yu


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



Can you move tests into a separate patch?

- Jie Yu


On Sept. 9, 2016, 3:26 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 9, 2016, 3:26 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-09 Thread Benjamin Bannier


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




src/tests/containerizer/isolator_tests.cpp (lines 1825 - 1826)


This should be fixed in `createTask` in a separate patch.



src/tests/containerizer/isolator_tests.cpp (lines 1886 - 1891)


This test case does succeed even though it shouldn't.

The error might be in how we handle capabilities if no task `uid` is given.



src/tests/containerizer/isolator_tests.cpp (lines 1909 - 1915)


This test case does succeed even though it shouldn't.

The error might be in how we handle capabilities if no task `uid` is given.



src/tests/containerizer/isolator_tests.cpp (lines 1955 - 1960)


This test case hangs waiting for `status2` where `status1` should already 
be failed. For some reason the `Failure` emitted from the isolator's `prepare` 
doesn't propagate properly.


- Benjamin Bannier


On Sept. 9, 2016, 5:26 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 9, 2016, 5:26 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-08 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [51697, 50270, 50271]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 8, 2016, 4:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 8, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-08 Thread Benjamin Bannier



> On Sept. 8, 2016, 11:12 a.m., Jay Guo wrote:
> > Could list test steps to verify functionalities? I tried following steps:
> > // start master
> > $ mesos-master --ip= --work_dir=
> > // start agent
> > $ sudo mesos-agent --master=: 
> > --work_dir= --isolation=linux/capabilities 
> > --allowed_capabilities='{"capabilities":["CHOWN"]}'
> > // mesos-execute
> > $ mesos-execute --master=: --name="test" 
> > --command="touch foo && chown nobody foo"
> > 
> > I'm expecting that I could `chown` with `CAP_CHOWN` however I got 
> > `operation not permitted`. I could be misunderstanding this completely, 
> > please advise

Your confusion comes from the fact that you assume this patch would allow to 
assign any capabilities to a container. The command you gave would also not 
work under plain Linux had you given an unpreviledged user `CAP_CHOWN` since 
`chown` neither makes direct use of capabilities by itself (it does not link 
against `libcap*`), nor has file capabilities. There is nothing in the current 
patch which would allow it to suddenly gain capability features. To achieve the 
effect you expected you ccould e.g., add `cap_chown` to `chown`'s file-based 
capabilities,

$ sudo /sbin/setcap cap_chown+ep `which chown`

With this addition and above agent invocation the task would succeed, while if 
you had specified an emtpy capability set (`--allowed_capabilities='{}'`) it 
would have failed, just like one would expect.


> On Sept. 8, 2016, 11:12 a.m., Jay Guo wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 1802
> > 
> >
> > I think we need more comprehensive tests for all the cases listed in 
> > the matrix in design doc. For instance, a test case for forbidden 
> > operations.

Agreed, I'll add a parameterized test capturing that matrix.


- Benjamin


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


On Sept. 8, 2016, 6:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 8, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-08 Thread Benjamin Bannier


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

(Updated Sept. 8, 2016, 6:04 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Addressed some of Jay's review comments.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
16dd3a19145b9764273cdb9a8899e353c98730e5 
  src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
  src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
  src/slave/containerizer/mesos/containerizer.cpp 
89b7e8db38916d69d9b2d4fe305d4397b0859a10 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
f8056ca08029feed5f164d4f94e24d521183bdfc 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-08 Thread Jay Guo


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



Could list test steps to verify functionalities? I tried following steps:
// start master
$ mesos-master --ip= --work_dir=
// start agent
$ sudo mesos-agent --master=: 
--work_dir= --isolation=linux/capabilities 
--allowed_capabilities='{"capabilities":["CHOWN"]}'
// mesos-execute
$ mesos-execute --master=: --name="test" 
--command="touch foo && chown nobody foo"

I'm expecting that I could `chown` with `CAP_CHOWN` however I got `operation 
not permitted`. I could be misunderstanding this completely, please advise


src/slave/containerizer/mesos/isolators/linux/capabilities.cpp (line 62)


We need a check here in case that `flags.allowed_capabilities` is not set, 
otherwise it would fail assertion in `get()` and exit.



src/tests/containerizer/isolator_tests.cpp (line 1802)


I think we need more comprehensive tests for all the cases listed in the 
matrix in design doc. For instance, a test case for forbidden operations.


- Jay Guo


On Sept. 7, 2016, 4:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 7, 2016, 4:46 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-07 Thread Benjamin Bannier



> On Sept. 6, 2016, 9 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/capabilities.hpp, line 41
> > 
> >
> > are we consistent on 'override' keyword? I'd suggest we be consistent 
> > with other isolators for now.

We consistently do not use `override`. I changed this code to explicitly be 
`virtual` as documentation.


> On Sept. 6, 2016, 9 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/capabilities.cpp, lines 45-56
> > 
> >
> > This looks unreadable to me. Can you use if/else instead?

Done.


> On Sept. 6, 2016, 9 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/capabilities.cpp, line 46
> > 
> >
> > I think we should use containerConfig.container_info now as we plan to 
> > supported nested container.

Done.


> On Sept. 6, 2016, 9 p.m., Jie Yu wrote:
> > include/mesos/slave/containerizer.proto, line 144
> > 
> >
> > the two sentenses here look a little duplicate?

I meant them to be orthogonal. Could you suggest a better alternative?


- Benjamin


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


On Sept. 7, 2016, 6:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 7, 2016, 6:46 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-07 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [51697, 50270, 50271]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 7, 2016, 4:46 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 7, 2016, 4:46 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-07 Thread Benjamin Bannier


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

(Updated Sept. 7, 2016, 6:46 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Addressed jieyu's review comments.


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


Repository: mesos


Description (updated)
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
16dd3a19145b9764273cdb9a8899e353c98730e5 
  src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
  src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
  src/slave/containerizer/mesos/containerizer.cpp 
89b7e8db38916d69d9b2d4fe305d4397b0859a10 
  src/slave/containerizer/mesos/isolators/linux/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
f8056ca08029feed5f164d4f94e24d521183bdfc 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-06 Thread Jie Yu


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




src/slave/containerizer/mesos/isolators/capabilities.hpp (line 34)


LinuxCapabilitiesIsolatorProcess


- Jie Yu


On Sept. 6, 2016, 3:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 6, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-06 Thread Jie Yu


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




include/mesos/slave/containerizer.proto (line 144)


the two sentenses here look a little duplicate?



include/mesos/slave/containerizer.proto (line 145)


s/executor/container/

This will be re-used to launch nested container (not tied to an executor)



src/CMakeLists.txt (line 161)


I'd suggest we create a linux folder in isolators and put 
capabilities.hpp|cpp there. We might want to add more linux specific isolators 
(e.g., sysctl, ulimit)



src/slave/containerizer/mesos/containerizer.cpp (line 331)


linux/capabilities



src/slave/containerizer/mesos/isolators/capabilities.hpp (line 41)


are we consistent on 'override' keyword? I'd suggest we be consistent with 
other isolators for now.



src/slave/containerizer/mesos/isolators/capabilities.hpp (line 44)


`s/flags_/_flags/`



src/slave/containerizer/mesos/isolators/capabilities.cpp (line 38)


I'd simply:
```
return new MesosIsolator(Owned(
new LinuxCapabilitiesIsolatorProcess(flags)));
```



src/slave/containerizer/mesos/isolators/capabilities.cpp (line 43)


put each parameter in one line, like you did in the header.



src/slave/containerizer/mesos/isolators/capabilities.cpp (lines 45 - 56)


This looks unreadable to me. Can you use if/else instead?



src/slave/containerizer/mesos/isolators/capabilities.cpp (line 46)


I think we should use containerConfig.container_info now as we plan to 
supported nested container.


- Jie Yu


On Sept. 6, 2016, 3:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Sept. 6, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, Jay Guo and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 16dd3a19145b9764273cdb9a8899e353c98730e5 
>   src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
>   src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 89b7e8db38916d69d9b2d4fe305d4397b0859a10 
>   src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f8056ca08029feed5f164d4f94e24d521183bdfc 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-09-06 Thread Benjamin Bannier


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

(Updated Sept. 6, 2016, 5:04 p.m.)


Review request for mesos, Jay Guo and Jie Yu.


Changes
---

Rebased, extended tests.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
16dd3a19145b9764273cdb9a8899e353c98730e5 
  src/CMakeLists.txt 01ef494f7120156de3b826d7def76fb30bcc61b5 
  src/Makefile.am 15b9a63822eca8d0b428191940026756fba7821e 
  src/slave/containerizer/mesos/containerizer.cpp 
89b7e8db38916d69d9b2d4fe305d4397b0859a10 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
f8056ca08029feed5f164d4f94e24d521183bdfc 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-08-25 Thread Benjamin Bannier


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

(Updated Aug. 25, 2016, 5:18 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
16dd3a19145b9764273cdb9a8899e353c98730e5 
  src/Makefile.am 69e56551f0adca6d6a9811cafea9a8d3c56d1df9 
  src/slave/containerizer/mesos/containerizer.cpp 
e87292f7dc3d51f8d4a5ff2e1b9e0090748cdee6 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
2725eb0f76f4e2668381c0d6686a99649f4f9f25 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-08-15 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [50266, 50889, 51042, 51043, 50269, 50270, 50271]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 15, 2016, 3:57 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Aug. 15, 2016, 3:57 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 5ed894d18258f6516866a2acd343c03281039c3c 
>   src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> f917db78157f799efc00b0c7af0230ddb6f26b0b 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-08-15 Thread Benjamin Bannier


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

(Updated Aug. 15, 2016, 5:57 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Cleanup.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
5ed894d18258f6516866a2acd343c03281039c3c 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
f917db78157f799efc00b0c7af0230ddb6f26b0b 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-08-12 Thread Mesos ReviewBot


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



Patch looks great!

Reviews applied: [50266, 50889, 51042, 51043, 50269, 50270, 50271]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 12, 2016, 4:48 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Aug. 12, 2016, 4:48 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
>   src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> 4f047ae6b2e85e177e8b73d60b9dfca913c832a5 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-08-12 Thread Benjamin Bannier


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

(Updated Aug. 12, 2016, 6:48 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
4f047ae6b2e85e177e8b73d60b9dfca913c832a5 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-08-10 Thread Mesos ReviewBot


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



Bad patch!

Reviews applied: [50271, 50270, 50269, 50889, 50266]

Failed command: ./support/apply-review.sh -n -r 50270

Error:
2016-08-10 23:58:36 URL:https://reviews.apache.org/r/50270/diff/raw/ 
[9465/9465] -> "50270.patch" [1]
error: patch failed: src/launcher/executor.cpp:110
error: src/launcher/executor.cpp: patch does not apply
error: patch failed: src/slave/flags.cpp:92
error: src/slave/flags.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14740/console

- Mesos ReviewBot


On Aug. 10, 2016, 5:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50271/
> ---
> 
> (Updated Aug. 10, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5275
> https://issues.apache.org/jira/browse/MESOS-5275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This isolator evaluates agent allowed capabilities and passes net
> capabilities on to `mesos-containerizer` which enforces the
> capabilities.
> 
> Capability information is passed via a new field in
> `ContainerLaunchInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
>   src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
>   src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
>   src/tests/containerizer/isolator_tests.cpp 
> 827c9f06b97191a5b74bb8fa2ef716c4282e7518 
> 
> Diff: https://reviews.apache.org/r/50271/diff/
> 
> 
> Testing
> ---
> 
> `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o 
> optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 50271: Created an isolator for Linux capabilities.

2016-08-10 Thread Benjamin Bannier


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

(Updated Aug. 10, 2016, 7:03 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This isolator evaluates agent allowed capabilities and passes net
capabilities on to `mesos-containerizer` which enforces the
capabilities.

Capability information is passed via a new field in
`ContainerLaunchInfo`.


Diffs (updated)
-

  include/mesos/slave/isolator.proto e945514fbdd49b3b35e200a7fc2e1cd446c1af91 
  src/Makefile.am 1a9b083493612cf610b80ac5a1c11c29d6302933 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
  src/slave/containerizer/mesos/isolators/capabilities.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/capabilities.cpp PRE-CREATION 
  src/tests/containerizer/isolator_tests.cpp 
827c9f06b97191a5b74bb8fa2ef716c4282e7518 

Diff: https://reviews.apache.org/r/50271/diff/


Testing
---

`make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o optimizations)


Thanks,

Benjamin Bannier

44 matches

Mail list logo