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 45082: Implemented cleanup() method of "network/cni" isolator.
Date Wed, 30 Mar 2016 17:04:00 GMT

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


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