----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70757/#review216755 -----------------------------------------------------------
Fix it, then Ship it! Just some minor formatting style nits. src/slave/containerizer/mesos/isolators/linux/nnp.hpp Lines 21 (patched) <https://reviews.apache.org/r/70757/#comment303994> These should be the other way round and newline separated: ``` #include "slave/flags.hpp" #include "slave/containerizer/mesos/isolator.hpp" ``` src/slave/containerizer/mesos/isolators/linux/nnp.cpp Lines 18 (patched) <https://reviews.apache.org/r/70757/#comment303995> You don't need `flags.hpp` again because it is in the isolator header. src/slave/containerizer/mesos/isolators/linux/nnp.cpp Lines 22 (patched) <https://reviews.apache.org/r/70757/#comment303996> This should come before "common/kernel_version.hpp" https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#order-of-includes src/slave/containerizer/mesos/isolators/linux/nnp.cpp Lines 48 (patched) <https://reviews.apache.org/r/70757/#comment303992> Nit: Newline here (I can add it when I commit). src/tests/containerizer/linux_nnp_isolator_tests.cpp Lines 19 (patched) <https://reviews.apache.org/r/70757/#comment303997> Need to fix order of includes (see c++ style guide). src/tests/containerizer/linux_nnp_isolator_tests.cpp Lines 57 (patched) <https://reviews.apache.org/r/70757/#comment303993> We can clean up the includes and using statements: ``` $ git diff diff --git a/src/tests/containerizer/linux_nnp_isolator_tests.cpp b/src/tests/containerizer/linux_nnp_isolator_tests.cpp index 3afd6c08b..8045acfec 100644 --- a/src/tests/containerizer/linux_nnp_isolator_tests.cpp +++ b/src/tests/containerizer/linux_nnp_isolator_tests.cpp @@ -14,31 +14,26 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include <mesos/mesos.hpp> +#include <map> +#include <string> -#include "common/kernel_version.hpp" +#include <mesos/mesos.hpp> -#include <process/owned.hpp> #include <process/gtest.hpp> +#include <process/owned.hpp> -#include <stout/error.hpp> -#include <stout/gtest.hpp> -#include <stout/os.hpp> -#include <stout/path.hpp> #include <stout/try.hpp> +#include "common/kernel_version.hpp" + #include "slave/containerizer/mesos/containerizer.hpp" -#include "slave/containerizer/mesos/isolators/linux/nnp.hpp" -#include "tests/cluster.hpp" -#include "tests/environment.hpp" #include "tests/mesos.hpp" #include "tests/containerizer/docker_archive.hpp" using process::Future; using process::Owned; -using process::PID; using std::map; using std::string; @@ -47,14 +42,9 @@ using mesos::internal::master::Master; using mesos::internal::slave::Containerizer; using mesos::internal::slave::Fetcher; -using mesos::internal::slave::LinuxNNPIsolatorProcess; using mesos::internal::slave::MesosContainerizer; -using mesos::internal::slave::Slave; - -using mesos::master::detector::MasterDetector; using mesos::slave::ContainerTermination; -using mesos::slave::Isolator; namespace mesos { namespace internal { ``` - James Peach On July 18, 2019, 8:36 p.m., Jacob Janco wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70757/ > ----------------------------------------------------------- > > (Updated July 18, 2019, 8:36 p.m.) > > > Review request for mesos, Andrei Budnik, Gilbert Song, Jie Yu, and James > Peach. > > > Bugs: MESOS-9770 > https://issues.apache.org/jira/browse/MESOS-9770 > > > Repository: mesos > > > Description > ------- > > This patch adds the isolation capability of flipping the > NO_NEW_PRIVILEGES bit for process control. > > > Diffs > ----- > > include/mesos/slave/containerizer.proto > 2d04f3c182a4274dda527a3da56c894c3c892a12 > src/CMakeLists.txt eb4e2ac699121ac16e2ac58e4606b26aae906ad8 > src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 > src/slave/containerizer/mesos/containerizer.cpp > c9a369bcdbfc1676912430c3e84fa567bbd7f766 > src/slave/containerizer/mesos/isolators/linux/nnp.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/linux/nnp.cpp PRE-CREATION > src/slave/containerizer/mesos/launch.cpp > 0419e8e0fbf154396ab5fb5026d77b94cc9fca5b > src/tests/CMakeLists.txt 5eb9c65f30d9d6ade289f9d47b18d73908b1f1db > src/tests/containerizer/linux_nnp_isolator_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/70757/diff/15/ > > > Testing > ------- > > > Thanks, > > Jacob Janco > >
