diff --git a/ipam/allocate.go b/ipam/allocate.go index d6dd2f10..d891f7fc 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -26,6 +26,8 @@ func (g *allocate) Try(alloc *Allocator) bool { } if addr, found := alloc.lookupOwned(g.ident, g.r); found { + // If we had heard that this container died, resurrect it + delete(alloc.dead, g.ident) // delete is no-op if key not in map g.resultChan <- allocateResult{addr, nil} return true } diff --git a/ipam/allocator.go b/ipam/allocator.go index ef0e0752..069ea126 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -21,8 +21,9 @@ const ( msgRingUpdate msgSpaceRequestDenied - tickInterval = time.Second * 5 - MinSubnetSize = 4 // first and last addresses are excluded, so 2 would be too small + tickInterval = time.Second * 5 + MinSubnetSize = 4 // first and last addresses are excluded, so 2 would be too small + containerDiedTimeout = time.Second * 5 ) // operation represents something which Allocator wants to do, but @@ -51,6 +52,7 @@ type Allocator struct { nicknames map[mesh.PeerName]string // so we can map nicknames for rmpeer pendingAllocates []operation // held until we get some free space pendingClaims []operation // held until we know who owns the space + dead map[string]time.Time // containers we heard were dead, and when gossip mesh.Gossip // our link to the outside world for sending messages paxos *paxos.Node paxosActive bool @@ -70,6 +72,7 @@ func NewAllocator(ourName mesh.PeerName, ourUID mesh.PeerUID, ourNickname string paxos: paxos.NewNode(ourName, ourUID, quorum), nicknames: map[mesh.PeerName]string{ourName: ourNickname}, isKnownPeer: isKnownPeer, + dead: make(map[string]time.Time), now: time.Now, } } @@ -223,10 +226,28 @@ func (alloc *Allocator) Claim(ident string, addr address.Address, noErrorOnUnkno return <-resultChan } -// ContainerDied is provided to satisfy the updater interface; does a 'Delete' underneath. Sync. +// ContainerDied called from the updater interface. Async. func (alloc *Allocator) ContainerDied(ident string) { - if err := alloc.Delete(ident); err == nil { - alloc.debugln("Container", ident, "died; released addresses") + alloc.actionChan <- func() { + if _, found := alloc.lookupOwned(ident, alloc.universe); found { + alloc.debugln("Container", ident, "died; noting to remove later") + alloc.dead[ident] = alloc.now() + } + // Also remove any pending ops + alloc.cancelOpsFor(&alloc.pendingAllocates, ident) + alloc.cancelOpsFor(&alloc.pendingClaims, ident) + } +} + +func (alloc *Allocator) removeDeadContainers() { + cutoff := alloc.now().Add(-containerDiedTimeout) + for ident, timeOfDeath := range alloc.dead { + if timeOfDeath.Before(cutoff) { + if err := alloc.delete(ident); err == nil { + alloc.debugln("Removed addresses for container", ident) + } + delete(alloc.dead, ident) + } } } @@ -236,25 +257,24 @@ func (alloc *Allocator) ContainerStarted(ident string) {} func (alloc *Allocator) Delete(ident string) error { errChan := make(chan error) alloc.actionChan <- func() { - addrs, found := alloc.owned[ident] - for _, addr := range addrs { - alloc.space.Free(addr) - } - delete(alloc.owned, ident) - - // Also remove any pending ops - found = alloc.cancelOpsFor(&alloc.pendingAllocates, ident) || found - found = alloc.cancelOpsFor(&alloc.pendingClaims, ident) || found - - if !found { - errChan <- fmt.Errorf("Delete: no addresses for %s", ident) - return - } - errChan <- nil + errChan <- alloc.delete(ident) } return <-errChan } +func (alloc *Allocator) delete(ident string) error { + addrs, found := alloc.owned[ident] + for _, addr := range addrs { + alloc.space.Free(addr) + } + delete(alloc.owned, ident) + + if !found { + return fmt.Errorf("Delete: no addresses for %s", ident) + } + return nil +} + // Free (Sync) - release single IP address for container func (alloc *Allocator) Free(ident string, addrToFree address.Address) error { errChan := make(chan error) @@ -514,6 +534,7 @@ func (alloc *Allocator) actorLoop(actionChan <-chan func()) { if alloc.paxosActive { alloc.propose() } + alloc.removeDeadContainers() alloc.tryPendingOps() } diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 766e5041..c9eef59c 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -69,8 +69,18 @@ func TestAllocFree(t *testing.T) { require.Equal(t, testAddr2, addr4.String(), "address") alloc.ContainerDied(container2) + + // Resurrect + addr1c, err := alloc.Allocate(container2, cidr1.HostRange(), returnFalse) + require.NoError(t, err) + require.Equal(t, addr1b, addr1c, "address") + alloc.ContainerDied(container3) - require.Equal(t, address.Offset(spaceSize), alloc.NumFreeAddresses(subnet)) + alloc.Encode() // sync up + // Move the clock forward and clear out the dead container + alloc.actionChan <- func() { alloc.now = func() time.Time { return time.Now().Add(containerDiedTimeout * 2) } } + alloc.actionChan <- func() { alloc.removeDeadContainers() } + require.Equal(t, address.Offset(spaceSize-1), alloc.NumFreeAddresses(subnet)) } func TestBootstrap(t *testing.T) { diff --git a/site/features.md b/site/features.md index a6115196..9a0074ac 100644 --- a/site/features.md +++ b/site/features.md @@ -156,6 +156,19 @@ connect to. The individual IP addresses given to containers must, of course, be unique - if you pick an address that the automatic allocator has already assigned you will receive a warning. +If you restart a container, it will retain the same IP addresses on +the weave network: + + host1$ docker run --name a1 -tdi ubuntu + f76b09a9fcfee04551dbb8d951d9a83e7e7d55126b02fd9f44f9f8a5f07d7c96 + host1$ weave ps a1 + a1 1e:dc:2a:db:ef:ff 10.32.0.3/12 + host1$ docker restart a1 + host1$ weave ps a1 + a1 16:c0:6f:5d:c5:73 10.32.0.3/12 + +(note that the IP address is held for a limited time - currently five seconds) + ### Naming and discovery Named containers are automatically registered in diff --git a/test/640_proxy_restart_reattaches_test.sh b/test/640_proxy_restart_reattaches_test.sh index 034d4704..584b3be2 100755 --- a/test/640_proxy_restart_reattaches_test.sh +++ b/test/640_proxy_restart_reattaches_test.sh @@ -2,8 +2,6 @@ . ./config.sh -C1=10.2.0.78 -C2=10.2.0.34 NAME=seetwo.weave.local check_attached() { @@ -26,8 +24,9 @@ wait_for_proxy() { start_suite "Proxy restart reattaches networking to containers" WEAVE_DOCKER_ARGS=--restart=always WEAVEPROXY_DOCKER_ARGS=--restart=always weave_on $HOST1 launch -proxy_start_container $HOST1 -e WEAVE_CIDR=$C2/24 -di --name=c2 --restart=always -h $NAME -proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C1/24 -di --name=c1 --restart=always +proxy_start_container $HOST1 -di --name=c2 --restart=always -h $NAME +proxy_start_container_with_dns $HOST1 -di --name=c1 --restart=always +C2=$(container_ip $HOST1 c2) proxy docker_on $HOST1 restart -t=1 c2 check_attached @@ -42,11 +41,13 @@ run_on $HOST1 sudo kill -KILL $(docker_on $HOST1 inspect --format='{{.State.Pid} sleep 1 check_attached -# Disabled because Docker 1.8 often fails to restart weave or weaveproxy # Restart docker itself, using different commands for systemd- and upstart-managed. -#run_on $HOST1 sh -c "command -v systemctl >/dev/null && sudo systemctl restart docker || sudo service docker restart" -#wait_for_proxy $HOST1 -#check_attached +run_on $HOST1 sh -c "command -v systemctl >/dev/null && sudo systemctl restart docker || sudo service docker restart" +wait_for_proxy $HOST1 +sleep 5 # allow for re-tries of attach +# Re-fetch the IP since it is not retained on docker restart +C2=$(container_ip $HOST1 c2) || (echo "container c2 has no IP address" 2>&1; exit 1) +check_attached # Restarting proxy shouldn't kill unattachable containers proxy_start_container $HOST1 -di --name=c3 --restart=always # Use ipam, so it won't be attachable w/o weave