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 54717: Modified `network/cni` isolator for dynamic addition of CNI networks.
Date Thu, 15 Dec 2016 22:32:03 GMT

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 596 - 599)
<https://reviews.apache.org/r/54717/#comment230361>

    I would probably just get rid of this check here. It looks to me that even if we did the
check here, things might change in 'attach'. Why bother checking it here?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1152 - 1172)
<https://reviews.apache.org/r/54717/#comment230363>

    I would introduce a helper:
    ```
    Try<JSON::Object> networkConfigJson = getNetworkConfigJson(networkName);
    ```
    
    The helper will cross check the name in the config with the cache, and do a reload if
necessary.


- Jie Yu


On Dec. 14, 2016, 12:28 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54717/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 12:28 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-6567
>     https://issues.apache.org/jira/browse/MESOS-6567
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If the `network/cni` isolator sees a cache-miss during the `prepare`
> phase, it will try to look for the CNI network on disk before giving
> up. This allows for the dynamic addition of CNI networks without the
> need for agent restart.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp b8fc755a8dd4757d904f7e97a71d3cf7f29d2033

>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp ddb4d33e9ecd0ffd118e8e68cb7ec90346b08049

> 
> Diff: https://reviews.apache.org/r/54717/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Manual testing with addition of a non-existent CNI configuration.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


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