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 Tue, 29 Mar 2016 10:10:36 GMT


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


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