From 34aa2e32d856070cfe86207462c07c6d0e0d083f Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Mon, 22 Jun 2015 14:16:46 +0100 Subject: [PATCH 01/69] [proxy] Only prepend weavewait if not already present This way `docker commit` will not stack up weavewaits. --- proxy/create_container_interceptor.go | 5 ++++- test/610_proxy_wait_for_weave_test.sh | 11 ++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index e12d5fa4..db9433ef 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -89,7 +89,10 @@ func (i *createContainerInterceptor) setWeaveWaitEntrypoint(container *docker.Co } } - container.Entrypoint = append(weaveWaitEntrypoint, container.Entrypoint...) + if len(container.Entrypoint) == 0 || container.Entrypoint[0] != weaveWaitEntrypoint[0] { + container.Entrypoint = append(weaveWaitEntrypoint, container.Entrypoint...) + } + return nil } diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index d13d3699..49d023e9 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -2,6 +2,10 @@ . ./config.sh +entrypoint() { + docker_on $HOST1 inspect --format="{{.Config.Entrypoint}}" "$@" +} + start_suite "Proxy waits for weave to be ready before running container commands" weave_on $HOST1 launch-proxy BASE_IMAGE=busybox @@ -10,6 +14,11 @@ if (docker_on $HOST1 images $BASE_IMAGE | grep -q $BASE_IMAGE); then docker_on $HOST1 rmi $BASE_IMAGE fi -assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.1/24' $BASE_IMAGE $CHECK_ETHWE_UP" +assert_raises "proxy docker_on $HOST1 run --name c1 -e 'WEAVE_CIDR=10.2.1.1/24' $BASE_IMAGE $CHECK_ETHWE_UP" + +# Check committed containers only have one weavewait prepended +COMMITTED_IMAGE=$(proxy docker_on $HOST1 commit c1) +assert_raises "proxy docker_on $HOST1 run --name c2 $COMMITTED_IMAGE" +assert "entrypoint c2" "$(entrypoint $COMMITTED_IMAGE)" end_suite From b2f62cc635d505f1af5af63073944c23de24675c Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 22 Jun 2015 20:09:45 +0100 Subject: [PATCH 02/69] show test failure output step towards fixing #970 --- test/600_proxy_docker_py_test.sh | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/600_proxy_docker_py_test.sh b/test/600_proxy_docker_py_test.sh index b61de7a8..045c225b 100755 --- a/test/600_proxy_docker_py_test.sh +++ b/test/600_proxy_docker_py_test.sh @@ -6,11 +6,16 @@ start_suite "Run docker-py test suite against the proxy" weave_on $HOST1 launch-proxy --no-default-ipam assert_raises "docker_on $HOST1 ps | grep weaveproxy" -assert_raises "docker_on $HOST1 run \ - -e NOT_ON_HOST=true \ - -e DOCKER_HOST=tcp://172.17.42.1:12375 \ - -v /tmp:/tmp \ - -v /var/run/docker.sock:/var/run/docker.sock \ - joffrey/docker-py python tests/integration_test.py" + +if docker_on $HOST1 run \ + -e NOT_ON_HOST=true \ + -e DOCKER_HOST=tcp://172.17.42.1:12375 \ + -v /tmp:/tmp \ + -v /var/run/docker.sock:/var/run/docker.sock \ + joffrey/docker-py python tests/integration_test.py ; then + assert_raises "true" +else + assert_raises "false" +fi end_suite From 2e5e0a7730f17fd590b7cd35f1f1e231d89fd12a Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 22 Jun 2015 21:05:42 +0100 Subject: [PATCH 03/69] remove pointless assertion --- test/600_proxy_docker_py_test.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/test/600_proxy_docker_py_test.sh b/test/600_proxy_docker_py_test.sh index 045c225b..332ca191 100755 --- a/test/600_proxy_docker_py_test.sh +++ b/test/600_proxy_docker_py_test.sh @@ -5,7 +5,6 @@ start_suite "Run docker-py test suite against the proxy" weave_on $HOST1 launch-proxy --no-default-ipam -assert_raises "docker_on $HOST1 ps | grep weaveproxy" if docker_on $HOST1 run \ -e NOT_ON_HOST=true \ From 1a17b41bc36de5671433746da4f08981167de7bb Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 22 Jun 2015 21:33:47 +0100 Subject: [PATCH 04/69] reduce test output noise --- test/600_proxy_docker_py_test.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/600_proxy_docker_py_test.sh b/test/600_proxy_docker_py_test.sh index 332ca191..84d27608 100755 --- a/test/600_proxy_docker_py_test.sh +++ b/test/600_proxy_docker_py_test.sh @@ -4,6 +4,8 @@ start_suite "Run docker-py test suite against the proxy" +docker_on $HOST1 pull joffrey/docker-py >/dev/null + weave_on $HOST1 launch-proxy --no-default-ipam if docker_on $HOST1 run \ From 9f784b3c316d557e45345044a96ec5443d7941c4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 12:22:53 +0100 Subject: [PATCH 05/69] Report errors in unicasting gossip --- ipam/allocator.go | 4 ++-- router/gossip_channel.go | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 8b609b96..98f36f3d 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -575,14 +575,14 @@ func (alloc *Allocator) propose() { alloc.gossip.GossipBroadcast(alloc.Gossip()) } -func (alloc *Allocator) sendSpaceRequest(dest router.PeerName, r address.Range) { +func (alloc *Allocator) sendSpaceRequest(dest router.PeerName, r address.Range) error { buf := new(bytes.Buffer) enc := gob.NewEncoder(buf) if err := enc.Encode(r); err != nil { panic(err) } msg := router.Concat([]byte{msgSpaceRequest}, buf.Bytes()) - alloc.gossip.GossipUnicast(dest, msg) + return alloc.gossip.GossipUnicast(dest, msg) } func (alloc *Allocator) sendRingUpdate(dest router.PeerName) { diff --git a/router/gossip_channel.go b/router/gossip_channel.go index da7dc0ec..bbc177db 100644 --- a/router/gossip_channel.go +++ b/router/gossip_channel.go @@ -3,6 +3,7 @@ package router import ( "bytes" "encoding/gob" + "fmt" "log" "sync" ) @@ -58,7 +59,10 @@ func (c *GossipChannel) deliverUnicast(srcName PeerName, origPayload []byte, dec return err } if c.ourself.Name != destName { - return c.relayUnicast(destName, origPayload) + c.relayUnicast(destName, origPayload) + // ignore errors in passing on data; a problem between us and destination + // is not enough reason to break the connection from the source + return nil } var payload []byte if err := dec.Decode(&payload); err != nil { @@ -160,15 +164,18 @@ func (c *GossipChannel) GossipBroadcast(update GossipData) error { return c.relayBroadcast(c.ourself.Name, update) } -func (c *GossipChannel) relayUnicast(dstPeerName PeerName, buf []byte) error { +func (c *GossipChannel) relayUnicast(dstPeerName PeerName, buf []byte) (err error) { if relayPeerName, found := c.routes.UnicastAll(dstPeerName); !found { - c.log("unknown relay destination:", dstPeerName) + err = fmt.Errorf("unknown relay destination: %s", dstPeerName) } else if conn, found := c.ourself.ConnectionTo(relayPeerName); !found { - c.log("unable to find connection to relay peer", relayPeerName) + err = fmt.Errorf("unable to find connection to relay peer %s", relayPeerName) } else { conn.(ProtocolSender).SendProtocolMsg(ProtocolMsg{ProtocolGossipUnicast, buf}) } - return nil + if err != nil { + c.log(err) + } + return err } func (c *GossipChannel) relayBroadcast(srcName PeerName, update GossipData) error { From af9668f130f6b115a2d7b9a38c1384a90c19cdf8 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 12:47:38 +0100 Subject: [PATCH 06/69] Keep trying peers to ask for space until we find one that doesn't error --- ipam/allocate.go | 7 +++++-- ipam/ring/ring.go | 31 +++++++++++++++---------------- ipam/ring/ring_test.go | 37 ++++++++++++++++++++----------------- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/ipam/allocate.go b/ipam/allocate.go index 130380e9..c1dcda66 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -42,9 +42,12 @@ func (g *allocate) Try(alloc *Allocator) bool { } // out of space - if donor, err := alloc.ring.ChoosePeerToAskForSpace(g.r.Start, g.r.End); err == nil { + donors := alloc.ring.ChoosePeersToAskForSpace(g.r.Start, g.r.End) + for _, donor := range donors { alloc.debugln("Decided to ask peer", donor, "for space in range", g.r) - alloc.sendSpaceRequest(donor, g.r) + if err := alloc.sendSpaceRequest(donor, g.r); err == nil { + break + } } return false diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index fcdb362e..4908e571 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -392,9 +392,9 @@ func (r *Ring) ReportFree(freespace map[address.Address]address.Offset) { } } -// ChoosePeerToAskForSpace chooses a weighted-random peer to ask -// for space in the range [start, end). Assumes start 0 { + // Pick random peer, weighted by total free space + rn := rand.Int63n(int64(sum)) // note using 64-bit because rand.Intn uses signed int + for peername, space := range totalSpacePerPeer { + rn -= int64(space) + if rn < 0 { + result = append(result, peername) + sum -= space + delete(totalSpacePerPeer, peername) + break + } } } - - panic("Should never reach this point") + return } func (r *Ring) PickPeerForTransfer() router.PeerName { diff --git a/ipam/ring/ring_test.go b/ipam/ring/ring_test.go index ce63a38e..7c23ccdd 100644 --- a/ipam/ring/ring_test.go +++ b/ipam/ring/ring_test.go @@ -345,39 +345,42 @@ func TestGossip(t *testing.T) { func TestFindFree(t *testing.T) { ring1 := New(start, end, peer1name) - _, err := ring1.ChoosePeerToAskForSpace(start, end) - wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") + peers := ring1.ChoosePeersToAskForSpace(start, end) + wt.AssertTrue(t, len(peers) == 0, "Expected no peers") ring1.Entries = []*entry{{Token: start, Peer: peer1name}} - _, err = ring1.ChoosePeerToAskForSpace(start, end) - wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") + peers = ring1.ChoosePeersToAskForSpace(start, end) + wt.AssertTrue(t, len(peers) == 0, "Expected no peers") // We shouldn't return outselves ring1.ReportFree(map[address.Address]address.Offset{start: 10}) - _, err = ring1.ChoosePeerToAskForSpace(start, end) - wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") + peers = ring1.ChoosePeersToAskForSpace(start, end) + wt.AssertTrue(t, len(peers) == 0, "Expected no peers") ring1.Entries = []*entry{{Token: start, Peer: peer1name, Free: 1}, {Token: middle, Peer: peer1name, Free: 1}} - _, err = ring1.ChoosePeerToAskForSpace(start, end) - wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") + peers = ring1.ChoosePeersToAskForSpace(start, end) + wt.AssertTrue(t, len(peers) == 0, "Expected no peers") ring1.assertInvariants() // We should return others ring1.Entries = []*entry{{Token: start, Peer: peer2name, Free: 1}} - peer, err := ring1.ChoosePeerToAskForSpace(start, end) - wt.AssertSuccess(t, err) - wt.AssertEquals(t, peer, peer2name) + peers = ring1.ChoosePeersToAskForSpace(start, end) + wt.AssertTrue(t, len(peers) == 1, "Expected one peer") + wt.AssertEquals(t, peers[0], peer2name) ring1.Entries = []*entry{{Token: start, Peer: peer2name, Free: 1}, {Token: middle, Peer: peer3name, Free: 1}} - peer, err = ring1.ChoosePeerToAskForSpace(start, middle) - wt.AssertSuccess(t, err) - wt.AssertEquals(t, peer, peer2name) + peers = ring1.ChoosePeersToAskForSpace(start, middle) + wt.AssertTrue(t, len(peers) == 1, "Expected one peer") + wt.AssertEquals(t, peers[0], peer2name) - peer, err = ring1.ChoosePeerToAskForSpace(middle, end) - wt.AssertSuccess(t, err) - wt.AssertEquals(t, peer, peer3name) + peers = ring1.ChoosePeersToAskForSpace(middle, end) + wt.AssertTrue(t, len(peers) == 1, "Expected one peer") + wt.AssertEquals(t, peers[0], peer3name) + + peers = ring1.ChoosePeersToAskForSpace(start, end) + wt.AssertTrue(t, len(peers) == 2, "Expected two peers") ring1.assertInvariants() } From c58cc449c07941f6f38b49730e246c4822102902 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 13:19:49 +0100 Subject: [PATCH 07/69] More complex ChoosePeersToAskForSpace, with better complexity --- ipam/ring/ring.go | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index 4908e571..c411b163 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -395,10 +395,7 @@ func (r *Ring) ReportFree(freespace map[address.Address]address.Offset) { // ChoosePeerToAskForSpace returns all peers we can ask for space in // the range [start, end), in weighted-random order. Assumes start 0 { - // Pick random peer, weighted by total free space - rn := rand.Int63n(int64(sum)) // note using 64-bit because rand.Intn uses signed int - for peername, space := range totalSpacePerPeer { - rn -= int64(space) - if rn < 0 { - result = append(result, peername) - sum -= space - delete(totalSpacePerPeer, peername) - break - } + // Compute cumulative distribution of spaces + type spaceCDF struct { + sum address.Offset + peername router.PeerName + } + var cdf []spaceCDF + var sum address.Offset + for peername, space := range totalSpacePerPeer { + sum += space + cdf = append(cdf, spaceCDF{sum: sum, peername: peername}) + } + + for len(cdf) > 0 { + // Pick a number up to remaining free space + // (note using 64-bit because rand.Intn is signed int) + rn := rand.Int63n(int64(cdf[len(cdf)-1].sum)) + // Find where this number lands in the CDF, and take that peer + i := sort.Search(len(cdf), func(j int) bool { return int64(cdf[j].sum) > rn }) + result = append(result, cdf[i].peername) + // Remove this entry from the CDF and adjust all following sums + space := totalSpacePerPeer[cdf[i].peername] + for j := i; j < len(cdf); j++ { + cdf[j].sum -= space } + cdf = append(cdf[:i], cdf[i+1:]...) } return } From b472da18f04b65470e7197521e5c5bbd68918f50 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 14:04:37 +0100 Subject: [PATCH 08/69] Improve logging of unicast gossip errors --- ipam/allocate.go | 6 ++++-- router/gossip_channel.go | 11 +++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ipam/allocate.go b/ipam/allocate.go index c1dcda66..f2da3c74 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -44,8 +44,10 @@ func (g *allocate) Try(alloc *Allocator) bool { // out of space donors := alloc.ring.ChoosePeersToAskForSpace(g.r.Start, g.r.End) for _, donor := range donors { - alloc.debugln("Decided to ask peer", donor, "for space in range", g.r) - if err := alloc.sendSpaceRequest(donor, g.r); err == nil { + if err := alloc.sendSpaceRequest(donor, g.r); err != nil { + alloc.debugln("Problem asking peer", donor, "for space:", err) + } else { + alloc.debugln("Decided to ask peer", donor, "for space in range", g.r) break } } diff --git a/router/gossip_channel.go b/router/gossip_channel.go index bbc177db..39de093a 100644 --- a/router/gossip_channel.go +++ b/router/gossip_channel.go @@ -59,9 +59,11 @@ func (c *GossipChannel) deliverUnicast(srcName PeerName, origPayload []byte, dec return err } if c.ourself.Name != destName { - c.relayUnicast(destName, origPayload) - // ignore errors in passing on data; a problem between us and destination - // is not enough reason to break the connection from the source + if err := c.relayUnicast(destName, origPayload); err != nil { + // just log errors from relayUnicast; a problem between us and destination + // is not enough reason to break the connection from the source + c.log(err) + } return nil } var payload []byte @@ -172,9 +174,6 @@ func (c *GossipChannel) relayUnicast(dstPeerName PeerName, buf []byte) (err erro } else { conn.(ProtocolSender).SendProtocolMsg(ProtocolMsg{ProtocolGossipUnicast, buf}) } - if err != nil { - c.log(err) - } return err } From c76f640e2e67d9dfeb345269dc0671eaf0ec31f4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 14:48:17 +0100 Subject: [PATCH 09/69] Simpler implementation of weighted-random order --- ipam/ring/ring.go | 50 +++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index c411b163..da65014b 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -392,10 +392,23 @@ func (r *Ring) ReportFree(freespace map[address.Address]address.Offset) { } } -// ChoosePeerToAskForSpace returns all peers we can ask for space in +type weightedPeer struct { + weight float64 + peername router.PeerName +} +type weightedPeers []weightedPeer + +func (ws weightedPeers) Len() int { return len(ws) } +func (ws weightedPeers) Less(i, j int) bool { return ws[i].weight < ws[j].weight } +func (ws weightedPeers) Swap(i, j int) { ws[i], ws[j] = ws[j], ws[i] } + +// ChoosePeersToAskForSpace returns all peers we can ask for space in // the range [start, end), in weighted-random order. Assumes start 0 { - // Pick a number up to remaining free space - // (note using 64-bit because rand.Intn is signed int) - rn := rand.Int63n(int64(cdf[len(cdf)-1].sum)) - // Find where this number lands in the CDF, and take that peer - i := sort.Search(len(cdf), func(j int) bool { return int64(cdf[j].sum) > rn }) - result = append(result, cdf[i].peername) - // Remove this entry from the CDF and adjust all following sums - space := totalSpacePerPeer[cdf[i].peername] - for j := i; j < len(cdf); j++ { - cdf[j].sum -= space - } - cdf = append(cdf[:i], cdf[i+1:]...) + sort.Sort(ws) + // Reverse order (bigger weights come last after sort) and copy to result + for i := range ws { + result = append(result, ws[len(ws)-i-1].peername) } return } From 8a4b0d536eb92c2bbcc06eaa18299aabe362255f Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 15:30:49 +0100 Subject: [PATCH 10/69] Updated after review feedback --- ipam/ring/ring.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index da65014b..127175e8 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -398,13 +398,14 @@ type weightedPeer struct { } type weightedPeers []weightedPeer +// Note Less is using > so that bigger weights sort earlier +func (ws weightedPeers) Less(i, j int) bool { return ws[i].weight > ws[j].weight } func (ws weightedPeers) Len() int { return len(ws) } -func (ws weightedPeers) Less(i, j int) bool { return ws[i].weight < ws[j].weight } func (ws weightedPeers) Swap(i, j int) { ws[i], ws[j] = ws[j], ws[i] } // ChoosePeersToAskForSpace returns all peers we can ask for space in // the range [start, end), in weighted-random order. Assumes start Date: Thu, 25 Jun 2015 16:45:18 +0100 Subject: [PATCH 11/69] More idiomatic exitChan --- ipam/testutils_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 6768a121..2ffac518 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -205,7 +205,7 @@ type gossipMessage struct { sender *router.PeerName buf []byte // for unicast data router.GossipData // for broadcast - exitChan chan bool + exitChan chan struct{} } type TestGossipRouter struct { @@ -225,7 +225,7 @@ func (grouter *TestGossipRouter) GossipBroadcast(update router.GossipData) error func (grouter *TestGossipRouter) removePeer(peer router.PeerName) { gossipChan := grouter.gossipChans[peer] - resultChan := make(chan bool) + resultChan := make(chan struct{}) gossipChan <- gossipMessage{exitChan: resultChan} <-resultChan delete(grouter.gossipChans, peer) @@ -245,7 +245,7 @@ func (grouter *TestGossipRouter) connect(sender router.PeerName, gossiper router select { case message := <-gossipChan: if message.exitChan != nil { - message.exitChan <- true + close(message.exitChan) return } From 959e5f0082b42d9919df40f80c1c6fa89d6499b2 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 17:04:07 +0100 Subject: [PATCH 12/69] Use different object types for mock gossip messages --- ipam/allocator_test.go | 2 +- ipam/testutils_test.go | 38 ++++++++++++++++++++------------------ 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index d32e9174..2aaece78 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -168,7 +168,7 @@ func TestCancel(t *testing.T) { CIDR = "10.0.1.7/26" ) - router := TestGossipRouter{make(map[router.PeerName]chan gossipMessage), 0.0} + router := TestGossipRouter{make(map[router.PeerName]chan interface{}), 0.0} alloc1, subnet := makeAllocator("01:00:00:02:00:00", CIDR, 2) alloc1.SetInterfaces(router.connect(alloc1.ourName, alloc1)) diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 2ffac518..fb2af1fd 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -200,23 +200,26 @@ func AssertNothingSentErr(t *testing.T, ch <-chan error) { } // Router to convey gossip from one gossiper to another, for testing -type gossipMessage struct { - isUnicast bool - sender *router.PeerName - buf []byte // for unicast - data router.GossipData // for broadcast - exitChan chan struct{} +type unicastMessage struct { + sender *router.PeerName + buf []byte +} +type broadcastMessage struct { + data router.GossipData +} +type exitMessage struct { + exitChan chan struct{} } type TestGossipRouter struct { - gossipChans map[router.PeerName]chan gossipMessage + gossipChans map[router.PeerName]chan interface{} loss float32 // 0.0 means no loss } func (grouter *TestGossipRouter) GossipBroadcast(update router.GossipData) error { for _, gossipChan := range grouter.gossipChans { select { - case gossipChan <- gossipMessage{data: update}: + case gossipChan <- broadcastMessage{data: update}: default: // drop the message if we cannot send it } } @@ -226,7 +229,7 @@ func (grouter *TestGossipRouter) GossipBroadcast(update router.GossipData) error func (grouter *TestGossipRouter) removePeer(peer router.PeerName) { gossipChan := grouter.gossipChans[peer] resultChan := make(chan struct{}) - gossipChan <- gossipMessage{exitChan: resultChan} + gossipChan <- exitMessage{exitChan: resultChan} <-resultChan delete(grouter.gossipChans, peer) } @@ -237,27 +240,26 @@ type TestGossipRouterClient struct { } func (grouter *TestGossipRouter) connect(sender router.PeerName, gossiper router.Gossiper) router.Gossip { - gossipChan := make(chan gossipMessage, 100) + gossipChan := make(chan interface{}, 100) go func() { gossipTimer := time.Tick(10 * time.Second) for { select { - case message := <-gossipChan: - if message.exitChan != nil { + case gossip := <-gossipChan: + if message, isExit := gossip.(exitMessage); isExit { close(message.exitChan) return } - if rand.Float32() > (1.0 - grouter.loss) { continue } - - if message.isUnicast { + switch message := gossip.(type) { + case unicastMessage: if err := gossiper.OnGossipUnicast(*message.sender, message.buf); err != nil { panic(fmt.Sprintf("Error doing gossip unicast to %s: %s", sender, err)) } - } else { + case broadcastMessage: for _, msg := range message.data.Encode() { if _, err := gossiper.OnGossipBroadcast(msg); err != nil { panic(fmt.Sprintf("Error doing gossip broadcast to %s: %s", sender, err)) @@ -276,7 +278,7 @@ func (grouter *TestGossipRouter) connect(sender router.PeerName, gossiper router func (client TestGossipRouterClient) GossipUnicast(dstPeerName router.PeerName, buf []byte) error { select { - case client.router.gossipChans[dstPeerName] <- gossipMessage{isUnicast: true, sender: &client.sender, buf: buf}: + case client.router.gossipChans[dstPeerName] <- unicastMessage{sender: &client.sender, buf: buf}: default: // drop the message if we cannot send it common.Error.Printf("Dropping message") } @@ -289,7 +291,7 @@ func (client TestGossipRouterClient) GossipBroadcast(update router.GossipData) e func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, TestGossipRouter, address.Range) { - gossipRouter := TestGossipRouter{make(map[router.PeerName]chan gossipMessage), 0.0} + gossipRouter := TestGossipRouter{make(map[router.PeerName]chan interface{}), 0.0} allocs := make([]*Allocator, size) var subnet address.Range From c659e3448ad551996ff2a235a43f20efea3a2da4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 17:38:02 +0100 Subject: [PATCH 13/69] Add a flush() function to the mock gossip router, and call it where required --- ipam/allocator_test.go | 4 ++++ ipam/testutils_test.go | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 2aaece78..6cc9aae5 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -256,13 +256,17 @@ func TestTransfer(t *testing.T) { wt.AssertTrue(t, err == nil, "Failed to get address") router.GossipBroadcast(alloc2.Gossip()) + router.flush() router.GossipBroadcast(alloc3.Gossip()) + router.flush() router.removePeer(alloc2.ourName) router.removePeer(alloc3.ourName) alloc2.Stop() alloc3.Stop() + router.flush() wt.AssertSuccess(t, alloc1.AdminTakeoverRanges(alloc2.ourName.String())) wt.AssertSuccess(t, alloc1.AdminTakeoverRanges(alloc3.ourName.String())) + router.flush() wt.AssertEquals(t, alloc1.NumFreeAddresses(subnet), address.Offset(1022)) diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index fb2af1fd..8448c2a2 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -210,6 +210,9 @@ type broadcastMessage struct { type exitMessage struct { exitChan chan struct{} } +type flushMessage struct { + flushChan chan struct{} +} type TestGossipRouter struct { gossipChans map[router.PeerName]chan interface{} @@ -226,6 +229,14 @@ func (grouter *TestGossipRouter) GossipBroadcast(update router.GossipData) error return nil } +func (grouter *TestGossipRouter) flush() { + for _, gossipChan := range grouter.gossipChans { + flushChan := make(chan struct{}) + gossipChan <- flushMessage{flushChan: flushChan} + <-flushChan + } +} + func (grouter *TestGossipRouter) removePeer(peer router.PeerName) { gossipChan := grouter.gossipChans[peer] resultChan := make(chan struct{}) @@ -251,6 +262,9 @@ func (grouter *TestGossipRouter) connect(sender router.PeerName, gossiper router close(message.exitChan) return } + if message, isFlush := gossip.(flushMessage); isFlush { + close(message.flushChan) + } if rand.Float32() > (1.0 - grouter.loss) { continue } @@ -305,7 +319,7 @@ func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, TestGossipRou } gossipRouter.GossipBroadcast(allocs[size-1].Gossip()) - time.Sleep(1000 * time.Millisecond) + gossipRouter.flush() return allocs, gossipRouter, subnet } From 6725c8355f886e65518e0ef9c2073e05b6bb7ce0 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 17:41:18 +0100 Subject: [PATCH 14/69] Check the result of Allocate in TestFakeRouterSimple() --- ipam/allocator_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 6cc9aae5..33bed602 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -286,7 +286,8 @@ func TestFakeRouterSimple(t *testing.T) { alloc1 := allocs[0] //alloc2 := allocs[1] - alloc1.Allocate("foo", subnet, nil) + _, err := alloc1.Allocate("foo", subnet, nil) + wt.AssertTrue(t, err == nil, "Failed to get address") } func TestAllocatorFuzz(t *testing.T) { From c84c5dbcdfa4841b7b78519241fa721eb4cdfc89 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 25 Jun 2015 18:13:43 +0100 Subject: [PATCH 15/69] Make TestGossipRouter goroutine easier to follow --- ipam/testutils_test.go | 64 +++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 8448c2a2..73334e2d 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -201,7 +201,7 @@ func AssertNothingSentErr(t *testing.T, ch <-chan error) { // Router to convey gossip from one gossiper to another, for testing type unicastMessage struct { - sender *router.PeerName + sender router.PeerName buf []byte } type broadcastMessage struct { @@ -250,41 +250,47 @@ type TestGossipRouterClient struct { sender router.PeerName } -func (grouter *TestGossipRouter) connect(sender router.PeerName, gossiper router.Gossiper) router.Gossip { - gossipChan := make(chan interface{}, 100) +func (grouter *TestGossipRouter) run(gossiper router.Gossiper, gossipChan chan interface{}) { + gossipTimer := time.Tick(10 * time.Second) + for { + select { + case gossip := <-gossipChan: + switch message := gossip.(type) { + case exitMessage: + close(message.exitChan) + return - go func() { - gossipTimer := time.Tick(10 * time.Second) - for { - select { - case gossip := <-gossipChan: - if message, isExit := gossip.(exitMessage); isExit { - close(message.exitChan) - return - } - if message, isFlush := gossip.(flushMessage); isFlush { - close(message.flushChan) - } + case flushMessage: + close(message.flushChan) + + case unicastMessage: if rand.Float32() > (1.0 - grouter.loss) { continue } - switch message := gossip.(type) { - case unicastMessage: - if err := gossiper.OnGossipUnicast(*message.sender, message.buf); err != nil { - panic(fmt.Sprintf("Error doing gossip unicast to %s: %s", sender, err)) - } - case broadcastMessage: - for _, msg := range message.data.Encode() { - if _, err := gossiper.OnGossipBroadcast(msg); err != nil { - panic(fmt.Sprintf("Error doing gossip broadcast to %s: %s", sender, err)) - } + if err := gossiper.OnGossipUnicast(message.sender, message.buf); err != nil { + panic(fmt.Sprintf("Error doing gossip unicast to %s: %s", message.sender, err)) + } + + case broadcastMessage: + if rand.Float32() > (1.0 - grouter.loss) { + continue + } + for _, msg := range message.data.Encode() { + if _, err := gossiper.OnGossipBroadcast(msg); err != nil { + panic(fmt.Sprintf("Error doing gossip broadcast: %s", err)) } } - case <-gossipTimer: - grouter.GossipBroadcast(gossiper.Gossip()) } + case <-gossipTimer: + grouter.GossipBroadcast(gossiper.Gossip()) } - }() + } +} + +func (grouter *TestGossipRouter) connect(sender router.PeerName, gossiper router.Gossiper) router.Gossip { + gossipChan := make(chan interface{}, 100) + + go grouter.run(gossiper, gossipChan) grouter.gossipChans[sender] = gossipChan return TestGossipRouterClient{grouter, sender} @@ -292,7 +298,7 @@ func (grouter *TestGossipRouter) connect(sender router.PeerName, gossiper router func (client TestGossipRouterClient) GossipUnicast(dstPeerName router.PeerName, buf []byte) error { select { - case client.router.gossipChans[dstPeerName] <- unicastMessage{sender: &client.sender, buf: buf}: + case client.router.gossipChans[dstPeerName] <- unicastMessage{sender: client.sender, buf: buf}: default: // drop the message if we cannot send it common.Error.Printf("Dropping message") } From a938608cc87474452bd05f8ae545edc027ceeb4e Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 30 Jun 2015 15:09:06 +0100 Subject: [PATCH 16/69] Call weave reset before deleting containers, to improve clean-down --- test/config.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/config.sh b/test/config.sh index 3c3c0db4..9ca96777 100644 --- a/test/config.sh +++ b/test/config.sh @@ -170,8 +170,8 @@ assert_dns_ptr_record() { start_suite() { for host in $HOST1 $HOST2; do [ -z "$DEBUG" ] || echo "Cleaning up on $host: removing all containers and resetting weave" - rm_containers $host $(docker_on $host ps -aq 2>/dev/null) weave_on $host reset 2>/dev/null + rm_containers $host $(docker_on $host ps -aq 2>/dev/null) done whitely echo "$@" } From 3bd5ae56f01de45be6bf84bfea2fe7720675bcaa Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 2 Jul 2015 11:46:48 +0100 Subject: [PATCH 17/69] fix `make update` Trying to build the thing for which we want to fetch the dependencies is rather silly. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index bfd649ee..8973e892 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ all: $(EXPORTS) travis: $(EXES) -update: $(EXES) +update: go get -u -f -v -tags -netgo $(addprefix ./,$(dir $(EXES))) $(WEAVER_EXE) $(WEAVEDNS_EXE) $(WEAVEPROXY_EXE) $(NETCHECK_EXE): common/*.go common/*/*.go net/*.go From 3914c4c018afa48168804209abc5ec08285d1987 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Fri, 26 Jun 2015 16:40:10 +0100 Subject: [PATCH 18/69] Enforce maximum length on proxy hostname derivation --- proxy/create_container_interceptor.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index db9433ef..b7d8d0c2 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -13,6 +13,8 @@ import ( "github.com/weaveworks/weave/nameserver" ) +const MaxDockerHostname = 64 + type createContainerInterceptor struct{ proxy *Proxy } type createContainerRequestBody struct { @@ -110,9 +112,14 @@ func (i *createContainerInterceptor) setWeaveDNS(container *createContainerReque name := r.URL.Query().Get("name") if container.Hostname == "" && name != "" { - container.Hostname = name // Strip trailing period because it's unusual to see it used on the end of a host name - container.Domainname = strings.TrimSuffix(dnsDomain, ".") + trimmedDNSDomain := strings.TrimSuffix(dnsDomain, ".") + if len(name)+1+len(trimmedDNSDomain) > MaxDockerHostname { + Warning.Printf("Container name [%s] too long to be used as hostname", name) + } else { + container.Hostname = name + container.Domainname = trimmedDNSDomain + } } if len(container.HostConfig.DNSSearch) == 0 { From d1a887857d04cf095dfea3c4ca301544d334c122 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Fri, 26 Jun 2015 17:44:37 +0100 Subject: [PATCH 19/69] Enforce maximum length on weave run hostname derivation --- weave | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/weave b/weave index b36721b0..9574975d 100755 --- a/weave +++ b/weave @@ -693,8 +693,13 @@ dns_args() { DNS_ARGS="--dns $DOCKER_BRIDGE_IP" if [ -n "$NAME_ARG" -a -z "$HOSTNAME_SPECIFIED" ] ; then DOMAIN=$(dns_domain) - DNS_ARGS="$DNS_ARGS --hostname=$NAME_ARG.${DOMAIN%.}" - HOSTNAME_SPECIFIED=1 + HOSTNAME="$NAME_ARG.${DOMAIN%.}" + if [ ${#HOSTNAME} -gt 64 ] ; then + echo "Container name too long to be used as hostname" >&2 + else + DNS_ARGS="$DNS_ARGS --hostname=$HOSTNAME" + HOSTNAME_SPECIFIED=1 + fi fi if [ -z "$DNS_SEARCH_SPECIFIED" ] ; then if [ -z "$HOSTNAME_SPECIFIED" ] ; then From d427c99161174b59fc9d14c2c0e3b43cf15c4cb7 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Wed, 1 Jul 2015 14:35:39 +0100 Subject: [PATCH 20/69] Add smoke test --- test/270_use_name_as_hostname_test.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/270_use_name_as_hostname_test.sh b/test/270_use_name_as_hostname_test.sh index ccb0ff86..9d59bbe9 100755 --- a/test/270_use_name_as_hostname_test.sh +++ b/test/270_use_name_as_hostname_test.sh @@ -33,4 +33,8 @@ assert_expected_fqdn "$HOSTNAME" -h $HOSTNAME --name=$NAME assert_expected_fqdn "$HOSTNAME" --hostname=$HOSTNAME --name=$NAME assert_expected_fqdn "$HOSTNAME" --hostname $HOSTNAME --name=$NAME +# Ensure we can launch a container with a name that is too long for hostname derivation +SIXTY_FIVE_CHARS=01234567890123456789012345678901234567890123456789012345678901234 +assert_raises "start_container $HOST1 --name=$SIXTY_FIVE_CHARS" + end_suite From 8b35f5e8feb33c66cdc9d3dbcd88c81636151d73 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 3 Jul 2015 10:03:07 +0100 Subject: [PATCH 21/69] fix typo in comment --- ipam/ring/ring.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index 127175e8..ac964b8d 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -348,9 +348,9 @@ func (r *Ring) String() string { return buffer.String() } -// ReportFree is used by the allocator to tell the ring -// how many free ips are in a given range, so that ChoosePeerToAskForSpace -// can make more intelligent decisions. +// ReportFree is used by the allocator to tell the ring how many free +// ips are in a given range, so that ChoosePeersToAskForSpace can make +// more intelligent decisions. func (r *Ring) ReportFree(freespace map[address.Address]address.Offset) { r.assertInvariants() defer r.assertInvariants() From 6097dc8aaaef343e8795118844d3aec77b017dfa Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Wed, 1 Jul 2015 16:48:46 +0100 Subject: [PATCH 22/69] Add wait forever capability to EnsureInterface --- net/if.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/net/if.go b/net/if.go index 9cea43bc..3cf3baab 100644 --- a/net/if.go +++ b/net/if.go @@ -2,6 +2,7 @@ package net import ( "fmt" + "math" "net" "time" ) @@ -10,11 +11,17 @@ const ( sleepTime = 100 * time.Millisecond ) +// Wait `wait` seconds for an interface to come up. Pass zero to check once +// and return immediately, or a negative value to wait indefinitely. func EnsureInterface(ifaceName string, wait int) (iface *net.Interface, err error) { if iface, err = findInterface(ifaceName); err == nil || wait == 0 { return } - for i := time.Duration(wait) * time.Second / sleepTime; err != nil && i > 0; i-- { + i := int64(math.MaxInt64) + if wait > 0 { + i = int64(time.Duration(wait) * time.Second / sleepTime) + } + for ; err != nil && i > 0; i-- { time.Sleep(sleepTime) iface, err = findInterface(ifaceName) } From 227cb11f255204517b9fa9c1c068d81e48017496 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Wed, 1 Jul 2015 16:53:09 +0100 Subject: [PATCH 23/69] Router, DNS and proxy wait forever by default --- prog/weavedns/Dockerfile | 2 +- prog/weavedns/main.go | 2 +- prog/weaver/Dockerfile | 2 +- prog/weaver/main.go | 2 +- prog/weavewait/main.go | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/prog/weavedns/Dockerfile b/prog/weavedns/Dockerfile index d7af8837..883b34b8 100644 --- a/prog/weavedns/Dockerfile +++ b/prog/weavedns/Dockerfile @@ -3,4 +3,4 @@ MAINTAINER Weaveworks Inc WORKDIR /home/weave ADD ./weavedns /home/weave/ EXPOSE 53/udp -ENTRYPOINT ["/home/weave/weavedns", "-wait", "20"] +ENTRYPOINT ["/home/weave/weavedns"] diff --git a/prog/weavedns/main.go b/prog/weavedns/main.go index 21193da6..69832eaf 100644 --- a/prog/weavedns/main.go +++ b/prog/weavedns/main.go @@ -46,7 +46,7 @@ func main() { flag.StringVar(&ifaceName, "iface", "", "name of interface to use for multicast") flag.StringVar(&apiPath, "api", "unix:///var/run/docker.sock", "path to Docker API socket") flag.StringVar(&domain, "domain", weavedns.DefaultLocalDomain, "local domain (ie, 'weave.local.')") - flag.IntVar(&wait, "wait", 0, "number of seconds to wait for interface to be created and come up") + flag.IntVar(&wait, "wait", -1, "number of seconds to wait for interfaces to come up (0=don't wait, -1=wait forever)") flag.IntVar(&dnsPort, "dnsport", weavedns.DefaultServerPort, "port to listen to DNS requests") flag.StringVar(&httpIfaceName, "httpiface", "", "interface on which to listen for HTTP requests (defaults to empty string which listens on all interfaces)") flag.IntVar(&httpPort, "httpport", weavedns.DefaultHTTPPort, "port to listen to HTTP requests") diff --git a/prog/weaver/Dockerfile b/prog/weaver/Dockerfile index 437fc9e0..e91764e0 100644 --- a/prog/weaver/Dockerfile +++ b/prog/weaver/Dockerfile @@ -2,4 +2,4 @@ FROM scratch MAINTAINER Weaveworks Inc WORKDIR /home/weave ADD ./weaver /home/weave/ -ENTRYPOINT ["/home/weave/weaver", "-wait", "20"] +ENTRYPOINT ["/home/weave/weaver"] diff --git a/prog/weaver/main.go b/prog/weaver/main.go index d0ac8596..c17a7d46 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -64,7 +64,7 @@ func main() { flag.StringVar(&routerName, "name", "", "name of router (defaults to MAC of interface)") flag.StringVar(&nickName, "nickname", "", "nickname of peer (defaults to hostname)") flag.StringVar(&password, "password", "", "network password") - flag.IntVar(&wait, "wait", 0, "number of seconds to wait for interface to be created and come up (0 = don't wait)") + flag.IntVar(&wait, "wait", -1, "number of seconds to wait for interface to come up (0=don't wait, -1=wait forever)") flag.BoolVar(&debug, "debug", false, "enable debug logging") flag.BoolVar(&pktdebug, "pktdebug", false, "enable per-packet debug logging") flag.StringVar(&prof, "profile", "", "enable profiling and write profiles to given path") diff --git a/prog/weavewait/main.go b/prog/weavewait/main.go index 3cb78663..0acd5f74 100644 --- a/prog/weavewait/main.go +++ b/prog/weavewait/main.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "math" "os" "os/exec" "os/signal" @@ -17,20 +18,19 @@ func main() { } args := os.Args[1:] - signalWait := 20 + signalWait := time.Duration(math.MaxInt64) if args[0] == "-s" { signalWait = 0 args = args[1:] } - interfaceWait := 20 - signalWait usr2 := make(chan os.Signal) signal.Notify(usr2, syscall.SIGUSR2) select { case <-usr2: - case <-time.After(time.Duration(signalWait) * time.Second): + case <-time.After(signalWait): } - _, err := net.EnsureInterface("ethwe", interfaceWait) + _, err := net.EnsureInterface("ethwe", -1) checkErr(err) binary, err := exec.LookPath(args[0]) From a3a7a198702910afeac0bc2b8367bbd658167c4a Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 3 Jul 2015 14:28:18 +0100 Subject: [PATCH 24/69] simplifying refactor --- prog/weavewait/main.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/prog/weavewait/main.go b/prog/weavewait/main.go index 0acd5f74..9b1151bf 100644 --- a/prog/weavewait/main.go +++ b/prog/weavewait/main.go @@ -2,12 +2,10 @@ package main import ( "fmt" - "math" "os" "os/exec" "os/signal" "syscall" - "time" "github.com/weaveworks/weave/net" ) @@ -16,20 +14,16 @@ func main() { if len(os.Args) <= 1 { os.Exit(0) } - args := os.Args[1:] - signalWait := time.Duration(math.MaxInt64) + if args[0] == "-s" { - signalWait = 0 args = args[1:] + } else { + usr2 := make(chan os.Signal) + signal.Notify(usr2, syscall.SIGUSR2) + <-usr2 } - usr2 := make(chan os.Signal) - signal.Notify(usr2, syscall.SIGUSR2) - select { - case <-usr2: - case <-time.After(signalWait): - } _, err := net.EnsureInterface("ethwe", -1) checkErr(err) From 4dbd657a264657ec1a9fba8a26052af245662dce Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 3 Jul 2015 20:32:27 +0100 Subject: [PATCH 25/69] import gopacket from github instaead of google code The latter redirects to the former, which breaks `go get`. Closes #1071. --- prog/weaver/main.go | 2 +- router/ethernet_decoder.go | 4 ++-- router/forwarder.go | 4 ++-- router/pcap.go | 2 +- router/router.go | 2 +- router/udp_sender.go | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/prog/weaver/main.go b/prog/weaver/main.go index c17a7d46..03f2412c 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -11,8 +11,8 @@ import ( "runtime" "strings" - "code.google.com/p/gopacket/layers" "github.com/davecheney/profile" + "github.com/google/gopacket/layers" "github.com/gorilla/mux" . "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/common/docker" diff --git a/router/ethernet_decoder.go b/router/ethernet_decoder.go index 6588c0ad..4fe0481a 100644 --- a/router/ethernet_decoder.go +++ b/router/ethernet_decoder.go @@ -2,8 +2,8 @@ package router import ( "bytes" - "code.google.com/p/gopacket" - "code.google.com/p/gopacket/layers" + "github.com/google/gopacket" + "github.com/google/gopacket/layers" "log" "net" ) diff --git a/router/forwarder.go b/router/forwarder.go index a4cec472..e3386936 100644 --- a/router/forwarder.go +++ b/router/forwarder.go @@ -1,8 +1,8 @@ package router import ( - "code.google.com/p/gopacket" - "code.google.com/p/gopacket/layers" + "github.com/google/gopacket" + "github.com/google/gopacket/layers" "syscall" "time" ) diff --git a/router/pcap.go b/router/pcap.go index 761d4e1d..2b2c3b5d 100644 --- a/router/pcap.go +++ b/router/pcap.go @@ -1,8 +1,8 @@ package router import ( - "code.google.com/p/gopacket/pcap" "fmt" + "github.com/google/gopacket/pcap" ) type PcapIO struct { diff --git a/router/router.go b/router/router.go index ab671a0d..141dfc38 100644 --- a/router/router.go +++ b/router/router.go @@ -11,7 +11,7 @@ import ( "syscall" "time" - "code.google.com/p/gopacket/layers" + "github.com/google/gopacket/layers" . "github.com/weaveworks/weave/common" ) diff --git a/router/udp_sender.go b/router/udp_sender.go index 9e91ee0b..e591c9dc 100644 --- a/router/udp_sender.go +++ b/router/udp_sender.go @@ -1,8 +1,8 @@ package router import ( - "code.google.com/p/gopacket" - "code.google.com/p/gopacket/layers" + "github.com/google/gopacket" + "github.com/google/gopacket/layers" "log" "net" "syscall" From bf73029d152537ec5dd4a50d99c9f61e82f6007c Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 3 Jul 2015 20:41:35 +0100 Subject: [PATCH 26/69] cosmetic: goimports -w . --- ipam/allocate.go | 1 + ipam/paxos/paxos.go | 1 + ipam/paxos/paxos_test.go | 3 ++- router/crypto.go | 7 ++++--- router/ethernet_decoder.go | 5 +++-- router/forwarder.go | 5 +++-- router/handshake_test.go | 3 ++- router/mocks_test.go | 3 ++- router/pcap.go | 1 + router/peers_test.go | 3 ++- router/udp_sender.go | 5 +++-- 11 files changed, 24 insertions(+), 13 deletions(-) diff --git a/ipam/allocate.go b/ipam/allocate.go index f2da3c74..0e4a9e4b 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -2,6 +2,7 @@ package ipam import ( "fmt" + "github.com/weaveworks/weave/ipam/address" ) diff --git a/ipam/paxos/paxos.go b/ipam/paxos/paxos.go index a69b91da..61fda181 100644 --- a/ipam/paxos/paxos.go +++ b/ipam/paxos/paxos.go @@ -2,6 +2,7 @@ package paxos import ( "fmt" + "github.com/weaveworks/weave/router" ) diff --git a/ipam/paxos/paxos_test.go b/ipam/paxos/paxos_test.go index cd704c5c..c1d5bfe0 100644 --- a/ipam/paxos/paxos_test.go +++ b/ipam/paxos/paxos_test.go @@ -2,10 +2,11 @@ package paxos import ( "fmt" - "github.com/weaveworks/weave/router" "math/rand" "testing" "time" + + "github.com/weaveworks/weave/router" ) type TestNode struct { diff --git a/router/crypto.go b/router/crypto.go index 19b0bcfa..291603d1 100644 --- a/router/crypto.go +++ b/router/crypto.go @@ -1,15 +1,16 @@ package router import ( - "code.google.com/p/go-bit/bit" - "code.google.com/p/go.crypto/nacl/box" - "code.google.com/p/go.crypto/nacl/secretbox" "crypto/rand" "crypto/sha256" "encoding/binary" "encoding/gob" "fmt" "sync" + + "code.google.com/p/go-bit/bit" + "code.google.com/p/go.crypto/nacl/box" + "code.google.com/p/go.crypto/nacl/secretbox" ) func GenerateKeyPair() (publicKey, privateKey *[32]byte, err error) { diff --git a/router/ethernet_decoder.go b/router/ethernet_decoder.go index 4fe0481a..f04c53e4 100644 --- a/router/ethernet_decoder.go +++ b/router/ethernet_decoder.go @@ -2,10 +2,11 @@ package router import ( "bytes" - "github.com/google/gopacket" - "github.com/google/gopacket/layers" "log" "net" + + "github.com/google/gopacket" + "github.com/google/gopacket/layers" ) type EthernetDecoder struct { diff --git a/router/forwarder.go b/router/forwarder.go index e3386936..6c55c368 100644 --- a/router/forwarder.go +++ b/router/forwarder.go @@ -1,10 +1,11 @@ package router import ( - "github.com/google/gopacket" - "github.com/google/gopacket/layers" "syscall" "time" + + "github.com/google/gopacket" + "github.com/google/gopacket/layers" ) type ForwardedFrame struct { diff --git a/router/handshake_test.go b/router/handshake_test.go index a72bf17c..1630d343 100644 --- a/router/handshake_test.go +++ b/router/handshake_test.go @@ -1,8 +1,9 @@ package router import ( - wt "github.com/weaveworks/weave/testing" "testing" + + wt "github.com/weaveworks/weave/testing" ) func TestFieldValidator(t *testing.T) { diff --git a/router/mocks_test.go b/router/mocks_test.go index 674eaa3a..dc0ec6c3 100644 --- a/router/mocks_test.go +++ b/router/mocks_test.go @@ -6,8 +6,9 @@ package router import ( - wt "github.com/weaveworks/weave/testing" "testing" + + wt "github.com/weaveworks/weave/testing" ) // Add to peers a connection from peers.ourself to p diff --git a/router/pcap.go b/router/pcap.go index 2b2c3b5d..36962f20 100644 --- a/router/pcap.go +++ b/router/pcap.go @@ -2,6 +2,7 @@ package router import ( "fmt" + "github.com/google/gopacket/pcap" ) diff --git a/router/peers_test.go b/router/peers_test.go index daf1e944..bf221642 100644 --- a/router/peers_test.go +++ b/router/peers_test.go @@ -2,9 +2,10 @@ package router import ( "fmt" - wt "github.com/weaveworks/weave/testing" "math/rand" "testing" + + wt "github.com/weaveworks/weave/testing" ) // TODO we should also test: diff --git a/router/udp_sender.go b/router/udp_sender.go index e591c9dc..28ef917a 100644 --- a/router/udp_sender.go +++ b/router/udp_sender.go @@ -1,11 +1,12 @@ package router import ( - "github.com/google/gopacket" - "github.com/google/gopacket/layers" "log" "net" "syscall" + + "github.com/google/gopacket" + "github.com/google/gopacket/layers" ) type UDPSender interface { From 706f9ee35266c1188d1d5030f3fc7a39673c4830 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 6 Jul 2015 17:22:42 +0100 Subject: [PATCH 27/69] detect `--dns-search ` ...and not just `--dns-search=` Fixes #1086. --- weave | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weave b/weave index 9574975d..9b3efb21 100755 --- a/weave +++ b/weave @@ -664,7 +664,7 @@ dns_args() { WITHOUT_DNS= while [ $# -gt 0 ] ; do case "$1" in - --dns-search=*) + --dns-search|--dns-search=*) DNS_SEARCH_SPECIFIED=1 ;; --name) From 02715f88d5515fed5e017a926096af71ab3e55b8 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 6 Jul 2015 17:26:04 +0100 Subject: [PATCH 28/69] cosmetic: re-order option parsing to match the order in which we susequently process them. --- weave | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/weave b/weave index 9b3efb21..67d69d9f 100755 --- a/weave +++ b/weave @@ -664,8 +664,11 @@ dns_args() { WITHOUT_DNS= while [ $# -gt 0 ] ; do case "$1" in - --dns-search|--dns-search=*) - DNS_SEARCH_SPECIFIED=1 + --with-dns) + WITH_DNS=1 + ;; + --without-dns) + WITHOUT_DNS=1 ;; --name) NAME_ARG="$2" @@ -677,11 +680,8 @@ dns_args() { -h|--hostname|--hostname=*) HOSTNAME_SPECIFIED=1 ;; - --with-dns) - WITH_DNS=1 - ;; - --without-dns) - WITHOUT_DNS=1 + --dns-search|--dns-search=*) + DNS_SEARCH_SPECIFIED=1 ;; esac shift From 78c4b06492c0db83aad654aed84f8d81670d5647 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 8 Jul 2015 16:35:21 +0100 Subject: [PATCH 29/69] Don't wait for certainty before responding to Claim Conflicts: ipam/claim.go --- ipam/claim.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/ipam/claim.go b/ipam/claim.go index 8d96e6e2..750ad950 100644 --- a/ipam/claim.go +++ b/ipam/claim.go @@ -3,6 +3,7 @@ package ipam import ( "fmt" + "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/ipam/address" "github.com/weaveworks/weave/router" ) @@ -14,6 +15,18 @@ type claim struct { hasBeenCancelled func() bool } +func (c *claim) sendResult(result error) { + // Make sure we only send a result once, since listener stops listening after that + if c.resultChan != nil { + c.resultChan <- result + close(c.resultChan) + c.resultChan = nil + } + if result != nil { + common.Error.Println("[allocator] " + result.Error()) + } +} + // Try returns true for success (or failure), false if we need to try again later func (c *claim) Try(alloc *Allocator) bool { if (c.hasBeenCancelled)() { @@ -24,7 +37,7 @@ func (c *claim) Try(alloc *Allocator) bool { if !alloc.ring.Contains(c.addr) { // Address not within our universe; assume user knows what they are doing alloc.infof("Ignored address %s claimed by %s - not in our universe\n", c.addr, c.ident) - c.resultChan <- nil + c.sendResult(nil) return true } @@ -34,6 +47,7 @@ func (c *claim) Try(alloc *Allocator) bool { owner := alloc.ring.Owner(c.addr) if owner == router.UnknownPeerName { alloc.infof("Ring is empty; will try later.\n", c.addr, owner) + c.sendResult(nil) // don't make the caller wait return false } if owner != alloc.ourName { @@ -41,34 +55,34 @@ func (c *claim) Try(alloc *Allocator) bool { if found { name = " (" + name + ")" } - c.resultChan <- fmt.Errorf("address %s is owned by other peer %s%s", c.addr.String(), owner, name) + c.sendResult(fmt.Errorf("address %s is owned by other peer %s%s", c.addr.String(), owner, name)) return true } // We are the owner, check we haven't given it to another container existingIdent := alloc.findOwner(c.addr) if existingIdent == c.ident { // same identifier is claiming same address; that's OK - c.resultChan <- nil + c.sendResult(nil) return true } if existingIdent == "" { err := alloc.space.Claim(c.addr) if err != nil { - c.resultChan <- err + c.sendResult(err) return true } alloc.debugln("Claimed", c.addr, "for", c.ident) alloc.addOwned(c.ident, c.addr) - c.resultChan <- nil + c.sendResult(nil) return true } // Addr already owned by container on this machine - c.resultChan <- fmt.Errorf("address %s is already owned by %s", c.addr.String(), existingIdent) + c.sendResult(fmt.Errorf("address %s is already owned by %s", c.addr.String(), existingIdent)) return true } func (c *claim) Cancel() { - c.resultChan <- fmt.Errorf("Operation cancelled.") + c.sendResult(fmt.Errorf("Operation cancelled.")) } func (c *claim) String() string { From cf4ce0a07d311bed450c6a93e3d2fa1f425706b7 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 9 Jul 2015 10:42:30 +0100 Subject: [PATCH 30/69] Don't cancel address claim when caller closes socket --- ipam/allocator.go | 5 ++--- ipam/allocator_test.go | 2 +- ipam/claim.go | 12 +++--------- ipam/http.go | 3 +-- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 98f36f3d..9dee326a 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -202,10 +202,9 @@ func (alloc *Allocator) Lookup(ident string, r address.Range) (address.Address, } // Claim an address that we think we should own (Sync) -func (alloc *Allocator) Claim(ident string, addr address.Address, cancelChan <-chan bool) error { +func (alloc *Allocator) Claim(ident string, addr address.Address) error { resultChan := make(chan error) - op := &claim{resultChan: resultChan, ident: ident, addr: addr, - hasBeenCancelled: hasBeenCancelled(cancelChan)} + op := &claim{resultChan: resultChan, ident: ident, addr: addr} alloc.doOperation(op, &alloc.pendingClaims) return <-resultChan } diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 33bed602..8f6683ce 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -143,7 +143,7 @@ func TestAllocatorClaim(t *testing.T) { alloc.claimRingForTesting() addr1, _ := address.ParseIP(testAddr1) - err := alloc.Claim(container3, addr1, nil) + err := alloc.Claim(container3, addr1) wt.AssertNoErr(t, err) // Check we get this address back if we try an allocate addr3, _ := alloc.Allocate(container3, subnet, nil) diff --git a/ipam/claim.go b/ipam/claim.go index 750ad950..c1dbc529 100644 --- a/ipam/claim.go +++ b/ipam/claim.go @@ -9,10 +9,9 @@ import ( ) type claim struct { - resultChan chan<- error - ident string - addr address.Address - hasBeenCancelled func() bool + resultChan chan<- error + ident string + addr address.Address } func (c *claim) sendResult(result error) { @@ -29,11 +28,6 @@ func (c *claim) sendResult(result error) { // Try returns true for success (or failure), false if we need to try again later func (c *claim) Try(alloc *Allocator) bool { - if (c.hasBeenCancelled)() { - c.Cancel() - return true - } - if !alloc.ring.Contains(c.addr) { // Address not within our universe; assume user knows what they are doing alloc.infof("Ignored address %s claimed by %s - not in our universe\n", c.addr, c.ident) diff --git a/ipam/http.go b/ipam/http.go index 8164d8c8..c81ee2ba 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -19,14 +19,13 @@ func badRequest(w http.ResponseWriter, err error) { // HandleHTTP wires up ipams HTTP endpoints to the provided mux. func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CIDR, dockerCli *docker.Client) { router.Methods("PUT").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - closedChan := w.(http.CloseNotifier).CloseNotify() vars := mux.Vars(r) ident := vars["id"] ipStr := vars["ip"] if ip, err := address.ParseIP(ipStr); err != nil { badRequest(w, err) return - } else if err := alloc.Claim(ident, ip, closedChan); err != nil { + } else if err := alloc.Claim(ident, ip); err != nil { badRequest(w, fmt.Errorf("Unable to claim: %s", err)) return } From b93070f91ad1222404a330922fe0761ee05ae0b5 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 9 Jul 2015 12:51:52 +0100 Subject: [PATCH 31/69] Fix log message on peer move --- router/connection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/connection.go b/router/connection.go index adf9e04c..81c74d30 100644 --- a/router/connection.go +++ b/router/connection.go @@ -231,7 +231,7 @@ func (conn *LocalConnection) ReceivedHeartbeat(remoteUDPAddr *net.UDPAddr, connU if oldRemoteUDPAddr == nil { return conn.sendFastHeartbeats() } else if oldRemoteUDPAddr.String() != remoteUDPAddr.String() { - log.Println("Peer", conn.remote, "moved from", old, "to", remoteUDPAddr) + log.Println("Peer", conn.remote, "moved from", oldRemoteUDPAddr, "to", remoteUDPAddr) } return nil }) From b67c102fe5df93b502b788a0ad8c3013607ead4d Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Thu, 2 Jul 2015 11:58:26 +0100 Subject: [PATCH 32/69] Remove simple sleep from router accept --- router/router.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/router/router.go b/router/router.go index 141dfc38..1c7dcbb2 100644 --- a/router/router.go +++ b/router/router.go @@ -16,17 +16,12 @@ import ( ) const ( - macMaxAge = 10 * time.Minute // [1] - tcpAcceptDelay = 1 * time.Millisecond // [2] + macMaxAge = 10 * time.Minute // [1] ) // [1] should be greater than typical ARP cache expiries, i.e. > 3/2 * // /proc/sys/net/ipv4_neigh/*/base_reachable_time_ms on Linux -// [2] time to wait between accepting tcp connections. This guards -// against brute-force attacks on the password when encryption is in -// use. It is also a basic DoS defence. - type LogFrameFunc func(string, []byte, *layers.Ethernet) type Config struct { @@ -212,7 +207,6 @@ func (router *Router) listenTCP(localPort int) { continue } router.acceptTCP(tcpConn) - time.Sleep(tcpAcceptDelay) } }() } From 9634526a67b0aa553b5a1275987a8ae1ba8751b2 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Thu, 2 Jul 2015 11:58:40 +0100 Subject: [PATCH 33/69] Add token bucket implementation --- router/token_bucket.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 router/token_bucket.go diff --git a/router/token_bucket.go b/router/token_bucket.go new file mode 100644 index 00000000..254de6eb --- /dev/null +++ b/router/token_bucket.go @@ -0,0 +1,42 @@ +package router + +import ( + "time" +) + +type TokenBucket struct { + capacity int64 // Maximum capacity of bucket + tokenInterval time.Duration // Token replenishment rate + refillDuration time.Duration // Time to refill from empty + earliestUnspentToken time.Time +} + +func NewTokenBucket(capacity int64, tokenInterval time.Duration) *TokenBucket { + tb := TokenBucket{ + capacity: capacity, + tokenInterval: tokenInterval, + refillDuration: tokenInterval * time.Duration(capacity)} + + tb.earliestUnspentToken = tb.capacityToken() + + return &tb +} + +func (tb *TokenBucket) Wait() { + // If earliest unspent token is in the future, sleep until then + time.Sleep(tb.earliestUnspentToken.Sub(time.Now())) + + // Alternatively, enforce bucket capacity if necessary + capacityToken := tb.capacityToken() + if tb.earliestUnspentToken.Before(capacityToken) { + tb.earliestUnspentToken = capacityToken + } + + // 'Remove' a token from the bucket + tb.earliestUnspentToken = tb.earliestUnspentToken.Add(tb.tokenInterval) +} + +// Determine the historic token timestamp representing a full bucket +func (tb *TokenBucket) capacityToken() time.Time { + return time.Now().Add(-tb.refillDuration).Truncate(tb.tokenInterval) +} From f3c4a9fc6d723c57eb1273c194d11d319b8eb9d0 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Thu, 2 Jul 2015 12:03:08 +0100 Subject: [PATCH 34/69] Limit router accept rate with token bucket --- router/router.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/router/router.go b/router/router.go index 1c7dcbb2..b498690b 100644 --- a/router/router.go +++ b/router/router.go @@ -16,12 +16,18 @@ import ( ) const ( - macMaxAge = 10 * time.Minute // [1] + macMaxAge = 10 * time.Minute // [1] + acceptMaxTokens = 100 // [2] + acceptTokenDelay = 100 * time.Millisecond // [3] ) // [1] should be greater than typical ARP cache expiries, i.e. > 3/2 * // /proc/sys/net/ipv4_neigh/*/base_reachable_time_ms on Linux +// [2] capacity of token bucket for rate limiting accepts + +// [3] control rate at which new tokens are added to the bucket + type LogFrameFunc func(string, []byte, *layers.Ethernet) type Config struct { @@ -45,6 +51,7 @@ type Router struct { gossipChannels GossipChannels TopologyGossip Gossip UDPListener *net.UDPConn + acceptLimiter *TokenBucket } type PacketSource interface { @@ -76,6 +83,7 @@ func NewRouter(config Config, name PeerName, nickName string) *Router { router.Routes = NewRoutes(router.Ourself, router.Peers) router.ConnectionMaker = NewConnectionMaker(router.Ourself, router.Peers, router.Port, router.PeerDiscovery) router.TopologyGossip = router.NewGossip("topology", router) + router.acceptLimiter = NewTokenBucket(acceptMaxTokens, acceptTokenDelay) return router } @@ -207,6 +215,7 @@ func (router *Router) listenTCP(localPort int) { continue } router.acceptTCP(tcpConn) + router.acceptLimiter.Wait() } }() } From e2f8b6f9fdc8e770f68b8f46b43c92b1245b3246 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Thu, 9 Jul 2015 16:45:37 +0100 Subject: [PATCH 35/69] Document /dev/random blocking issue --- site/how-it-works.md | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/site/how-it-works.md b/site/how-it-works.md index 4e570c7c..70b97ff1 100644 --- a/site/how-it-works.md +++ b/site/how-it-works.md @@ -278,7 +278,7 @@ password, and the result is hashed through SHA256, to form the final ephemeral session key. Thus the supplied password is never exchanged directly, and is thoroughly mixed into the shared secret. Furthermore, the rate at which TCP connections are accepted is limited by weave to -1kHz, which twarts online dictionary attacks on reasonably strong +10Hz, which twarts online dictionary attacks on reasonably strong passwords. The shared key formed by Diffie-Hellman is 256 bits long, appending @@ -293,6 +293,39 @@ not be able to form valid ephemeral session keys. The same ephemeral session key is used for both TCP and UDP traffic between two peers. + Generating fresh keys for every connection +provides forward secrecy at the cost of placing a demand on the Linux +CSPRNG (accessed by `GenerateKey` via `/dev/urandom`) proportional to +the number of inbound connection attempts. Weave has accept throttling +to mitigate against denial of service attacks that seek to deplete the +CSPRNG entropy pool, however even at the lower bound of ten requests +per second there may not be enough entropy gathered on a headless +system to keep pace. + +Under such conditions, the consequences will be limited to slowing +down processes reading from the blocking `/dev/random` device as the +kernel waits for enough new entropy to be harvested. It is important +to note that contrary to intuition this low entropy state does not +compromise the ongoing use of `/dev/urandom` - [expert +opinion](http://blog.cr.yp.to/20140205-entropy.html) +asserts that as long as the CSPRNG is seeded with enough entropy (e.g. +256 bits) before random number generation commences then the output is +entirely safe for use as key material. + +By way of comparison, this is exactly how OpenSSL works - it reads 256 +bits of entropy at startup, and uses that to seed an internal CSPRNG +which is used thenceforth to generate keys. Whilst we could have taken +the same approach and built our own CSPRNG to work around the +potential `/dev/random` blocking issue, we thought it was much more +prudent to rely on the [heavily +scrutinised](http://eprint.iacr.org/2012/251.pdf) Linux random number +generator as [advised +here](http://cr.yp.to/highspeed/coolnacl-20120725.pdf) (page 10, +'Centralizing randomness'). The aforementioned notwithstanding, if +weave's demand on `/dev/urandom` is causing you problems with blocking +`/dev/random` reads, please get in touch with us - we'd love to hear +about your use case. + #### TCP TCP connection are only used to exchange topology information between From acd8bdec21391a4f48569849ebf600323fae3522 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Mon, 13 Jul 2015 15:01:21 +0100 Subject: [PATCH 36/69] [proxy] respect custom DOCKER_BRIDGE --- proxy/common.go | 9 ++++++++- weave | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/proxy/common.go b/proxy/common.go index 45ed79d8..7c7775c4 100644 --- a/proxy/common.go +++ b/proxy/common.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "net/http" + "os" "os/exec" "regexp" @@ -22,7 +23,13 @@ func callWeave(args ...string) ([]byte, error) { args = append([]string{"--local"}, args...) Debug.Print("Calling weave", args) cmd := exec.Command("./weave", args...) - cmd.Env = []string{"PROCFS=/hostproc", "PATH=/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"} + cmd.Env = []string{ + "PATH=/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "PROCFS=/hostproc", + } + if bridge := os.Getenv("DOCKER_BRIDGE"); bridge != "" { + cmd.Env = append(cmd.Env, fmt.Sprintf("DOCKER_BRIDGE=%s", bridge)) + } out, err := cmd.CombinedOutput() return out, err } diff --git a/weave b/weave index 67d69d9f..eee61860 100755 --- a/weave +++ b/weave @@ -1076,6 +1076,7 @@ case "$COMMAND" in -v /proc:/hostproc \ -e PROCFS=/hostproc \ -e WEAVE_CIDR=none \ + -e DOCKER_BRIDGE \ --entrypoint=/home/weave/weaveproxy \ $WEAVEPROXY_DOCKER_ARGS $EXEC_IMAGE $PROXY_ARGS) wait_for_log $PROXY_CONTAINER_NAME "proxy listening" From e4422205e830c09b6da89bef3fcecaa1c8e3b4ca Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 15 Jul 2015 14:35:26 +0100 Subject: [PATCH 37/69] [proxy] should not attempt weavewait in on weave container's execs --- proxy/create_exec_interceptor.go | 4 +++- test/610_proxy_wait_for_weave_test.sh | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/proxy/create_exec_interceptor.go b/proxy/create_exec_interceptor.go index 72e62730..33e655d9 100644 --- a/proxy/create_exec_interceptor.go +++ b/proxy/create_exec_interceptor.go @@ -29,7 +29,9 @@ func (i *createExecInterceptor) InterceptRequest(r *http.Request) error { return err } - if cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config); ok { + _, hasWeaveWait := container.Volumes["/w"] + cidrs, hasCIDRs := i.proxy.weaveCIDRsFromConfig(container.Config) + if hasCIDRs && hasWeaveWait { Info.Printf("Exec in container %s with WEAVE_CIDR \"%s\"", container.ID, strings.Join(cidrs, " ")) cmd := append(weaveWaitEntrypoint, "-s") options.Cmd = append(cmd, options.Cmd...) diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index 49d023e9..204f86cc 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -21,4 +21,8 @@ COMMITTED_IMAGE=$(proxy docker_on $HOST1 commit c1) assert_raises "proxy docker_on $HOST1 run --name c2 $COMMITTED_IMAGE" assert "entrypoint c2" "$(entrypoint $COMMITTED_IMAGE)" +# Check exec works on containers without weavewait +docker_on $HOST1 run -dit --name c3 $SMALL_IMAGE /bin/sh +assert_raises "proxy docker_on $HOST1 exec c3 true" + end_suite From b32c017f33cbece12fb99c67118428b32238303c Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 16 Jul 2015 13:19:56 +0100 Subject: [PATCH 38/69] fix detection of host netns when weave script runs inside container $$ refers to the pid in the container's process namespace, so using it as a reference into /hostproc is entirely wrong. Fixes #1162. --- weave | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weave b/weave index eee61860..37ee8210 100755 --- a/weave +++ b/weave @@ -359,7 +359,7 @@ netnsenter() { } connect_container_to_bridge() { - if [ -h "$PROCFS/$CONTAINER_PID/ns/net" -a -h "$PROCFS/$$/ns/net" -a "$(readlink $PROCFS/$CONTAINER_PID/ns/net)" = "$(readlink $PROCFS/$$/ns/net)" ] ; then + if [ -h "$PROCFS/$CONTAINER_PID/ns/net" -a -h "/proc/$$/ns/net" -a "$(readlink $PROCFS/$CONTAINER_PID/ns/net)" = "$(readlink /proc/$$/ns/net)" ] ; then echo "Container is running in the host network namespace, and therefore cannot be" >&2 echo "connected to weave. Perhaps the container was started with --net=host." >&2 return 1 From 1132d26d479cb1cbcf88c108ee03467bdbfa7e46 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 13:23:48 +0100 Subject: [PATCH 39/69] [proxy] do not weave attach containers in '--net=host|container:' --- proxy/create_container_interceptor.go | 2 +- proxy/create_exec_interceptor.go | 2 +- proxy/proxy.go | 7 ++++++- proxy/start_container_interceptor.go | 2 +- test/660_proxy_ipam_test.sh | 8 ++++++++ 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index b7d8d0c2..db365c61 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -46,7 +46,7 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { return err } - if cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config); ok { + if cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig); ok { Info.Printf("Creating container with WEAVE_CIDR \"%s\"", strings.Join(cidrs, " ")) if container.HostConfig == nil { container.HostConfig = &docker.HostConfig{} diff --git a/proxy/create_exec_interceptor.go b/proxy/create_exec_interceptor.go index 33e655d9..d8a15815 100644 --- a/proxy/create_exec_interceptor.go +++ b/proxy/create_exec_interceptor.go @@ -30,7 +30,7 @@ func (i *createExecInterceptor) InterceptRequest(r *http.Request) error { } _, hasWeaveWait := container.Volumes["/w"] - cidrs, hasCIDRs := i.proxy.weaveCIDRsFromConfig(container.Config) + cidrs, hasCIDRs := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) if hasCIDRs && hasWeaveWait { Info.Printf("Exec in container %s with WEAVE_CIDR \"%s\"", container.ID, strings.Join(cidrs, " ")) cmd := append(weaveWaitEntrypoint, "-s") diff --git a/proxy/proxy.go b/proxy/proxy.go index c391431f..8964b074 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -115,7 +115,12 @@ func (proxy *Proxy) ListenAndServe() error { return (&http.Server{Handler: proxy}).Serve(listener) } -func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config) ([]string, bool) { +func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, bool) { + if hostConfig != nil && + hostConfig.NetworkMode != "" && + hostConfig.NetworkMode != "bridge" { + return nil, false + } for _, e := range config.Env { if strings.HasPrefix(e, "WEAVE_CIDR=") { if e[11:] == "none" { diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index 4d54680d..ece4069f 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -21,7 +21,7 @@ func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { return err } - cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config) + cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) if !ok { Debug.Print("No Weave CIDR, ignoring") return nil diff --git a/test/660_proxy_ipam_test.sh b/test/660_proxy_ipam_test.sh index 11137995..ed1e589b 100755 --- a/test/660_proxy_ipam_test.sh +++ b/test/660_proxy_ipam_test.sh @@ -26,13 +26,21 @@ start $HOST1 --name=auto start $HOST1 --name=none -e WEAVE_CIDR=none start $HOST2 --name=zero -e WEAVE_CIDR= start $HOST2 --name=no-default +start $HOST1 --name=bridge --net=bridge +start $HOST1 --name=host --net=host +start $HOST1 --name=other --net=container:auto AUTO=$(container_ip $HOST1 auto) ZERO=$(container_ip $HOST2 zero) +BRIDGE=$(container_ip $HOST1 bridge) +OTHER=$(container_ip $HOST1 other) assert_raises "proxy exec_on $HOST1 auto $PING $ZERO" assert_raises "proxy exec_on $HOST2 zero $PING $AUTO" +assert_raises "proxy exec_on $HOST2 zero $PING $BRIDGE" +assert_raises "proxy exec_on $HOST2 zero $PING $OTHER" assert_no_ethwe $HOST1 none assert_no_ethwe $HOST2 no-default +assert_no_ethwe $HOST1 host end_suite From c13154eea9ce83256ee75c0842088706709c973a Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 13:26:47 +0100 Subject: [PATCH 40/69] updating comment as per golint --- proxy/create_container_interceptor.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index db365c61..da038b21 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -23,9 +23,9 @@ type createContainerRequestBody struct { MacAddress string `json:"MacAddress,omitempty" yaml:"MacAddress,omitempty"` } -// Replacement for docker.NoSuchImage, which does not contain the -// image name, which in turn breaks docker clients post 1.7.0 since -// they expect the image name to be present in errors. +// ErrNoSuchImage replaces docker.NoSuchImage, which does not contain the image +// name, which in turn breaks docker clients post 1.7.0 since they expect the +// image name to be present in errors. type ErrNoSuchImage struct { Name string } From 9619a27f8c042d2f0c8987c9a57fa9c8a08a9a9b Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 13:57:00 +0100 Subject: [PATCH 41/69] [proxy] adding log message for ignored containers --- proxy/proxy.go | 15 ++++++++++++--- proxy/start_container_interceptor.go | 4 ++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/proxy/proxy.go b/proxy/proxy.go index 8964b074..2e29b844 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -116,9 +116,7 @@ func (proxy *Proxy) ListenAndServe() error { } func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, bool) { - if hostConfig != nil && - hostConfig.NetworkMode != "" && - hostConfig.NetworkMode != "bridge" { + if !validNetworkMode(hostConfig) { return nil, false } for _, e := range config.Env { @@ -131,3 +129,14 @@ func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *dock } return nil, !proxy.NoDefaultIPAM } + +func validNetworkMode(hostConfig *docker.HostConfig) bool { + return networkMode(hostConfig) == "bridge" +} + +func networkMode(hostConfig *docker.HostConfig) string { + if hostConfig == nil || hostConfig.NetworkMode == "" { + return "bridge" + } + return hostConfig.NetworkMode +} diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index ece4069f..b020fd34 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -21,6 +21,10 @@ func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { return err } + if !validNetworkMode(container.HostConfig) { + Debug.Printf("Ignoring container %s with --net=%s", container.ID, networkMode(container.HostConfig)) + return nil + } cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) if !ok { Debug.Print("No Weave CIDR, ignoring") From c6fa9cb17edc6614d51ac55f8b0a2d1a047ae318 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 16 Jul 2015 14:42:20 +0100 Subject: [PATCH 42/69] refactor weaveCIDRsFromConfig to return an error message --- proxy/create_container_interceptor.go | 4 +++- proxy/create_exec_interceptor.go | 6 ++++-- proxy/proxy.go | 31 +++++++++++++-------------- proxy/start_container_interceptor.go | 10 +++------ 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index da038b21..574b5c01 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -46,7 +46,9 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { return err } - if cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig); ok { + if cidrs, err := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig); err != nil { + Info.Printf("Ignoring container due to %s", err) + } else { Info.Printf("Creating container with WEAVE_CIDR \"%s\"", strings.Join(cidrs, " ")) if container.HostConfig == nil { container.HostConfig = &docker.HostConfig{} diff --git a/proxy/create_exec_interceptor.go b/proxy/create_exec_interceptor.go index d8a15815..f15ed56f 100644 --- a/proxy/create_exec_interceptor.go +++ b/proxy/create_exec_interceptor.go @@ -30,8 +30,10 @@ func (i *createExecInterceptor) InterceptRequest(r *http.Request) error { } _, hasWeaveWait := container.Volumes["/w"] - cidrs, hasCIDRs := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) - if hasCIDRs && hasWeaveWait { + cidrs, err := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) + if err != nil { + Info.Printf("Ignoring container %s due to %s", container.ID, err) + } else if hasWeaveWait { Info.Printf("Exec in container %s with WEAVE_CIDR \"%s\"", container.ID, strings.Join(cidrs, " ")) cmd := append(weaveWaitEntrypoint, "-s") options.Cmd = append(cmd, options.Cmd...) diff --git a/proxy/proxy.go b/proxy/proxy.go index 2e29b844..d454a06c 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -2,6 +2,7 @@ package proxy import ( "crypto/tls" + "errors" "net" "net/http" "net/url" @@ -22,6 +23,10 @@ var ( containerCreateRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/create$") containerStartRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/[^/]*/(re)?start$") execCreateRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/[^/]*/exec$") + + ErrInvalidNetworkMode = errors.New("--net option") + ErrWeaveCIDRNone = errors.New("WEAVE_CIDR=none") + ErrNoDefaultIPAM = errors.New("--no-default-ipam option") ) type Proxy struct { @@ -115,28 +120,22 @@ func (proxy *Proxy) ListenAndServe() error { return (&http.Server{Handler: proxy}).Serve(listener) } -func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, bool) { - if !validNetworkMode(hostConfig) { - return nil, false +func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, error) { + if hostConfig != nil && + hostConfig.NetworkMode != "" && + hostConfig.NetworkMode != "bridge" { + return nil, ErrInvalidNetworkMode } for _, e := range config.Env { if strings.HasPrefix(e, "WEAVE_CIDR=") { if e[11:] == "none" { - return nil, false + return nil, ErrWeaveCIDRNone } - return strings.Fields(e[11:]), true + return strings.Fields(e[11:]), nil } } - return nil, !proxy.NoDefaultIPAM -} - -func validNetworkMode(hostConfig *docker.HostConfig) bool { - return networkMode(hostConfig) == "bridge" -} - -func networkMode(hostConfig *docker.HostConfig) string { - if hostConfig == nil || hostConfig.NetworkMode == "" { - return "bridge" + if proxy.NoDefaultIPAM { + return nil, ErrNoDefaultIPAM } - return hostConfig.NetworkMode + return nil, nil } diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index b020fd34..803ca869 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -21,13 +21,9 @@ func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { return err } - if !validNetworkMode(container.HostConfig) { - Debug.Printf("Ignoring container %s with --net=%s", container.ID, networkMode(container.HostConfig)) - return nil - } - cidrs, ok := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) - if !ok { - Debug.Print("No Weave CIDR, ignoring") + cidrs, err := i.proxy.weaveCIDRsFromConfig(container.Config, container.HostConfig) + if err != nil { + Info.Printf("Ignoring container %s due to %s", container.ID, err) return nil } Info.Printf("Attaching container %s with WEAVE_CIDR \"%s\" to weave network", container.ID, strings.Join(cidrs, " ")) From 5fc16a826732e92e976197eac40c4106c1768d98 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 17 Jul 2015 14:08:51 +0100 Subject: [PATCH 43/69] [proxy] use a waitgroup as it's more semantically accurate --- proxy/proxy_intercept.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/proxy/proxy_intercept.go b/proxy/proxy_intercept.go index 69f83c83..787ad6b0 100644 --- a/proxy/proxy_intercept.go +++ b/proxy/proxy_intercept.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "net/http/httputil" + "sync" "github.com/fsouza/go-dockerclient" . "github.com/weaveworks/weave/common" @@ -94,16 +95,15 @@ func doRawStream(w http.ResponseWriter, resp *http.Response, client *httputil.Cl return } - upDone := make(chan struct{}) - downDone := make(chan struct{}) - go copyStream(down, io.MultiReader(rem, up), upDone) - go copyStream(up, downBuf, downDone) - <-upDone - <-downDone + var wg sync.WaitGroup + wg.Add(2) + go copyStream(down, io.MultiReader(rem, up), &wg) + go copyStream(up, downBuf, &wg) + wg.Wait() } -func copyStream(dst io.Writer, src io.Reader, done chan struct{}) { - defer close(done) +func copyStream(dst io.Writer, src io.Reader, wg *sync.WaitGroup) { + defer wg.Done() if _, err := io.Copy(dst, src); err != nil { Warning.Print(err) } From 3f4b385b0f7f7ba2b0b3d42e314a91abd13d6d8a Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 17 Jul 2015 14:09:27 +0100 Subject: [PATCH 44/69] [proxy] If we can't CloseWrite a connection (tls), just Close it so clients don't hang --- proxy/proxy_intercept.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/proxy/proxy_intercept.go b/proxy/proxy_intercept.go index 787ad6b0..5dd109db 100644 --- a/proxy/proxy_intercept.go +++ b/proxy/proxy_intercept.go @@ -102,17 +102,23 @@ func doRawStream(w http.ResponseWriter, resp *http.Response, client *httputil.Cl wg.Wait() } -func copyStream(dst io.Writer, src io.Reader, wg *sync.WaitGroup) { +type closeWriter interface { + CloseWrite() error +} + +func copyStream(dst io.WriteCloser, src io.Reader, wg *sync.WaitGroup) { defer wg.Done() if _, err := io.Copy(dst, src); err != nil { Warning.Print(err) } - if c, ok := dst.(interface { - CloseWrite() error - }); ok { - if err := c.CloseWrite(); err != nil { - Warning.Printf("Error closing connection: %s", err) - } + var err error + if c, ok := dst.(closeWriter); ok { + err = c.CloseWrite() + } else { + err = dst.Close() + } + if err != nil { + Warning.Printf("Error closing connection: %s", err) } } From aa8466f688f18bf704eb68c5ccb1963c76afb247 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Tue, 21 Jul 2015 11:30:21 +0100 Subject: [PATCH 45/69] [proxy] Mount weavewait readonly so it can't be tampered with --- proxy/create_container_interceptor.go | 2 +- test/610_proxy_wait_for_weave_test.sh | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index 574b5c01..ee5c43ed 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -53,7 +53,7 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { if container.HostConfig == nil { container.HostConfig = &docker.HostConfig{} } - container.HostConfig.VolumesFrom = append(container.HostConfig.VolumesFrom, "weaveproxy") + container.HostConfig.VolumesFrom = append(container.HostConfig.VolumesFrom, "weaveproxy:ro") if container.Config == nil { container.Config = &docker.Config{} } diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index 204f86cc..ddc63b3d 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -25,4 +25,7 @@ assert "entrypoint c2" "$(entrypoint $COMMITTED_IMAGE)" docker_on $HOST1 run -dit --name c3 $SMALL_IMAGE /bin/sh assert_raises "proxy docker_on $HOST1 exec c3 true" +# Check we can't modify weavewait +assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.2/24' $BASE_IMAGE touch /w/w" 1 + end_suite From a14d4d9f8e8166b5fe4c7696bd1705cf555ccfd8 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Tue, 28 Jul 2015 17:18:58 +0100 Subject: [PATCH 46/69] Enforce docker bridge address assignment type on launch --- weave | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/weave b/weave index 37ee8210..80b49b70 100755 --- a/weave +++ b/weave @@ -93,6 +93,22 @@ check_docker_version() { fi } +enforce_docker_bridge_addr_assign_type() { + if ! ADDR_ASSIGN_TYPE=$(cat /sys/class/net/$DOCKER_BRIDGE/addr_assign_type 2>/dev/null) ; then + echo "Could not determine address assignment type of $DOCKER_BRIDGE" >&2 + return + fi + # From include/uapi/linux/netdevice.h + # #define NET_ADDR_PERM 0 /* address is permanent (default) */ + # #define NET_ADDR_RANDOM 1 /* address is generated randomly */ + # #define NET_ADDR_STOLEN 2 /* address is stolen from other device */ + # #define NET_ADDR_SET 3 /* address is set using dev_set_mac_address() */ + if [ $ADDR_ASSIGN_TYPE != 3 ] ; then + echo "Setting $DOCKER_BRIDGE MAC (mitigate https://github.com/docker/docker/issues/14908)" >&2 + ip link set dev $DOCKER_BRIDGE address $(random_mac) || true + fi +} + is_cidr() { echo "$1" | grep -E "^$CIDR_REGEXP$" >/dev/null } @@ -976,6 +992,7 @@ case "$COMMAND" in ;; launch) check_not_running $CONTAINER_NAME $BASE_IMAGE + enforce_docker_bridge_addr_assign_type create_bridge # We set the router name to the bridge mac since that is # stable across re-creations of the containers. From 5e9bd5424aa2cc32aa1755e3229989dfe63f5d64 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 29 Jul 2015 15:47:02 +0100 Subject: [PATCH 47/69] Removed all the imports from "code.google.com" --- prog/weaveproxy/main.go | 2 +- router/crypto.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/prog/weaveproxy/main.go b/prog/weaveproxy/main.go index c896d872..d110088b 100644 --- a/prog/weaveproxy/main.go +++ b/prog/weaveproxy/main.go @@ -5,7 +5,7 @@ import ( "os" "strings" - "code.google.com/p/getopt" + "github.com/pborman/getopt" . "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/proxy" ) diff --git a/router/crypto.go b/router/crypto.go index 291603d1..b66d2b73 100644 --- a/router/crypto.go +++ b/router/crypto.go @@ -8,9 +8,9 @@ import ( "fmt" "sync" - "code.google.com/p/go-bit/bit" - "code.google.com/p/go.crypto/nacl/box" - "code.google.com/p/go.crypto/nacl/secretbox" + "github.com/andybalholm/go-bit" + "golang.org/x/crypto/nacl/box" + "golang.org/x/crypto/nacl/secretbox" ) func GenerateKeyPair() (publicKey, privateKey *[32]byte, err error) { From e2389a4061d04f1baacd88f1438fb21102232d66 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 30 Jul 2015 11:52:23 +0100 Subject: [PATCH 48/69] [proxy] Don't try to attach if container failed to start --- proxy/start_container_interceptor.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proxy/start_container_interceptor.go b/proxy/start_container_interceptor.go index 803ca869..9dc1411d 100644 --- a/proxy/start_container_interceptor.go +++ b/proxy/start_container_interceptor.go @@ -16,6 +16,10 @@ func (i *startContainerInterceptor) InterceptRequest(r *http.Request) error { } func (i *startContainerInterceptor) InterceptResponse(r *http.Response) error { + if r.StatusCode < 200 || r.StatusCode >= 300 { // Docker didn't do the start + return nil + } + container, err := inspectContainerInPath(i.proxy.client, r.Request.URL.Path) if err != nil { return err From 4958b8d37a6bf631537c06046188dca9bc87c5ab Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 29 Jul 2015 15:51:51 +0100 Subject: [PATCH 49/69] [proxy] Mount /w from the host for weavewaited containers We set it at container create. Had tried also setting it at start, but it seems to cause weird behaviour, and there is no way to do it from the CLI. --- proxy/create_container_interceptor.go | 14 +++++++++++- proxy/proxy.go | 31 ++++++++++++++++++++++++--- test/610_proxy_wait_for_weave_test.sh | 5 +++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index ee5c43ed..5b6bfd7b 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -53,10 +53,10 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { if container.HostConfig == nil { container.HostConfig = &docker.HostConfig{} } - container.HostConfig.VolumesFrom = append(container.HostConfig.VolumesFrom, "weaveproxy:ro") if container.Config == nil { container.Config = &docker.Config{} } + i.addWeaveWaitVolume(container.HostConfig) if err := i.setWeaveWaitEntrypoint(container.Config); err != nil { return err } @@ -75,6 +75,18 @@ func (i *createContainerInterceptor) InterceptRequest(r *http.Request) error { return nil } +func (i *createContainerInterceptor) addWeaveWaitVolume(hostConfig *docker.HostConfig) { + var binds []string + for _, bind := range hostConfig.Binds { + s := strings.Split(bind, ":") + if len(s) >= 2 && s[1] == "/w" { + continue + } + binds = append(binds, bind) + } + hostConfig.Binds = append(binds, fmt.Sprintf("%s:/w:ro", i.proxy.weaveWaitVolume)) +} + func (i *createContainerInterceptor) setWeaveWaitEntrypoint(container *docker.Config) error { if len(container.Entrypoint) == 0 { image, err := i.proxy.client.InspectImage(container.Image) diff --git a/proxy/proxy.go b/proxy/proxy.go index d454a06c..b42a582d 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -3,6 +3,7 @@ package proxy import ( "crypto/tls" "errors" + "fmt" "net" "net/http" "net/url" @@ -32,9 +33,10 @@ var ( type Proxy struct { Config - dial func() (net.Conn, error) - client *docker.Client - dockerBridgeIP string + dial func() (net.Conn, error) + client *docker.Client + dockerBridgeIP string + weaveWaitVolume string } type Config struct { @@ -85,9 +87,32 @@ func NewProxy(c Config) (*Proxy, error) { return nil, err } + if err = p.findWeaveWaitVolume(); err != nil { + return nil, err + } + return p, nil } +func (proxy *Proxy) findWeaveWaitVolume() error { + container, err := proxy.client.InspectContainer("weaveproxy") + if err != nil { + return fmt.Errorf("Could not find the weavewait volume: %s", err) + } + + if container.Volumes == nil { + return fmt.Errorf("Could not find the weavewait volume") + } + + volume, ok := container.Volumes["/w"] + if !ok { + return fmt.Errorf("Could not find the weavewait volume") + } + + proxy.weaveWaitVolume = volume + return nil +} + func (proxy *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { Info.Printf("%s %s", r.Method, r.URL) path := r.URL.Path diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index ddc63b3d..1d5e1276 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -28,4 +28,9 @@ assert_raises "proxy docker_on $HOST1 exec c3 true" # Check we can't modify weavewait assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.2/24' $BASE_IMAGE touch /w/w" 1 +# Check only user-specified volumes and /w are mounted +proxy docker_on $HOST1 create --name c4 -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $BASE_IMAGE $CHECK_ETHWE_UP +proxy docker_on $HOST1 start -ai c4 +assert "docker_on $HOST1 inspect --format='{{range \$k, \$v := .Volumes}}{{\$k}} {{end}}' c4" "/srv1 /srv2 /w " + end_suite From 7cc53c4c6854c13a9e08fc3901753ba396161351 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Mon, 3 Aug 2015 12:28:17 +0100 Subject: [PATCH 50/69] [proxy] Handle case where no command is specified. The docker daemon can't do this because we set /w/w. --- prog/weavewait/main.go | 14 ++++++++------ proxy/create_container_interceptor.go | 9 +++++++++ test/610_proxy_wait_for_weave_test.sh | 5 +++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/prog/weavewait/main.go b/prog/weavewait/main.go index 9b1151bf..d88e89f3 100644 --- a/prog/weavewait/main.go +++ b/prog/weavewait/main.go @@ -7,16 +7,14 @@ import ( "os/signal" "syscall" - "github.com/weaveworks/weave/net" + weavenet "github.com/weaveworks/weave/net" + "github.com/weaveworks/weave/proxy" ) func main() { - if len(os.Args) <= 1 { - os.Exit(0) - } args := os.Args[1:] - if args[0] == "-s" { + if len(args) > 0 && args[0] == "-s" { args = args[1:] } else { usr2 := make(chan os.Signal) @@ -24,9 +22,13 @@ func main() { <-usr2 } - _, err := net.EnsureInterface("ethwe", -1) + _, err := weavenet.EnsureInterface("ethwe", -1) checkErr(err) + if len(args) == 0 { + checkErr(proxy.ErrNoCommandSpecified) + } + binary, err := exec.LookPath(args[0]) checkErr(err) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index 5b6bfd7b..42b60ed1 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -3,6 +3,7 @@ package proxy import ( "bytes" "encoding/json" + "errors" "fmt" "io/ioutil" "net/http" @@ -15,6 +16,10 @@ import ( const MaxDockerHostname = 64 +var ( + ErrNoCommandSpecified = errors.New("No command specified") +) + type createContainerInterceptor struct{ proxy *Proxy } type createContainerRequestBody struct { @@ -105,6 +110,10 @@ func (i *createContainerInterceptor) setWeaveWaitEntrypoint(container *docker.Co } } + if len(container.Entrypoint) == 0 && len(container.Cmd) == 0 { + return ErrNoCommandSpecified + } + if len(container.Entrypoint) == 0 || container.Entrypoint[0] != weaveWaitEntrypoint[0] { container.Entrypoint = append(weaveWaitEntrypoint, container.Entrypoint...) } diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index 1d5e1276..8abd4440 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -33,4 +33,9 @@ proxy docker_on $HOST1 create --name c4 -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAV proxy docker_on $HOST1 start -ai c4 assert "docker_on $HOST1 inspect --format='{{range \$k, \$v := .Volumes}}{{\$k}} {{end}}' c4" "/srv1 /srv2 /w " +# Check errors are returned (when docker returns an error code) +assert_raises "docker -H tcp://$HOST1:12375 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE foo 2>&1 | grep 'exec: \"foo\": executable file not found in \$PATH'" +# Check errors still happen when no command is specified +assert_raises "docker -H tcp://$HOST1:12375 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE 2>&1 | grep 'Error response from daemon: No command specified'" + end_suite From 67aae051d67b6137c81532d4ed4cb6bcbc50f8e0 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Wed, 5 Aug 2015 13:35:35 +0100 Subject: [PATCH 51/69] send ARP update for *all* container IPs; not just the first one This was missed in #498. --- weave | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/weave b/weave index 80b49b70..2ea86d3b 100755 --- a/weave +++ b/weave @@ -449,7 +449,9 @@ attach() { netnsenter ip route add 224.0.0.0/4 dev $CONTAINER_IFNAME fi - arp_update $CONTAINER_IFNAME $1 "netnsenter" + for ADDR in "$@" ; do + arp_update $CONTAINER_IFNAME $ADDR "netnsenter" + done } detach() { From c3b653b1b912c8eb11c5f0ed7d3d9da3715856dc Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Sat, 8 Aug 2015 22:01:45 +0100 Subject: [PATCH 52/69] ignore DuplicateError when inserting mDNS answer in zone When we issue two concurrent mDNS queries for the same name, the answer to the second one encounters a DuplicateError when inserting into the zone. We would return that error, which causes the query would fail. And we'd cache that fact. All bad. So ignore DuplicateError. Fixes #1270. --- nameserver/zone_lookup.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nameserver/zone_lookup.go b/nameserver/zone_lookup.go index 5c5aacd6..00d5aee0 100644 --- a/nameserver/zone_lookup.go +++ b/nameserver/zone_lookup.go @@ -103,9 +103,11 @@ func (zone *ZoneDb) domainLookup(target string, lfun ZoneLookupFunc) (res []Zone for _, answer := range lanswers { r, err := remoteIdent.addIPToName(answer, now) if err != nil { - zone.mx.Unlock() - Warning.Printf("[zonedb] '%s' insertion for %s failed: %s", answer, target, err) - return nil, err + if _, ok := err.(DuplicateError); !ok { + zone.mx.Unlock() + Warning.Printf("[zonedb] '%s' insertion for %s failed: %s", answer, target, err) + return nil, err + } } uniq.add(r) } From f4e7c8521972b5f40f37b76c62975922e43c08f4 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 10 Aug 2015 13:27:06 +0100 Subject: [PATCH 53/69] warn on -iprange overlap Closes #1276. --- weave | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/weave b/weave index 2ea86d3b..2f4d1bdd 100755 --- a/weave +++ b/weave @@ -1041,10 +1041,15 @@ case "$COMMAND" in if [ -z "$IPRANGE_SPECIFIED" ] ; then IPRANGE="10.128.0.0/10" if command_exists netcheck && ! netcheck $IPRANGE ; then - echo "Default -iprange $IPRANGE overlaps with existing route." >&2 - echo "Please pick another range and set it on all hosts." >&2 + echo "ERROR: Default -iprange $IPRANGE overlaps with existing route." >&2 + echo "You must pick another range and set it on all hosts." >&2 exit 1 fi + else + if command_exists netcheck && ! netcheck $IPRANGE ; then + echo "WARNING: Specified -iprange $IPRANGE overlaps with existing route." >&2 + echo "Unless this is deliberate, you must pick another range and set it on all hosts." >&2 + fi fi # Set WEAVE_DOCKER_ARGS in the environment in order to supply # additional parameters, such as resource limits, to docker From 9e65cc20cbf3eea47e3274975db303ee3309e2d9 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 10 Aug 2015 17:33:45 +0100 Subject: [PATCH 54/69] cosmetic --- weave | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/weave b/weave index 2f4d1bdd..e7573cb1 100755 --- a/weave +++ b/weave @@ -1041,13 +1041,13 @@ case "$COMMAND" in if [ -z "$IPRANGE_SPECIFIED" ] ; then IPRANGE="10.128.0.0/10" if command_exists netcheck && ! netcheck $IPRANGE ; then - echo "ERROR: Default -iprange $IPRANGE overlaps with existing route." >&2 + echo "ERROR: Default -iprange $IPRANGE overlaps with existing route on host." >&2 echo "You must pick another range and set it on all hosts." >&2 exit 1 fi else if command_exists netcheck && ! netcheck $IPRANGE ; then - echo "WARNING: Specified -iprange $IPRANGE overlaps with existing route." >&2 + echo "WARNING: Specified -iprange $IPRANGE overlaps with existing route on host." >&2 echo "Unless this is deliberate, you must pick another range and set it on all hosts." >&2 fi fi From f8639a57e7beae807130b62db1acaaa59afaf420 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 11 Aug 2015 14:02:23 +0100 Subject: [PATCH 55/69] prevent proxy from interferring with `weave run` ...which could result in containers unexpectedly getting multiple IP addresses. Fixes #1279. --- test/680_proxy_weave_run_test.sh | 16 ++++++++++++++++ weave | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100755 test/680_proxy_weave_run_test.sh diff --git a/test/680_proxy_weave_run_test.sh b/test/680_proxy_weave_run_test.sh new file mode 100755 index 00000000..1ed61da7 --- /dev/null +++ b/test/680_proxy_weave_run_test.sh @@ -0,0 +1,16 @@ +#! /bin/bash + +. ./config.sh + +C1=10.2.1.13 + +start_suite "Start containers with 'weave run' via proxy" + +weave_on $HOST1 launch +weave_on $HOST1 launch-proxy + +proxy start_container $HOST1 $C1/24 --name=c1 +# check we get exactly one IP back, the one specified +assert "weave_on $HOST1 ps c1 | cut -d ' ' -f 3-" $C1/24 + +end_suite diff --git a/weave b/weave index e7573cb1..7a1ab8d5 100755 --- a/weave +++ b/weave @@ -158,7 +158,7 @@ if [ "$1" = "run" ] ; then shift $(dns_arg_count "$@") collect_cidr_args "$@" shift $CIDR_ARG_COUNT - CONTAINER=$(docker $DOCKER_CLIENT_ARGS run $DNS_ARGS -d "$@") + CONTAINER=$(docker $DOCKER_CLIENT_ARGS run -e WEAVE_CIDR=none $DNS_ARGS -d "$@") exec_remote attach $CIDR_ARGS --or-die $CONTAINER echo $CONTAINER exit 0 @@ -1160,7 +1160,7 @@ case "$COMMAND" in shift $(dns_arg_count "$@") collect_cidr_args "$@" shift $CIDR_ARG_COUNT - CONTAINER=$(docker run $DNS_ARGS -d "$@") + CONTAINER=$(docker run -e WEAVE_CIDR=none $DNS_ARGS -d "$@") create_bridge ipam_cidrs_or_die $CONTAINER $CIDR_ARGS with_container_netns_or_die $CONTAINER attach $ALL_CIDRS From e2fec3d007c432a24e278616cc33a81b9931d265 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 12 Aug 2015 11:14:51 +0100 Subject: [PATCH 56/69] fix intermittent test failure Tests were running in the same shell, so function definitions were leaking between tests. --- test/run_all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_all.sh b/test/run_all.sh index b0dddbd4..b0d71e18 100755 --- a/test/run_all.sh +++ b/test/run_all.sh @@ -36,7 +36,7 @@ echo Running $TESTS for t in $TESTS; do echo greyly echo "---= Running $t =---" - . $t + (. $t) # Report test runtime when running on circle, to help scheduler if [ -n "$CIRCLECI" ]; then From 2576e819a7916806a82d42533876436ceab418ec Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 12 Aug 2015 16:05:31 +0100 Subject: [PATCH 57/69] Revert "fix intermittent test failure" This reverts commit e2fec3d007c432a24e278616cc33a81b9931d265. Was causing a failure on circleci --- test/run_all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_all.sh b/test/run_all.sh index b0d71e18..b0dddbd4 100755 --- a/test/run_all.sh +++ b/test/run_all.sh @@ -36,7 +36,7 @@ echo Running $TESTS for t in $TESTS; do echo greyly echo "---= Running $t =---" - (. $t) + . $t # Report test runtime when running on circle, to help scheduler if [ -n "$CIRCLECI" ]; then From 74eef4dedae617c2aab34f91963a43cd056a84b3 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 12 Aug 2015 11:18:19 +0100 Subject: [PATCH 58/69] [proxy] mount all tls volumes as readonly --- weave | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weave b/weave index 7a1ab8d5..f6e0f667 100755 --- a/weave +++ b/weave @@ -882,7 +882,7 @@ ipam_cidrs_or_die() { # TODO: Handle args with spaces tls_arg() { filename=${3:-$2} - PROXY_VOLUMES="$PROXY_VOLUMES -v $1:/home/weave/tls/$filename.pem" + PROXY_VOLUMES="$PROXY_VOLUMES -v $1:/home/weave/tls/$filename.pem:ro" PROXY_ARGS="$PROXY_ARGS --tls$2 /home/weave/tls/$filename.pem" } From f70dc6660a6079a24ce40c9432423379bca7b0eb Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 12 Aug 2015 13:36:50 +0100 Subject: [PATCH 59/69] [proxy] lock docker api usage to 1.15 --- proxy/proxy.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/proxy/proxy.go b/proxy/proxy.go index b42a582d..5a421538 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -82,7 +82,11 @@ func NewProxy(c Config) (*Proxy, error) { Error.Fatalf("Could not configure tls for proxy: %s", err) } - p.client, err = docker.NewClient(p.DockerAddr) + // We pin the protocol version to 1.15 (which corresponds to Docker 1.3.x; + // the earliest version supported by weave) in order to insulate ourselves + // from breaking changes to the API, as happened in 1.20 (Docker 1.8.0) when + // the presentation of volumes changed in inspect. + p.client, err = docker.NewVersionedClient(p.DockerAddr, "1.15") if err != nil { return nil, err } From 4961e8368642c6ea6ac78f5cc9b7ab77ecae6da7 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 12 Aug 2015 14:02:36 +0100 Subject: [PATCH 60/69] [proxy] support new docker 1.8 --net="default" --- proxy/proxy.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxy/proxy.go b/proxy/proxy.go index 5a421538..83664800 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -25,9 +25,8 @@ var ( containerStartRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/[^/]*/(re)?start$") execCreateRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/[^/]*/exec$") - ErrInvalidNetworkMode = errors.New("--net option") - ErrWeaveCIDRNone = errors.New("WEAVE_CIDR=none") - ErrNoDefaultIPAM = errors.New("--no-default-ipam option") + ErrWeaveCIDRNone = errors.New("WEAVE_CIDR=none") + ErrNoDefaultIPAM = errors.New("--no-default-ipam option") ) type Proxy struct { @@ -152,8 +151,9 @@ func (proxy *Proxy) ListenAndServe() error { func (proxy *Proxy) weaveCIDRsFromConfig(config *docker.Config, hostConfig *docker.HostConfig) ([]string, error) { if hostConfig != nil && hostConfig.NetworkMode != "" && + hostConfig.NetworkMode != "default" && hostConfig.NetworkMode != "bridge" { - return nil, ErrInvalidNetworkMode + return nil, fmt.Errorf("--net option: %q", hostConfig.NetworkMode) } for _, e := range config.Env { if strings.HasPrefix(e, "WEAVE_CIDR=") { From bf2b456af179282b8a7cb81daf14c8b3443ec006 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 13 Aug 2015 13:36:22 +0100 Subject: [PATCH 61/69] Don't use .Volumes in tests, as it's gone in docker 1.8 --- test/610_proxy_wait_for_weave_test.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index 8abd4440..bebce97e 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -29,9 +29,9 @@ assert_raises "proxy docker_on $HOST1 exec c3 true" assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.2/24' $BASE_IMAGE touch /w/w" 1 # Check only user-specified volumes and /w are mounted -proxy docker_on $HOST1 create --name c4 -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $BASE_IMAGE $CHECK_ETHWE_UP -proxy docker_on $HOST1 start -ai c4 -assert "docker_on $HOST1 inspect --format='{{range \$k, \$v := .Volumes}}{{\$k}} {{end}}' c4" "/srv1 /srv2 /w " +dirs_sans_proxy=$(docker_on $HOST1 run --rm -it $BASE_IMAGE mount | wc -l) +dirs_with_proxy=$(proxy docker_on $HOST1 run --rm -it -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $BASE_IMAGE mount | wc -l) +assert "$((dirs_with_proxy-3))" $dirs_sans_proxy # Check errors are returned (when docker returns an error code) assert_raises "docker -H tcp://$HOST1:12375 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE foo 2>&1 | grep 'exec: \"foo\": executable file not found in \$PATH'" From 94d2f84364046a3743a06e5e557ec5c07205ae0a Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 13 Aug 2015 13:50:29 +0100 Subject: [PATCH 62/69] refactor: use `proxy` abstraction --- test/610_proxy_wait_for_weave_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index 8abd4440..7569c4f4 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -34,8 +34,8 @@ proxy docker_on $HOST1 start -ai c4 assert "docker_on $HOST1 inspect --format='{{range \$k, \$v := .Volumes}}{{\$k}} {{end}}' c4" "/srv1 /srv2 /w " # Check errors are returned (when docker returns an error code) -assert_raises "docker -H tcp://$HOST1:12375 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE foo 2>&1 | grep 'exec: \"foo\": executable file not found in \$PATH'" +assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE foo 2>&1 | grep 'exec: \"foo\": executable file not found in \$PATH'" # Check errors still happen when no command is specified -assert_raises "docker -H tcp://$HOST1:12375 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE 2>&1 | grep 'Error response from daemon: No command specified'" +assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE 2>&1 | grep 'Error response from daemon: No command specified'" end_suite From 84c5c52090244d205710671fb49bcf8534607c17 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 13 Aug 2015 14:05:17 +0100 Subject: [PATCH 63/69] fix broken test silliness --- test/610_proxy_wait_for_weave_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index 106843d8..b218816e 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -31,7 +31,7 @@ assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.2/24' $BASE_IMAG # Check only user-specified volumes and /w are mounted dirs_sans_proxy=$(docker_on $HOST1 run --rm -it $BASE_IMAGE mount | wc -l) dirs_with_proxy=$(proxy docker_on $HOST1 run --rm -it -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $BASE_IMAGE mount | wc -l) -assert "$((dirs_with_proxy-3))" $dirs_sans_proxy +assert "echo $((dirs_with_proxy-3))" $dirs_sans_proxy # Check errors are returned (when docker returns an error code) assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE foo 2>&1 | grep 'exec: \"foo\": executable file not found in \$PATH'" From a3b4d6cdbfcc68bec36b6973cff54ef21c1af50b Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 13 Aug 2015 14:06:43 +0100 Subject: [PATCH 64/69] consistency: use $SMALL_IMAGE instead of $BASE_IMAGE whenever possible --- test/610_proxy_wait_for_weave_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index b218816e..20b2c62a 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -29,8 +29,8 @@ assert_raises "proxy docker_on $HOST1 exec c3 true" assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.2/24' $BASE_IMAGE touch /w/w" 1 # Check only user-specified volumes and /w are mounted -dirs_sans_proxy=$(docker_on $HOST1 run --rm -it $BASE_IMAGE mount | wc -l) -dirs_with_proxy=$(proxy docker_on $HOST1 run --rm -it -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $BASE_IMAGE mount | wc -l) +dirs_sans_proxy=$(docker_on $HOST1 run --rm -it $SMALL_IMAGE mount | wc -l) +dirs_with_proxy=$(proxy docker_on $HOST1 run --rm -it -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE mount | wc -l) assert "echo $((dirs_with_proxy-3))" $dirs_sans_proxy # Check errors are returned (when docker returns an error code) From 90ed76853c7e8c4d84cc9bcc8f3dd1b0c006bf1c Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 13 Aug 2015 14:36:30 +0100 Subject: [PATCH 65/69] fix test breakage on master, and only when running via `run_all.sh`. Weird. --- test/610_proxy_wait_for_weave_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/610_proxy_wait_for_weave_test.sh b/test/610_proxy_wait_for_weave_test.sh index 20b2c62a..de3af8b4 100755 --- a/test/610_proxy_wait_for_weave_test.sh +++ b/test/610_proxy_wait_for_weave_test.sh @@ -29,8 +29,8 @@ assert_raises "proxy docker_on $HOST1 exec c3 true" assert_raises "proxy docker_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.2/24' $BASE_IMAGE touch /w/w" 1 # Check only user-specified volumes and /w are mounted -dirs_sans_proxy=$(docker_on $HOST1 run --rm -it $SMALL_IMAGE mount | wc -l) -dirs_with_proxy=$(proxy docker_on $HOST1 run --rm -it -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE mount | wc -l) +dirs_sans_proxy=$(docker_on $HOST1 run --rm $SMALL_IMAGE mount | wc -l) +dirs_with_proxy=$(proxy docker_on $HOST1 run --rm -v /tmp/1:/srv1 -v /tmp/2:/srv2 -e 'WEAVE_CIDR=10.2.1.3/24' $SMALL_IMAGE mount | wc -l) assert "echo $((dirs_with_proxy-3))" $dirs_sans_proxy # Check errors are returned (when docker returns an error code) From 65578211ef459a9eee8d1defa1b974ce9ecead83 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 13 Aug 2015 15:01:15 +0100 Subject: [PATCH 66/69] extract test functions for "start container via proxy" This also eliminates the redefinition of 'start_container' by some of the tests, which causes failures when multiple tests are run with `run_all.sh`. Fixes #1294. --- test/630_proxy_dns_test.sh | 4 ++-- test/635_proxy_dns_unqualified_test.sh | 12 ++++-------- test/640_proxy_restart_reattaches_test.sh | 4 ++-- test/660_proxy_ipam_test.sh | 20 +++++++------------- test/config.sh | 12 ++++++++++++ 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/test/630_proxy_dns_test.sh b/test/630_proxy_dns_test.sh index d3b9ce04..67a844b7 100755 --- a/test/630_proxy_dns_test.sh +++ b/test/630_proxy_dns_test.sh @@ -8,8 +8,8 @@ C1_NAME=c1.weave.local C2_NAME=seetwo.weave.local boot_containers() { - proxy docker_on $HOST1 run -e WEAVE_CIDR=$C2/24 -dt --name=c2 -h $C2_NAME $DNS_IMAGE /bin/sh - proxy docker_on $HOST1 run -e WEAVE_CIDR=$C1/24 -dt --name=c1 $DNS_IMAGE /bin/sh + proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C2/24 -dt --name=c2 -h $C2_NAME + proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C1/24 -dt --name=c1 } kill_containers() { diff --git a/test/635_proxy_dns_unqualified_test.sh b/test/635_proxy_dns_unqualified_test.sh index b4ffbd18..b3ba410c 100755 --- a/test/635_proxy_dns_unqualified_test.sh +++ b/test/635_proxy_dns_unqualified_test.sh @@ -9,19 +9,15 @@ C4=10.2.0.99 DOMAIN=weave.local NAME=seeone.$DOMAIN -start_container() { - proxy docker_on $HOST1 run "$@" -dt $DNS_IMAGE /bin/sh -} - start_suite "Resolve unqualified names" weave_on $HOST1 launch-dns 10.2.254.1/24 weave_on $HOST1 launch-proxy -start_container -e WEAVE_CIDR=$C1/24 --name=c1 -h $NAME -start_container -e WEAVE_CIDR=$C2/24 --name=c2 -h seetwo.$DOMAIN -start_container -e WEAVE_CIDR=$C3/24 --name=c3 --dns-search=$DOMAIN -container=$(start_container -e WEAVE_CIDR=$C4/24) +proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C1/24 --name=c1 -h $NAME +proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C2/24 --name=c2 -h seetwo.$DOMAIN +proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C3/24 --name=c3 --dns-search=$DOMAIN +container=$(proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C4/24) check() { assert "proxy exec_on $HOST1 $1 getent hosts seeone | tr -s ' '" "$C1 $NAME" diff --git a/test/640_proxy_restart_reattaches_test.sh b/test/640_proxy_restart_reattaches_test.sh index b85e87c4..7f0166ed 100755 --- a/test/640_proxy_restart_reattaches_test.sh +++ b/test/640_proxy_restart_reattaches_test.sh @@ -10,8 +10,8 @@ start_suite "Proxy restart reattaches networking to containers" launch_dns_on $HOST1 10.2.254.1/24 weave_on $HOST1 launch-proxy --with-dns -proxy docker_on $HOST1 run -e WEAVE_CIDR=$C2/24 -dt --name=c2 -h $NAME $SMALL_IMAGE /bin/sh -proxy docker_on $HOST1 run -e WEAVE_CIDR=$C1/24 -dt --name=c1 $DNS_IMAGE /bin/sh +proxy_start_container $HOST1 -e WEAVE_CIDR=$C2/24 -dt --name=c2 -h $NAME +proxy_start_container_with_dns $HOST1 -e WEAVE_CIDR=$C1/24 -dt --name=c1 proxy docker_on $HOST1 restart c2 assert_raises "proxy exec_on $HOST1 c2 $CHECK_ETHWE_UP" diff --git a/test/660_proxy_ipam_test.sh b/test/660_proxy_ipam_test.sh index ed1e589b..16ea7490 100755 --- a/test/660_proxy_ipam_test.sh +++ b/test/660_proxy_ipam_test.sh @@ -4,12 +4,6 @@ UNIVERSE=10.2.2.0/24 -start() { - host=$1 - shift - proxy docker_on "$host" run "$@" -dt $SMALL_IMAGE /bin/sh -} - assert_no_ethwe() { assert_raises "container_ip $1 $2" 1 assert_raises "proxy exec_on $1 $2 ip link show | grep -v ethwe" @@ -22,13 +16,13 @@ weave_on $HOST2 launch -iprange $UNIVERSE $HOST1 weave_on $HOST1 launch-proxy weave_on $HOST2 launch-proxy --no-default-ipam -start $HOST1 --name=auto -start $HOST1 --name=none -e WEAVE_CIDR=none -start $HOST2 --name=zero -e WEAVE_CIDR= -start $HOST2 --name=no-default -start $HOST1 --name=bridge --net=bridge -start $HOST1 --name=host --net=host -start $HOST1 --name=other --net=container:auto +proxy_start_container $HOST1 --name=auto +proxy_start_container $HOST1 --name=none -e WEAVE_CIDR=none +proxy_start_container $HOST2 --name=zero -e WEAVE_CIDR= +proxy_start_container $HOST2 --name=no-default +proxy_start_container $HOST1 --name=bridge --net=bridge +proxy_start_container $HOST1 --name=host --net=host +proxy_start_container $HOST1 --name=other --net=container:auto AUTO=$(container_ip $HOST1 auto) ZERO=$(container_ip $HOST2 zero) diff --git a/test/config.sh b/test/config.sh index 9ca96777..053b3c49 100644 --- a/test/config.sh +++ b/test/config.sh @@ -120,6 +120,18 @@ start_container_with_dns() { weave_on $host run --with-dns "$@" -t $DNS_IMAGE /bin/sh } +proxy_start_container() { + host=$1 + shift 1 + proxy docker_on $host run "$@" -dt $SMALL_IMAGE /bin/sh +} + +proxy_start_container_with_dns() { + host=$1 + shift 1 + proxy docker_on $host run "$@" -dt $DNS_IMAGE /bin/sh +} + rm_containers() { host=$1 shift From d2205424915a9cbd998d52236eea4577c2c297d1 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Mon, 17 Aug 2015 08:49:40 +0000 Subject: [PATCH 67/69] Add sudo apt-get update to circle.yml --- circle.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/circle.yml b/circle.yml index 2a01b19f..b4d1b47e 100644 --- a/circle.yml +++ b/circle.yml @@ -26,6 +26,7 @@ dependencies: docker save weaveworks/weave-build >$WEAVE_BUILD; fi post: + - sudo apt-get update - sudo apt-get install bc - pip install requests - curl https://sdk.cloud.google.com | bash From c50c90ea8d378d084affbfea53c2ff602d4777f2 Mon Sep 17 00:00:00 2001 From: Alvaro Saurin Date: Mon, 17 Aug 2015 13:45:17 +0200 Subject: [PATCH 68/69] Try to compress too long replies (when forwarding). --- nameserver/server.go | 3 +++ nameserver/server_test.go | 25 ++++++++++++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/nameserver/server.go b/nameserver/server.go index a7b84bc7..26a0a607 100644 --- a/nameserver/server.go +++ b/nameserver/server.go @@ -395,6 +395,9 @@ func (s *DNSServer) notUsHandler(proto dnsProtocol) dns.HandlerFunc { } Debug.Printf("[dns msgid %d] Given answer by %s for query %s", r.MsgHdr.Id, server, q.Name) + if reply.Len() > maxLen { + reply.Compress = true + } w.WriteMsg(reply) return } diff --git a/nameserver/server_test.go b/nameserver/server_test.go index feba9997..08a3cfdb 100644 --- a/nameserver/server_test.go +++ b/nameserver/server_test.go @@ -3,12 +3,14 @@ package nameserver import ( "encoding/binary" "fmt" + "math/rand" "net" "testing" "time" "github.com/miekg/dns" . "github.com/weaveworks/weave/common" + "github.com/weaveworks/weave/ipam/address" wt "github.com/weaveworks/weave/testing" ) @@ -32,11 +34,12 @@ func TestUDPDNSServer(t *testing.T) { setupForTest(t) const ( - successTestName = "test1.weave.local." - failTestName = "fail.weave.local." - nonLocalName = "weave.works." - testAddr1 = "10.2.2.1" - containerID = "somecontainer" + successTestName = "test1.weave.local." + failTestName = "fail.weave.local." + compressedTestName = "compressed.weave.works." + nonLocalName = "weave.works." + testAddr1 = "10.2.2.1" + containerID = "somecontainer" ) testCIDR1 := testAddr1 + "/24" @@ -72,6 +75,15 @@ func TestUDPDNSServer(t *testing.T) { m.Answer[0] = &dns.PTR{Hdr: dns.RR_Header{Name: m.Question[0].Name, Rrtype: dns.TypePTR, Class: dns.ClassINET, Ttl: 0}, Ptr: "ns1.google.com."} } else if q.Name == testRDNSfail && q.Qtype == dns.TypePTR { m.Rcode = dns.RcodeNameError + } else if q.Name == compressedTestName { + // Construct a response that is >512 when uncompressed, <512 when compressed + header := makeHeader(m, &q, 300) + for m.Len() <= minUDPSize { + ip := address.Address(rand.Uint32()).IP4() + m.Answer = append(m.Answer, &dns.A{Hdr: *header, A: ip}) + } + m.Compress = true + wt.AssertTrue(t, m.Len() <= minUDPSize, fmt.Sprintf("length: %d", m.Len())) } w.WriteMsg(m) } @@ -125,6 +137,9 @@ func TestUDPDNSServer(t *testing.T) { assertExchange(t, testRDNSnonlocal, dns.TypePTR, testPort, 1, -1, 0) + _, r = assertExchange(t, compressedTestName, dns.TypeA, testPort, -1, -1, 0) + wt.AssertTrue(t, r.Len() > minUDPSize, "unexpected message length") + // Not testing MDNS functionality of server here (yet), since it // needs two servers, each listening on its own address } From 8c5248da43acb2fea20a9626ef36f65b45f43760 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 18 Aug 2015 08:27:26 +0100 Subject: [PATCH 69/69] Add release 1.0.2 --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59351392..9b9cbc06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## Release 1.0.2 + +This release fixes a number of bugs, including some security +vulnerabilities in the Weave Docker API proxy, hangs and failures in +address allocation, and sporadic failures in name resolution. + +More details in the +[change log](https://github.com/weaveworks/weave/issues?q=milestone%3A1.0.2). + +The release is fully compatible with other 1.0.x versions, so existing +clusters can be upgraded incrementally. + ## Release 1.0.1 This is a bug fix release, addressing the following issue: