----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44706/#review125825 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 353) <https://reviews.apache.org/r/44706/#comment188699> Should we return a Failure here instead? src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 395) <https://reviews.apache.org/r/44706/#comment188702> There's a problem with the current code. 'collect' (in isolate()) on 'attach' will fail if any of the call to the plugin (ADD) fails. And we'll call 'cleanup' right after that, it's likely that another call to the plugin (ADD) is still pending. I don't think calling DEL while an ADD is still pending is gonna work. Therefore, I think we should use 'await' in isolate instead of 'collect' so that we can make sure all of them are in termimal state before we return the result. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 450) <https://reviews.apache.org/r/44706/#comment188698> You want to read 'stderr' as well. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 525) <https://reviews.apache.org/r/44706/#comment188701> Could you please add a comment about the fact that destroy cannot happen in the middle of attach and `_attach` because the containerizer will wait for isolate to finish before destroying the container. Also, add a CHECK(infos.container(containerId)); - Jie Yu On March 26, 2016, 2:53 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44706/ > ----------------------------------------------------------- > > (Updated March 26, 2016, 2:53 p.m.) > > > Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu. > > > Bugs: MESOS-4759 > https://issues.apache.org/jira/browse/MESOS-4759 > > > Repository: mesos > > > Description > ------- > > Implemented isolate() method of "network/cni" isolator. > > > Diffs > ----- > > src/CMakeLists.txt 7bda2ac684f38da94e334f0cef843614687ae4aa > src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp > b1b7205f4f10b6dc256fcc4ecb3210105c1240b4 > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 7cda5715814a0cfc4b394eb04437831e6dc44e3f > src/slave/containerizer/mesos/isolators/network/cni/paths.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/network/cni/paths.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/44706/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Qian Zhang > >
