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

Reply via email to