----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53354/#review154782 -----------------------------------------------------------
See my comments for the filesystem isolator. You probably should do a scan on other isolators as well to make sure the isolator behave correctly for debug class container, and do proper validation. src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (line 330) <https://reviews.apache.org/r/53354/#comment224470> Can you make sure if the container is in DEBUG class, the rootfs is not set? src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 383 - 399) <https://reviews.apache.org/r/53354/#comment224471> Do you need to do this if the container class is DEBUG? Since you are entering the mount namespace of the parent and the parent container has already done this, do you need this? src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (line 408) <https://reviews.apache.org/r/53354/#comment224472> I think with the validation I mentioned above, you probably should add a CHECK to make sure it's not a debug container. src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (line 435) <https://reviews.apache.org/r/53354/#comment224473> Do we support volume for debug container? We probably should add a validation too. src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 608 - 610) <https://reviews.apache.org/r/53354/#comment224475> Can you add a note here saying that debug class container has to be a nested contaienr. Therefore, update won't be called for debug container. - Jie Yu On Nov. 1, 2016, 10:29 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53354/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2016, 10:29 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6464 > https://issues.apache.org/jira/browse/MESOS-6464 > > > Repository: mesos > > > Description > ------- > > The namespace-related isolators now do different things depending on > whether they are launching a "normal" nested container or a "debug" > nested container. Normal nested containers clone a new mount namespace > as well as a new pid namespace. Debug nested cotnainers do not -- they > simply inherit these namespaces from their parent. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > 67cc595278f124cdf518d2f4fcfb257439f067e2 > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp > af9f3736b487b595e8768e56ce60dc4823db28a1 > src/slave/containerizer/mesos/isolators/filesystem/linux.cpp > df16b8fee6799a69c7d96f33a5049bd9787c48f5 > src/slave/containerizer/mesos/isolators/filesystem/shared.cpp > a1283e5ee92c916baaf9fca8ce314d597e8421b3 > src/slave/containerizer/mesos/isolators/gpu/isolator.cpp > e3756c920081f2944bf4b640edf0a83f42784586 > src/slave/containerizer/mesos/isolators/namespaces/pid.cpp > 0d9ec57d9aa83bcc6cc2e5a8d75f2e2251179b1b > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 939142e36b926d9e4201d35dedd25e32e9f8c63c > src/slave/containerizer/mesos/isolators/network/port_mapping.cpp > 48202fb5bf1ede71b80760844c6d8a36ca7c700c > src/slave/containerizer/mesos/isolators/volume/image.cpp > 210e67ad0d84f52135e77184f21e574c9e31628d > src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp > 7b976d29226c3e0a4d52922e9d2f7e685de72297 > src/slave/containerizer/mesos/linux_launcher.cpp > 0305d14c1f791c93edcd3b32786b483b15f40a2d > src/tests/containerizer/nested_mesos_containerizer_tests.cpp > e6c690c411f57138207044f31b4816bd4090c1b7 > > Diff: https://reviews.apache.org/r/53354/diff/ > > > Testing > ------- > > make -j check > (Some tests are still fialing though -- need to debug) > > > Thanks, > > Kevin Klues > >
