----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53688/#review157875 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp (line 73) <https://reviews.apache.org/r/53688/#comment228488> Shouldn't containers (top-level containers) have the ability to specify if they want an IPC namespace or not. Seems like the minute we turn on the IPC isolator we change the default behavior of Mesos where every task/container starts getting a new IPC namespace without it explicitly intending. Was thinking, it might be better to introduce a flag in `MesosInfo` to explicitly ask for an IPC namespace. This would maintain backward compatibility? src/tests/containerizer/isolator_tests.cpp (line 107) <https://reviews.apache.org/r/53688/#comment228476> s/readPath/readValue src/tests/containerizer/isolator_tests.cpp (line 196) <https://reviews.apache.org/r/53688/#comment228478> We should add a comment here to explain the logic being used to test the creation of a new IPC namespace. It wasn't obvious to me on the first pass till learnt about the fact that certain /proc interfaces are unique to each IPC namespace: http://man7.org/linux/man-pages/man7/namespaces.7.html src/tests/containerizer/isolator_tests.cpp (line 198) <https://reviews.apache.org/r/53688/#comment228484> Why do we need the `filesystem/linux` isolator? src/tests/containerizer/isolator_tests.cpp (line 207) <https://reviews.apache.org/r/53688/#comment228479> s/pid/ipc src/tests/containerizer/isolator_tests.cpp (line 221) <https://reviews.apache.org/r/53688/#comment228482> Might be useful to store `static_cast<uint64_t>(::getpid())` at the begining of the test. Seems to be used at multiple places. - Avinash sridharan On Dec. 2, 2016, 5:43 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53688/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2016, 5:43 p.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-6557 > https://issues.apache.org/jira/browse/MESOS-6557 > > > Repository: mesos > > > Description > ------- > > Implement a namespace/ipc isolator. > > > Diffs > ----- > > src/CMakeLists.txt ea6e399c40d3b2cda195091dc7e74230ff3f68fd > src/Makefile.am 7750ed756d60aa61225667c129df35c6ec70f239 > src/slave/containerizer/mesos/containerizer.cpp > 7dde6fc7c20ecd3543891e7d33230d0eaf9460a2 > src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp PRE-CREATION > src/tests/containerizer/isolator_tests.cpp > da4627846730abd3a817c3d538ff5676c3c1ef45 > > Diff: https://reviews.apache.org/r/53688/diff/ > > > Testing > ------- > > Make check on Fedora 24. > > > Thanks, > > James Peach > >
