Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docker networks to the Overlay Topology #1584

Merged
merged 3 commits into from
Jun 15, 2016
Merged

Conversation

2opremio
Copy link
Contributor

Fixes #1563

@2opremio 2opremio force-pushed the 1563-read-docker-networks branch from ba04f34 to 4181b29 Compare June 14, 2016 14:53
@2opremio 2opremio force-pushed the 1563-read-docker-networks branch from 4181b29 to 6f0a31d Compare June 14, 2016 14:58
r.networks = r.networks[:0]
for i := range networks {
r.networks = append(r.networks, &networks[i])
}

This comment was marked as abuse.

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor

tomwilkie commented Jun 14, 2016

I don't understand how this fixed that issue. Could I have a sentence or two explaining it please?

@tomwilkie
Copy link
Contributor

Okay explained f2f. Let me think for 20mins.

@2opremio
Copy link
Contributor Author

As part of the review feedback commit I have changed the registry to store (and walk) images instead of image pointers (we are not modifying them so AFAIU there was no reason to use pointers).

@tomwilkie
Copy link
Contributor

@idcrosby @2opremio as discussed, need a reproduction for this pls.

@tomwilkie
Copy link
Contributor

Try to reproduce the issue (without this PR):

$ docker network create foo
$ docker run --net=foo -d --name nginx nginx
$ docker run --net=foo -d --name client alpine /bin/sh -c "while true; do \
wget http://nginx:80/ -O - >/dev/null || true; \
sleep 1; \
done"

But the edges appeared.

@tomwilkie
Copy link
Contributor

Okay if you use the weavemesh driver I can indeed reproduce:

$ weave launch
$ docker network create --driver=weavemesh foo
$ docker run --net=foo -d --name nginx nginx
$ docker run --net=foo -d --name client alpine /bin/sh -c "while true; do \
wget http://nginx:80/ -O - >/dev/null || true; \
sleep 1; \
done"

@tomwilkie
Copy link
Contributor

/cc @rade

screen shot 2016-06-15 at 16 48 57

@tomwilkie
Copy link
Contributor

back to fons as didn't fix the above repro.

@tomwilkie tomwilkie assigned 2opremio and unassigned tomwilkie Jun 15, 2016
@2opremio
Copy link
Contributor Author

Meh, the problem is that the go client for some reason is not filling in the IPAMConfig field:

docker.Network{
        Name:   "foo",
        ID:     "fdbd13e9e24280f38e5a3810b8b55352c1d1cd099ad34a143414fe3e39a01501",
        Scope:  "local",
        Driver: "weavemesh",
        IPAM:   docker.IPAMOptions{Driver: "default", Config: []docker.IPAMConfig(nil)},
        Containers: map[string]docker.Endpoint{
                "d42139eae4697224b5deefbdaaefb927345241b62b55ca7f294e81cdc046bd7a": docker.Endpoint{
                        Name:        "nginx",
                        ID:          "30dfb5ec8abf5c882701b154e9e87439c2f2286154f72a913a34b25c3f58c569",
                        MacAddress:  "",
                        IPv4Address: "172.18.0.3/16",
                        IPv6Address: ""},
                "ec3ab14f5543c0567c84030c2585a0786aa0ad140f99dc7db3fc16bde3149b29": docker.Endpoint{
                        Name:        "client",
                        ID:          "50004dbcaca6bdca3fda7dcc24526fa61be7e23aac02a22dcc7f96df56ca0ced",
                        MacAddress:  "",
                        IPv4Address: "172.18.0.2/16",
                        IPv6Address: ""}},
        Options:  map[string]string{},
        Internal: false,
}
$ docker network inspect foo -o json                                                                                                                                
[
    {
        "Name": "foo",
        "Id": "fdbd13e9e24280f38e5a3810b8b55352c1d1cd099ad34a143414fe3e39a01501",
        "Scope": "local",
        "Driver": "weavemesh",
        "IPAM": {
            "Driver": "default",
            "Options": {},
            "Config": [
                {
                    "Subnet": "172.18.0.0/16",
                    "Gateway": "172.18.0.1/16"
                }
            ]
        },
        "Containers": {
            "d42139eae4697224b5deefbdaaefb927345241b62b55ca7f294e81cdc046bd7a": {
                "Name": "nginx",
                "EndpointID": "30dfb5ec8abf5c882701b154e9e87439c2f2286154f72a913a34b25c3f58c569",
                "MacAddress": "",
                "IPv4Address": "172.18.0.3/16",
                "IPv6Address": ""
            },
            "ec3ab14f5543c0567c84030c2585a0786aa0ad140f99dc7db3fc16bde3149b29": {
                "Name": "client",
                "EndpointID": "50004dbcaca6bdca3fda7dcc24526fa61be7e23aac02a22dcc7f96df56ca0ced",
                "MacAddress": "",
                "IPv4Address": "172.18.0.2/16",
                "IPv6Address": ""
            }
        },
        "Options": {}
    }
]

My bad: https://twitter.com/ThePracticalDev/status/687672086152753152

@2opremio
Copy link
Contributor Author

Updating the docker client made it work.

@tomwilkie
Copy link
Contributor

LGTM

@2opremio 2opremio merged commit 5d07b99 into master Jun 15, 2016
@2opremio 2opremio deleted the 1563-read-docker-networks branch June 15, 2016 18:08
2opremio pushed a commit that referenced this pull request Jun 15, 2016
They both passed tests separately and were no source-line conflicts ... but shit happens.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants