diff --git a/ipam/allocator.go b/ipam/allocator.go index 6ad7a540..8f1a699f 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -210,9 +210,9 @@ func (alloc *Allocator) doOperation(op operation, ops *[]operation) { // already succeeded. If it is on the queue, we call // cancel on it, allowing callers waiting for the resultChans // to unblock. -func (alloc *Allocator) cancelOp(op operation, ops *[]operation) { +func (alloc *Allocator) cancelOp(opToCancel operation, ops *[]operation) { for i, op := range *ops { - if op == op { + if op == opToCancel { *ops = append((*ops)[:i], (*ops)[i+1:]...) op.Cancel() break diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 273d7cb1..e31ba8eb 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -186,6 +186,7 @@ func TestAllocatorClaim(t *testing.T) { alloc.Prime() // Do an allocate on the other peer, which we will try to claim later addrx, err := allocs[0].Allocate(container1, subnet, true, returnFalse) + require.NoError(t, err) // Should not get the address we pre-claimed require.NotEqual(t, addrx, preAddr) router.Flush() diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 29797047..1c00060e 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -52,12 +52,12 @@ func TestLowlevel(t *testing.T) { s := New() require.Equal(t, address.Count(0), s.NumFreeAddresses()) - ok, got := s.Allocate(address.NewRange(0, 1000)) + ok, _ := s.Allocate(address.NewRange(0, 1000)) require.False(t, ok, "allocate in empty space should fail") s.Add(100, 100) require.Equal(t, address.Count(100), s.NumFreeAddresses()) - ok, got = s.Allocate(address.NewRange(0, 1000)) + ok, got := s.Allocate(address.NewRange(0, 1000)) require.True(t, ok && got == 100, "allocate") require.Equal(t, address.Count(99), s.NumFreeAddresses()) require.NoError(t, s.Claim(150)) @@ -68,6 +68,7 @@ func TestLowlevel(t *testing.T) { wt.AssertErrorInterface(t, (*error)(nil), s.Free(100), "double free") r, ok := s.Donate(address.NewRange(0, 1000)) + require.True(t, ok, "donate") require.Equal(t, address.NewRange(0xa0, 0x20), r, "donate") // test Donate when addresses are scarce @@ -78,6 +79,7 @@ func TestLowlevel(t *testing.T) { require.NoError(t, s.Claim(0)) require.NoError(t, s.Claim(2)) r, ok = s.Donate(address.NewRange(0, 1000)) + require.True(t, ok, "donate") require.Equal(t, address.NewRange(1, 1), r, "donate") r, ok = s.Donate(address.NewRange(0, 1000)) require.True(t, !ok, "donate should fail") diff --git a/plugin/net/cni.go b/plugin/net/cni.go index fcf208fc..0af6e108 100644 --- a/plugin/net/cni.go +++ b/plugin/net/cni.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net" + "os" "strings" "syscall" @@ -53,6 +54,9 @@ func (c *CNIPlugin) getIP(ipamType string, args *skel.CmdArgs) (newResult *curre if err != nil { return nil, err } + if result == nil { + return nil, fmt.Errorf("Received no usable result from IPAM plugin") + } newResult, err = current.NewResultFromResult(result) // Check if ipam returned no results without error if err == nil && len(newResult.IPs) == 0 { @@ -87,6 +91,10 @@ func (c *CNIPlugin) CmdAdd(args *skel.CmdArgs) error { if err == weavenet.ErrBridgeNoIP { bridgeArgs := *args bridgeArgs.ContainerID = "weave:expose" + // It would be better if libcni let us send just the desired parameters, + // but there is a bug: https://github.com/containernetworking/cni/issues/410 + // so just blank out the one we want to change + os.Setenv("CNI_CONTAINERID", bridgeArgs.ContainerID) bridgeIPResult, err := c.getIP(conf.IPAM.Type, &bridgeArgs) if err != nil { return fmt.Errorf("unable to allocate IP address for bridge: %s", err) diff --git a/prog/plugin/main.go b/prog/plugin/main.go index 5c3c368c..6e36d8ec 100644 --- a/prog/plugin/main.go +++ b/prog/plugin/main.go @@ -94,7 +94,7 @@ func run(dockerClient *docker.Client, weave *weaveapi.Client, address, meshAddre } go serveStatus(statusListener) sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, os.Interrupt, os.Kill, syscall.SIGTERM) + signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) select { case sig := <-sigChan: diff --git a/prog/weaveutil/docker_tls_args.go b/prog/weaveutil/docker_tls_args.go index 300d2428..9278b722 100644 --- a/prog/weaveutil/docker_tls_args.go +++ b/prog/weaveutil/docker_tls_args.go @@ -45,7 +45,6 @@ func dockerTLSArgs(args []string) error { switch { case arg == "-d" || arg == "daemon": isDaemon = true - break case arg == "--tls", arg == "--tlsverify": tlsArgs = append(tlsArgs, arg) case strings.HasPrefix(arg, "--tls"): diff --git a/prog/weaveutil/unique_id.go b/prog/weaveutil/unique_id.go index 2f12d71e..c475cd20 100644 --- a/prog/weaveutil/unique_id.go +++ b/prog/weaveutil/unique_id.go @@ -16,6 +16,6 @@ func uniqueID(args []string) error { if err != nil { return err } - fmt.Printf(uid) + fmt.Print(uid) return nil } diff --git a/router/sleeve.go b/router/sleeve.go index f4b01b27..44e36a11 100644 --- a/router/sleeve.go +++ b/router/sleeve.go @@ -6,7 +6,6 @@ package router import ( - "bytes" "encoding/binary" "fmt" "io" @@ -1039,6 +1038,9 @@ func (sender *udpSenderDF) dial() error { laddr := &net.IPAddr{IP: sender.localIP} raddr := &net.IPAddr{IP: sender.remoteIP} s, err := net.DialIP("ip4:UDP", laddr, raddr) + if err != nil { + return err + } f, err := s.File() if err != nil { @@ -1059,7 +1061,7 @@ func (sender *udpSenderDF) dial() error { func (sender *udpSenderDF) send(msg []byte, raddr *net.UDPAddr) error { // Ensure we have a socket sending to the right IP address - if sender.socket == nil || !bytes.Equal(sender.remoteIP, raddr.IP) { + if sender.socket == nil || !sender.remoteIP.Equal(raddr.IP) { sender.remoteIP = raddr.IP if err := sender.dial(); err != nil { return err @@ -1111,7 +1113,7 @@ func (sender *udpSenderDF) close() error { } func udpAddrsEqual(a *net.UDPAddr, b *net.UDPAddr) bool { - return bytes.Equal(a.IP, b.IP) && a.Port == b.Port && a.Zone == b.Zone + return a.IP.Equal(b.IP) && a.Port == b.Port && a.Zone == b.Zone } func allZeros(s []byte) bool { diff --git a/test/830_cni_plugin_test.sh b/test/830_cni_plugin_test.sh index f33485fe..dd6dc7fd 100755 --- a/test/830_cni_plugin_test.sh +++ b/test/830_cni_plugin_test.sh @@ -16,11 +16,23 @@ run_on $HOST1 sudo mkdir -p /opt/cni/bin weave_on $HOST1 setup-cni weave_on $HOST1 launch +C0=$(docker_on $HOST1 run --net=none --name=c0 -dt $SMALL_IMAGE /bin/sh) C1=$(docker_on $HOST1 run --net=none --privileged --name=c1 -dt $SMALL_IMAGE /bin/sh) C2=$(docker_on $HOST1 run --net=none --name=c2 -dt $SMALL_IMAGE /bin/sh) # Enable unsolicited ARPs so that ping after the address reuse does not time out exec_on $HOST1 c1 sysctl -w net.ipv4.conf.all.arp_accept=1 +# Contrived example to trigger the bug in #2839 +cni_connect $HOST1 c0 </dev/null) || PLUGIN_FILTER=cat rm_containers $host $(docker_on $host ps -aq 2>/dev/null | $PLUGIN_FILTER) weave_on $host reset 2>/dev/null + run_on $host sudo rm -f /opt/cni/bin/weave-plugin-latest /opt/cni/bin/weave-net /opt/cni/bin/weave-ipam /etc/cni/net.d/10-weave.conf done whitely echo "$@" }