From df52117bfa24cfb8508cb1f451f7e8470fcd7122 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 11 Oct 2019 11:20:35 +0000 Subject: [PATCH 1/5] Remove unused code --- ipam/allocator.go | 15 --------------- ipam/allocator_test.go | 2 -- ipam/paxos/paxos_test.go | 10 ---------- ipam/testutils_test.go | 25 ------------------------- nameserver/dns_test.go | 2 -- net/ipsec/ipsec.go | 8 -------- npc/namespace.go | 7 ------- plugin/skel/listener.go | 18 ------------------ prog/kube-utils/peerlist_test.go | 2 -- proxy/common.go | 2 -- test/tls/generate_certs.go | 2 -- testing/util.go | 7 ------- 12 files changed, 100 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 6dbd7f37..c4048de7 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -207,21 +207,6 @@ func (alloc *Allocator) doOperation(op operation, ops *[]operation) { } } -// Given an operation, remove it from the pending queue -// Note the op may not be on the queue; it may have -// 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(opToCancel operation, ops *[]operation) { - for i, op := range *ops { - if op == opToCancel { - *ops = append((*ops)[:i], (*ops)[i+1:]...) - op.Cancel() - break - } - } -} - // Cancel all operations in a queue func (alloc *Allocator) cancelOps(ops *[]operation) { for _, op := range *ops { diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index e73c61d2..a60c72d4 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -17,8 +17,6 @@ import ( const ( testStart1 = "10.0.1.0" - testStart2 = "10.0.2.0" - testStart3 = "10.0.3.0" ) func returnFalse() bool { return false } diff --git a/ipam/paxos/paxos_test.go b/ipam/paxos/paxos_test.go index a78e4203..014e7e7f 100644 --- a/ipam/paxos/paxos_test.go +++ b/ipam/paxos/paxos_test.go @@ -56,16 +56,6 @@ func (m *Model) addLink(a, b *TestNode) { m.readyLinks = append(m.readyLinks, &ab, &ba) } -func (m *Model) linkExists(a, b *TestNode) bool { - for _, l := range a.links { - if l.to == b { - return true - } - } - - return false -} - type TestParams struct { // Number of nodes nodeCount uint diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 8761d793..3039dd50 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -24,14 +24,6 @@ func (m *mockMessage) String() string { return fmt.Sprintf("-> %s [%x]", m.dst, m.buf) } -func toStringArray(messages []mockMessage) []string { - out := make([]string, len(messages)) - for i := range out { - out[i] = messages[i].String() - } - return out -} - type mockGossipComms struct { sync.RWMutex *testing.T @@ -100,14 +92,6 @@ func (m *mockGossipComms) GossipUnicast(dstPeerName mesh.PeerName, buf []byte) e return nil } -func ExpectMessage(alloc *Allocator, dst string, msgType byte, buf []byte) { - m := alloc.gossip.(*mockGossipComms) - dstPeerName, _ := mesh.PeerNameFromString(dst) - m.Lock() - m.messages = append(m.messages, mockMessage{dstPeerName, msgType, buf}) - m.Unlock() -} - func ExpectBroadcastMessage(alloc *Allocator, buf []byte) { m := alloc.gossip.(*mockGossipComms) m.Lock() @@ -208,15 +192,6 @@ func AssertNothingSent(t *testing.T, ch <-chan bool) { } } -func AssertNothingSentErr(t *testing.T, ch <-chan error) { - select { - case val := <-ch: - require.FailNow(t, fmt.Sprintf("Unexpected value on channel: %v", val)) - default: - // no message received - } -} - func makeNetworkOfAllocators(size int, cidr string, preClaims ...[]PreClaim) ([]*Allocator, *gossip.TestRouter, address.CIDR) { gossipRouter := gossip.NewTestRouter(0.0) allocs := make([]*Allocator, size) diff --git a/nameserver/dns_test.go b/nameserver/dns_test.go index 43cb117d..aa9a2804 100644 --- a/nameserver/dns_test.go +++ b/nameserver/dns_test.go @@ -41,9 +41,7 @@ func TestTruncation(t *testing.T) { defer dnsserver.Stop() // Add 100 mappings to nameserver - addrs := []address.Address{} for i := address.Address(0); i < 100; i++ { - addrs = append(addrs, i) nameserver.AddEntry("foo.weave.local.", "", mesh.UnknownPeerName, i) } diff --git a/net/ipsec/ipsec.go b/net/ipsec/ipsec.go index 5e429251..9aacbac2 100644 --- a/net/ipsec/ipsec.go +++ b/net/ipsec/ipsec.go @@ -430,14 +430,6 @@ func (ipsec *IPSec) removeDropNonEncrypted(srcIP, dstIP net.IP, udpPort int, inS return ipsec.resetRules(rules, true) } -func (ipsec *IPSec) removeDropNonEncryptedInbound(srcIP, dstIP net.IP, inSPI SPI) error { - r := ruleMarkInboundESP(srcIP, dstIP, inSPI) - if err := ipsec.ipt.Delete(r.table, r.chain, r.rulespec...); err != nil { - return errors.Wrap(err, fmt.Sprintf("iptables delete unique (%s, %s, %s)", r.table, r.chain, r.rulespec)) - } - return nil -} - // xfrm func xfrmAllocSpiState(srcIP, dstIP net.IP) *netlink.XfrmState { diff --git a/npc/namespace.go b/npc/namespace.go index c9184cff..13e59651 100644 --- a/npc/namespace.go +++ b/npc/namespace.go @@ -158,13 +158,6 @@ func (ns *ns) addToDefaultAllowIfNoMatching(pod *coreapi.Pod, policyType policyT return nil } -func (ns *ns) checkLocalPod(obj *coreapi.Pod) bool { - if obj.Spec.NodeName != ns.nodeName { - return false - } - return true -} - func (ns *ns) addPod(obj *coreapi.Pod) error { ns.pods[uid(obj)] = obj diff --git a/plugin/skel/listener.go b/plugin/skel/listener.go index 24bd4705..929c1303 100644 --- a/plugin/skel/listener.go +++ b/plugin/skel/listener.go @@ -201,24 +201,6 @@ func (listener *listener) leaveEndpoint(w http.ResponseWriter, r *http.Request) emptyOrErrorResponse(w, listener.d.LeaveEndpoint(&l)) } -func (listener *listener) discoverNew(w http.ResponseWriter, r *http.Request) { - var disco api.DiscoveryNotification - if err := json.NewDecoder(r.Body).Decode(&disco); err != nil { - sendError(w, "Could not decode JSON encoded payload", http.StatusBadRequest) - return - } - emptyOrErrorResponse(w, listener.d.DiscoverNew(&disco)) -} - -func (listener *listener) discoverDelete(w http.ResponseWriter, r *http.Request) { - var disco api.DiscoveryNotification - if err := json.NewDecoder(r.Body).Decode(&disco); err != nil { - sendError(w, "Could not decode JSON encoded payload", http.StatusBadRequest) - return - } - emptyOrErrorResponse(w, listener.d.DiscoverDelete(&disco)) -} - // === func (listener *listener) getIpamCapabilities(w http.ResponseWriter, r *http.Request) { diff --git a/prog/kube-utils/peerlist_test.go b/prog/kube-utils/peerlist_test.go index c5e47fee..2228282f 100644 --- a/prog/kube-utils/peerlist_test.go +++ b/prog/kube-utils/peerlist_test.go @@ -16,8 +16,6 @@ const ( peerName3 = "03:00:00:00:00:00" nodeName1 = "fake-node-1" nodeName2 = "fake-node-2" - - testIters = 1000 ) func TestPeerListBasic(t *testing.T) { diff --git a/proxy/common.go b/proxy/common.go index b54816dc..88dd8354 100644 --- a/proxy/common.go +++ b/proxy/common.go @@ -17,8 +17,6 @@ import ( var ( containerIDRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/([^/]*)/.*") weaveWaitEntrypoint = []string{"/w/w"} - weaveEntrypoint = "/home/weave/weaver" - weaveContainerName = "/weave" Log = common.Log ) diff --git a/test/tls/generate_certs.go b/test/tls/generate_certs.go index 319384ad..a6e3b274 100644 --- a/test/tls/generate_certs.go +++ b/test/tls/generate_certs.go @@ -15,8 +15,6 @@ const ( caCertPath = "ca.pem" clientKeyPath = "key.pem" clientCertPath = "cert.pem" - serverKeyPath = "server-key.pem" - serverCertPath = "server.pem" ) func main() { diff --git a/testing/util.go b/testing/util.go index f87dba65..dd18c3f1 100644 --- a/testing/util.go +++ b/testing/util.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "reflect" - "runtime" "testing" "github.com/stretchr/testify/require" @@ -29,12 +28,6 @@ func AssertErrorType(t *testing.T, expected interface{}, actual error, desc stri } } -func stackTrace(all bool) string { - buf := make([]byte, 1<<20) - stacklen := runtime.Stack(buf, all) - return string(buf[:stacklen]) -} - // TrimTestArgs finds the first -- in os.Args and trim all args before // that, returning true when a -- was in fact found. func TrimTestArgs() bool { From 00bc020498ffd7814f1bb0337cd46424d02b92d9 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 11 Oct 2019 11:22:24 +0000 Subject: [PATCH 2/5] Use Context instead of deprecated http.CloseNotifier interface --- ipam/http.go | 17 +++++++++-------- ipam/http_test.go | 5 ++++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/ipam/http.go b/ipam/http.go index 87cd8be7..ec5d2b45 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -1,6 +1,7 @@ package ipam import ( + "context" "encoding/json" "fmt" "net/http" @@ -41,7 +42,7 @@ func writeAddresses(w http.ResponseWriter, cidrs []address.CIDR) { } } -func hasBeenCancelled(dockerCli *docker.Client, closedChan <-chan bool, ident string, checkAlive bool) func() bool { +func hasBeenCancelled(dockerCli *docker.Client, closedChan <-chan struct{}, ident string, checkAlive bool) func() bool { return func() bool { select { case <-closedChan: @@ -63,9 +64,9 @@ func cancellationErr(w http.ResponseWriter, err error) bool { return false } -func (alloc *Allocator) handleHTTPAllocate(dockerCli *docker.Client, w http.ResponseWriter, ident string, checkAlive bool, subnet address.CIDR) { +func (alloc *Allocator) handleHTTPAllocate(ctx context.Context, dockerCli *docker.Client, w http.ResponseWriter, ident string, checkAlive bool, subnet address.CIDR) { addr, err := alloc.Allocate(ident, subnet, checkAlive, - hasBeenCancelled(dockerCli, w.(http.CloseNotifier).CloseNotify(), ident, checkAlive)) + hasBeenCancelled(dockerCli, ctx.Done(), ident, checkAlive)) if err != nil { if !cancellationErr(w, err) { badRequest(w, err) @@ -75,9 +76,9 @@ func (alloc *Allocator) handleHTTPAllocate(dockerCli *docker.Client, w http.Resp fmt.Fprintf(w, "%s/%d", addr, subnet.PrefixLen) } -func (alloc *Allocator) handleHTTPClaim(dockerCli *docker.Client, w http.ResponseWriter, ident string, cidr address.CIDR, checkAlive, noErrorOnUnknown bool) { +func (alloc *Allocator) handleHTTPClaim(ctx context.Context, dockerCli *docker.Client, w http.ResponseWriter, ident string, cidr address.CIDR, checkAlive, noErrorOnUnknown bool) { err := alloc.Claim(ident, cidr, checkAlive, noErrorOnUnknown, - hasBeenCancelled(dockerCli, w.(http.CloseNotifier).CloseNotify(), ident, checkAlive)) + hasBeenCancelled(dockerCli, ctx.Done(), ident, checkAlive)) if err != nil { if !cancellationErr(w, err) { badRequest(w, fmt.Errorf("Unable to claim: %s", err)) @@ -99,7 +100,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID ident := vars["id"] checkAlive := r.FormValue("check-alive") == "true" noErrorOnUnknown := r.FormValue("noErrorOnUnknown") == "true" - alloc.handleHTTPClaim(dockerCli, w, ident, cidr, checkAlive, noErrorOnUnknown) + alloc.handleHTTPClaim(r.Context(), dockerCli, w, ident, cidr, checkAlive, noErrorOnUnknown) } }) @@ -164,13 +165,13 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID router.Methods("POST").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) if subnet, ok := parseCIDR(w, vars["ip"]+"/"+vars["prefixlen"], true); ok { - alloc.handleHTTPAllocate(dockerCli, w, vars["id"], r.FormValue("check-alive") == "true", subnet) + alloc.handleHTTPAllocate(r.Context(), dockerCli, w, vars["id"], r.FormValue("check-alive") == "true", subnet) } }) router.Methods("POST").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) - alloc.handleHTTPAllocate(dockerCli, w, vars["id"], r.FormValue("check-alive") == "true", defaultSubnet) + alloc.handleHTTPAllocate(r.Context(), dockerCli, w, vars["id"], r.FormValue("check-alive") == "true", defaultSubnet) }) router.Methods("DELETE").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/ipam/http_test.go b/ipam/http_test.go index 14ba7d41..058c5d94 100644 --- a/ipam/http_test.go +++ b/ipam/http_test.go @@ -1,6 +1,7 @@ package ipam import ( + "context" "fmt" "io" "io/ioutil" @@ -158,11 +159,13 @@ func TestHTTPCancel(t *testing.T) { // Ask the http server for a new address req, _ := http.NewRequest("POST", allocURL(port, testCIDR1, containerID), nil) + ctx, cancel := context.WithCancel(context.Background()) + req = req.WithContext(ctx) // On another goroutine, wait for a bit then cancel the request go func() { time.Sleep(100 * time.Millisecond) common.Log.Debug("Cancelling allocate") - http.DefaultTransport.(*http.Transport).CancelRequest(req) + cancel() }() res, _ := http.DefaultClient.Do(req) From c19762468904a48d8485d2a19384656981bdaafc Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 11 Oct 2019 11:30:06 +0000 Subject: [PATCH 3/5] Fix up incorrect output formatting --- ipam/http.go | 2 +- test/images/network-tester/webserver.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipam/http.go b/ipam/http.go index ec5d2b45..416c1e86 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -204,6 +204,6 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID if alloc.tracker != nil { tracker = alloc.tracker.String() } - fmt.Fprintf(w, tracker) + fmt.Fprintln(w, tracker) }) } diff --git a/test/images/network-tester/webserver.go b/test/images/network-tester/webserver.go index 4315a2aa..ff1342b5 100644 --- a/test/images/network-tester/webserver.go +++ b/test/images/network-tester/webserver.go @@ -91,7 +91,7 @@ func (s *State) serveClientIP(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) } - fmt.Fprintf(w, host) + fmt.Fprintln(w, host) } // serveRead writes our json encoded state From 28e482af1462891a522eab3bf3d1dd9b5af21174 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 11 Oct 2019 11:32:12 +0000 Subject: [PATCH 4/5] Avoid some deprecated APIs --- prog/weave-npc/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index ab6ffeba..2942b885 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -248,7 +248,7 @@ func root(cmd *cobra.Command, args []string) { npc := npc.New(nodeName, ipt, ips, client) - nsController := makeController(client.Core().RESTClient(), "namespaces", &coreapi.Namespace{}, + nsController := makeController(client.CoreV1().RESTClient(), "namespaces", &coreapi.Namespace{}, cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { handleError(npc.AddNamespace(obj.(*coreapi.Namespace))) @@ -268,7 +268,7 @@ func root(cmd *cobra.Command, args []string) { handleError(npc.UpdateNamespace(old.(*coreapi.Namespace), new.(*coreapi.Namespace))) }}) - podController := makeController(client.Core().RESTClient(), "pods", &coreapi.Pod{}, + podController := makeController(client.CoreV1().RESTClient(), "pods", &coreapi.Pod{}, cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { handleError(npc.AddPod(obj.(*coreapi.Pod))) From 34dc4dabce472e8a001f970960b7cdea05c00b00 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 11 Oct 2019 11:32:58 +0000 Subject: [PATCH 5/5] Clean up some lint warnings I ran golangci-lint and it pointed out a few things to clean up. --- ipam/ring/ring.go | 2 +- ipam/space/space_test.go | 4 ++-- npc/selector.go | 2 +- prog/kube-utils/main.go | 2 +- proxy/common.go | 2 +- router/overlay_switch.go | 4 ++-- router/pcap.go | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index 43322a7f..e1cb1bcd 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -560,7 +560,7 @@ func (r *Ring) ChoosePeersToAskForSpace(start, end address.Address) []mesh.PeerN break } // Ignore ranges with no free space - if entry.Free <= 0 { + if entry.Free == 0 { continue } diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 1c00060e..e123287d 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -73,7 +73,7 @@ func TestLowlevel(t *testing.T) { // test Donate when addresses are scarce s = New() - r, ok = s.Donate(address.NewRange(0, 1000)) + _, ok = s.Donate(address.NewRange(0, 1000)) require.True(t, !ok, "donate on empty space should fail") s.Add(0, 3) require.NoError(t, s.Claim(0)) @@ -81,7 +81,7 @@ func TestLowlevel(t *testing.T) { 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)) + _, ok = s.Donate(address.NewRange(0, 1000)) require.True(t, !ok, "donate should fail") } diff --git a/npc/selector.go b/npc/selector.go index e6738a30..dfe73aee 100644 --- a/npc/selector.go +++ b/npc/selector.go @@ -59,7 +59,7 @@ func (spec *selectorSpec) getRuleSpec(src bool) ([]string, string) { } rule := []string{"-m", "set", "--match-set", string(spec.ipsetName), dir} - comment := "anywhere" + var comment string if spec.nsName != "" { comment = fmt.Sprintf("pods: namespace: %s, selector: %s", spec.nsName, spec.key) } else { diff --git a/prog/kube-utils/main.go b/prog/kube-utils/main.go index 6803ff11..fe1f05f2 100644 --- a/prog/kube-utils/main.go +++ b/prog/kube-utils/main.go @@ -370,7 +370,7 @@ func main() { } if runReclaimDaemon { // Handle SIGINT and SIGTERM - ch := make(chan os.Signal) + ch := make(chan os.Signal, 1) signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM) stopCh := make(chan struct{}) rand.Seed(time.Now().UnixNano()) diff --git a/proxy/common.go b/proxy/common.go index 88dd8354..280cc0c8 100644 --- a/proxy/common.go +++ b/proxy/common.go @@ -15,7 +15,7 @@ import ( ) var ( - containerIDRegexp = regexp.MustCompile("^(/v[0-9\\.]*)?/containers/([^/]*)/.*") + containerIDRegexp = regexp.MustCompile(`^(/v[0-9.]*)?/containers/([^/]*)/.*`) weaveWaitEntrypoint = []string{"/w/w"} Log = common.Log diff --git a/router/overlay_switch.go b/router/overlay_switch.go index b99d621a..0ef113f7 100644 --- a/router/overlay_switch.go +++ b/router/overlay_switch.go @@ -334,13 +334,13 @@ func (fwd *overlaySwitchForwarder) healthCheck(index int, healthy bool) { fwd.lock.Lock() defer fwd.lock.Unlock() - if healthy == true && fwd.forwarders[index].onHold { + if healthy && fwd.forwarders[index].onHold { log.Debug(fwd.logPrefix(), "Adding "+fwd.forwarders[index].overlayName+" to the list of forwarders") fwd.forwarders[index].onHold = false fwd.chooseBest() } - if healthy == false && !fwd.forwarders[index].onHold { + if !healthy && !fwd.forwarders[index].onHold { log.Debug(fwd.logPrefix(), "Removing "+fwd.forwarders[index].overlayName+" from the list of forwarders") fwd.forwarders[index].onHold = true fwd.chooseBest() diff --git a/router/pcap.go b/router/pcap.go index c4859277..21f84329 100644 --- a/router/pcap.go +++ b/router/pcap.go @@ -147,7 +147,7 @@ func (p *Pcap) sniff(readHandle *pcap.Handle, consumer Consumer) { // in order to prevent the next capture from // overwriting the data pktLen := len(pkt) - pktCopy := make([]byte, pktLen, pktLen) + pktCopy := make([]byte, pktLen) copy(pktCopy, pkt) fop.Process(pktCopy, dec, false)