----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46798/#review141649 -----------------------------------------------------------
src/launcher/executor.cpp (line 90) <https://reviews.apache.org/r/46798/#comment207006> I would much prefer this to not force each and every caller to deal with different function signatures. We should probably try to find a way to use identical signatures on every platform, and then possible make the underlying implementation do what is actually supported. src/launcher/executor.cpp (line 376) <https://reviews.apache.org/r/46798/#comment207007> It seems we should be able to do away with conditional compilation here if we'd push the check for platform support into the capabilities implementation. src/launcher/executor.cpp (line 397) <https://reviews.apache.org/r/46798/#comment207008> Same here, we should try to remove the conditional compilation and instead push the checks for platform support into the capabilities infrastructure. src/slave/containerizer/mesos/containerizer.cpp (line 270) <https://reviews.apache.org/r/46798/#comment207019> This could again be simplified by pushing the check for platform support into the capabilities infrastructure. src/slave/containerizer/mesos/containerizer.cpp (line 271) <https://reviews.apache.org/r/46798/#comment207020> I think moving the capabilities check out of the loop would improve readibilty of this loop. src/slave/containerizer/mesos/containerizer.cpp (line 1299) <https://reviews.apache.org/r/46798/#comment207021> Remove conditional compilation once member exists for all platforms. src/slave/containerizer/mesos/launch.cpp (line 31) <https://reviews.apache.org/r/46798/#comment207015> Sort alphabetically. src/slave/containerizer/mesos/launch.cpp (line 49) <https://reviews.apache.org/r/46798/#comment207046> As currently written this needs to be compiled conditionally as `Capabilities` is not defined on non-Linux platforms. src/slave/containerizer/mesos/launch.cpp (line 280) <https://reviews.apache.org/r/46798/#comment207017> Why do we need to require a shell command here? src/slave/containerizer/mesos/launch.cpp (line 281) <https://reviews.apache.org/r/46798/#comment207018> The nesting of these preprocessor checks (where one checks a negative and the other for a positive) on top of a already deeply control flow makes this really hard to read. I believe this could again be simplified by pushing the check for platform support into the capabilities infrastructure. - Benjamin Bannier On May 26, 2016, 5:03 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46798/ > ----------------------------------------------------------- > > (Updated May 26, 2016, 5:03 p.m.) > > > Review request for mesos and Jie Yu. > > > Repository: mesos > > > Description > ------- > > This change introduces linux capability based security for unified > containerizer. A new agent flag \`allowed_capabilities\` has been > introduced to override the default capabilities of the user or the > capabilities requested by the user. > > This feature is only available on linux. > > > Diffs > ----- > > src/common/parse.hpp 19e56dbeb765f8bec92e0a3615f6f7c12466fa9e > src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 > src/slave/containerizer/mesos/containerizer.hpp > a1a00020668f6da8d611f26e5637afffc87d09ba > src/slave/containerizer/mesos/containerizer.cpp > 75e5a32a3e70ec60a6800e21a621673184ea0956 > src/slave/containerizer/mesos/launch.hpp > c716e0396736d1f2f60ec31540f12f4f7597d081 > src/slave/containerizer/mesos/launch.cpp > e22106b014c871e2184a15c2ab154a0674874e47 > src/slave/flags.hpp 80ba2887448e91c40ae68fc2d9f0c0bee1a49f48 > src/slave/flags.cpp b7df8f760d0f75459f1e80e3d8e18d49a3995df8 > src/tests/container_logger_tests.cpp > efadceafca5721bce4dbffadb35f54fd5365abb0 > src/tests/containerizer/docker_volume_isolator_tests.cpp > c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 > src/tests/containerizer/filesystem_isolator_tests.cpp > 4293416ac8434e9eb7e80724480a54936a2fe24a > src/tests/containerizer/mesos_containerizer_tests.cpp > 09742ff21513dc2570684d384b257868dd57a9ce > > Diff: https://reviews.apache.org/r/46798/diff/ > > > Testing > ------- > > make check; used mesos cli to test end to end functionality. > > > Thanks, > > Jojy Varghese > >
