> On March 21, 2016, 11:57 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp, lines 328-336
> > <https://reviews.apache.org/r/44706/diff/2/?file=1307515#file1307515line328>
> >
> >     why do we need this dispatch ? The dispatch is to itself, so seems a 
> > bit odd. Can we invoke the `subprocess` for the plugin in _connect directly 
> > ?
> >     
> >     Basically not sure why we need connect & _connect.
> 
> Qian Zhang wrote:
>     The idea is similar with how `MesosContainerizer` call each isolator: 
> https://github.com/apache/mesos/blob/0.28.0/src/slave/containerizer/mesos/containerizer.cpp#L1171:L1181
>     
>     I'd like to handle each call to a CNI plugin in a separate libprocess 
> dispatch event, so that's why I call `connect` via `dispatch`.
> 
> Avinash sridharan wrote:
>     In the example you gave the `isolator` does a dispatch on an isolator 
> process. So the `MesosContainerizer` effectively does a dispatch on a 
> separate libprocess thread, which is the intended behavior. Here it seems a 
> bit odd that dispatch is scheduling an event on itself. Functionally nothing 
> wrong, but this is not idiomatic and we should avoid this.

Actually, when I wrote these code, instead of using `dispatch()`, I was 
thinking to directly call `subprocess()` to invoke CNI plugin in the 
`foreachvalue(NetworkInfo& networkInfo, infos[containerId]->networkInfos)` 
loop, and then use `defer()` to checkpoint the output return by the CNI plugin. 
However, the problem with this approach is, say a container wants to join two 
CNI networks (e.g., net1 and net2), if `subprocess()` for net1 succeeds, but 
`subprocess()` for net2 fails for a reason (it will return an `Error`), in this 
case we need to directly return a `Failure` in `isolate()`, but the issue is no 
one will be responsible for checkpointing the output for net1 which will cause 
IP leak since when we cleanup this container, we will not call CNI plugin to 
disconnect the container from net1 due to no checkpointed data. That's why I'd 
like to handle each call to a CNI plugin in a separate libprocess dispatch 
event, then the failure of one call will not affect any other calls.


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44706/#review124554
-----------------------------------------------------------


On March 23, 2016, 10:22 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 10:22 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 0bd7a978306488b687a7e2eeeb8a5c9766d43548 
>   src/Makefile.am 6552e48eab2708a28dd69adba3ec759cb5aeca4c 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 3878a7e85fe24175b3bd5e3a6268cf32a07f2d8b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 9552312f9ba1e4df6564cfb737cc41e041cf4407 
>   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
> 
>

Reply via email to