Re: Review Request 44531: Fixed parsing network ip address with docker.

[Joris Van Remoortere]

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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Timothy Chen]

> On March 10, 2016, 11:09 p.m., Jie Yu wrote:
> > src/docker/docker.cpp, lines 336-341
> > 
> >
> > To be consistent, maybe just do `if (!ipAddressValue.isSome())` here as 
> > well?

For now I think I'll leave it as is since the depecreated API should always 
work for now, and therefore we don't expect errors.


- Timothy


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Jie Yu]

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


Fix it, then Ship it!





src/docker/docker.cpp (lines 306 - 307)


s/networkModeValue/networkMode/



src/docker/docker.cpp (lines 311 - 312)


Can you make this less jagged:
```
VLOG(1) << "Unable to detect HostConfig.NetworkMode, "
<< "attempting deprecated IP field";
```



src/docker/docker.cpp (line 315)


Can you add a NOTE about the fact that we rely on docker containerizer to 
always set `--net` (basically the discussion in the review thread)



src/docker/docker.cpp (line 316)


you can use networkMode->value here



src/docker/docker.cpp (lines 336 - 341)


To be consistent, maybe just do `if (!ipAddressValue.isSome())` here as 
well?


- Jie Yu


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Guangya Liu]

> On 三月 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`
> 
> Travis Hegner wrote:
> Do you know if it's possible to run a container with "--net=default" and 
> have the container running in something _other_ than "brige" mode? Is there 
> an option you can pass to the daemon to set the "default" networkMode to 
> something other than "bridge"?
> 
> Dan Osborne wrote:
> @Travis I'm unable to reproduce the behavior you are describing when 
> launching containers with docker 1.10.2:
> 
> ```
> [vagrant@mesos-02 ~]$ docker run --name=test1 -d --net=bridge busybox 
> sleep 360
> 52e8b0a1951a851708194278d55ea02065d405c518c35c69d52eb325f2682a05
> [vagrant@mesos-02 ~]$ docker inspect 
> --format='{{.HostConfig.NetworkMode}}' test1
> bridge
> ```
> 
> It appropriately returns 'bridge', not 'default'.
> 
> Travis Hegner wrote:
> Yes, when you pass bridge, networkMode is bridge. It's when you pass 
> "default" or omit the net parameter altogether, then networkMode is 
> "default", but the actual object name in the Networks list is "bridge".
> 
> If anyone can confirm that mesos currently **always** supplies a --net 
> parameter, and it is never currently set to "default", then this issue is not 
> a problem, as a mesos container will never have a mismatched "networkMode" 
> and actual "Network.[name]".
> 
> Dan Osborne wrote:
> Ah, thank you for clarifiying, I figured I wasn't understanding correctly.
> 
> I can confirm that mesos always supplies a --net parameter when launching 
> a docker container, and it is always host, bridge, or none. See [Line 514 in 
> docker.cpp](https://github.com/apache/mesos/blob/0.28.0-rc1/src/docker/docker.cpp#L514)
> 
> Guangya Liu wrote:
> Yes, Mesos only support host, bridge and None, and the default value is 
> host for now. Mesos do not support `default` now, so I think that this patch 
> is ok for current logic.
> 
> Guangya Liu wrote:
> Related code are here: 
> https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1625-L1631
> 
> Travis Hegner wrote:
> Also, looking at these lines: 
> https://github.com/apache/mesos/blob/master/src/docker/docker.cpp#L514-L524 
> proves that in the docker run command "--net" is always used, and only ever 
> used with the above linked enum types.
> 
> When user defined networks are supported, we will have to add a special 
> case for "default", since arbitrary names can be passed in, but until then 
> this issue is a non-issue for mesos.

Yes, it is not a problem for mesos now, I filed a issue to trace this here 
https://issues.apache.org/jira/browse/MESOS-4904


- Guangya


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


On 三月 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated 三月 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: 

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Travis Hegner]

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


Ship it!




Ship It!

- Travis Hegner


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Travis Hegner]

> On March 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`
> 
> Travis Hegner wrote:
> Do you know if it's possible to run a container with "--net=default" and 
> have the container running in something _other_ than "brige" mode? Is there 
> an option you can pass to the daemon to set the "default" networkMode to 
> something other than "bridge"?
> 
> Dan Osborne wrote:
> @Travis I'm unable to reproduce the behavior you are describing when 
> launching containers with docker 1.10.2:
> 
> ```
> [vagrant@mesos-02 ~]$ docker run --name=test1 -d --net=bridge busybox 
> sleep 360
> 52e8b0a1951a851708194278d55ea02065d405c518c35c69d52eb325f2682a05
> [vagrant@mesos-02 ~]$ docker inspect 
> --format='{{.HostConfig.NetworkMode}}' test1
> bridge
> ```
> 
> It appropriately returns 'bridge', not 'default'.
> 
> Travis Hegner wrote:
> Yes, when you pass bridge, networkMode is bridge. It's when you pass 
> "default" or omit the net parameter altogether, then networkMode is 
> "default", but the actual object name in the Networks list is "bridge".
> 
> If anyone can confirm that mesos currently **always** supplies a --net 
> parameter, and it is never currently set to "default", then this issue is not 
> a problem, as a mesos container will never have a mismatched "networkMode" 
> and actual "Network.[name]".
> 
> Dan Osborne wrote:
> Ah, thank you for clarifiying, I figured I wasn't understanding correctly.
> 
> I can confirm that mesos always supplies a --net parameter when launching 
> a docker container, and it is always host, bridge, or none. See [Line 514 in 
> docker.cpp](https://github.com/apache/mesos/blob/0.28.0-rc1/src/docker/docker.cpp#L514)
> 
> Guangya Liu wrote:
> Yes, Mesos only support host, bridge and None, and the default value is 
> host for now. Mesos do not support `default` now, so I think that this patch 
> is ok for current logic.
> 
> Guangya Liu wrote:
> Related code are here: 
> https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1625-L1631

Also, looking at these lines: 
https://github.com/apache/mesos/blob/master/src/docker/docker.cpp#L514-L524 
proves that in the docker run command "--net" is always used, and only ever 
used with the above linked enum types.

When user defined networks are supported, we will have to add a special case 
for "default", since arbitrary names can be passed in, but until then this 
issue is a non-issue for mesos.


- Travis


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Guangya Liu]

> On 三月 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`
> 
> Travis Hegner wrote:
> Do you know if it's possible to run a container with "--net=default" and 
> have the container running in something _other_ than "brige" mode? Is there 
> an option you can pass to the daemon to set the "default" networkMode to 
> something other than "bridge"?
> 
> Dan Osborne wrote:
> @Travis I'm unable to reproduce the behavior you are describing when 
> launching containers with docker 1.10.2:
> 
> ```
> [vagrant@mesos-02 ~]$ docker run --name=test1 -d --net=bridge busybox 
> sleep 360
> 52e8b0a1951a851708194278d55ea02065d405c518c35c69d52eb325f2682a05
> [vagrant@mesos-02 ~]$ docker inspect 
> --format='{{.HostConfig.NetworkMode}}' test1
> bridge
> ```
> 
> It appropriately returns 'bridge', not 'default'.
> 
> Travis Hegner wrote:
> Yes, when you pass bridge, networkMode is bridge. It's when you pass 
> "default" or omit the net parameter altogether, then networkMode is 
> "default", but the actual object name in the Networks list is "bridge".
> 
> If anyone can confirm that mesos currently **always** supplies a --net 
> parameter, and it is never currently set to "default", then this issue is not 
> a problem, as a mesos container will never have a mismatched "networkMode" 
> and actual "Network.[name]".
> 
> Dan Osborne wrote:
> Ah, thank you for clarifiying, I figured I wasn't understanding correctly.
> 
> I can confirm that mesos always supplies a --net parameter when launching 
> a docker container, and it is always host, bridge, or none. See [Line 514 in 
> docker.cpp](https://github.com/apache/mesos/blob/0.28.0-rc1/src/docker/docker.cpp#L514)
> 
> Guangya Liu wrote:
> Yes, Mesos only support host, bridge and None, and the default value is 
> host for now. Mesos do not support `default` now, so I think that this patch 
> is ok for current logic.

Related code are here: 
https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1625-L1631


- Guangya


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


On 三月 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated 三月 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Guangya Liu]

> On 三月 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`
> 
> Travis Hegner wrote:
> Do you know if it's possible to run a container with "--net=default" and 
> have the container running in something _other_ than "brige" mode? Is there 
> an option you can pass to the daemon to set the "default" networkMode to 
> something other than "bridge"?
> 
> Dan Osborne wrote:
> @Travis I'm unable to reproduce the behavior you are describing when 
> launching containers with docker 1.10.2:
> 
> ```
> [vagrant@mesos-02 ~]$ docker run --name=test1 -d --net=bridge busybox 
> sleep 360
> 52e8b0a1951a851708194278d55ea02065d405c518c35c69d52eb325f2682a05
> [vagrant@mesos-02 ~]$ docker inspect 
> --format='{{.HostConfig.NetworkMode}}' test1
> bridge
> ```
> 
> It appropriately returns 'bridge', not 'default'.
> 
> Travis Hegner wrote:
> Yes, when you pass bridge, networkMode is bridge. It's when you pass 
> "default" or omit the net parameter altogether, then networkMode is 
> "default", but the actual object name in the Networks list is "bridge".
> 
> If anyone can confirm that mesos currently **always** supplies a --net 
> parameter, and it is never currently set to "default", then this issue is not 
> a problem, as a mesos container will never have a mismatched "networkMode" 
> and actual "Network.[name]".
> 
> Dan Osborne wrote:
> Ah, thank you for clarifiying, I figured I wasn't understanding correctly.
> 
> I can confirm that mesos always supplies a --net parameter when launching 
> a docker container, and it is always host, bridge, or none. See [Line 514 in 
> docker.cpp](https://github.com/apache/mesos/blob/0.28.0-rc1/src/docker/docker.cpp#L514)

Yes, Mesos only support host, bridge and None, and the default value is host 
for now. Mesos do not support `default` now, so I think that this patch is ok 
for current logic.


- Guangya


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


On 三月 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated 三月 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Dan Osborne]

> On March 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`
> 
> Travis Hegner wrote:
> Do you know if it's possible to run a container with "--net=default" and 
> have the container running in something _other_ than "brige" mode? Is there 
> an option you can pass to the daemon to set the "default" networkMode to 
> something other than "bridge"?
> 
> Dan Osborne wrote:
> @Travis I'm unable to reproduce the behavior you are describing when 
> launching containers with docker 1.10.2:
> 
> ```
> [vagrant@mesos-02 ~]$ docker run --name=test1 -d --net=bridge busybox 
> sleep 360
> 52e8b0a1951a851708194278d55ea02065d405c518c35c69d52eb325f2682a05
> [vagrant@mesos-02 ~]$ docker inspect 
> --format='{{.HostConfig.NetworkMode}}' test1
> bridge
> ```
> 
> It appropriately returns 'bridge', not 'default'.
> 
> Travis Hegner wrote:
> Yes, when you pass bridge, networkMode is bridge. It's when you pass 
> "default" or omit the net parameter altogether, then networkMode is 
> "default", but the actual object name in the Networks list is "bridge".
> 
> If anyone can confirm that mesos currently **always** supplies a --net 
> parameter, and it is never currently set to "default", then this issue is not 
> a problem, as a mesos container will never have a mismatched "networkMode" 
> and actual "Network.[name]".

Ah, thank you for clarifiying, I figured I wasn't understanding correctly.

I can confirm that mesos always supplies a --net parameter when launching a 
docker container, and it is always host, bridge, or none. See [Line 514 in 
docker.cpp](https://github.com/apache/mesos/blob/0.28.0-rc1/src/docker/docker.cpp#L514)


- Dan


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Travis Hegner]

> On March 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`
> 
> Travis Hegner wrote:
> Do you know if it's possible to run a container with "--net=default" and 
> have the container running in something _other_ than "brige" mode? Is there 
> an option you can pass to the daemon to set the "default" networkMode to 
> something other than "bridge"?
> 
> Dan Osborne wrote:
> @Travis I'm unable to reproduce the behavior you are describing when 
> launching containers with docker 1.10.2:
> 
> ```
> [vagrant@mesos-02 ~]$ docker run --name=test1 -d --net=bridge busybox 
> sleep 360
> 52e8b0a1951a851708194278d55ea02065d405c518c35c69d52eb325f2682a05
> [vagrant@mesos-02 ~]$ docker inspect 
> --format='{{.HostConfig.NetworkMode}}' test1
> bridge
> ```
> 
> It appropriately returns 'bridge', not 'default'.

Yes, when you pass bridge, networkMode is bridge. It's when you pass "default" 
or omit the net parameter altogether, then networkMode is "default", but the 
actual object name in the Networks list is "bridge".

If anyone can confirm that mesos currently **always** supplies a --net 
parameter, and it is never currently set to "default", then this issue is not a 
problem, as a mesos container will never have a mismatched "networkMode" and 
actual "Network.[name]".


- Travis


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Dan Osborne]

> On March 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`
> 
> Travis Hegner wrote:
> Do you know if it's possible to run a container with "--net=default" and 
> have the container running in something _other_ than "brige" mode? Is there 
> an option you can pass to the daemon to set the "default" networkMode to 
> something other than "bridge"?

@Travis I'm unable to reproduce the behavior you are describing when launching 
containers with docker 1.10.2:

```
[vagrant@mesos-02 ~]$ docker run --name=test1 -d --net=bridge busybox sleep 360
52e8b0a1951a851708194278d55ea02065d405c518c35c69d52eb325f2682a05
[vagrant@mesos-02 ~]$ docker inspect --format='{{.HostConfig.NetworkMode}}' 
test1
bridge
```

It appropriately returns 'bridge', not 'default'.


- Dan


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Dan Osborne]

> On March 8, 2016, 11:51 p.m., Dan Osborne wrote:
> > Ship It!

I ran a test cluster and found no regression for launching containers networked 
with bridge and host.


- Dan


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Travis Hegner]

> On March 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.
> 
> Guangya Liu wrote:
> I was using 1.10.0, and the default value of net is `--net=default` and 
> it also works if I run the command `docker run -it --net default ubuntu:14.04 
> /bin/bash`

Do you know if it's possible to run a container with "--net=default" and have 
the container running in something _other_ than "brige" mode? Is there an 
option you can pass to the daemon to set the "default" networkMode to something 
other than "bridge"?


- Travis


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44531]

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 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Guangya Liu]

> On 三月 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.
> 
> Travis Hegner wrote:
> I've found this to be true with at least docker 1.10.2.
> 
> Is it possible to launch a container with "--net default" (or ommitted) 
> and have it not be a "bridge" in general with docker? I haven't determined 
> that yet. If not, then we could statically test for "default" and set the 
> string to "bridge" every time. Another thing to consider, if mesos /always/ 
> launches a container with an explicit "--net bridge" or "--net host" 
> parameter (until UDNs are implemented anyway), then we aren't really in 
> danger of exposing the behavior described in this issue. Launching the 
> container with an explicit "--net bridge" sets the NetworkMode field to 
> "bridge" as well.

I was using 1.10.0, and the default value of net is `--net=default` and it also 
works if I run the command `docker run -it --net default ubuntu:14.04 /bin/bash`


- Guangya


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


On 三月 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated 三月 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Travis Hegner]

> On March 9, 2016, 2:21 a.m., Guangya Liu wrote:
> > src/docker/docker.cpp, lines 306-307
> > 
> >
> > I found that the NetworkMode is `default` when using `bridge` mode, so 
> > the value of `networkModeValue` would be `default` but not `bridge`, and 
> > this will cause line 315 constrcut a invalid addressLocation as 
> > `NetworkSettings.Networks.default.IPAddress` which will always failed and 
> > thus fall to old API.

I've found this to be true with at least docker 1.10.2.

Is it possible to launch a container with "--net default" (or ommitted) and 
have it not be a "bridge" in general with docker? I haven't determined that 
yet. If not, then we could statically test for "default" and set the string to 
"bridge" every time. Another thing to consider, if mesos /always/ launches a 
container with an explicit "--net bridge" or "--net host" parameter (until UDNs 
are implemented anyway), then we aren't really in danger of exposing the 
behavior described in this issue. Launching the container with an explicit 
"--net bridge" sets the NetworkMode field to "bridge" as well.


- Travis


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


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Guangya Liu]

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




src/docker/docker.cpp (lines 306 - 307)


I found that the NetworkMode is `default` when using `bridge` mode, so the 
value of `networkModeValue` would be `default` but not `bridge`, and this will 
cause line 315 constrcut a invalid addressLocation as 
`NetworkSettings.Networks.default.IPAddress` which will always failed and thus 
fall to old API.


- Guangya Liu


On 三月 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated 三月 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Dan Osborne]

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


Ship it!




Ship It!

- Dan Osborne


On March 8, 2016, 10:54 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44531/
> ---
> 
> (Updated March 8, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Dan Osborne, Jie Yu, and Travis Hegner.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed parsing network ip address with docker.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/44531/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Timothy Chen]

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

(Updated March 8, 2016, 9:58 p.m.)


Review request for mesos, Jie Yu and Travis Hegner.


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


Repository: mesos


Description
---

Fixed parsing network ip address with docker.


Diffs
-

  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 44531: Fixed parsing network ip address with docker.

[Timothy Chen]

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

(Updated March 8, 2016, 9:58 p.m.)


Review request for mesos, Jie Yu and Travis Hegner.


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


Repository: mesos


Description
---

Fixed parsing network ip address with docker.


Diffs
-

  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

make check


Thanks,

Timothy Chen