Re: Review Request 42516: Add support for user-defined networks.
--- 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 <e...@il.ibm.com> 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.
--- 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 <e...@il.ibm.com> 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.
> On March 29, 2016, 6:01 p.m., Avinash sridharan wrote: > > src/docker/docker.cpp, line 568 > > <https://reviews.apache.org/r/42516/diff/19/?file=1318097#file1318097line568> > > > > 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 <e...@il.ibm.com> > > > 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.
--- 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 <e...@il.ibm.com> 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.
> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote:
> > src/docker/docker.cpp, line 562
> > <https://reviews.apache.org/r/42516/diff/14/?file=1316581#file1316581line562>
> >
> > 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 <e...@il.ibm.com>
>
>
> 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.
--- 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 <e...@il.ibm.com> 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.
> On March 29, 2016, 8:54 a.m., Guangya Liu wrote: > > src/docker/docker.cpp, line 567 > > <https://reviews.apache.org/r/42516/diff/16/?file=1317647#file1317647line567> > > > > 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 <e...@il.ibm.com> > > > 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.
--- 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 <e...@il.ibm.com> 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.
> On March 29, 2016, 8:54 a.m., Guangya Liu wrote: > > src/docker/docker.cpp, line 567 > > <https://reviews.apache.org/r/42516/diff/16/?file=1317647#file1317647line567> > > > > 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 <e...@il.ibm.com> > > > 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.
> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote: > > src/docker/docker.cpp, line 562 > > <https://reviews.apache.org/r/42516/diff/14/?file=1316581#file1316581line562> > > > > 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 <e...@il.ibm.com> > > > 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.
--- 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 <e...@il.ibm.com> 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.
> On March 29, 2016, 7:51 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 553-572
> > <https://reviews.apache.org/r/42516/diff/15/?file=1317644#file1317644line553>
> >
> > 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 <e...@il.ibm.com>
>
>
> 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.
--- 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 <e...@il.ibm.com> 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.
> On March 28, 2016, 3:13 p.m., Avinash sridharan wrote: > > src/docker/docker.cpp, line 562 > > <https://reviews.apache.org/r/42516/diff/14/?file=1316581#file1316581line562> > > > > 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 <e...@il.ibm.com> > > > 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.
> On March 28, 2016, 3:43 p.m., Guangya Liu wrote: > > src/docker/docker.cpp, lines 554-572 > > <https://reviews.apache.org/r/42516/diff/14/?file=1316581#file1316581line554> > > > > 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 <e...@il.ibm.com> > > > 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.
> On March 28, 2016, 3:43 p.m., Guangya Liu wrote: > > src/docker/docker.cpp, lines 554-572 > > <https://reviews.apache.org/r/42516/diff/14/?file=1316581#file1316581line554> > > > > 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 <e...@il.ibm.com> > > > 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.
> On March 28, 2016, 3:43 p.m., Guangya Liu wrote: > > src/docker/docker.cpp, lines 554-572 > > <https://reviews.apache.org/r/42516/diff/14/?file=1316581#file1316581line554> > > > > 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 <e...@il.ibm.com> > > > 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.
--- 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 <e...@il.ibm.com> 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: Cleaned up code style and fixed typos in comments.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42516/ --- (Updated March 28, 2016, 10 a.m.) Review request for mesos and Timothy Chen. Changes --- Change the code to use the new NetworkInfo structure Summary (updated) - Cleaned up code style and fixed typos in comments. Bugs: MESOS-4369 https://issues.apache.org/jira/browse/MESOS-4369 Repository: mesos Description (updated) --- Review: https://reviews.apache.org/r/43033/ Fixed a leak of a `ZooKeeperMasterContender` in a test case. Review: https://reviews.apache.org/r/43065/ Added EXPECT_NO_FUTURE_DISPATCHES to libprocess. This allows a unit test to assert that a given libprocess message will not be dispatched. Review: https://reviews.apache.org/r/43034/ Decrease countCacheEntries in FetcherCacheTest.SimpleEviction. Review: https://reviews.apache.org/r/43316/ Synchronized v1/ and unversioned copies of `mesos.proto`. No functional changes, but various comments were slightly out of sync. Review: https://reviews.apache.org/r/43617/ Added documentation for multiple-disk support. Review: https://reviews.apache.org/r/43583 Improved Multiple Disk documentation. Review: https://reviews.apache.org/r/43633/ Removed the duplicate "active" field in json schema of `Framework`. The new `jsonify` library is a writer-based approach, and does not keep track of the fields that have been written out so far. The previous version of `summarize(framework)` and `model(framework)` had a duplicate `"active"` field which was de-duplicated since they simply get inserted to a `std::map`, overriding the previous value. In the `jsonify` case, this pattern results in duplicate key in the JSON output. Although the presence of duplicate keys is technically not __invalid__ according to the JSON specification, some JSON libraries disallow them. As such, we should generate JSON outputs without duplicate keys. Review: https://reviews.apache.org/r/43159/ Defined `status` method for `ComposingContainerizer`. Review: https://reviews.apache.org/r/43564/ Implemented `status` method in `ComposingContainerizer`. This method will be used by the agent to retrieve `ContainerStatus` from the `Containerizer`, that was responsible for launching the container. Review: https://reviews.apache.org/r/43565/ Fixed minor style issues in command_utils.cpp. Review: https://reviews.apache.org/r/43638/ Added missing JIRA ticket in CHANGELOG for 0.27.0. Updated CHANGELOG for 0.27.1-rc1 release. Added examples for documentation on master/slave state.json. We are working on a more standardized way of representing (and thus more easily documenting) our internal JSON objects. Until we have this, there is no point in trying to exhaustively document the state.json (or any other JSON) endpoints. Including an example and noting that it is not exhaustive is better than it was before though. Review: https://reviews.apache.org/r/43618/ Added generated docs from updates to state.json documentation. Review: https://reviews.apache.org/r/43619/ Speed up FetcherCacheTest.Local* test by reduce loop. Review: https://reviews.apache.org/r/42696/ Fixed flakiness in ContainerLoggerTest.DefaultToSandbox. The test needs to wait for the task to finish before examining the resulting files. Review: https://reviews.apache.org/r/42842/ Added common compression utilities. Added support for GZIP compress and uncompress. Review: https://reviews.apache.org/r/43546/ Introduced Appc image fetcher. Added implementation for simple image discovery for Appc images. Review: https://reviews.apache.org/r/43336/ Introduced `Sequence` in container. The `Sequence` will be used to serialize the invocation for status requests from isolators for a given container. Review: https://reviews.apache.org/r/43672/ Searialized invocation of `await` in the status method. Review: https://reviews.apache.org/r/43673/ Added per container SNMP statistics. Review: https://reviews.apache.org/r/38117/ Documented how the replicated log works. This is closely based on an (unpublished) blog post by Jie Yu. Review: https://reviews.apache.org/r/43712 Fixed persistent volumes with docker tasks. Review: https://reviews.apache.org/r/43015 Fixed symlink generation for DESTDIR installs. The install-data-hook was missing the '$(DESTDIR)' prefix. Review: https://reviews.apache.org/r/43741 Fixed compilation issue with clang. Extended allocator interface to support dynamic weights. Review: https://reviews.apache.org/r/41597/ Test cases for dynamic weights + allocation behaviour. Review: https://reviews.apache.org/r/41672/ Fixed typo in fetcher docs. Review: https://reviews.apache.org/r/43737/ Fixed some style issues in docker store. Added a NOTE in docker store about caching. Added note about resending of KillTaskMessages
Re: Review Request 42516: Add support for user-defined networks.
> On Feb. 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > <https://reviews.apache.org/r/42516/diff/12/?file=1251685#file1251685line1543>
> >
> > 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 <e...@il.ibm.com>
>
>
> 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 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43093/#review122802 --- src/docker/docker.cpp (lines 304 - 305) <https://reviews.apache.org/r/43093/#comment184893> Please note that a container can be attached to more then one network! In that case HostConfig.NetworkMode will contain the first NW the container was attached two. The desired IP, however, might be the one on a different NW in the NetworkSettings.Networks array. - Ezra Silvera On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43093/ > --- > > (Updated Feb. 17, 2016, 10:52 p.m.) > > > Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen. > > > Bugs: MESOS-4370 > https://issues.apache.org/jira/browse/MESOS-4370 > > > Repository: mesos > > > Description > --- > > Fixes [MESOS-4370] > > > Diffs > - > > src/docker/docker.cpp b4b8d3e > > Diff: https://reviews.apache.org/r/43093/diff/ > > > Testing > --- > > This patch will first query the docker API for the HostConfig.NetworkMode, > which is populated with the network name. (Essentially what was passed in > --net to the docker run command). This name is then used as a key in > NetworkSettings.Networks..IPAddress to get the IP address that is > currently in use by the container. > > It appears that even though the docker API has been set up to allow for > multiple networks, our testing has indicated that it's still only applying > one network to the container (the last one via the --net argument on the run > line). I can only speculate that the docker API will change again in the near > future, but I can't speculate how, so at least this fixes the problem as it > stands right now. > > Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04. > > > Thanks, > > Travis Hegner > >
Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker
> On March 9, 2016, 8:48 p.m., Ezra Silvera wrote: > > Please note that a container can be attached to more then one network! In > > that case HostConfig.NetworkMode will contain the first NW the container > > was attached two. The desired IP, however, might be the one on a different > > NW in the NetworkSettings.Networks array. Please ignore this. I opened a specific issue instead. - Ezra --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43093/#review122796 --- On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43093/ > --- > > (Updated Feb. 17, 2016, 10:52 p.m.) > > > Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen. > > > Bugs: MESOS-4370 > https://issues.apache.org/jira/browse/MESOS-4370 > > > Repository: mesos > > > Description > --- > > Fixes [MESOS-4370] > > > Diffs > - > > src/docker/docker.cpp b4b8d3e > > Diff: https://reviews.apache.org/r/43093/diff/ > > > Testing > --- > > This patch will first query the docker API for the HostConfig.NetworkMode, > which is populated with the network name. (Essentially what was passed in > --net to the docker run command). This name is then used as a key in > NetworkSettings.Networks..IPAddress to get the IP address that is > currently in use by the container. > > It appears that even though the docker API has been set up to allow for > multiple networks, our testing has indicated that it's still only applying > one network to the container (the last one via the --net argument on the run > line). I can only speculate that the docker API will change again in the near > future, but I can't speculate how, so at least this fixes the problem as it > stands right now. > > Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04. > > > Thanks, > > Travis Hegner > >
Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43093/#review122796 --- Please note that a container can be attached to more then one network! In that case HostConfig.NetworkMode will contain the first NW the container was attached two. The desired IP, however, might be the one on a different NW in the NetworkSettings.Networks array. - Ezra Silvera On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43093/ > --- > > (Updated Feb. 17, 2016, 10:52 p.m.) > > > Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen. > > > Bugs: MESOS-4370 > https://issues.apache.org/jira/browse/MESOS-4370 > > > Repository: mesos > > > Description > --- > > Fixes [MESOS-4370] > > > Diffs > - > > src/docker/docker.cpp b4b8d3e > > Diff: https://reviews.apache.org/r/43093/diff/ > > > Testing > --- > > This patch will first query the docker API for the HostConfig.NetworkMode, > which is populated with the network name. (Essentially what was passed in > --net to the docker run command). This name is then used as a key in > NetworkSettings.Networks..IPAddress to get the IP address that is > currently in use by the container. > > It appears that even though the docker API has been set up to allow for > multiple networks, our testing has indicated that it's still only applying > one network to the container (the last one via the --net argument on the run > line). I can only speculate that the docker API will change again in the near > future, but I can't speculate how, so at least this fixes the problem as it > stands right now. > > Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04. > > > Thanks, > > Travis Hegner > >
Re: Review Request 42516: Add support for user-defined networks.
> On Feb. 18, 2016, 10:44 p.m., Jie Yu wrote:
> > include/mesos/v1/mesos.proto, lines 1543-1544
> > <https://reviews.apache.org/r/42516/diff/12/?file=1251685#file1251685line1543>
> >
> > 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 <e...@il.ibm.com>
>
>
> 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.
--- 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 <e...@il.ibm.com> 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.
> 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 <e...@il.ibm.com> > > > 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.
> 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 <e...@il.ibm.com> > > > 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.
> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote: > > src/docker/docker.cpp, line 529 > > <https://reviews.apache.org/r/42516/diff/9/?file=1239370#file1239370line529> > > > > 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 <e...@il.ibm.com> > > > 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.
> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote: > > src/docker/docker.cpp, line 529 > > <https://reviews.apache.org/r/42516/diff/9/?file=1239370#file1239370line529> > > > > 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 <e...@il.ibm.com> > > > 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.
> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote: > > src/docker/docker.cpp, line 529 > > <https://reviews.apache.org/r/42516/diff/9/?file=1239370#file1239370line529> > > > > 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 <e...@il.ibm.com> > > > 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.
--- 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 <e...@il.ibm.com> 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.
--- 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 <e...@il.ibm.com> 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.
> On Feb. 10, 2016, 1:05 p.m., Guangya Liu wrote: > > src/docker/docker.cpp, line 526 > > <https://reviews.apache.org/r/42516/diff/9/?file=1239370#file1239370line526> > > > > 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 <e...@il.ibm.com> > > > 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.
> On Feb. 10, 2016, 1:05 p.m., Guangya Liu wrote: > > src/docker/docker.cpp, line 526 > > <https://reviews.apache.org/r/42516/diff/9/?file=1239370#file1239370line526> > > > > 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 <e...@il.ibm.com> > > > 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.
> On Feb. 10, 2016, 1:05 p.m., Guangya Liu wrote: > > src/docker/docker.cpp, line 526 > > <https://reviews.apache.org/r/42516/diff/9/?file=1239370#file1239370line526> > > > > 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 <e...@il.ibm.com> > > > 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.
--- 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 <e...@il.ibm.com> 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.
--- 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 <e...@il.ibm.com> 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.
--- 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 <e...@il.ibm.com> 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.
--- 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 <e...@il.ibm.com> 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.
> On Jan. 24, 2016, 6:58 a.m., Shuai Lin wrote: > > src/docker/docker.cpp, line 519 > > <https://reviews.apache.org/r/42516/diff/4/?file=1218946#file1218946line519> > > > > 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 <e...@il.ibm.com> > > > 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.
--- 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 <e...@il.ibm.com> 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.
--- 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 <e...@il.ibm.com> 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.
--- 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 <e...@il.ibm.com> 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.
> On Jan. 20, 2016, 7:41 a.m., Guangya Liu wrote: > > src/docker/docker.cpp, lines 539-541 > > <https://reviews.apache.org/r/42516/diff/1/?file=1201989#file1201989line539> > > > > 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 <e...@il.ibm.com> > > 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.
--- 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 <e...@il.ibm.com> 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.
> 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 <e...@il.ibm.com> > > 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.
--- 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 <e...@il.ibm.com> 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.
> 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 > > <https://reviews.apache.org/r/42516/diff/1/?file=1201987#file1201987line1469> > > > > 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 <e...@il.ibm.com> > > > 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 > >
