Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [47463]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 19, 2016, 8:05 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 19, 2016, 8:05 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Avinash sridharan


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, line 247
> > 
> >
> > and vice versa unless the executor uses HTTP API?
> 
> Avinash sridharan wrote:
> Even with the HTTP API the requirement of routeability (in both 
> directions) would still hold right?
> 
> Vinod Kone wrote:
> With HTTP API, agent doesn't directly make connections to the executor. 
> It just sends messages on the connection opened by the executor.

Well .. TCP is bi-directional so you will still need routeability in both 
directions at the IP layer?


- Avinash


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


On May 19, 2016, 8:05 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 19, 2016, 8:05 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Vinod Kone


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, line 247
> > 
> >
> > and vice versa unless the executor uses HTTP API?
> 
> Avinash sridharan wrote:
> Even with the HTTP API the requirement of routeability (in both 
> directions) would still hold right?

With HTTP API, agent doesn't directly make connections to the executor. It just 
sends messages on the connection opened by the executor.


- Vinod


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


On May 19, 2016, 8:05 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 19, 2016, 8:05 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Avinash sridharan

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

(Updated May 19, 2016, 8:05 p.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, Kevin Klues, Neil Conway, 
Qian Zhang, and Vinod Kone.


Changes
---

Addressing Neil's comments.


Bugs: MESOS-4771
https://issues.apache.org/jira/browse/MESOS-4771


Repository: mesos


Description
---

Added documentation for `network/cni` isolator.


Diffs (updated)
-

  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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Neil Conway

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




docs/cni.md (line 4)


remove comma after MesosContainerizer



docs/cni.md (line 10)


why emphasis on "probably"?



docs/cni.md (line 140)


Remove space before colon



docs/cni.md (line 166)


"container's"


- Neil Conway


On May 19, 2016, 6:38 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 19, 2016, 6:38 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Avinash sridharan

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

(Updated May 19, 2016, 6:38 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 (updated)
-

  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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Avinash sridharan


> On May 19, 2016, 2:47 p.m., Neil Conway wrote:
> > docs/cni.md, line 3
> > 
> >
> > Phrasing is awkward: who is "we", and when "have we introduced" the CNI 
> > isolator? Better would be: "The Mesos `network/cni` isolator allows 
> > containers launched using the `MesosContainerizer` to be attached to 
> > several different types of IP networks."

I revamped the introduction completely. Would be great if you can have another 
look at it.


> On May 19, 2016, 2:47 p.m., Neil Conway wrote:
> > docs/cni.md, line 40
> > 
> >
> > Saying "network namespace" three times in one sentence seems 
> > regrettable.

Re-wrote this paragraph.


- Avinash


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


On May 19, 2016, 6:38 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 19, 2016, 6:38 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Avinash sridharan


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, lines 100-101
> > 
> >
> > 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?
> 
> Avinash sridharan wrote:
> Thanks !! This is an important point that I missed. Currently we are not 
> checkpointing the CNI configuration for a given container. This implies that 
> if the current CNI config is modified or deleted, it won't affect container 
> operation, or even Agent restart, but it will impact deletion of containers. 
> Since the plugin information in the CNI config might have changed, resulting 
> in the plugin throwing an error when it tries to delete the veth and release 
> IP address from the container network namespace.
> 
> I will add a `### Limiations` section describing this behavior. We have 
> https://issues.apache.org/jira/browse/MESOS-5310 to address this issue.

Added a ### Limitations section.


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, line 247
> > 
> >
> > and vice versa unless the executor uses HTTP API?

Even with the HTTP API the requirement of routeability (in both directions) 
would still hold right?


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, line 255
> > 
> >
> > s/existing// ?

s/existing/egressing


- Avinash


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


On May 19, 2016, 6:38 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 19, 2016, 6:38 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Neil Conway

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




docs/cni.md (line 3)


Phrasing is awkward: who is "we", and when "have we introduced" the CNI 
isolator? Better would be: "The Mesos `network/cni` isolator allows containers 
launched using the `MesosContainerizer` to be attached to several different 
types of IP networks."



docs/cni.md (line 40)


Saying "network namespace" three times in one sentence seems regrettable.



docs/cni.md (line 57)


"and" twice.


- Neil Conway


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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-19 Thread Tomasz Janiszewski

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




docs/cni.md (line 5)


Add link to [Mesos Containerizer](mesos-containerizer.md) or References 
section with links to other docs


- Tomasz Janiszewski


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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-18 Thread Avinash sridharan


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, line 5
> > 
> >
> > Should we make it explicit that this is not supported for 
> > DockerContainerizer?

I don't think we should be explicit about the `DockerContainerizer` here. I 
mention in the introduction that the `network/cni` isolator is for the 
`MesosContainerizer`. The title also states that this is for Mesos containers. 
We are also going to have a separate page for `Networking with 
DockerContainerizer`. Was also thinking that in the home page we can link this 
page using the title "Networking with MesosContainerizer".


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, lines 100-101
> > 
> >
> > 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?

Thanks !! This is an important point that I missed. Currently we are not 
checkpointing the CNI configuration for a given container. This implies that if 
the current CNI config is modified or deleted, it won't affect container 
operation, or even Agent restart, but it will impact deletion of containers. 
Since the plugin information in the CNI config might have changed, resulting in 
the plugin throwing an error when it tries to delete the veth and release IP 
address from the container network namespace.

I will add a `### Limiations` section describing this behavior. We have 
https://issues.apache.org/jira/browse/MESOS-5310 to address this issue.


> On May 18, 2016, 6:39 p.m., Vinod Kone wrote:
> > docs/cni.md, lines 114-119
> > 
> >
> > 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

Yes. In all the above cases the behavior will be that the container will use 
the host network namespace, effectively joining the host naetwork.


- Avinash


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


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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-18 Thread Vinod Kone

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


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



docs/cni.md (line 36)


s/the users/users/



docs/cni.md (lines 84 - 85)


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



docs/cni.md (line 94)


s/An important point to note is/Note/



docs/cni.md (lines 100 - 101)


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)


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)


s/provisioner/provider/ ?



docs/cni.md (line 200)


s/of frameworks/of a framework/



docs/cni.md (line 247)


and vice versa unless the executor uses HTTP API?



docs/cni.md (line 255)


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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [47463]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 11:27 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 11:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Avinash sridharan

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

(Updated May 17, 2016, 11:27 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.


Changes
---

Fixed review comments.


Bugs: MESOS-4771
https://issues.apache.org/jira/browse/MESOS-4771


Repository: mesos


Description
---

Added documentation for `network/cni` isolator.


Diffs (updated)
-

  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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Avinash sridharan


> On May 17, 2016, 10:17 p.m., Jie Yu wrote:
> > docs/cni.md, line 133
> > 
> >
> > I would mention that nsenter can also be used to achieve the same goal. 
> > One just need to get the pid (which can be get from the bind mount as well).
> > 
> > You don't have to show the commands, just mention it in the text.

Mentioned MESOS-5278 instead of `nsenter`.


- Avinash


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


On May 17, 2016, 11:27 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 11:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Jie Yu

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


Fix it, then Ship it!





docs/cni.md (line 2)


one blank line between the paragraph and the subtitle. Please fix all other 
places.



docs/cni.md (line 133)


I would mention that nsenter can also be used to achieve the same goal. One 
just need to get the pid (which can be get from the bind mount as well).

You don't have to show the commands, just mention it in the text.



docs/cni.md (line 176)


I would typically introduce a blank line above in such cases. Please fix 
them all.



docs/cni.md (line 190)


2 lines part between subsections. Please fix them all.



docs/cni.md (line 221)


kill this line.



docs/cni.md (line 223)


Ditto on adding a new line above.


- Jie Yu


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Avinash sridharan


> On May 17, 2016, 7:47 p.m., Michael Cambria wrote:
> > docs/cni.md, line 54
> > 
> >
> > This reads like the plugin is always responsible for creating veth 
> > pairs.  Is this always the case or just applicable for e.g. bridge network 
> > types?

The plugin is always responsible for creating the `veth` pair.


- Avinash


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


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Michael Cambria

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




docs/cni.md (line 54)


This reads like the plugin is always responsible for creating veth pairs.  
Is this always the case or just applicable for e.g. bridge network types?


- Michael Cambria


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Michael Cambria

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




docs/cni.md (line 23)


macvlan network is still listed in the Networking Recipes, but this section 
has been reoved.


- Michael Cambria


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Michael Cambria

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



Networking Recipes still lists macvlan as a recipe, prior edit removed this 
section

- Michael Cambria


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [47463]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Tomasz Janiszewski

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




docs/cni.md (line 47)


Missing space before `(`


- Tomasz Janiszewski


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> 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
> 
>



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Avinash sridharan

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

(Updated May 17, 2016, 3:22 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.


Changes
---

Added reviewers.


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 (updated)
---

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



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Avinash sridharan

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

(Updated May 17, 2016, 3:06 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Updated JIRA.


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.


Thanks,

Avinash sridharan



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Avinash sridharan

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

(Updated May 17, 2016, 3:04 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Changes
---

Remove the "macvlan" section.


Repository: mesos


Description
---

Added documentation for `network/cni` isolator.


Diffs (updated)
-

  docs/cni.md PRE-CREATION 

Diff: https://reviews.apache.org/r/47463/diff/


Testing
---

Build the documentation website and verified the rendering.


Thanks,

Avinash sridharan



Re: Review Request 47463: Added documentation for `network/cni` isolator.

2016-05-17 Thread Avinash sridharan

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

(Updated May 17, 2016, 3:02 p.m.)


Review request for mesos, Jie Yu and Qian Zhang.


Repository: mesos


Description
---

Added documentation for `network/cni` isolator.


Diffs
-

  docs/cni.md PRE-CREATION 

Diff: https://reviews.apache.org/r/47463/diff/


Testing (updated)
---

Build the documentation website and verified the rendering.


Thanks,

Avinash sridharan