----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71172/#review216976 -----------------------------------------------------------
Ship it! This part of code is fragile. Probably we need a unit test. The complication comes from the factor that we only checkpointed partial information. Probably we should just check point all containers. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp Lines 1445-1457 (original), 1447-1472 (patched) <https://reviews.apache.org/r/71172/#comment304202> Can review this patch https://reviews.apache.org/r/65987/ again and make sure there is no similar logic mistake in other methods src/slave/containerizer/mesos/isolators/network/cni/cni.cpp Lines 1445-1457 (original), 1447-1472 (patched) <https://reviews.apache.org/r/71172/#comment304203> another option is: ``` if (isNestedContainer) { if (!infos.contains(containerId) || infos[containerId]->joinsParentsNetwork) { return status(containerId.parent()); } } if (!infos.contains(containerId)) { return ContainerStatus(); } ``` up to you to pick the style that is easier to read. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp Lines 1448-1461 (patched) <https://reviews.apache.org/r/71172/#comment304201> if a nested container has its own network namespace, I guess its INFO will be checkpointed? - Gilbert Song On July 29, 2019, 1:40 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71172/ > ----------------------------------------------------------- > > (Updated July 29, 2019, 1:40 a.m.) > > > Review request for mesos, Andrei Budnik and Gilbert Song. > > > Bugs: MESOS-9868 > https://issues.apache.org/jira/browse/MESOS-9868 > > > Repository: mesos > > > Description > ------- > > Looked up parent's IP for the non-checkpointed nested containers. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5 > > > Diff: https://reviews.apache.org/r/71172/diff/1/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Qian Zhang > >
