Re: Review Request 42516: Add support for user-defined networks.

2016-04-07 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On March 31, 2016, 11:17 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 31, 2016, 11:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-31 Thread Ezra Silvera

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

(Updated March 31, 2016, 11:17 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Fix comments from Haosdent Huang and Guangya Liu


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-30 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 March 30, 2016, 7:52 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 30, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-30 Thread haosdent huang

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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (line 556)


I think could remove `this->`


- haosdent huang


On March 30, 2016, 7:52 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 30, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-30 Thread Guangya Liu

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


Fix it, then Ship it!




Ship It!


src/docker/docker.cpp (lines 568 - 569)


I think this can be simplified as:

return Failure("Only a single network can be defined in Docker run");

No need to add period to the end.


- Guangya Liu


On 三月 30, 2016, 7:52 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 30, 2016, 7:52 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-30 Thread Ezra Silvera

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

(Updated March 30, 2016, 7:52 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Fixed the  error message in case multiple networks are specified during "run"


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera


> On March 29, 2016, 6:01 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 568
> > 
> >
> > Maybe s/ Found .../Found multiple networks specified for this 
> > container. We can only attach docker containers to a single network/
> > 
> > We try to avoid variable names in `Error` since it becomes cryptic. 
> > Also, I guess the limiation is of the `DockerContainerizer` rather than 
> > `Docker`?

No, the limitation is from Docker. During "run/create" container you can 
specify only single netwrok. Only this command is supported n the contenerizer 
as it consume resources. From Docker you can later on call "network connect 
..." which allows you to connect the containe to aditional/multiple network. 
1) The "connect" command is not supported by the contenerizer
2) The implementation we now reviw is for the "run" command for which docker 
supports only a single network.


- Ezra


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


On March 29, 2016, 4:33 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 March 29, 2016, 4:33 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Avinash sridharan

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




src/docker/docker.cpp (line 568)


Maybe s/ Found .../Found multiple networks specified for this container. We 
can only attach docker containers to a single network/

We try to avoid variable names in `Error` since it becomes cryptic. Also, I 
guess the limiation is of the `DockerContainerizer` rather than `Docker`?


- Avinash sridharan


On March 29, 2016, 4:33 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 4:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera

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

(Updated March 29, 2016, 4:33 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Don't allow multiple network_info elements during "docker run"


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera


> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.
> 
> Ezra Silvera wrote:
> I'm not sure I follow you point ...  Indeed in Docker run it's not 
> allowed to specify multiple networks this is the reason we use the first 
> element in the  array.  In general if you are not going to populate this 
> structure through inspect I'm not sure you even need an array there ..
> 
> Ezra Silvera wrote:
> Let me know if you think we should do something specific in this code, or 
> it was just a general remark about NetworkInfo
> 
> Avinash sridharan wrote:
> Should have been clearer. The `NetworkInfo` field in `ContainerInfo` is a 
> repeated field. So the framework can potentially ask the container to join 
> multiple networks (this is supported in CNI 
> https://issues.apache.org/jira/browse/MESOS-4641) . We should explicitly 
> disallow it over here. So may be an `if` condition :
> if (containerInfo.network_infos_size() > 1) {
>return Failure();
> }
> 
> Avinash sridharan wrote:
> Just to add. The reason I think raising this error is important is that 
> we should explicitly inform the Framework that they are trying to do 
> something that is not supported, rather than making an assumption that the 
> first network specified by the `Framework` was the network it intended the 
> container to join. This is ofcourse valid only in case multiple user-networks 
> have been specified.

I fully agree. I currently print just a warning   I also think we should fail 
the operation!


- Ezra


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


On March 29, 2016, 11:08 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Avinash sridharan


> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.
> 
> Ezra Silvera wrote:
> I'm not sure I follow you point ...  Indeed in Docker run it's not 
> allowed to specify multiple networks this is the reason we use the first 
> element in the  array.  In general if you are not going to populate this 
> structure through inspect I'm not sure you even need an array there ..
> 
> Ezra Silvera wrote:
> Let me know if you think we should do something specific in this code, or 
> it was just a general remark about NetworkInfo
> 
> Avinash sridharan wrote:
> Should have been clearer. The `NetworkInfo` field in `ContainerInfo` is a 
> repeated field. So the framework can potentially ask the container to join 
> multiple networks (this is supported in CNI 
> https://issues.apache.org/jira/browse/MESOS-4641) . We should explicitly 
> disallow it over here. So may be an `if` condition :
> if (containerInfo.network_infos_size() > 1) {
>return Failure();
> }

Just to add. The reason I think raising this error is important is that we 
should explicitly inform the Framework that they are trying to do something 
that is not supported, rather than making an assumption that the first network 
specified by the `Framework` was the network it intended the container to join. 
This is ofcourse valid only in case multiple user-networks have been specified.


- Avinash


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


On March 29, 2016, 11:08 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Avinash sridharan


> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.
> 
> Ezra Silvera wrote:
> I'm not sure I follow you point ...  Indeed in Docker run it's not 
> allowed to specify multiple networks this is the reason we use the first 
> element in the  array.  In general if you are not going to populate this 
> structure through inspect I'm not sure you even need an array there ..
> 
> Ezra Silvera wrote:
> Let me know if you think we should do something specific in this code, or 
> it was just a general remark about NetworkInfo

Should have been clearer. The `NetworkInfo` field in `ContainerInfo` is a 
repeated field. So the framework can potentially ask the container to join 
multiple networks (this is supported in CNI 
https://issues.apache.org/jira/browse/MESOS-4641) . We should explicitly 
disallow it over here. So may be an `if` condition :
if (containerInfo.network_infos_size() > 1) {
   return Failure();
}


- Avinash


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


On March 29, 2016, 11:08 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 11:08 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera

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

(Updated March 29, 2016, 11:08 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Changes
---

Typo Warnning -->  Warning


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera


> On March 29, 2016, 8:54 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 567
> > 
> >
> > Can you please add some comments here: If there are multiple network 
> > infos, the docker containerizer will only take the first network info.
> > 
> > It is a tricky part here, if an operator defined many network infos and 
> > only the first one do not have network name, error will return here.
> 
> Ezra Silvera wrote:
> We need to resolve this "many network infos" once and for all :-)  
> In Docker you can't specify more then a single network during the 
> run/create command. We are handling here the run command so in this case we 
> expect a single NetworkInfo element, anything else will be an error. 
> Currently we choose to just ignore it, we can actually fail the operation for 
> elements > 1 if you think it will be more clear.
> 
> Guangya Liu wrote:
> I think that logging some warning message may be good enough if there are 
> multiple networks.
> 
> Can you please also add @jieyu as he is also tracking this issue and we 
> can get some comments from him ;-)

Add a warnning. Keep in mind that Only single network_info can be used for 
Docker run.


- Ezra


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


On March 29, 2016, 10:46 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 10:46 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera

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

(Updated March 29, 2016, 10:46 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Add warnning for multiple network_infos.


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Guangya Liu


> On 三月 29, 2016, 8:54 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 567
> > 
> >
> > Can you please add some comments here: If there are multiple network 
> > infos, the docker containerizer will only take the first network info.
> > 
> > It is a tricky part here, if an operator defined many network infos and 
> > only the first one do not have network name, error will return here.
> 
> Ezra Silvera wrote:
> We need to resolve this "many network infos" once and for all :-)  
> In Docker you can't specify more then a single network during the 
> run/create command. We are handling here the run command so in this case we 
> expect a single NetworkInfo element, anything else will be an error. 
> Currently we choose to just ignore it, we can actually fail the operation for 
> elements > 1 if you think it will be more clear.

I think that logging some warning message may be good enough if there are 
multiple networks.

Can you please also add @jieyu as he is also tracking this issue and we can get 
some comments from him ;-)


- Guangya


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


On 三月 29, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 29, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera


> On March 29, 2016, 8:54 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 567
> > 
> >
> > Can you please add some comments here: If there are multiple network 
> > infos, the docker containerizer will only take the first network info.
> > 
> > It is a tricky part here, if an operator defined many network infos and 
> > only the first one do not have network name, error will return here.

We need to resolve this "many network infos" once and for all :-)  
In Docker you can't specify more then a single network during the run/create 
command. We are handling here the run command so in this case we expect a 
single NetworkInfo element, anything else will be an error. Currently we choose 
to just ignore it, we can actually fail the operation for elements > 1 if you 
think it will be more clear.


- Ezra


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


On March 29, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Guangya Liu

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




src/docker/docker.cpp (line 567)


Can you please add some comments here: If there are multiple network infos, 
the docker containerizer will only take the first network info.

It is a tricky part here, if an operator defined many network infos and 
only the first one do not have network name, error will return here.



src/docker/docker.cpp (line 570)


blank line here.


- Guangya Liu


On 三月 29, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 29, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera


> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.
> 
> Ezra Silvera wrote:
> I'm not sure I follow you point ...  Indeed in Docker run it's not 
> allowed to specify multiple networks this is the reason we use the first 
> element in the  array.  In general if you are not going to populate this 
> structure through inspect I'm not sure you even need an array there ..

Let me know if you think we should do something specific in this code, or it 
was just a general remark about NetworkInfo


- Ezra


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


On March 29, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera

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

(Updated March 29, 2016, 8:34 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fixing comments from Guangya Liu


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera


> On March 29, 2016, 7:51 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 553-572
> > 
> >
> > I prefer the following format:
> > 
> > case ContainerInfo::DockerInfo::USER: {
> >   // User defined networks require docker version >= 1.9.0.
> >   Try validateVersion =
> > this->validateVersion(Version(1, 9, 0));
> > 
> >   if (validateVersion.isError()) {
> > return Failure("User defined networks require Docker "
> >"version 1.9.0 or higher");
> >   }
> > 
> >   if (containerInfo.network_infos_size() == 0) {
> > return Failure("No network info found in container info");
> >   }
> > 
> >   const NetworkInfo& networkInfo = containerInfo.network_infos(0);
> >   if(!networkInfo.has_name()){
> > return Failure("No network name found in network info");
> >   }
> >   network = networkInfo.name();
> >   break;
> > }
> > 
> > The main changes are:
> > 1) Add a period to the end of the comments in L554, also keep 2 spaces.
> > 2) Add a blank space after L556.
> > 2) Put `break` into the `{}` block.
> > 3) Updated indent in L559.

Thanks. Fixed.


- Ezra


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


On March 29, 2016, 7:31 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 29, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Guangya Liu

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




src/docker/docker.cpp (lines 553 - 572)


I prefer the following format:

case ContainerInfo::DockerInfo::USER: {
  // User defined networks require docker version >= 1.9.0.
  Try validateVersion =
this->validateVersion(Version(1, 9, 0));

  if (validateVersion.isError()) {
return Failure("User defined networks require Docker "
   "version 1.9.0 or higher");
  }

  if (containerInfo.network_infos_size() == 0) {
return Failure("No network info found in container info");
  }

  const NetworkInfo& networkInfo = containerInfo.network_infos(0);
  if(!networkInfo.has_name()){
return Failure("No network name found in network info");
  }
  network = networkInfo.name();
  break;
}

The main changes are:
1) Add a period to the end of the comments in L554, also keep 2 spaces.
2) Add a blank space after L556.
2) Put `break` into the `{}` block.
3) Updated indent in L559.


- Guangya Liu


On 三月 29, 2016, 7:31 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 29, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera

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

(Updated March 29, 2016, 7:31 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fix Indentation


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-29 Thread Ezra Silvera


> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > 
> >
> > We should make sure the user is not trying to specify  more than one 
> > network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
> > multiple user networks but the container can be connected to more than one 
> > network only after being started (using `docker network connect`) which is 
> > kind of useless. Docker 1.10 apparently allows you to attach container to 
> > multiple network but not using the `docker run` command, so doesn't fit the 
> > model for `DockerContainerizer`.

I'm not sure I follow you point ...  Indeed in Docker run it's not allowed to 
specify multiple networks this is the reason we use the first element in the  
array.  In general if you are not going to populate this structure through 
inspect I'm not sure you even need an array there ..


- Ezra


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Ezra Silvera


> On March 28, 2016, 3:43 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 554-572
> > 
> >
> > 2 spaces indent, you can take 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
> > reference.
> > 
> > I think that we still need update `DockerInfo` by adding `network_name` 
> > and always using `network_name` from `DockerInfo` first; if no 
> > `network_name` in `DockerInfo`, use `name` from `NetworkInfo`
> 
> Ezra Silvera wrote:
> Why? I thought the point was to use NetworkInfo instead of network_name . 
> Filling the NetworkInfo in case of user-defined network should be mandatory.
> 
> Avinash sridharan wrote:
> I agree with Ezra on this. We shouldn't have multiple fields in the 
> protobuf trying to give the same information. The idea of introducing the 
> `name` field was to specify the user-defined network. Duplicating this field 
> in Docker doesn't help.
> 
> Guangya Liu wrote:
> You can take a look at @Jie Yu's comments: I think what I am proposing is 
> that: we add a NetworkInfo.name, and if DOckerInfo.network is not set and 
> NetworkInfo.name is set, the docker containerizer will do 
> --net=.
> 
> I think that keeping a network name in `DockerInfo` is more suitable for 
> Docker containerizer.
> 
> Ezra Silvera wrote:
> You can't use DockerInfo.network because it is an enum that always has a 
> value. 
> This is why we added the network_name and everything worked perfectly 
> E2E, we then removed it because you were pointing/asking to use the new 
> NetworkInfo.
> And if we have that we field why at all we need to check the NetworkInfo 
> for ?   Those are both new fields that are going to be filled by the 
> framework.

Just to clarify the previous comment - as Avinash sridharan said - we need one 
of these fields. Let's just agree which and push this patch :-)


- Ezra


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Ezra Silvera


> On March 28, 2016, 3:43 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 554-572
> > 
> >
> > 2 spaces indent, you can take 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
> > reference.
> > 
> > I think that we still need update `DockerInfo` by adding `network_name` 
> > and always using `network_name` from `DockerInfo` first; if no 
> > `network_name` in `DockerInfo`, use `name` from `NetworkInfo`
> 
> Ezra Silvera wrote:
> Why? I thought the point was to use NetworkInfo instead of network_name . 
> Filling the NetworkInfo in case of user-defined network should be mandatory.
> 
> Avinash sridharan wrote:
> I agree with Ezra on this. We shouldn't have multiple fields in the 
> protobuf trying to give the same information. The idea of introducing the 
> `name` field was to specify the user-defined network. Duplicating this field 
> in Docker doesn't help.
> 
> Guangya Liu wrote:
> You can take a look at @Jie Yu's comments: I think what I am proposing is 
> that: we add a NetworkInfo.name, and if DOckerInfo.network is not set and 
> NetworkInfo.name is set, the docker containerizer will do 
> --net=.
> 
> I think that keeping a network name in `DockerInfo` is more suitable for 
> Docker containerizer.

You can't use DockerInfo.network because it is an enum that always has a value. 
This is why we added the network_name and everything worked perfectly E2E, we 
then removed it because you were pointing/asking to use the new NetworkInfo.
And if we have that we field why at all we need to check the NetworkInfo for ?  
 Those are both new fields that are going to be filled by the framework.


- Ezra


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Avinash sridharan


> On March 28, 2016, 3:43 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 554-572
> > 
> >
> > 2 spaces indent, you can take 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
> > reference.
> > 
> > I think that we still need update `DockerInfo` by adding `network_name` 
> > and always using `network_name` from `DockerInfo` first; if no 
> > `network_name` in `DockerInfo`, use `name` from `NetworkInfo`
> 
> Ezra Silvera wrote:
> Why? I thought the point was to use NetworkInfo instead of network_name . 
> Filling the NetworkInfo in case of user-defined network should be mandatory.
> 
> Guangya Liu wrote:
> You can take a look at @Jie Yu's comments: I think what I am proposing is 
> that: we add a NetworkInfo.name, and if DOckerInfo.network is not set and 
> NetworkInfo.name is set, the docker containerizer will do 
> --net=.
> 
> I think that keeping a network name in `DockerInfo` is more suitable for 
> Docker containerizer.

I agree with Ezra on this. We shouldn't have multiple fields in the protobuf 
trying to give the same information. The idea of introducing the `name` field 
was to specify the user-defined network. Duplicating this field in Docker 
doesn't help.


- Avinash


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Guangya Liu


> On 三月 28, 2016, 3:43 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 554-572
> > 
> >
> > 2 spaces indent, you can take 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
> > reference.
> > 
> > I think that we still need update `DockerInfo` by adding `network_name` 
> > and always using `network_name` from `DockerInfo` first; if no 
> > `network_name` in `DockerInfo`, use `name` from `NetworkInfo`
> 
> Ezra Silvera wrote:
> Why? I thought the point was to use NetworkInfo instead of network_name . 
> Filling the NetworkInfo in case of user-defined network should be mandatory.
> 
> Avinash sridharan wrote:
> I agree with Ezra on this. We shouldn't have multiple fields in the 
> protobuf trying to give the same information. The idea of introducing the 
> `name` field was to specify the user-defined network. Duplicating this field 
> in Docker doesn't help.

You can take a look at @Jie Yu's comments: I think what I am proposing is that: 
we add a NetworkInfo.name, and if DOckerInfo.network is not set and 
NetworkInfo.name is set, the docker containerizer will do 
--net=.

I think that keeping a network name in `DockerInfo` is more suitable for Docker 
containerizer.


- Guangya


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


On 三月 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Ezra Silvera


> On March 28, 2016, 3:43 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 554-572
> > 
> >
> > 2 spaces indent, you can take 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
> > reference.
> > 
> > I think that we still need update `DockerInfo` by adding `network_name` 
> > and always using `network_name` from `DockerInfo` first; if no 
> > `network_name` in `DockerInfo`, use `name` from `NetworkInfo`

Why? I thought the point was to use NetworkInfo instead of network_name . 
Filling the NetworkInfo in case of user-defined network should be mandatory.


- Ezra


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


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Guangya Liu

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




src/docker/docker.cpp (lines 554 - 572)


2 spaces indent, you can take 
https://github.com/apache/mesos/blob/master/src/master/http.cpp#L482 as 
reference.

I think that we still need update `DockerInfo` by adding `network_name` and 
always using `network_name` from `DockerInfo` first; if no `network_name` in 
`DockerInfo`, use `name` from `NetworkInfo`


- Guangya Liu


On 三月 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 三月 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Avinash sridharan

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




src/docker/docker.cpp (line 555)


Indentation seems off ? I guess youc an align it with `ContainerInfo` above?



src/docker/docker.cpp (line 556)


We generally don't use `this`. Maybe

Try validate = validateVersion(...) ?



src/docker/docker.cpp (line 562)


We should make sure the user is not trying to specify  more than one 
network for this container (multiple `NetworkInfo`). Docker 1.9 supports 
multiple user networks but the container can be connected to more than one 
network only after being started (using `docker network connect`) which is kind 
of useless. Docker 1.10 apparently allows you to attach container to multiple 
network but not using the `docker run` command, so doesn't fit the model for 
`DockerContainerizer`.


- Avinash sridharan


On March 28, 2016, 10:51 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated March 28, 2016, 10:51 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
>   include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
>   src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-28 Thread Ezra Silvera

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

(Updated March 28, 2016, 10:51 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fix patch and diff. Previouse one included, by mistake, the rebase as well


Summary (updated)
-

Add support for user-defined networks.


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


Repository: mesos


Description (updated)
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 
  src/docker/docker.cpp 4d35513cdd9c044d37d876a6db7dd9321ceaca53 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-03-23 Thread Ezra Silvera


> On Feb. 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `
> 
> Jie Yu wrote:
> What i am suggesting is to add a singular name field:
> ```
> message NetworkInfo {
>   optional string name;
> }
> ```
> 
> NetworkInfo itself is repeated in ContainerInfo
> 
> Guangya Liu wrote:
> Thanks Yu Jie, still a bit confused, can you please explain more? :-)
> 
> What is the advantage of adding the following instead of `optional string 
> network_name = 8;`? 
> 
> message NetworkInfo {
>   optional string name;
> }
> 
> Also once add a singular `NetworkInfo`, do we still need to add the 
> `optional string network_name` field?
> 
> Qian Zhang wrote:
> I think what we are going to do is to add a singular `name` field in the 
> existing `NetworkInfo` message rather than to add a singular `NetworkInfo` 
> message.
> 
> Guangya Liu wrote:
> @Ezra, FYI, There is a JIRA 
> https://issues.apache.org/jira/browse/MESOS-4758 trying to add `name` to 
> `NetworkInfo`
> 
> Ezra Silvera wrote:
> @gyliu - is https://issues.apache.org/jira/browse/MESOS-4758 already 
> merged?  If not I suggest to merge this change and then when that request is 
> merged we can easily go and change the code to use that field. It seems we 
> keep moving gin circles here on a relatively simple change which prohibit us 
> to use user-networks from our Swarm based cloud..
> 
> Guangya Liu wrote:
> @Ezra, not yet. I think that you can get some comments from @jieyu for 
> this.
> 
> Jie Yu wrote:
> @Ezra @gyliu, sorry about the delay. MESOS-4758 just got merged.
> 
> Guangya Liu wrote:
> @Ezra,I think that you can rebase ur patch now,  it would be great if 
> your patch can catch up 0.28 rc1

@gyliu, thanks. Yes we are now updating opur code to use the networkInfo ...


- Ezra


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


On Feb. 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-10 Thread Guangya Liu


> On 二月 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `
> 
> Jie Yu wrote:
> What i am suggesting is to add a singular name field:
> ```
> message NetworkInfo {
>   optional string name;
> }
> ```
> 
> NetworkInfo itself is repeated in ContainerInfo
> 
> Guangya Liu wrote:
> Thanks Yu Jie, still a bit confused, can you please explain more? :-)
> 
> What is the advantage of adding the following instead of `optional string 
> network_name = 8;`? 
> 
> message NetworkInfo {
>   optional string name;
> }
> 
> Also once add a singular `NetworkInfo`, do we still need to add the 
> `optional string network_name` field?
> 
> Qian Zhang wrote:
> I think what we are going to do is to add a singular `name` field in the 
> existing `NetworkInfo` message rather than to add a singular `NetworkInfo` 
> message.
> 
> Guangya Liu wrote:
> @Ezra, FYI, There is a JIRA 
> https://issues.apache.org/jira/browse/MESOS-4758 trying to add `name` to 
> `NetworkInfo`
> 
> Ezra Silvera wrote:
> @gyliu - is https://issues.apache.org/jira/browse/MESOS-4758 already 
> merged?  If not I suggest to merge this change and then when that request is 
> merged we can easily go and change the code to use that field. It seems we 
> keep moving gin circles here on a relatively simple change which prohibit us 
> to use user-networks from our Swarm based cloud..
> 
> Guangya Liu wrote:
> @Ezra, not yet. I think that you can get some comments from @jieyu for 
> this.
> 
> Jie Yu wrote:
> @Ezra @gyliu, sorry about the delay. MESOS-4758 just got merged.

@Ezra,I think that you can rebase ur patch now,  it would be great if your 
patch can catch up 0.28 rc1


- Guangya


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


On 二月 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-10 Thread Jie Yu


> On Feb. 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `
> 
> Jie Yu wrote:
> What i am suggesting is to add a singular name field:
> ```
> message NetworkInfo {
>   optional string name;
> }
> ```
> 
> NetworkInfo itself is repeated in ContainerInfo
> 
> Guangya Liu wrote:
> Thanks Yu Jie, still a bit confused, can you please explain more? :-)
> 
> What is the advantage of adding the following instead of `optional string 
> network_name = 8;`? 
> 
> message NetworkInfo {
>   optional string name;
> }
> 
> Also once add a singular `NetworkInfo`, do we still need to add the 
> `optional string network_name` field?
> 
> Qian Zhang wrote:
> I think what we are going to do is to add a singular `name` field in the 
> existing `NetworkInfo` message rather than to add a singular `NetworkInfo` 
> message.
> 
> Guangya Liu wrote:
> @Ezra, FYI, There is a JIRA 
> https://issues.apache.org/jira/browse/MESOS-4758 trying to add `name` to 
> `NetworkInfo`
> 
> Ezra Silvera wrote:
> @gyliu - is https://issues.apache.org/jira/browse/MESOS-4758 already 
> merged?  If not I suggest to merge this change and then when that request is 
> merged we can easily go and change the code to use that field. It seems we 
> keep moving gin circles here on a relatively simple change which prohibit us 
> to use user-networks from our Swarm based cloud..
> 
> Guangya Liu wrote:
> @Ezra, not yet. I think that you can get some comments from @jieyu for 
> this.

@Ezra @gyliu, sorry about the delay. MESOS-4758 just got merged.


- Jie


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


On Feb. 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-09 Thread Guangya Liu


> On 二月 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `
> 
> Jie Yu wrote:
> What i am suggesting is to add a singular name field:
> ```
> message NetworkInfo {
>   optional string name;
> }
> ```
> 
> NetworkInfo itself is repeated in ContainerInfo
> 
> Guangya Liu wrote:
> Thanks Yu Jie, still a bit confused, can you please explain more? :-)
> 
> What is the advantage of adding the following instead of `optional string 
> network_name = 8;`? 
> 
> message NetworkInfo {
>   optional string name;
> }
> 
> Also once add a singular `NetworkInfo`, do we still need to add the 
> `optional string network_name` field?
> 
> Qian Zhang wrote:
> I think what we are going to do is to add a singular `name` field in the 
> existing `NetworkInfo` message rather than to add a singular `NetworkInfo` 
> message.
> 
> Guangya Liu wrote:
> @Ezra, FYI, There is a JIRA 
> https://issues.apache.org/jira/browse/MESOS-4758 trying to add `name` to 
> `NetworkInfo`
> 
> Ezra Silvera wrote:
> @gyliu - is https://issues.apache.org/jira/browse/MESOS-4758 already 
> merged?  If not I suggest to merge this change and then when that request is 
> merged we can easily go and change the code to use that field. It seems we 
> keep moving gin circles here on a relatively simple change which prohibit us 
> to use user-networks from our Swarm based cloud..

@Ezra, not yet. I think that you can get some comments from @jieyu for this.


- Guangya


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


On 二月 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-03-09 Thread Ezra Silvera


> On Feb. 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `
> 
> Jie Yu wrote:
> What i am suggesting is to add a singular name field:
> ```
> message NetworkInfo {
>   optional string name;
> }
> ```
> 
> NetworkInfo itself is repeated in ContainerInfo
> 
> Guangya Liu wrote:
> Thanks Yu Jie, still a bit confused, can you please explain more? :-)
> 
> What is the advantage of adding the following instead of `optional string 
> network_name = 8;`? 
> 
> message NetworkInfo {
>   optional string name;
> }
> 
> Also once add a singular `NetworkInfo`, do we still need to add the 
> `optional string network_name` field?
> 
> Qian Zhang wrote:
> I think what we are going to do is to add a singular `name` field in the 
> existing `NetworkInfo` message rather than to add a singular `NetworkInfo` 
> message.
> 
> Guangya Liu wrote:
> @Ezra, FYI, There is a JIRA 
> https://issues.apache.org/jira/browse/MESOS-4758 trying to add `name` to 
> `NetworkInfo`

@gyliu - is https://issues.apache.org/jira/browse/MESOS-4758 already merged?  
If not I suggest to merge this change and then when that request is merged we 
can easily go and change the code to use that field. It seems we keep moving 
gin circles here on a relatively simple change which prohibit us to use 
user-networks from our Swarm based cloud..


- Ezra


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


On Feb. 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-24 Thread Guangya Liu


> On 二月 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `
> 
> Jie Yu wrote:
> What i am suggesting is to add a singular name field:
> ```
> message NetworkInfo {
>   optional string name;
> }
> ```
> 
> NetworkInfo itself is repeated in ContainerInfo
> 
> Guangya Liu wrote:
> Thanks Yu Jie, still a bit confused, can you please explain more? :-)
> 
> What is the advantage of adding the following instead of `optional string 
> network_name = 8;`? 
> 
> message NetworkInfo {
>   optional string name;
> }
> 
> Also once add a singular `NetworkInfo`, do we still need to add the 
> `optional string network_name` field?
> 
> Qian Zhang wrote:
> I think what we are going to do is to add a singular `name` field in the 
> existing `NetworkInfo` message rather than to add a singular `NetworkInfo` 
> message.

@Ezra, FYI, There is a JIRA https://issues.apache.org/jira/browse/MESOS-4758 
trying to add `name` to `NetworkInfo`


- Guangya


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


On 二月 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-18 Thread Guangya Liu


> On 二月 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `
> 
> Jie Yu wrote:
> What i am suggesting is to add a singular name field:
> ```
> message NetworkInfo {
>   optional string name;
> }
> ```
> 
> NetworkInfo itself is repeated in ContainerInfo

Thanks Yu Jie, still a bit confused, can you please explain more? :-)

What is the advantage of adding the following instead of `optional string 
network_name = 8;`? 

message NetworkInfo {
  optional string name;
}

Also once add a singular `NetworkInfo`, do we still need to add the `optional 
string network_name` field?


- Guangya


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


On 二月 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-18 Thread Jie Yu


> On Feb. 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.
> 
> Guangya Liu wrote:
> Does it make sense to use the `repeated string groups` field in 
> NetworkInfo? I think that we can treate the groups as different user defined 
> networks. If we added `repeated string names` field, then what are the 
> difference between those two fields?
> 
> `
> // A group is the name given to a set of logically-related interfaces that
> // are allowed to communicate among themselves. Network traffic is allowed
> // between two container interfaces that share at least one network group.
> // For example, one might want to create separate groups for isolating 
> dev,
> // testing, qa and prod deployment environments.
> repeated string groups = 3;
> `

What i am suggesting is to add a singular name field:
```
message NetworkInfo {
  optional string name;
}
```

NetworkInfo itself is repeated in ContainerInfo


- Jie


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


On Feb. 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-18 Thread Guangya Liu


> On 二月 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > 
> >
> > We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
> > frameworks to specify the network they want to join. Note that  NetworkInfo 
> > in ContainerInfo is repeated which allows us to express the situation where 
> > a container wants to join multiple networks.
> > 
> > As we did for ContainerInfo.volumes, I think command configurations for 
> > a container should go to top level. This also avoids the confusion that 
> > 'network_name' is set in DockerInfo while there's another NetworkInfo.name.
> > 
> > I think what I am proposing is that: we add a NetworkInfo.name, and if 
> > DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
> > containerizer will do --net=.

Does it make sense to use the `repeated string groups` field in NetworkInfo? I 
think that we can treate the groups as different user defined networks. If we 
added `repeated string names` field, then what are the difference between those 
two fields?

`
// A group is the name given to a set of logically-related interfaces that
// are allowed to communicate among themselves. Network traffic is allowed
// between two container interfaces that share at least one network group.
// For example, one might want to create separate groups for isolating dev,
// testing, qa and prod deployment environments.
repeated string groups = 3;
`


- Guangya


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


On 二月 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-18 Thread Jie Yu

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




include/mesos/v1/mesos.proto (lines 1543 - 1544)


We're going to introduce a ContainerInfo.NetworkInfo.name to allow 
frameworks to specify the network they want to join. Note that  NetworkInfo in 
ContainerInfo is repeated which allows us to express the situation where a 
container wants to join multiple networks.

As we did for ContainerInfo.volumes, I think command configurations for a 
container should go to top level. This also avoids the confusion that 
'network_name' is set in DockerInfo while there's another NetworkInfo.name.

I think what I am proposing is that: we add a NetworkInfo.name, and if 
DOckerInfo.network is not set and NetworkInfo.name is set, the docker 
containerizer will do --net=.


- Jie Yu


On Feb. 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-16 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 Feb. 16, 2016, 12:39 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 16, 2016, 12:39 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
>   include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-16 Thread Ezra Silvera

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

(Updated Feb. 16, 2016, 12:39 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fixing typos


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
  include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-02-16 Thread Ezra Silvera


> On Feb. 16, 2016, 4:04 a.m., Timothy Chen wrote:
> > Thinking about it a bit more I think it's fine without a unit test as it 
> > requires docker network create. Did you test this manually and made sure it 
> > worked?
> > Once you update the comments I can merge it.

I fixed all the typos and committed the changes.  BTW, I rebased my code to the 
latest mesos code here:

https://github.com/ezrasilvera/mesos/tree/dockerUserNetwork

We tested the code against Docker 1.8.0 and got the excepted error (require 
version greater then 1.9.0) and  verified that it's working against Docker 
1.9.0 and 1.10 
Once you merge it we can also go ahead and make the needed changes in SWARM to 
support this.


- Ezra


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


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-15 Thread Timothy Chen

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



Thinking about it a bit more I think it's fine without a unit test as it 
requires docker network create. Did you test this manually and made sure it 
worked?
Once you update the comments I can merge it.

- Timothy Chen


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-15 Thread Timothy Chen

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




include/mesos/v1/mesos.proto (line 1534)


Remove extra space.



src/docker/docker.cpp (line 521)


Capitalize the first letter of your comment ( // User defined...)



src/docker/docker.cpp (line 528)


Space between next if



src/docker/docker.cpp (line 530)


Fix the formatting, need to move one more char to the right.


- Timothy Chen


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-12 Thread Travis Hegner


> On Feb. 12, 2016, 3:25 p.m., Travis Hegner wrote:
> > Please have a look at https://reviews.apache.org/r/43093/. Should these 
> > patches be combined, or worked on together? Adding support for user defined 
> > networks, should also include support for proper IP address detection via 
> > those networks yes? Docker 1.9.0 not only introduced user defined networks, 
> > it also introduced custom network drivers and deprecated the existing 
> > "NetworkSettings.IPAddress" field which mesos uses to identify the 
> > container's IP.
> 
> Ezra Silvera wrote:
> We can add the change into our patch as we are already doing all the 
> relevant checks + defined a dedicated new field for the network name. 
> We would simply use "NetworkSettings.Networks..IPAddress 
> for docker version >= 1.9 ..   I'll push another commit shortly.
> Is that OK ?

That would be fantastic. My code was not yet fully baked, so hopefully it 
doesn't hinder your progress too much here. I'd be happy to help with the 
review proces, but I won't be available until Monday to do so.

Also, somewhat unrelated, but I've noticed that when health checks are enabled 
for a docker container, it seems to change the detected IPAddress for that 
container. Perhaps there is another execution path somewhere that does IP 
detection with different code? I've only just started investigating that issue, 
and don't have much more info than that, so feel free to ignore that part until 
I can investigate further.


- Travis


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


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-12 Thread Ezra Silvera


> On Feb. 12, 2016, 3:25 p.m., Travis Hegner wrote:
> > Please have a look at https://reviews.apache.org/r/43093/. Should these 
> > patches be combined, or worked on together? Adding support for user defined 
> > networks, should also include support for proper IP address detection via 
> > those networks yes? Docker 1.9.0 not only introduced user defined networks, 
> > it also introduced custom network drivers and deprecated the existing 
> > "NetworkSettings.IPAddress" field which mesos uses to identify the 
> > container's IP.

We can add the change into our patch as we are already doing all the relevant 
checks + defined a dedicated new field for the network name. 
We would simply use "NetworkSettings.Networks..IPAddress for 
docker version >= 1.9 ..   I'll push another commit shortly.
Is that OK ?


- Ezra


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


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-12 Thread Travis Hegner

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



Please have a look at https://reviews.apache.org/r/43093/. Should these patches 
be combined, or worked on together? Adding support for user defined networks, 
should also include support for proper IP address detection via those networks 
yes? Docker 1.9.0 not only introduced user defined networks, it also introduced 
custom network drivers and deprecated the existing "NetworkSettings.IPAddress" 
field which mesos uses to identify the container's IP.

- Travis Hegner


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-12 Thread Guangya Liu


> On 二月 12, 2016, 3:25 p.m., Travis Hegner wrote:
> > Please have a look at https://reviews.apache.org/r/43093/. Should these 
> > patches be combined, or worked on together? Adding support for user defined 
> > networks, should also include support for proper IP address detection via 
> > those networks yes? Docker 1.9.0 not only introduced user defined networks, 
> > it also introduced custom network drivers and deprecated the existing 
> > "NetworkSettings.IPAddress" field which mesos uses to identify the 
> > container's IP.
> 
> Ezra Silvera wrote:
> We can add the change into our patch as we are already doing all the 
> relevant checks + defined a dedicated new field for the network name. 
> We would simply use "NetworkSettings.Networks..IPAddress 
> for docker version >= 1.9 ..   I'll push another commit shortly.
> Is that OK ?
> 
> Travis Hegner wrote:
> That would be fantastic. My code was not yet fully baked, so hopefully it 
> doesn't hinder your progress too much here. I'd be happy to help with the 
> review proces, but I won't be available until Monday to do so.
> 
> Also, somewhat unrelated, but I've noticed that when health checks are 
> enabled for a docker container, it seems to change the detected IPAddress for 
> that container. Perhaps there is another execution path somewhere that does 
> IP detection with different code? I've only just started investigating that 
> issue, and don't have much more info than that, so feel free to ignore that 
> part until I can investigate further.

Please note that https://reviews.apache.org/r/43032/ may introduce docker 
version and save it in the Docker class, if that finished, then this patch can 
just leverage the docker version directly.


- Guangya


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


On 二月 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Klaus Ma

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




src/docker/docker.cpp (line 529)


Also check whether `network_name()` is empty string `""`.


please add unit test cases for this patches; refer to `bridge` test cases.

- Klaus Ma


On Feb. 10, 2016, 4:34 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Ezra Silvera


> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.
> 
> Ezra Silvera wrote:
> I think we already covered this issue in previous review comments. Is 
> there anything special you think is missing ?
> 
> Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)
> 
> Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
> We added a new value "USER" which is identical in terms of code flow 
> to HOST and NONE so I'm not sure we should add a special test for USER while 
> all other values are not tested.
> Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
> The same is true for port mapping - maybe we had to better clarify it 
> on the comments - we didn't change any functionality at all and didn't touch 
> any code related to port mapping or network functionality. We are simply 
> passing a new value for the  --net parameter to the docker engine, nothing 
> else has changed.
> What do you think?

My comment was for teh testing. We will add a test for "" string (we actually 
started with that and switch to the current test due to review comments. :-)


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Ezra Silvera


> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.
> 
> Ezra Silvera wrote:
> I think we already covered this issue in previous review comments. Is 
> there anything special you think is missing ?
> 
> Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)
> 
> Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
> We added a new value "USER" which is identical in terms of code flow 
> to HOST and NONE so I'm not sure we should add a special test for USER while 
> all other values are not tested.
> Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
> The same is true for port mapping - maybe we had to better clarify it 
> on the comments - we didn't change any functionality at all and didn't touch 
> any code related to port mapping or network functionality. We are simply 
> passing a new value for the  --net parameter to the docker engine, nothing 
> else has changed.
> What do you think?
> 
> Ezra Silvera wrote:
> My comment was for teh testing. We will add a test for "" string (we 
> actually started with that and switch to the current test due to review 
> comments. :-)
> 
> Klaus Ma wrote:
> For the test, using user-defined network is fine (similar to test using 
> BRIDGE). For the empty string, current code is only check whether 
> `network_name()` is set; if it's set to `""`, I'm not sure the behaviour but 
> I think we it's better to return error before launching docker command line 
> with empty string.

I agree.


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Ezra Silvera


> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.

I think we already covered this issue in previous review comments. Is there 
anything special you think is missing ?

Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)

Guangya Liu, I went through the testing functions and I'm not sure we 
actually need to add test for this.  Currently there are 3 possible values that 
can be passed BRIDGE, HOST, NONE. There isn't any test that check the NONE and 
HOST values. In fact there is  not even a functionality test for BRIDGE value 
but simply when creating containers the BRIDGE value is used.
We added a new value "USER" which is identical in terms of code flow to 
HOST and NONE so I'm not sure we should add a special test for USER while all 
other values are not tested.
Further more in order to create a container with USER we will need to 
create the actual user-network on the docker engine - this will require us to 
add the functionality for  "network create" into docker.cpp. This functionality 
is not needed at all for the mesos and will be used only  for this test.
The same is true for port mapping - maybe we had to better clarify it on 
the comments - we didn't change any functionality at all and didn't touch any 
code related to port mapping or network functionality. We are simply passing a 
new value for the  --net parameter to the docker engine, nothing else has 
changed.
What do you think?


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Ezra Silvera

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

(Updated Feb. 11, 2016, 1:51 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Loaded the correct diff


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Ezra Silvera

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

(Updated Feb. 11, 2016, 1:14 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Change error message + add a test for network_name=""


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-02-11 Thread Klaus Ma


> On Feb. 11, 2016, 5:34 p.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.
> 
> Ezra Silvera wrote:
> I think we already covered this issue in previous review comments. Is 
> there anything special you think is missing ?
> 
> Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)
> 
> Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
> We added a new value "USER" which is identical in terms of code flow 
> to HOST and NONE so I'm not sure we should add a special test for USER while 
> all other values are not tested.
> Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
> The same is true for port mapping - maybe we had to better clarify it 
> on the comments - we didn't change any functionality at all and didn't touch 
> any code related to port mapping or network functionality. We are simply 
> passing a new value for the  --net parameter to the docker engine, nothing 
> else has changed.
> What do you think?
> 
> Ezra Silvera wrote:
> My comment was for teh testing. We will add a test for "" string (we 
> actually started with that and switch to the current test due to review 
> comments. :-)

For the test, using user-defined network is fine (similar to test using 
BRIDGE). For the empty string, current code is only check whether 
`network_name()` is set; if it's set to `""`, I'm not sure the behaviour but I 
think we it's better to return error before launching docker command line with 
empty string.


- Klaus


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


On Feb. 10, 2016, 4:34 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Guangya Liu


> On 二月 10, 2016, 1:05 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 526
> > 
> >
> > I prefer that we use "higher than" or "greater than" instead of ">=", 
> > we can wait for the comments from Tim.
> 
> Ezra Silvera wrote:
> The problem is that "higher or equal to" seemed too awkward to me

Just saw that @tnachen already showed his comments above about this and his 
suggestion is `User network mode requires Docker version higher than 1.9.0`


- Guangya


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


On 二月 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Ezra Silvera


> On Feb. 10, 2016, 1:05 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 526
> > 
> >
> > I prefer that we use "higher than" or "greater than" instead of ">=", 
> > we can wait for the comments from Tim.

The problem is that "higher or equal to" seemed too awkward to me


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Ezra Silvera


> On Feb. 10, 2016, 1:05 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 526
> > 
> >
> > I prefer that we use "higher than" or "greater than" instead of ">=", 
> > we can wait for the comments from Tim.
> 
> Ezra Silvera wrote:
> The problem is that "higher or equal to" seemed too awkward to me
> 
> Guangya Liu wrote:
> Just saw that @tnachen already showed his comments above about this and 
> his suggestion is `User network mode requires Docker version higher than 
> 1.9.0`

The problem is the 1.9.0 is also OK so it's "higher or equal to " which I'm not 
so happy with :-)


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Ezra Silvera


> On Feb. 10, 2016, 1:05 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 526
> > 
> >
> > I prefer that we use "higher than" or "greater than" instead of ">=", 
> > we can wait for the comments from Tim.
> 
> Ezra Silvera wrote:
> The problem is that "higher or equal to" seemed too awkward to me
> 
> Guangya Liu wrote:
> Just saw that @tnachen already showed his comments above about this and 
> his suggestion is `User network mode requires Docker version higher than 
> 1.9.0`
> 
> Ezra Silvera wrote:
> The problem is the 1.9.0 is also OK so it's "higher or equal to " which 
> I'm not so happy with :-)
> 
> Guangya Liu wrote:
> What about `User network mode requires Docker version not smaller than 
> 1.9.0` ?

"User defined networks require Docker version 1.9.0 or higher"


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Guangya Liu


> On 二月 10, 2016, 1:05 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 526
> > 
> >
> > I prefer that we use "higher than" or "greater than" instead of ">=", 
> > we can wait for the comments from Tim.
> 
> Ezra Silvera wrote:
> The problem is that "higher or equal to" seemed too awkward to me
> 
> Guangya Liu wrote:
> Just saw that @tnachen already showed his comments above about this and 
> his suggestion is `User network mode requires Docker version higher than 
> 1.9.0`
> 
> Ezra Silvera wrote:
> The problem is the 1.9.0 is also OK so it's "higher or equal to " which 
> I'm not so happy with :-)

What about `User network mode requires Docker version not smaller than 1.9.0` ?


- Guangya


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


On 二月 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread haosdent huang

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


Ship it!




Ship It!

- haosdent huang


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Ezra Silvera

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

(Updated Feb. 10, 2016, 8:34 a.m.)


Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Guangya Liu

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




src/docker/docker.cpp (line 526)


I prefer that we use "higher than" or "greater than" instead of ">=", we 
can wait for the comments from Tim.


- Guangya Liu


On 二月 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Guangya Liu


> On 二月 10, 2016, 1:05 p.m., Guangya Liu wrote:
> > src/docker/docker.cpp, line 526
> > 
> >
> > I prefer that we use "higher than" or "greater than" instead of ">=", 
> > we can wait for the comments from Tim.
> 
> Ezra Silvera wrote:
> The problem is that "higher or equal to" seemed too awkward to me
> 
> Guangya Liu wrote:
> Just saw that @tnachen already showed his comments above about this and 
> his suggestion is `User network mode requires Docker version higher than 
> 1.9.0`
> 
> Ezra Silvera wrote:
> The problem is the 1.9.0 is also OK so it's "higher or equal to " which 
> I'm not so happy with :-)
> 
> Guangya Liu wrote:
> What about `User network mode requires Docker version not smaller than 
> 1.9.0` ?
> 
> Ezra Silvera wrote:
> "User defined networks require Docker version 1.9.0 or higher"

This looks better, thanks.


- Guangya


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


On 二月 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-10 Thread Timothy Chen


> On Feb. 9, 2016, 2:17 a.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 525
> > 
> >
> > Can you put a more elaborate message here?
> > i.e: "User network mode requires Docker version higher than 1.9.0."

Btw any kind of test we can add around this change?


- Timothy


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-09 Thread Ezra Silvera

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

(Updated Feb. 9, 2016, 7:25 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Fixed error message


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-02-09 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 Feb. 9, 2016, 7:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 9, 2016, 7:25 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-09 Thread Guangya Liu

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




src/docker/docker.cpp (line 521)


s/version >=  1.9.0/version >= 1.9.0

one space



src/docker/docker.cpp (line 526)


Remove the period in the end.


- Guangya Liu


On 二月 9, 2016, 7:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 二月 9, 2016, 7:25 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-08 Thread Timothy Chen

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




src/docker/docker.cpp (line 525)


Can you put a more elaborate message here?
i.e: "User network mode requires Docker version higher than 1.9.0."


- Timothy Chen


On Feb. 4, 2016, 10:04 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 4, 2016, 10:04 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
>   src/docker/docker.cpp a83172674b385a59cfd6f344836740a5719f954e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-02 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

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 Feb. 2, 2016, 1:57 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 2, 2016, 1:57 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
>   src/docker/docker.cpp a83172674b385a59cfd6f344836740a5719f954e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-02-02 Thread Ezra Silvera

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

(Updated Feb. 2, 2016, 1:53 p.m.)


Review request for mesos and TimothyIL TimothyIL.


Changes
---

Validate engine version >= 1.9.0


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
  src/docker/docker.cpp a83172674b385a59cfd6f344836740a5719f954e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-02-02 Thread Ezra Silvera

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

(Updated Feb. 2, 2016, 1:57 p.m.)


Review request for mesos and TimothyIL TimothyIL.


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
  src/docker/docker.cpp a83172674b385a59cfd6f344836740a5719f954e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-01-31 Thread Guangya Liu

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




src/docker/docker.cpp (lines 517 - 522)


The user network was introduced in docker 1.9, is it possible to add some 
error handlingn here to handle the case if end user is using docker version 
before 1.9?


- Guangya Liu


On 一月 27, 2016, 7:05 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 一月 27, 2016, 7:05 a.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
>   include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-31 Thread Guangya Liu

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



BTW: I think that you meant to have tim Chen as shephard, his id is tnachen, 
you may want to update the shephard for this.

- Guangya Liu


On 一月 27, 2016, 7:05 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 一月 27, 2016, 7:05 a.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
>   include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-27 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 27, 2016, 7:05 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 27, 2016, 7:05 a.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
>   include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-26 Thread Ezra Silvera


> On Jan. 24, 2016, 6:58 a.m., Shuai Lin wrote:
> > src/docker/docker.cpp, line 519
> > 
> >
> > typo, s/sempty/empty :)

good catch. thanks :-)


- Ezra


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


On Jan. 24, 2016, 6:42 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 24, 2016, 6:42 a.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
>   include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-26 Thread Ezra Silvera

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

(Updated Jan. 27, 2016, 7:05 a.m.)


Review request for mesos and TimothyIL TimothyIL.


Changes
---

fix type smpty-->empty


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
  include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-01-23 Thread Shuai Lin

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




src/docker/docker.cpp (line 517)


+1 for checking `dockerInfo.network_name` when `network` is set to `USER`


- Shuai Lin


On Jan. 20, 2016, 12:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 20, 2016, 12:25 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> Review: https://reviews.apache.org/r/42549
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-23 Thread Shuai Lin

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


Fix it, then Ship it!





src/docker/docker.cpp (line 518)


nit: use `dockerInfo.has_network_name()` could be more intuitive.


- Shuai Lin


On Jan. 23, 2016, 8:45 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 23, 2016, 8:45 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-23 Thread Ezra Silvera

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

(Updated Jan. 23, 2016, 8:45 p.m.)


Review request for mesos and TimothyIL TimothyIL.


Changes
---

Verify that network_name is not empty


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


Repository: mesos


Description (updated)
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-01-23 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 23, 2016, 8:45 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 23, 2016, 8:45 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-23 Thread Ezra Silvera

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

(Updated Jan. 24, 2016, 6:42 a.m.)


Review request for mesos and TimothyIL TimothyIL.


Changes
---

use has_network_name()


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
  include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-01-23 Thread Shuai Lin

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


Fix it, then Ship it!





src/docker/docker.cpp (line 519)


typo, s/sempty/empty :)


- Shuai Lin


On Jan. 24, 2016, 6:42 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 24, 2016, 6:42 a.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
>   include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-23 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42516]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 24, 2016, 6:42 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 24, 2016, 6:42 a.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 96b911fb370223933df52f9370897871827d2247 
>   include/mesos/v1/mesos.proto 0501dfa27ed610666226953591a902eac4c295f8 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-20 Thread Qian Zhang

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



src/docker/docker.cpp (line 517)


Do we need to check if `dockerInfo.network_name()` is empty? What will 
happend if we pass `--net ` into `docker run"


- Qian Zhang


On Jan. 20, 2016, 8:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 20, 2016, 8:25 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> Review: https://reviews.apache.org/r/42549
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-20 Thread Ezra Silvera


> On Jan. 20, 2016, 7:41 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 539-541
> > 
> >
> > I think that you may also want some test cases to verify port mapping 
> > plus user defined network.

We agree that currently it seems we can't add a unit-test


- Ezra


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


On Jan. 20, 2016, 12:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 20, 2016, 12:25 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> Review: https://reviews.apache.org/r/42549
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-20 Thread Guangya Liu


> On 一月 19, 2016, 11:29 p.m., Guangya Liu wrote:
> > A unit test also needed, please refer to 
> > https://github.com/apache/mesos/blob/master/src/tests/containerizer/docker_containerizer_tests.cpp#L297
> >  for detail
> 
> Ezra Silvera wrote:
> Thanks. I'll have a look. We will probably need to add support for 
> "create network" in order to be able to test the created network.
> 
> Ezra Silvera wrote:
> Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
> We added a new value "USER" which is identical in terms of code flow to 
> HOST and NONE so I'm not sure we should add a special test for USER while all 
> other values are not tested.
> Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
> The same is true for port mapping - maybe we had to better clarify it on 
> the comments - we didn't change any functionality at all and didn't touch any 
> code related to port mapping or network functionality. We are simply passing 
> a new value for the  --net parameter to the docker engine, nothing else has 
> changed.
> What do you think?

Yes, I also checked the code seems we cannot add the unit test for now, thanks 
for the clarification.


- Guangya


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


On 一月 20, 2016, 12:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 一月 20, 2016, 12:25 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> Review: https://reviews.apache.org/r/42549
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-20 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [42516]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 20, 2016, 12:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 20, 2016, 12:25 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> Review: https://reviews.apache.org/r/42549
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-20 Thread Ezra Silvera

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

(Updated Jan. 20, 2016, 12:25 p.m.)


Review request for mesos and TimothyIL TimothyIL.


Changes
---

fix typos


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


Repository: mesos


Description (updated)
---

Signed-off-by: Ezra Silvera 

Review: https://reviews.apache.org/r/42549


Diffs (updated)
-

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-01-20 Thread Ezra Silvera


> On Jan. 19, 2016, 11:29 p.m., Guangya Liu wrote:
> > A unit test also needed, please refer to 
> > https://github.com/apache/mesos/blob/master/src/tests/containerizer/docker_containerizer_tests.cpp#L297
> >  for detail
> 
> Ezra Silvera wrote:
> Thanks. I'll have a look. We will probably need to add support for 
> "create network" in order to be able to test the created network.

Guangya Liu, I went through the testing functions and I'm not sure we actually 
need to add test for this.  Currently there are 3 possible values that can be 
passed BRIDGE, HOST, NONE. There isn't any test that check the NONE and HOST 
values. In fact there is  not even a functionality test for BRIDGE value but 
simply when creating containers the BRIDGE value is used.
We added a new value "USER" which is identical in terms of code flow to HOST 
and NONE so I'm not sure we should add a special test for USER while all other 
values are not tested.
Further more in order to create a container with USER we will need to create 
the actual user-network on the docker engine - this will require us to add the 
functionality for  "network create" into docker.cpp. This functionality is not 
needed at all for the mesos and will be used only  for this test.
The same is true for port mapping - maybe we had to better clarify it on the 
comments - we didn't change any functionality at all and didn't touch any code 
related to port mapping or network functionality. We are simply passing a new 
value for the  --net parameter to the docker engine, nothing else has changed.
What do you think?


- Ezra


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


On Jan. 20, 2016, 12:25 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 20, 2016, 12:25 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> Review: https://reviews.apache.org/r/42549
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Review Request 42516: Add support for user-defined networks.

2016-01-19 Thread Ezra Silvera

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

Review request for mesos and TimothyIL TimothyIL.


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


Repository: mesos


Description
---

Since docker 1.9, Docker now supports user-defined networks (both local and 
overlays). The user can then create containers that need to be attached to 
these networks (e.g., docker run --net=my-network
In order to support user-defined network we need to make 2 changes:
1a) Add a new network type in mesos.proto so that frameworks will be able to 
pass this network type
1b) Add a field to add the actual network name. Till now this was not needed as 
users could not define their i own names.
2) Change the executer so it supports processing port mapping also for 
user-define network (instead of just for legacy bridge)

Note that using the new user-defined network (both local and overlay) is 
considered the prefered Docker networking option.

Signed-off-by: Ezra Silvera 


Diffs
-

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera



Re: Review Request 42516: Add support for user-defined networks.

2016-01-19 Thread Ezra Silvera


> On Jan. 19, 2016, 11:29 p.m., Guangya Liu wrote:
> > A unit test also needed, please refer to 
> > https://github.com/apache/mesos/blob/master/src/tests/containerizer/docker_containerizer_tests.cpp#L297
> >  for detail

Thanks. I'll have a look. We will probably need to add support for "create 
network" in order to be able to test the created network.


> On Jan. 19, 2016, 11:29 p.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, line 1469
> > 
> >
> > What about name it as OVERLAY?

We choose USER because it is defined by Docker as "user defined network" and 
can be either overlay or local bridge. You can see it here:
https://docs.docker.com/engine/userguide/networking/dockernetworks/#user-defined-networks


- Ezra


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


On Jan. 19, 2016, 6:53 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Jan. 19, 2016, 6:53 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since docker 1.9, Docker now supports user-defined networks (both local and 
> overlays). The user can then create containers that need to be attached to 
> these networks (e.g., docker run --net=my-network
> In order to support user-defined network we need to make 2 changes:
> 1a) Add a new network type in mesos.proto so that frameworks will be able to 
> pass this network type
> 1b) Add a field to add the actual network name. Till now this was not needed 
> as users could not define their i own names.
> 2) Change the executer so it supports processing port mapping also for 
> user-define network (instead of just for legacy bridge)
> 
> Note that using the new user-defined network (both local and overlay) is 
> considered the prefered Docker networking option.
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



Re: Review Request 42516: Add support for user-defined networks.

2016-01-19 Thread Guangya Liu


> On 一月 19, 2016, 11:29 p.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, line 1469
> > 
> >
> > What about name it as OVERLAY?
> 
> Ezra Silvera wrote:
> We choose USER because it is defined by Docker as "user defined network" 
> and can be either overlay or local bridge. You can see it here:
> 
> https://docs.docker.com/engine/userguide/networking/dockernetworks/#user-defined-networks

Good to know, thanks.


- Guangya


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


On 一月 19, 2016, 6:53 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated 一月 19, 2016, 6:53 p.m.)
> 
> 
> Review request for mesos and TimothyIL TimothyIL.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since docker 1.9, Docker now supports user-defined networks (both local and 
> overlays). The user can then create containers that need to be attached to 
> these networks (e.g., docker run --net=my-network
> In order to support user-defined network we need to make 2 changes:
> 1a) Add a new network type in mesos.proto so that frameworks will be able to 
> pass this network type
> 1b) Add a field to add the actual network name. Till now this was not needed 
> as users could not define their i own names.
> 2) Change the executer so it supports processing port mapping also for 
> user-define network (instead of just for legacy bridge)
> 
> Note that using the new user-defined network (both local and overlay) is 
> considered the prefered Docker networking option.
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
>   include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>



  1   2   >