mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 47463: Added documentation for `network/cni` isolator.
Date Wed, 18 May 2016 18:39:45 GMT

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


Fix it, then Ship it!




LGTM. I'm assuming Jie already verified the technical correctness.


docs/cni.md (line 5)
<https://reviews.apache.org/r/47463/#comment198385>

    Should we make it explicit that this is not supported for DockerContainerizer?



docs/cni.md (line 36)
<https://reviews.apache.org/r/47463/#comment198375>

    s/the users/users/



docs/cni.md (lines 84 - 85)
<https://reviews.apache.org/r/47463/#comment198377>

    The second sentence "The operator..." seems redundant? I suggest to kill it.



docs/cni.md (line 94)
<https://reviews.apache.org/r/47463/#comment198378>

    s/An important point to note is/Note/



docs/cni.md (lines 100 - 101)
<https://reviews.apache.org/r/47463/#comment198379>

    What happens if a network that is in use (by containers) is removed from the config and
agent restarted? What if the network config is updated? Is that all safe?



docs/cni.md (lines 114 - 119)
<https://reviews.apache.org/r/47463/#comment198382>

    Do all these configs result in the same networking behavior?
    
    -- network/cni flag is disabled
    -- NetworkInfo is not set and flag is enabled
    --- NetworkInfo is not set and flag is disabled
    -- NetworkInfo is set but name is not and flag is enabled
    -- NetworkInfo is set but name is not and flag is disabled
    
    -- all the above options with 0.28.0; without the flag ofcourse



docs/cni.md (line 197)
<https://reviews.apache.org/r/47463/#comment198384>

    s/provisioner/provider/ ?



docs/cni.md (line 200)
<https://reviews.apache.org/r/47463/#comment198386>

    s/of frameworks/of a framework/



docs/cni.md (line 247)
<https://reviews.apache.org/r/47463/#comment198387>

    and vice versa unless the executor uses HTTP API?



docs/cni.md (line 255)
<https://reviews.apache.org/r/47463/#comment198388>

    s/existing// ?


- Vinod Kone


On May 18, 2016, 1:31 p.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> -----------------------------------------------------------
> 
> (Updated May 18, 2016, 1:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, Kevin Klues, Neil Conway, Qian Zhang,
and Vinod Kone.
> 
> 
> Bugs: MESOS-4771
>     https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -----
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> -------
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


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