mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 44706: Implemented isolate() method of "network/cni" isolator.
Date Tue, 29 Mar 2016 05:33:23 GMT

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 353)
<https://reviews.apache.org/r/44706/#comment188699>

    Should we return a Failure here  instead?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 395)
<https://reviews.apache.org/r/44706/#comment188702>

    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.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 450)
<https://reviews.apache.org/r/44706/#comment188698>

    You want to read 'stderr' as well.



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 525)
<https://reviews.apache.org/r/44706/#comment188701>

    Could you please add a comment about the fact that destroy cannot happen in the middle
of attach and `_attach` because the containerizer will wait for isolate to finish before destroying
the container.
    
    Also, add a CHECK(infos.container(containerId));


- Jie Yu


On March 26, 2016, 2:53 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44706/
> -----------------------------------------------------------
> 
> (Updated March 26, 2016, 2:53 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