----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50271/#review147886 -----------------------------------------------------------
include/mesos/slave/containerizer.proto (line 144) <https://reviews.apache.org/r/50271/#comment215177> the two sentenses here look a little duplicate? include/mesos/slave/containerizer.proto (line 145) <https://reviews.apache.org/r/50271/#comment215176> s/executor/container/ This will be re-used to launch nested container (not tied to an executor) src/CMakeLists.txt (line 161) <https://reviews.apache.org/r/50271/#comment215178> 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) <https://reviews.apache.org/r/50271/#comment215180> linux/capabilities src/slave/containerizer/mesos/isolators/capabilities.hpp (line 41) <https://reviews.apache.org/r/50271/#comment215182> 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) <https://reviews.apache.org/r/50271/#comment215183> `s/flags_/_flags/` src/slave/containerizer/mesos/isolators/capabilities.cpp (line 38) <https://reviews.apache.org/r/50271/#comment215184> I'd simply: ``` return new MesosIsolator(Owned<MesosIsolatorProcess>( new LinuxCapabilitiesIsolatorProcess(flags))); ``` src/slave/containerizer/mesos/isolators/capabilities.cpp (line 43) <https://reviews.apache.org/r/50271/#comment215185> put each parameter in one line, like you did in the header. src/slave/containerizer/mesos/isolators/capabilities.cpp (lines 45 - 56) <https://reviews.apache.org/r/50271/#comment215189> This looks unreadable to me. Can you use if/else instead? src/slave/containerizer/mesos/isolators/capabilities.cpp (line 46) <https://reviews.apache.org/r/50271/#comment215190> 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 > >
