> On March 29, 2016, 1:33 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 354 > > <https://reviews.apache.org/r/44706/diff/4/?file=1315484#file1315484line354> > > > > Should we return a Failure here instead?
I think we should not return a Failure because there can be containers which does not opt in CNI networks in which case "network/cni" isolator should act as a no-op. > On March 29, 2016, 1:33 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 396 > > <https://reviews.apache.org/r/44706/diff/4/?file=1315484#file1315484line396> > > > > 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. Agree, will fix it soon. > On March 29, 2016, 1:33 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, line 451 > > <https://reviews.apache.org/r/44706/diff/4/?file=1315484#file1315484line451> > > > > You want to read 'stderr' as well. But as per CNI spec (https://github.com/appc/cni/blob/master/SPEC.md#result), CNI plugins will never write to "stderr", they will always write to "stdout" in both success and failure cases. - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44706/#review125825 ----------------------------------------------------------- On March 29, 2016, 3:04 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44706/ > ----------------------------------------------------------- > > (Updated March 29, 2016, 3:04 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 > >
