----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50271/#review150223 -----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp (line 63) <https://reviews.apache.org/r/50271/#comment218112> this is linux only as we agreed, right? src/slave/containerizer/mesos/launch.cpp (lines 114 - 116) <https://reviews.apache.org/r/50271/#comment218113> You need to move this out as well? src/slave/flags.hpp (line 104) <https://reviews.apache.org/r/50271/#comment218111> I thought we agree that this is a linux only feature as the isolator is only available on linux? src/tests/containerizer/isolator_tests.cpp (lines 43 - 47) <https://reviews.apache.org/r/50271/#comment218114> Please create a separate test files for this: `src/tests/containerizer/capabilities_isolator_tests.cpp` src/tests/containerizer/isolator_tests.cpp (line 375) <https://reviews.apache.org/r/50271/#comment219263> Rename this to `TestConfig` or `TestSetup`? src/tests/containerizer/isolator_tests.cpp (line 378) <https://reviews.apache.org/r/50271/#comment219258> I suggest we take `Option<vector<Capability>>` here so that the caller can do: ``` TestConfig( {NET_RAW, SYS_ADMIN}, {...}, ... ) ``` src/tests/containerizer/isolator_tests.cpp (line 380) <https://reviews.apache.org/r/50271/#comment219261> I would remove this and leave a TODO. src/tests/containerizer/isolator_tests.cpp (line 381) <https://reviews.apache.org/r/50271/#comment219262> 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. src/tests/containerizer/isolator_tests.cpp (line 395) <https://reviews.apache.org/r/50271/#comment219264> s/os/stream/ src/tests/containerizer/isolator_tests.cpp (lines 573 - 578) <https://reviews.apache.org/r/50271/#comment219260> This can be ``` TestConfig({}, None(), false) ``` src/tests/containerizer/isolator_tests.cpp (line 576) <https://reviews.apache.org/r/50271/#comment219259> I would use 'using capabilities::xxx' in the begining of this file to reduce verbosity here. src/tests/containerizer/isolator_tests.cpp (lines 657 - 659) <https://reviews.apache.org/r/50271/#comment219265> Should we fix those? src/tests/containerizer/isolator_tests.cpp (lines 665 - 682) <https://reviews.apache.org/r/50271/#comment219266> I am wondering if we can parameterize this as well? - Jie Yu On Sept. 29, 2016, 4:20 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50271/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2016, 4:20 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 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 > >
