Fix various issues with refactors

Makefile

@@ -191,6 +191,7 @@ gofmt: ## Tells you what files need to be gofmt'd.
 
 gofmt-fix: ## Fixes files that need to be gofmt'd.
 	gofmt -s -w $(shell find . -not \( \( -wholename '*/vendor/*' \) -prune \) -name '*.go')
+	goimports -w $(shell find . -not \( \( -wholename '*/vendor/*' \) -prune \) -name '*.go')
 
 # List of all file_moq.go files which would need to be regenerated
 # from file.go if changed

build/test-scripts/unit_test_timing.py

@@ -0,0 +1,34 @@
+#!/usr/bin/env python
+
+# Taken from: https://rotational.io/blog/speeding-up-go-tests/ this python script assists in parsing Golang JSON unit
+# tests and sorting them by the amount of time taken
+#
+# To use, run Go unit tests via the following:
+# go test -v -json -count 1 github.com/cloudnativelabs/kube-router/v2/cmd/kube-router/ github.com/cloudnativelabs/kube-router/v2/pkg/... >testing_output.json
+#
+# Then run this script via:
+# build/test-scripts/unit_test_timing.py testing_output.json
+
+import json
+import sys
+
+if __name__ == "__main__":
+    tests = []
+
+    with open(sys.argv[1], 'r') as f:
+        for line in f:
+            data = json.loads(line)
+            if data['Action'] != 'pass':
+                continue
+
+            if 'Test' not in data:
+                continue
+
+            if data['Elapsed'] < 0.1:
+                continue
+
+            tests.append(data)
+
+    tests.sort(key=lambda d: d['Elapsed'], reverse=True)
+    for t in tests:
+        print(f"{t['Elapsed']:0.3f}s\t{t['Package']} {t['Test']}")

pkg/bgp/id.go

@@ -37,7 +37,7 @@ func GenerateRouterID(nodeIPAware utils.NodeIPAware, configRouterID string) (str
 	if nodeIPAware.GetPrimaryNodeIP().To4() == nil {
 		return "", errors.New("router-id must be specified when primary node IP is an IPv6 address")
 	}
-	return configRouterID, nil
+	return nodeIPAware.GetPrimaryNodeIP().String(), nil
 }
 
 // ValidateCommunity takes in a string and attempts to parse a BGP community out of it in a way that is similar to

pkg/controllers/lballoc/lballoc.go

@@ -38,6 +38,7 @@ type LoadBalancerController struct {
 	clientset    kubernetes.Interface
 	isDefault    bool
 	syncPeriod   time.Duration
+	unitTestWG   *sync.WaitGroup
 }
 
 func getNamespace() (namespace string, err error) {
@@ -346,6 +347,12 @@ func appendIngressIP(svc *v1core.Service, ip net.IP) {
 }
 
 func (lbc *LoadBalancerController) updateService(svc *v1core.Service, ips ...net.IP) {
+	// This is only non-nil during certain unit tests that need to understand when this goroutine is finished to remove
+	// chance of race conditions
+	if lbc.unitTestWG != nil {
+		defer lbc.unitTestWG.Done()
+	}
+
 	if lbc.clientset == nil {
 		panic("clientset")
 	}
@@ -399,6 +406,11 @@ func (lbc *LoadBalancerController) allocateService(svc *v1core.Service) error {
 		return errors.New("unable to allocate dual-stack addresses: " + err.Error())
 	}
 
+	// This is only non-nil during certain unit tests that need to understand when this goroutine is finished to remove
+	// chance of race conditions
+	if lbc.unitTestWG != nil {
+		lbc.unitTestWG.Add(1)
+	}
 	go lbc.updateService(svc, ipv4, ipv6)
 	return nil
 }

pkg/controllers/lballoc/lballoc_test.go

@@ -3,6 +3,7 @@ package lballoc
 import (
 	"errors"
 	"net"
+	"sync"
 	"testing"
 	"time"
 
@@ -572,7 +573,8 @@ func TestAppendIngressIP(t *testing.T) {
 
 func TestAllocateService(t *testing.T) {
 	mlbc := &LoadBalancerController{
-		clientset: fake.NewSimpleClientset(),
+		clientset:  fake.NewSimpleClientset(),
+		unitTestWG: &sync.WaitGroup{},
 	}
 	ir4, ir6 := makeIPRanges("127.127.127.127/30", "ffff::/80")
 	mlbc.ipv4Ranges = ir4
@@ -586,6 +588,7 @@ func TestAllocateService(t *testing.T) {
 		t.Fatalf("expected %v, got %s", nil, err)
 	}
 
+	mlbc.unitTestWG.Wait()
 	svc = makeTestService()
 	mlbc.ipv4Ranges = newipRanges(nil)
 	fp := v1core.IPFamilyPolicyRequireDualStack

pkg/controllers/routing/ecmp_vip_test.go

@@ -903,6 +903,7 @@ func Test_getVIPsForService(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
 			for _, serviceAdvertisedIP := range test.serviceAdvertisedIPs {
 				endpoints := serviceAdvertisedIP.endpoints
 				clientset := fake.NewSimpleClientset()

pkg/utils/node.go

@@ -85,18 +85,16 @@ type NodeAware interface {
 // GetNodeIPv4Addrs returns the node's IPv4 addresses as defined by the Kubernetes Node Object.
 func (n *KRNode) GetNodeIPv4Addrs() []net.IP {
 	var nodeIPs []net.IP
-	for _, ip := range n.NodeIPv4Addrs {
-		nodeIPs = append(nodeIPs, ip...)
-	}
+	nodeIPs = append(nodeIPs, n.NodeIPv4Addrs[apiv1.NodeInternalIP]...)
+	nodeIPs = append(nodeIPs, n.NodeIPv4Addrs[apiv1.NodeExternalIP]...)
 	return nodeIPs
 }
 
 // GetNodeIPv6Addrs returns the node's IPv6 addresses as defined by the Kubernetes Node Object.
 func (n *KRNode) GetNodeIPv6Addrs() []net.IP {
 	var nodeIPs []net.IP
-	for _, ip := range n.NodeIPv6Addrs {
-		nodeIPs = append(nodeIPs, ip...)
-	}
+	nodeIPs = append(nodeIPs, n.NodeIPv6Addrs[apiv1.NodeInternalIP]...)
+	nodeIPs = append(nodeIPs, n.NodeIPv6Addrs[apiv1.NodeExternalIP]...)
 	return nodeIPs
 }
 
@@ -196,12 +194,10 @@ func (n *LocalKRNode) GetNodeMTU() (int, error) {
 // Node Object.
 func (n *KRNode) GetNodeIPAddrs() []net.IP {
 	var nodeIPs []net.IP
-	for _, ip := range n.NodeIPv4Addrs {
-		nodeIPs = append(nodeIPs, ip...)
-	}
-	for _, ip := range n.NodeIPv6Addrs {
-		nodeIPs = append(nodeIPs, ip...)
-	}
+	ipv4IPs := n.GetNodeIPv4Addrs()
+	nodeIPs = append(nodeIPs, ipv4IPs...)
+	ipv6IPs := n.GetNodeIPv6Addrs()
+	nodeIPs = append(nodeIPs, ipv6IPs...)
 	return nodeIPs
 }