Add network restore to support docker live restore container #1244

aboch · 2016-06-11T00:39:18Z

This is an extract from #1135 retaining the changes in libnetwork core to support the network restore, with few modifications:

network drivers state restore not controlled by libnetwork
so no changes to driverapi
active sandboxes passed via config option
no change to controller.New() input and return params

Plus other commits which bring the endpoint persist logic to the existing network drivers and which reserve with the ipam driver the addresses of the existing local endpoints.

Note: this change has the advantage it can be merged irrespective of docker side changes

~~@coolljt0725 changes in overlay driver to restore the osl sandbox will be pushed as part of a separate commit.~~

Signed-off-by: Lei Jitang [email protected]
Signed-off-by: Alessandro Boch [email protected]

aboch · 2016-06-12T02:04:37Z

Corresponding changes to @coolljt0725 docker/docker commit can be found at https://github.com/aboch/docker/commit/5f3a9a7f2ed5df06aca88a28ac26ca59a9486069:

daemon to pass active sandbox list along with options via SandboxOption
do not modify sandbox port mapping config on restore

coolljt0725 · 2016-06-12T03:01:06Z

sandbox_store.go

@@ -198,7 +201,7 @@ func (c *controller) sandboxCleanup() {
 			dbExists:    true,
 		}



@aboch let's do

// restore sb.config sb.processOptions(option...) sb.restorePath()

here
instead of at https://github.com/docker/libnetwork/pull/1244/files#diff-ffc9c06e306285f214919ed203907012R707
because sb.Key() relay on sb.config.useDefaultSandBox
I had this change on local but forget to push to github in time :)

Yes, I am refactoring that code. I am now restoring in cleanupSandboxes()
Thanks for the suggestion

@aboch nice refactor :)

coolljt0725 · 2016-06-12T06:25:38Z

@aboch should we also rename the defaultScopes https://github.com/docker/libnetwork/blob/master/datastore/datastore.go#L134?
Once this pr is merged, the host and none network is persist to local db, if we downgrade the docker version, the the daemon will failed to start on creating host and none because we doesn't check if host and none exist before creation host and none.

chenchun · 2016-06-12T06:36:25Z

controller.go.orig should be deleted.

aboch · 2016-06-12T07:03:53Z

@coolljt0725 AFAIK host and none network are not saved to store. Maybe I missed where this got enabled.

aboch · 2016-06-12T07:04:03Z

@chenchun yes

coolljt0725 · 2016-06-12T07:11:24Z

@aboch host and none network are enabled to save to store in daemon side
https://github.com/aboch/docker/commit/5f3a9a7f2ed5df06aca88a28ac26ca59a9486069#diff-b56258ecab11f7b797306907327cc904R605

aboch · 2016-06-12T07:20:13Z

@aboch host and none network are enabled to save to store in daemon side
aboch/docker@5f3a9a7#diff-b56258ecab11f7b797306907327cc904R605

Ah, thanks @coolljt0725 That needs attention. Will think about it tomorrow.
Actually it could be fine, id will be unique, there isn't and there wasn't any check for existance by name in libnetwork or in docker/docker before creating null and host networks, if I have not missed that as well.

chenchun · 2016-06-12T07:23:27Z

drivers/bridge/bridge.go

@@ -1009,6 +1012,10 @@ func (d *driver) CreateEndpoint(nid, eid string, ifInfo driverapi.InterfaceInfo,
 		}
 	}

+	if err := d.storeUpdate(endpoint); err != nil {
+		logrus.Warnf("Failed to save bridge endpoint %s to store: %v", ep.id[0:7], err)


Should we return error here given we will invoke necessary rollback functions on error ?

The roll-back function should not be invoked given the if scope of err.
I do not want to fail the container start if we cannot save it to the bridge store.
The container may not even live long enough to go through a daemon reload.

Is there a problem in failing the operation if the storeUpdate fails ?
I think that will be consistent with other store failures in the create path.
(Its understandable that we can warn on Delete - since there is no recovery path).

We can fail, just that we will fail even if the user did not start the daemon with the restore flag.
Anyhow, I think that if the save to store fails for this, it would fail for many others, so I do not think the system would be usable anyway. I will make the change.

coolljt0725 · 2016-06-12T12:45:03Z

some test cases about this on daemon side is moby/moby@4cfcd60, I'll add more test cases tomorrow. I'm busy today and got no time to test this and look into this, I'll do this tomorrow.

aboch · 2016-06-12T15:35:23Z

iptables/iptables.go

+	// TODO: Do we need this change ?
+	// check if the rule exists
+	var (
+		doCheck bool


@coolljt0725 Do you think we still need this change ?

@aboch This is not need any more since we will remove the iptables during bridge driver initialization in https://github.com/docker/libnetwork/blob/master/drivers/bridge/bridge.go#L381

Thanks, that's what I thought. Thanks for confirming.

mavenugo · 2016-06-12T18:04:56Z

@aboch overall I think the refactor has simplified the interactions. Yes, it adds the states to the store. I guess that is okay given the requirement.

@coolljt0725 wdyt ?

coolljt0725 · 2016-06-13T01:48:33Z

@mavenugo I'm agree with this refactor. it's much more simple 👍

mavenugo · 2016-06-13T04:01:01Z

@coolljt0725 Please let us know if you got a chance to test this out and / or added more test cases

sanimej · 2016-06-13T04:12:51Z

restoring the overlay network sandbox has some issues. After daemon restart starting another container on an existing network fails. Will update the PR when its addressed.

sanimej · 2016-06-13T06:52:45Z

@aboch For overlay networks, after a daemon restart I was getting this error when starting a new container..

could not add veth pair inside the network sandbox: error setting interface "veth93e7d95" master to "br": Device does not exist

This was a because of a bug if the osl sandbox restore logic. Its a one liner..

sanimej@fccee4c

You can make this change in the overlay commit included in the PR

aboch · 2016-06-13T06:59:29Z

Thanks @sanimej will update shortly

coolljt0725 · 2016-06-13T08:23:43Z

@mavenugo working on it :)

coolljt0725 · 2016-06-13T08:25:29Z

sandbox_store.go

+			sb.isStub = false
+			isRestore = true
+			opts := val.([]SandboxOption)
+			sb.processOptions(opts...)


missing sb.restorePath() here, this will cause docker network connect /disconnect failed on old running container
docker network connect foo 882d7eb8a982 Error response from daemon: rename /var/lib/docker/containers/882d7eb8a982986ac37d75dc2ca5fe18cc0cdc37c8307124de73aace68954362/hash708549390 : no such file or directory

Thanks. Verified.

coolljt0725 · 2016-06-13T11:40:01Z

The daemon tests case for networking restore is coolljt0725/docker@52d5507
the test cases cover bridge host none network mode, and docker network connect/disconnect, and docker stats , embed dns. All these tests passed with commit sanimej/libnetwork@fccee4c and change in comment https://github.com/docker/libnetwork/pull/1244/files#r66754900
the ipvlan , macvlan and overlay doesn't cover.
I'll test these driver manually

coolljt0725 · 2016-06-13T12:54:49Z

drivers/overlay/ov_network.go

+	if key == "" {
+		return fmt.Errorf("failed to find old sandbox key  %s", osl.GenerateKey(fmt.Sprintf("%d-", n.initEpoch)+n.id))
+	}
+	sbox, nerr := osl.NewSandbox(key, false, true)


this should be osl.NewSandbox(key, true, true)

coolljt0725 · 2016-06-13T12:57:25Z

overlay work for me with commit sanimej/libnetwork@fccee4c and change in comment https://github.com/docker/libnetwork/pull/1244/files#r66786053

I'll test ipvlan and macvlan tomorrow

mavenugo · 2016-06-14T00:46:50Z

@aboch @coolljt0725 @sanimej thanks a lot for reducing the complexity and making it clean/modular. 👍

LGTM

coolljt0725 · 2016-06-14T01:49:22Z

@sanimej I got this error Could not open netlink handle during vni population for ns /var/run/docker/netns/2-3e0a937d28: failed to set into network namespace 23 while creating netlink socket: invalid argument when I try to start a container with overlay network, but it works fine yesterday.
am I missing something?

chenchun · 2016-06-14T01:50:20Z

sandbox.go

+			ifaceOptions[j] = append(ifaceOptions[j], sb.osSbox.InterfaceOptions().MacAddress(i.mac))
+		}
+		Ifaces[fmt.Sprintf("%s+%s", i.srcName, i.dstPrefix)] = ifaceOptions[j]
+		if joinInfo != nil {


Missing LinkLocalAddresses?

Saw this, but it is not really needed. FWIW only i.addr is needed as it is being evaluated during the restore.

For consistency, I added the link-local addresses. Thanks.

coolljt0725 · 2016-06-14T02:02:33Z

one more thing I concern about is shall we pass the active endpoints to driver on driver initialization so that we can clean up used driver endpoints?
the driver endpoints and the sandbox endpoints are stored separate, if daemon shutdown ungracefully, the driver endpoints and the sandbox endpoints could not be consistent, so it may cause fail to delete network with ActiveEndpointsError.

mavenugo · 2016-06-14T02:12:44Z

@coolljt0725 ActiveEndpointsError will be seen ONLY if the libnetwork core endpoint is left stale in the store. It will not be impacted by driver states.

Also, as you know, the libnetwork core performs stale endpoint and sandbox cleanup on bootup and also makes sure that the driver endpoint states are cleaned up (via the deleteEndpoint call) before cleaning up its own states. Hence, the cleanup functionality at bootup should handle the stale endpoint issues for ungraceful restart scenarios.

Do you see it behaving otherwise ? Did you hit the ActiveEndpointsError due to this change ?

aboch · 2016-06-14T02:18:36Z

@coolljt0725 Agree with @mavenugo, libnetwork endpoint removal from store will happen only if driver endpoint removal has succeeded. And libnetwork core will perform stale endpoint removal at each boot. So this ensures we won;t end up in acase where drivers has stale endpoints but libnetwork does not.

aboch · 2016-06-14T02:26:28Z

@sanimej I got this error Could not open netlink handle during vni population for ns /var/run/docker/netns/2-3e0a937d28: failed to set into network namespace 23 while creating netlink socket: invalid argument when I try to start a container with overlay network, but it works fine yesterday.
am I missing something?

@coolljt0725 I have not seen this problem and I have tested several cases in overlay. Given few changes/refactoring have happened, make sure to start from a clean state.

I updated https://github.com/aboch/docker/commits/rst with the latest changes and add the --live-restore commit changes.

Did you do anything special or just start a container on overlay network after a daemon restart ?

coolljt0725 · 2016-06-14T02:31:27Z

@mavenugo @aboch no, I didn't hit this error for now, just my concern :)
you explanation make it clear 👍

coolljt0725 · 2016-06-14T02:35:09Z

@coolljt0725 I have not seen this problem and I have tested several cases in overlay. Given few changes/refactoring have happened, make sure to start from a clean state.
I updated https://github.com/aboch/docker/commits/rst with the latest changes and add the --live-restore commit changes.
Did you do anything special or just start a container on overlay network after a daemon restart ?

I just can't run a container with a overlay network without daemon restart
I'll try again with https://github.com/aboch/docker/commits/rst

sanimej · 2016-06-14T03:33:00Z

@coolljt0725

I got this error Could not open netlink handle during vni population for ns /var/run/docker/netns/2-3e0a937d28: failed to set into network namespace 23 while creating netlink socket: invalid argument when I try to start a container with overlay network, but it works fine yesterday.

I didn't see this error in my testing.

I just can't run a container with a overlay network without daemon restart

Do you mean containers work after a daemon restart ? If you didn't see it before and it happens now it could be because of the new changes.

Can you try from a clean state and with a new network ?

coolljt0725 · 2016-06-14T03:36:36Z

@sanimej when I reboot my system and also delete the old states, it worked :)
@aboch @mavenugo for now, the overlay , ipvlan macvlan also work for me. so LGTM 👍

icecrime · 2016-06-14T03:45:16Z

Thanks a lot for all the hard work on this everyone <3

Signed-off-by: Lei Jitang <[email protected]>

Signed-off-by: Alessandro Boch <[email protected]>

- also in overlay/encryprion.go Signed-off-by: Alessandro Boch <[email protected]>

Signed-off-by: Santhosh Manohar <[email protected]>

Signed-off-by: Alessandro Boch <[email protected]>

chenchun · 2016-06-14T16:21:06Z

Tested bridge/overlay network with https://github.com/aboch/docker/commits/rst manually. Everything seems fine except that MASQUERADE rules duplicates if restarting daemon with live containers in bridge network. We didn't clean POSTROUTING chain during bridge driver init and we recreate MASQUERADE rules on AllocatePort.

aboch · 2016-06-14T16:29:36Z

Thanks @chenchun. If it is ok with you, we can take care of the duplicate iptables rules as a subsequent bug fix.

chenchun · 2016-06-14T16:37:21Z

That's ok.

aboch · 2016-06-14T17:05:04Z

@coolljt0725 The error you were hitting Could not open netlink handle during vni population for ns /var/run/docker/netns/2-3e0a937d28: failed to set into network namespace 23 while creating netlink socket: invalid argument was likely due a stale netns. Latest commit which restart the sandbox GC should have fixed it.

mrjana · 2016-06-14T20:33:20Z

@coolljt0725 @aboch @sanimej @mavenugo Thanks for the marathon effort in getting this done in time before 1.12-RC1. The design and changes LGTM.

GordonTheTurtle added the status/0-triage label Jun 11, 2016

This was referenced Jun 11, 2016

WIP: Networks drivers to persist endpoints #1239

Closed

Add network restore to support docker live restore container #1135

Closed

coolljt0725 reviewed Jun 12, 2016
View reviewed changes

chenchun reviewed Jun 12, 2016
View reviewed changes

aboch reviewed Jun 12, 2016
View reviewed changes

mavenugo added status/2-needs-code-review and removed status/0-triage labels Jun 12, 2016

coolljt0725 reviewed Jun 13, 2016
View reviewed changes

chenchun reviewed Jun 14, 2016
View reviewed changes

coolljt0725 and others added 7 commits June 13, 2016 23:48

Add network restore to support docker live restore container

be153a1

Signed-off-by: Lei Jitang <[email protected]>

Reserve addresses of existing local endpoints

bd0e260

Signed-off-by: Alessandro Boch <[email protected]>

Bridge driver to persist endpoints

d22b5eb

Signed-off-by: Alessandro Boch <[email protected]>

IPvlan and macvlan driver to persist endpoints

6beab9e

Signed-off-by: Alessandro Boch <[email protected]>

Populate nlHandle and use it on netns restore

c86a414

- also in overlay/encryprion.go Signed-off-by: Alessandro Boch <[email protected]>

Persist and restore overlay endpoints to handle daemon restart

04fde2f

Signed-off-by: Santhosh Manohar <[email protected]>

Make sure sandbox GC is started after live-restore

241e751

Signed-off-by: Alessandro Boch <[email protected]>

aboch mentioned this pull request Jun 14, 2016

Daemon to support network restore and Macvlan driver exits experimental moby/moby#23524

Merged

mrjana merged commit 452dff1 into moby:master Jun 14, 2016

sainath14 mentioned this pull request Jun 14, 2016

Delete endpoint from network map for macvlan driver upon endpoint deletion #1262

Merged

aboch deleted the restore branch June 27, 2016 18:39

andyxning mentioned this pull request Dec 20, 2016

Proposal: Restore running container network settings for containerd integration to support hot upgrade #975

Closed

		@@ -198,7 +201,7 @@ func (c *controller) sandboxCleanup() {
		dbExists: true,
		}

Add network restore to support docker live restore container #1244

Add network restore to support docker live restore container #1244

Conversation

aboch commented Jun 11, 2016 • edited Loading

aboch commented Jun 12, 2016

Choose a reason for hiding this comment

aboch Jun 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coolljt0725 commented Jun 12, 2016

chenchun commented Jun 12, 2016

aboch commented Jun 12, 2016

aboch commented Jun 12, 2016

coolljt0725 commented Jun 12, 2016

aboch commented Jun 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coolljt0725 commented Jun 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mavenugo commented Jun 12, 2016

coolljt0725 commented Jun 13, 2016

mavenugo commented Jun 13, 2016

sanimej commented Jun 13, 2016

sanimej commented Jun 13, 2016

aboch commented Jun 13, 2016

coolljt0725 commented Jun 13, 2016

coolljt0725 Jun 13, 2016 • edited Loading

Choose a reason for hiding this comment

aboch Jun 13, 2016 • edited Loading

Choose a reason for hiding this comment

coolljt0725 commented Jun 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coolljt0725 commented Jun 13, 2016

mavenugo commented Jun 14, 2016

coolljt0725 commented Jun 14, 2016

Choose a reason for hiding this comment

aboch Jun 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coolljt0725 commented Jun 14, 2016

mavenugo commented Jun 14, 2016

aboch commented Jun 14, 2016 • edited Loading

aboch commented Jun 14, 2016 • edited Loading

coolljt0725 commented Jun 14, 2016

coolljt0725 commented Jun 14, 2016 • edited Loading

sanimej commented Jun 14, 2016

coolljt0725 commented Jun 14, 2016 • edited Loading

icecrime commented Jun 14, 2016

chenchun commented Jun 14, 2016 • edited Loading

aboch commented Jun 14, 2016

chenchun commented Jun 14, 2016

aboch commented Jun 14, 2016

mrjana commented Jun 14, 2016

aboch commented Jun 11, 2016 •

edited

Loading

aboch Jun 12, 2016 •

edited

Loading

coolljt0725 Jun 13, 2016 •

edited

Loading

aboch Jun 13, 2016 •

edited

Loading

coolljt0725 commented Jun 13, 2016 •

edited

Loading

aboch Jun 14, 2016 •

edited

Loading

aboch commented Jun 14, 2016 •

edited

Loading

aboch commented Jun 14, 2016 •

edited

Loading

coolljt0725 commented Jun 14, 2016 •

edited

Loading

coolljt0725 commented Jun 14, 2016 •

edited

Loading

chenchun commented Jun 14, 2016 •

edited

Loading