mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhang...@cn.ibm.com>
Subject Re: Review Request 44706: Implemented isolate() method of "network/cni" isolator.
Date Thu, 24 Mar 2016 06:58:07 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message