Fix unstable parallel tests (#202)

* update interface tests and configuration messages * little debug * little debug on both errors * print all devs * list of devices * debug func * handle interface close * debug socks * debug socks * if ports match * use random assigned ports * remove unused const * close management client connection when stopping engine * GracefulStop when management clients are closed * enable workflows on PRs too * remove iface_test debug code
netbirdio · Jan 25, 2022 · 0739038 · 0739038
1 parent 8ab6eb1
commit 0739038
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 47 deletions.
diff --git a/.github/workflows/golang-test-build.yml b/.github/workflows/golang-test-build.yml
@@ -1,5 +1,5 @@
 name: Test Build On Platforms
-on: push
+on: [push,pull_request]
 jobs:
   test_build:
     strategy:

diff --git a/.github/workflows/golang-test-darwin.yml b/.github/workflows/golang-test-darwin.yml
@@ -1,5 +1,5 @@
 name: Test Code Darwin
-on: push
+on: [push,pull_request]
 jobs:
   test:
     strategy:

diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml
@@ -1,5 +1,5 @@
 name: Test Code Linux
-on: push
+on: [push,pull_request]
 jobs:
   test:
     strategy:

diff --git a/.github/workflows/golang-test-windows.yml b/.github/workflows/golang-test-windows.yml
@@ -1,5 +1,5 @@
 name: Test Code Windows
-on: push
+on: [push,pull_request]
 jobs:
   pre:
     runs-on: ubuntu-latest
@@ -47,5 +47,5 @@ jobs:
       - name: Install modules
         run: go mod tidy
 
-      - name: Test build
+      - name: Test
         run: go test -tags=load_wgnt_from_rsrc ./...
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
@@ -1,5 +1,5 @@
 name: golangci-lint
-on: push
+on: [push,pull_request]
 jobs:
   golangci:
     name: lint

diff --git a/client/internal/engine.go b/client/internal/engine.go
@@ -96,7 +96,13 @@ func (e *Engine) Stop() error {
 	e.syncMsgMux.Lock()
 	defer e.syncMsgMux.Unlock()
 
-	err := e.removeAllPeers()
+	err := e.mgmClient.Close()
+	if err != nil {
+		log.Errorf("failed closing engine's management client: %v", err)
+		return err
+	}
+
+	err = e.removeAllPeers()
 	if err != nil {
 		return err
 	}

diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go
@@ -297,7 +297,7 @@ func TestEngine_MultiplePeers(t *testing.T) {
 		t.Fatal(err)
 		return
 	}
-	defer mgmtServer.Stop()
+	defer mgmtServer.GracefulStop()
 
 	setupKey := "A2C8E62B-38F5-4553-B31E-DD66C696CEBB"
 

diff --git a/go.mod b/go.mod
@@ -28,6 +28,7 @@ require (
 )
 
 require (
+	github.com/cakturk/go-netstat v0.0.0-20200220111822-e5b49efee7a5
 	github.com/magiconair/properties v1.8.5
 	github.com/rs/xid v1.3.0
 	github.com/stretchr/testify v1.7.0

diff --git a/go.sum b/go.sum
@@ -66,6 +66,8 @@ github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
+github.com/cakturk/go-netstat v0.0.0-20200220111822-e5b49efee7a5 h1:BjkPE3785EwPhhyuFkbINB+2a1xATwk8SNDWnJiD41g=
+github.com/cakturk/go-netstat v0.0.0-20200220111822-e5b49efee7a5/go.mod h1:jtAfVaU/2cu1+wdSRPWE2c1N2qeAA3K4RH9pYgqwets=
 github.com/cenkalti/backoff/v4 v4.1.2 h1:6Yo7N8UP2K6LWZnW94DLVSSrbobcWdVzAYOisuDPIFo=
 github.com/cenkalti/backoff/v4 v4.1.2/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=

diff --git a/iface/configuration.go b/iface/configuration.go
@@ -1,6 +1,7 @@
 package iface
 
 import (
+	"fmt"
 	log "github.com/sirupsen/logrus"
 	"golang.zx2c4.com/wireguard/wgctrl"
 	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
@@ -45,7 +46,11 @@ func (w *WGIface) Configure(privateKey string, port int) error {
 		ListenPort:   &port,
 	}
 
-	return w.configureDevice(config)
+	err = w.configureDevice(config)
+	if err != nil {
+		return fmt.Errorf("received error \"%v\" while configuring interface %s with port %d", err, w.Name, port)
+	}
+	return nil
 }
 
 // GetListenPort returns the listening port of the Wireguard endpoint
@@ -96,8 +101,11 @@ func (w *WGIface) UpdatePeer(peerKey string, allowedIps string, keepAlive time.D
 	config := wgtypes.Config{
 		Peers: []wgtypes.PeerConfig{peer},
 	}
-
-	return w.configureDevice(config)
+	err = w.configureDevice(config)
+	if err != nil {
+		return fmt.Errorf("received error \"%v\" while updating peer on interface %s with settings: allowed ips %s, endpoint %s", err, w.Name, allowedIps, endpoint.String())
+	}
+	return nil
 }
 
 // RemovePeer removes a Wireguard Peer from the interface iface
@@ -117,6 +125,9 @@ func (w *WGIface) RemovePeer(peerKey string) error {
 	config := wgtypes.Config{
 		Peers: []wgtypes.PeerConfig{peer},
 	}
-
-	return w.configureDevice(config)
+	err = w.configureDevice(config)
+	if err != nil {
+		return fmt.Errorf("received error \"%v\" while removing peer %s from interface %s", err, peerKey, w.Name)
+	}
+	return nil
 }
diff --git a/iface/iface_test.go b/iface/iface_test.go
@@ -12,7 +12,7 @@ import (
 
 // keep darwin compability
 const (
-	WgPort = 51000
+	WgIntNumber = 2000
 )
 
 var (
@@ -30,8 +30,8 @@ func init() {
 
 //
 func Test_CreateInterface(t *testing.T) {
-	ifaceName := "utun999"
-	wgIP := "10.99.99.1/24"
+	ifaceName := fmt.Sprintf("utun%d", WgIntNumber+1)
+	wgIP := "10.99.99.1/32"
 	iface, err := NewWGIface(ifaceName, wgIP, DefaultMTU)
 	if err != nil {
 		t.Fatal(err)
@@ -59,8 +59,8 @@ func Test_CreateInterface(t *testing.T) {
 }
 
 func Test_Close(t *testing.T) {
-	ifaceName := "utun1004"
-	wgIP := "10.99.99.50/24"
+	ifaceName := fmt.Sprintf("utun%d", WgIntNumber+2)
+	wgIP := "10.99.99.2/32"
 	iface, err := NewWGIface(ifaceName, wgIP, DefaultMTU)
 	if err != nil {
 		t.Fatal(err)
@@ -87,8 +87,8 @@ func Test_Close(t *testing.T) {
 }
 
 func Test_ConfigureInterface(t *testing.T) {
-	ifaceName := "utun1000"
-	wgIP := "10.99.99.10/24"
+	ifaceName := fmt.Sprintf("utun%d", WgIntNumber+3)
+	wgIP := "10.99.99.5/30"
 	iface, err := NewWGIface(ifaceName, wgIP, DefaultMTU)
 	if err != nil {
 		t.Fatal(err)
@@ -104,7 +104,11 @@ func Test_ConfigureInterface(t *testing.T) {
 		}
 	}()
 
-	err = iface.Configure(key, WgPort+1)
+	port, err := iface.GetListenPort()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = iface.Configure(key, *port)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -130,8 +134,8 @@ func Test_ConfigureInterface(t *testing.T) {
 }
 
 func Test_UpdatePeer(t *testing.T) {
-	ifaceName := "utun1001"
-	wgIP := "10.99.99.20/24"
+	ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4)
+	wgIP := "10.99.99.9/30"
 	iface, err := NewWGIface(ifaceName, wgIP, DefaultMTU)
 	if err != nil {
 		t.Fatal(err)
@@ -146,12 +150,16 @@ func Test_UpdatePeer(t *testing.T) {
 			t.Error(err)
 		}
 	}()
-	err = iface.Configure(key, WgPort+2)
+	port, err := iface.GetListenPort()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = iface.Configure(key, *port)
 	if err != nil {
 		t.Fatal(err)
 	}
 	keepAlive := 15 * time.Second
-	allowedIP := "10.99.99.2/32"
+	allowedIP := "10.99.99.10/32"
 	endpoint, err := net.ResolveUDPAddr("udp", "127.0.0.1:9900")
 	if err != nil {
 		t.Fatal(err)
@@ -185,8 +193,8 @@ func Test_UpdatePeer(t *testing.T) {
 }
 
 func Test_RemovePeer(t *testing.T) {
-	ifaceName := "utun1003"
-	wgIP := "10.99.99.40/24"
+	ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4)
+	wgIP := "10.99.99.13/30"
 	iface, err := NewWGIface(ifaceName, wgIP, DefaultMTU)
 	if err != nil {
 		t.Fatal(err)
@@ -201,12 +209,16 @@ func Test_RemovePeer(t *testing.T) {
 			t.Error(err)
 		}
 	}()
-	err = iface.Configure(key, WgPort+3)
+	port, err := iface.GetListenPort()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = iface.Configure(key, *port)
 	if err != nil {
 		t.Fatal(err)
 	}
 	keepAlive := 15 * time.Second
-	allowedIP := "10.99.99.2/32"
+	allowedIP := "10.99.99.14/32"
 
 	err = iface.UpdatePeer(peerPubKey, allowedIP, keepAlive, nil, nil)
 	if err != nil {
@@ -223,25 +235,15 @@ func Test_RemovePeer(t *testing.T) {
 }
 
 func Test_ConnectPeers(t *testing.T) {
-	peer1ifaceName := fmt.Sprintf("utun%d", 400)
-	peer1wgIP := "10.99.99.100/24"
+	peer1ifaceName := fmt.Sprintf("utun%d", WgIntNumber+400)
+	peer1wgIP := "10.99.99.17/30"
 	peer1Key, _ := wgtypes.GeneratePrivateKey()
-	peer1Port := WgPort + 4
-
-	peer1endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("127.0.0.1:%d", peer1Port))
-	if err != nil {
-		t.Fatal(err)
-	}
+	//peer1Port := WgPort + 4
 
 	peer2ifaceName := fmt.Sprintf("utun%d", 500)
-	peer2wgIP := "10.99.99.200/24"
+	peer2wgIP := "10.99.99.18/30"
 	peer2Key, _ := wgtypes.GeneratePrivateKey()
-	peer2Port := WgPort + 5
-
-	peer2endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("127.0.0.1:%d", peer2Port))
-	if err != nil {
-		t.Fatal(err)
-	}
+	//peer2Port := WgPort + 5
 
 	keepAlive := 1 * time.Second
 
@@ -253,6 +255,15 @@ func Test_ConnectPeers(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	peer1Port, err := iface1.GetListenPort()
+	if err != nil {
+		t.Fatal(err)
+	}
+	peer1endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("127.0.0.1:%d", *peer1Port))
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	iface2, err := NewWGIface(peer2ifaceName, peer2wgIP, DefaultMTU)
 	if err != nil {
 		t.Fatal(err)
@@ -261,6 +272,14 @@ func Test_ConnectPeers(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	peer2Port, err := iface2.GetListenPort()
+	if err != nil {
+		t.Fatal(err)
+	}
+	peer2endpoint, err := net.ResolveUDPAddr("udp", fmt.Sprintf("127.0.0.1:%d", *peer2Port))
+	if err != nil {
+		t.Fatal(err)
+	}
 	defer func() {
 		err = iface1.Close()
 		if err != nil {
@@ -271,11 +290,12 @@ func Test_ConnectPeers(t *testing.T) {
 			t.Error(err)
 		}
 	}()
-	err = iface1.Configure(peer1Key.String(), peer1Port)
+
+	err = iface1.Configure(peer1Key.String(), *peer1Port)
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = iface2.Configure(peer2Key.String(), peer2Port)
+	err = iface2.Configure(peer2Key.String(), *peer2Port)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/management/server/management_proto_test.go b/management/server/management_proto_test.go
@@ -110,7 +110,7 @@ func Test_SyncProtocol(t *testing.T) {
 		t.Fatal(err)
 		return
 	}
-	defer mgmtServer.Stop()
+	defer mgmtServer.GracefulStop()
 
 	client, clientConn, err := createRawClient(fmt.Sprintf("localhost:%d", mport))
 	if err != nil {