----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45082/#review126130 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 625) <https://reviews.apache.org/r/45082/#comment189029> space after `foreachkey`. Please fix all occurances. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 629) <https://reviews.apache.org/r/45082/#comment189028> Should we use 'await' here to make sure all plugin DEL finishes/failed before returnning result? You can do: ``` return await(futures) .then(_cleanup); ``` No need for onAny here since await is guaranteed to have a suceeded future. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 631) <https://reviews.apache.org/r/45082/#comment189031> Please assign the parameters here. You're off by 1 space. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 685) <https://reviews.apache.org/r/45082/#comment189030> You can use `s->status()` and `s->out().get()` here. ALso, no need for process:: namespace prefix. src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 739) <https://reviews.apache.org/r/45082/#comment189034> I think in `_cleanup`, I think if any of the plugin DEL succeeds, we should remove it's corresponding directory so that we don't retry DEL again. Otherwise, we should not so that during recover, we can retry it's deletion. I think we should not umount unless all plugin DEL succeeds. - Jie Yu On March 30, 2016, 8:36 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45082/ > ----------------------------------------------------------- > > (Updated March 30, 2016, 8:36 a.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 cleanup() method of "network/cni" isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp > 873e0c52475f4868e611bd24a6782ad5eb261a99 > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > 1c8e231813c0579b79681c5d18b1f799a727ead7 > > Diff: https://reviews.apache.org/r/45082/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Qian Zhang > >
