Replace existing docker networking code and integrate with libnetwork #13060

mrjana · 2015-05-07T17:09:06Z

This PR attempts to integrate libnetwork with three main goals:

Modularize docker networking code to a separate package
Introduce the first implementation of Container Network Model and make docker purely rely on that
Allow more kinds of network drivers to integrated with ease and possibly no change to docker core at all

Please refer to following libnetwork docs for more info:

All dead code has been removed.

crosbymichael · 2015-05-07T17:38:05Z

you need to rebase ;)

mrjana · 2015-05-07T17:48:02Z

@crosbymichael done :-)

jessfraz · 2015-05-07T17:48:56Z

started test on lxc ;) feeling brave

LK4D4 · 2015-05-07T17:51:42Z

@mrjana Also, you should separate commit message body from title with empty line

calavera · 2015-05-07T17:54:33Z

very nice at first glance, thanks for vendoring the code in a separated commit, much better for reviewing ⚡

cpuguy83 · 2015-05-08T12:57:37Z

Test failure.

mavenugo · 2015-05-08T15:37:21Z

@cpuguy83 thanks. am looking into it.

mavenugo · 2015-05-10T19:53:13Z

hack/vendor.sh

@@ -53,6 +54,10 @@ clone git github.com/kr/pty 05017fcccf
 clone git github.com/tchap/go-patricia v2.1.0
 clone hg code.google.com/p/go.net 84a4013f96e0
 clone hg code.google.com/p/gosqlite 74691fb6f837
+clone git github.com/go-fsnotify/fsnotify v1.2.0
+clone git github.com/go-check/check 64131543e7896d5bcc6bd5a76287eb75ea96c673


can you pls remove the redundant go-fsnotify and go-check dependencies ?

mrjana · 2015-05-11T04:04:41Z

ping @crosbymichael @jfrazelle @LK4D4 @calavera @cpuguy83

jessfraz · 2015-05-11T20:01:30Z

Please note that the code not needed in docker any more like the networkdriver and portmapper haven't been removed yet. It can be removed in subsequent PRs once this one gets merged.

So is that code dead or is it still being called.

I think if it is dead we may as well remove it now, but that is just my opinion, I like large minus (-) ;)

jessfraz · 2015-05-11T20:04:20Z

as far as design goes I like it, it is clean and simple.

crosbymichael · 2015-05-11T20:04:52Z

Yes, don't leave dead code around, it will just be confusing for anyone changing code after you. Don't comment things out either, just delete, everything is in git so we can always see what it was before.

mrjana · 2015-05-11T21:38:06Z

@crosbymichael Sure, let me cleanup dead code.

@jfrazelle I ran lxc integration tests in my dev machine and it had only 7 failures(which was the same on stock 1.7.0-dev in my machine). Does lxc test fail intermittently?

mrjana · 2015-05-12T02:33:47Z

@jfrazelle @crosbymichael @LK4D4
Updated the PR with all the comments taken care

LK4D4 · 2015-05-12T17:00:40Z

With last commit it is definetely better.

LK4D4 · 2015-05-19T15:22:33Z

I found panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x30 pc=0x75206f]

goroutine 107 [running]:
github.com/docker/libnetwork.(*endpoint).Leave(0xc2082bcf30, 0xc2084753c0, 0x40, 0x0, 0x0, 0x0, 0x0, 0x0)
        /go/src/github.com/docker/docker/vendor/src/github.com/docker/libnetwork/endpoint.go:357 +0x2cf
github.com/docker/docker/daemon.(*Container).ReleaseNetwork(0xc208310000)
        /go/src/github.com/docker/docker/daemon/container.go:961 +0x45f
github.com/docker/docker/daemon.(*Container).cleanup(0xc208310000)
        /go/src/github.com/docker/docker/daemon/container.go:975 +0x36
github.com/docker/docker/daemon.(*containerMonitor).Close(0xc2081557a0, 0x0, 0x0)
        /go/src/github.com/docker/docker/daemon/monitor.go:86 +0x4a
github.com/docker/docker/daemon.func·031()
        /go/src/github.com/docker/docker/daemon/monitor.go:117 +0xbb
github.com/docker/docker/daemon.(*containerMonitor).Start(0xc2081557a0, 0x0, 0x0)
        /go/src/github.com/docker/docker/daemon/monitor.go:180 +0x7af
github.com/docker/docker/daemon.*containerMonitor.Start·fm(0x0, 0x0)
        /go/src/github.com/docker/docker/daemon/container.go:1584 +0x39
github.com/docker/docker/pkg/promise.func·001()
        /go/src/github.com/docker/docker/pkg/promise/promise.go:8 +0x2f
created by github.com/docker/docker/pkg/promise.Go
        /go/src/github.com/docker/docker/pkg/promise/promise.go:9 +0xfb

I created two containers with --net=host and then leave from both. Also you probably want to write test for this situation :)

LK4D4 · 2015-05-19T15:32:21Z

After daemon crash I can't start old container with:

Error response from daemon: Cannot start container c6732738abba: open /var/run/docker/netns/c6732738abba: permission denied
Error: failed to start containers: [c6732738abba]

In logs:

ERRO[0023] leaving endpoint failed: no container attached to the endpoint

LK4D4 · 2015-05-19T15:44:54Z

Last error probably was here forever, but now it became little more severe. I think we need to call container.cleanup() in daemon.register instead of just container.Unmount

icecrime · 2015-05-19T15:49:25Z

On the good side of things, tests are green with --userland-proxy=false.

estesp · 2015-05-19T17:27:24Z

@mrjana thanks for the resolv.conf test update--it looks good. Based on that, it appears that any kind of live watching of the host resolver config is now replaced from an implementation perspective by DNS setup step during container start? Given we had no way to do live update of running containers, this seems fully reasonable and at functional parity with today's implementation.

mrjana · 2015-05-19T17:32:36Z

@estesp Yes, that was the only design change which functionally doesn't change anything.

mrjana · 2015-05-19T20:15:55Z

@LK4D4 Thanks for catching the panic issue. Pushed a new set of diffs with the fix for that and a test case for that in libnetwork as well. Also rebased.

/cc @icecrime @crosbymichael

LK4D4 · 2015-05-19T20:59:35Z

Error response from daemon: Cannot start container c6732738abba: open /var/run/docker/netns/c6732738abba: permission denied
Error: failed to start containers: [c6732738abba]

still here.
And I don't see that you added some code to fix it.
I reproduced like

docker run -d busybox sleep 1000
kill docker with 4 ctrl-C
start docker
docker start old container

LK4D4 · 2015-05-19T20:59:45Z

also somehow you need rebase again :/ And port #12437

Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>

- Updated Dockerfile to satisfy libnetwork GOPATH requirements. - Reworked daemon to allocate network resources using libnetwork. - Reworked remove link code to also update network resources in libnetwork. - Adjusted the exec driver command population to reflect libnetwork design. - Adjusted the exec driver create command steps. - Updated a few test cases to reflect the change in design. - Removed the dns setup code from docker as resolv.conf is entirely managed in libnetwork. - Integrated with lxc exec driver. Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>

As part of this some generic packages like iptables, etchosts and resolvconf have also been moved to libnetwork. Even though they can still be consumed in a generic fashion they will reside and be maintained from within the libnetwork project. Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>

mrjana · 2015-05-20T00:02:34Z

@LK4D4 I pushed new diffs which attempts to fix the issue that you are having. BTW, I was not able to reproduce this issue at all in my local machine. Probably something to do with the version of kernel you are running.

In any case the way I've fixed it is to to remove the namespace path file if it exists during a new join for an old container and recreate the namespace for the container from scratch. Please test it in your machine and see if it works ok.

This is rebased again.

jessfraz · 2015-05-20T00:04:03Z

testing now on 4.0.4

On Tue, May 19, 2015 at 5:03 PM, Jana Radhakrishnan <
notifications@github.com> wrote:

@LK4D4 https://github.com/LK4D4 I pushed new diffs which attempts to
fix the issue that you are having. BTW, I was not able to reproduce this
issue at all in my local machine. Probably something to do with the version
of kernel you are running.

In any case the way I've fixed it is to to remove the namespace path file
if it exists during a new join for an old container and recreate the
namespace for the container from scratch. Please test it in your machine
and see if it works ok.

This is rebased again.

—
Reply to this email directly or view it on GitHub
#13060 (comment).

mrjana · 2015-05-20T00:05:14Z

Thanks @jfrazelle

jessfraz · 2015-05-20T00:19:09Z

LGTM

icecrime · 2015-05-20T00:23:23Z

LGTM

Replace existing docker networking code and integrate with libnetwork

mavenugo · 2015-05-20T00:31:06Z

Thanks @mrjana @icecrime @LK4D4 @crosbymichael @tiborvass @jfrazelle @estesp @ashahab-altiscale @stevvooe @cpuguy83 @aboch @dave-tucker @nerdalert for a good review & merge.

🎉 🎈 🎆

fgbreel · 2015-05-20T14:27:59Z

Finally! 👍

noizu · 2015-07-08T03:30:20Z

Is there a rough estimate of when this extension may, if approved,make it into the stable branch? 6 months, 12 months, etc.

This perfectly answers some scenarios I have but I need to decide if I implement custom work arounds and leverage third party tools or work on other components of the system until this is in place or those other work arounds need to be done.

Excellent work meanwhile. Simplified elastic-ip like routing between containers is definitely a major hole in the existing feature set.

stevvooe · 2015-07-08T05:15:29Z

@noizu This isn't really the right venue to ask this sort of question. I suggest that you use either the mailing list (docker-dev@googlegroups.org) or IRC #docker-dev.

GordonTheTurtle added the status/0-triage label May 7, 2015

mavenugo added the Networking label May 7, 2015

mrjana force-pushed the cnm_integ branch from f4a6ab8 to bd82bbf Compare May 7, 2015 17:31

mrjana force-pushed the cnm_integ branch from bd82bbf to 3712685 Compare May 7, 2015 17:47

jessfraz added status/1-design-review and removed status/0-triage labels May 7, 2015

mrjana force-pushed the cnm_integ branch 3 times, most recently from 60a3e7f to 0e1c92e Compare May 8, 2015 06:32

mrjana force-pushed the cnm_integ branch 2 times, most recently from 0f5f203 to 72597dd Compare May 10, 2015 19:13

mavenugo reviewed May 10, 2015
View reviewed changes

mrjana force-pushed the cnm_integ branch from 72597dd to 65cf477 Compare May 11, 2015 03:32

mrjana force-pushed the cnm_integ branch from 65cf477 to 4075dc2 Compare May 12, 2015 01:28

bru7us mentioned this pull request May 12, 2015

Proposal: Port Allocations from a Host Range and Host Port Persistence Policies #12622

Closed

mrjana force-pushed the cnm_integ branch from 4075dc2 to 51d02da Compare May 13, 2015 04:18

LK4D4 mentioned this pull request May 19, 2015

Fix Put without Get in overlay #13325

Merged

mrjana force-pushed the cnm_integ branch from 7f1b2b4 to aec7b8c Compare May 19, 2015 18:59

mrjana added 3 commits May 19, 2015 22:40

Vendoring in libnetwork, netns, netlink

272f8cd

Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>

mrjana force-pushed the cnm_integ branch from aec7b8c to f12f51b Compare May 19, 2015 22:45

icecrime pushed a commit that referenced this pull request May 20, 2015

Merge pull request #13060 from mrjana/cnm_integ

496bc46

Replace existing docker networking code and integrate with libnetwork

icecrime merged commit 496bc46 into moby:master May 20, 2015

tiwillia mentioned this pull request Oct 7, 2015

[TO_STAGE] oo-accept-node Report errors if selinux contexts on $GEAR_BASE_DIR are incorrect openshift/origin-server#6254

Closed

aboch mentioned this pull request Nov 23, 2015

don't try to use default route MTU as container MTU #18108

Merged

aboch mentioned this pull request Feb 29, 2016

IP Aliases support moby/libnetwork#976

Closed

thaJeztah mentioned this pull request Sep 22, 2022

libnetwork/resolvconf: removed unused GetIfChanged() and GetLastModified() #44179

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace existing docker networking code and integrate with libnetwork #13060

Replace existing docker networking code and integrate with libnetwork #13060

mrjana commented May 7, 2015

crosbymichael commented May 7, 2015

mrjana commented May 7, 2015

jessfraz commented May 7, 2015

LK4D4 commented May 7, 2015

calavera commented May 7, 2015

cpuguy83 commented May 8, 2015

mavenugo commented May 8, 2015

mavenugo May 10, 2015

mrjana commented May 11, 2015

jessfraz commented May 11, 2015

jessfraz commented May 11, 2015

crosbymichael commented May 11, 2015

mrjana commented May 11, 2015

mrjana commented May 12, 2015

LK4D4 commented May 12, 2015

LK4D4 commented May 19, 2015

LK4D4 commented May 19, 2015

LK4D4 commented May 19, 2015

icecrime commented May 19, 2015

estesp commented May 19, 2015

mrjana commented May 19, 2015

mrjana commented May 19, 2015

LK4D4 commented May 19, 2015

LK4D4 commented May 19, 2015

mrjana commented May 20, 2015

jessfraz commented May 20, 2015

mrjana commented May 20, 2015

jessfraz commented May 20, 2015

icecrime commented May 20, 2015

mavenugo commented May 20, 2015

fgbreel commented May 20, 2015

noizu commented Jul 8, 2015

stevvooe commented Jul 8, 2015

Replace existing docker networking code and integrate with libnetwork #13060

Replace existing docker networking code and integrate with libnetwork #13060

Conversation

mrjana commented May 7, 2015

crosbymichael commented May 7, 2015

mrjana commented May 7, 2015

jessfraz commented May 7, 2015

LK4D4 commented May 7, 2015

calavera commented May 7, 2015

cpuguy83 commented May 8, 2015

mavenugo commented May 8, 2015

mavenugo May 10, 2015

Choose a reason for hiding this comment

mrjana commented May 11, 2015

jessfraz commented May 11, 2015

jessfraz commented May 11, 2015

crosbymichael commented May 11, 2015

mrjana commented May 11, 2015

mrjana commented May 12, 2015

LK4D4 commented May 12, 2015

LK4D4 commented May 19, 2015

LK4D4 commented May 19, 2015

LK4D4 commented May 19, 2015

icecrime commented May 19, 2015

estesp commented May 19, 2015

mrjana commented May 19, 2015

mrjana commented May 19, 2015

LK4D4 commented May 19, 2015

LK4D4 commented May 19, 2015

mrjana commented May 20, 2015

jessfraz commented May 20, 2015

mrjana commented May 20, 2015

jessfraz commented May 20, 2015

icecrime commented May 20, 2015

mavenugo commented May 20, 2015

fgbreel commented May 20, 2015

noizu commented Jul 8, 2015

stevvooe commented Jul 8, 2015