----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53586/#review155617 -----------------------------------------------------------
Fix it, then Ship it! src/slave/containerizer/mesos/containerizer.cpp (line 162) <https://reviews.apache.org/r/53586/#comment225617> Let's move this function to `src/slave/containerizer/mesos/utils.hpp|cpp` src/slave/containerizer/mesos/containerizer.cpp (line 177) <https://reviews.apache.org/r/53586/#comment225622> for primitive types, we don't use const ref src/slave/containerizer/mesos/launch.cpp (line 412) <https://reviews.apache.org/r/53586/#comment225635> insert a blank line below src/tests/containerizer/nested_mesos_containerizer_tests.cpp (line 363) <https://reviews.apache.org/r/53586/#comment225637> Why this? Can you just let the agent detect the resources on the box? src/tests/containerizer/nested_mesos_containerizer_tests.cpp (lines 401 - 403) <https://reviews.apache.org/r/53586/#comment225639> You can use 'createDockerImage' src/tests/containerizer/nested_mesos_containerizer_tests.cpp (lines 416 - 418) <https://reviews.apache.org/r/53586/#comment225640> Can you use 'createContainerInfo' src/tests/containerizer/nested_mesos_containerizer_tests.cpp (lines 433 - 436) <https://reviews.apache.org/r/53586/#comment225661> Is this necessary? I think once we get a TASK_RUNNING, the subprocess should have been already launched? - Jie Yu On Nov. 10, 2016, 8:25 p.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53586/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2016, 8:25 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-6543 > https://issues.apache.org/jira/browse/MESOS-6543 > > > Repository: mesos > > > Description > ------- > > Until we switch over to the default (a.k.a. "pod" executor) for > launching command tasks, we need to special case which `pid` we use > for entering the `mnt` namespace of a parent container. Specifically, > we need to enter the `mnt` namespace of the process representing the > command task itself, not the `mnt` namespace of the `init` process of > the container or the `executor` of the container because these run in > the same `mnt` namespace as the agent (not the task). > > Unfortunately, there is no easy way to get the `pid` of tasks launched > with the command executor because we only checkpoint the `pid` of the > `init` process of these containers. For now, we compensate for this by > simply walking the process tree from the container's `init` process up > to 2-levels down (where the task process would exist) and look to see > if any process along the way has a different `mnt` namespace. If it > does, we return a reference to its `pid` as the `pid` for entering the > `mnt` namespace of the container. Otherwise, we return the `init` > process's `pid`. > > We then pass this pid to the `mesos-containerizer launch` binary and > have it set the namespace, rather than letting the `ns::clone()` call > do it for us. This is important because otherwise we wouldn't be able > to find the `mesos-containerizer launch` itself (it only exists in the > host mount namespace!). > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > 44225ebf63d8dd93be9b60fff496c74dc6c3a5ad > src/slave/containerizer/mesos/launch.hpp > 8b23c1b6df6bc1fdd987af5a4469664356e7f27a > src/slave/containerizer/mesos/launch.cpp > 377a9d94aa780ab598b1c2034c10ce25a4e02cbe > src/slave/containerizer/mesos/utils.hpp > f24215b52b5fa95321b15b57468660aab4d1aefc > src/slave/containerizer/mesos/utils.cpp > 237aea4510dc80e9b3d39d577aa702dfb1f554db > src/tests/containerizer/nested_mesos_containerizer_tests.cpp > e6c690c411f57138207044f31b4816bd4090c1b7 > > Diff: https://reviews.apache.org/r/53586/diff/ > > > Testing > ------- > > make -j check > sudo src/mesos-tests > > > Thanks, > > Kevin Klues > >
