From e66412da1b700b4beb96deba51bf329c5f4c98c7 Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Fri, 18 Jul 2025 19:12:44 +0200 Subject: [PATCH] add race flag to client tests using for now a temp fixed for ice --- .github/workflows/golang-test-darwin.yml | 2 +- .github/workflows/golang-test-linux.yml | 2 +- .github/workflows/golang-test-windows.yml | 2 +- client/firewall/uspfilter/conntrack/udp.go | 9 ++++++ .../firewall/uspfilter/conntrack/udp_test.go | 4 +-- client/internal/dns/upstream_test.go | 18 +++++++++--- client/internal/engine.go | 7 ++++- client/internal/lazyconn/activity/manager.go | 8 ++++++ .../lazyconn/activity/manager_test.go | 7 +++-- client/internal/peer/notifier_test.go | 28 +++++++++++++++++-- go.mod | 2 +- go.sum | 4 +-- 12 files changed, 76 insertions(+), 17 deletions(-) diff --git a/.github/workflows/golang-test-darwin.yml b/.github/workflows/golang-test-darwin.yml index 4571ce753..94c93592b 100644 --- a/.github/workflows/golang-test-darwin.yml +++ b/.github/workflows/golang-test-darwin.yml @@ -42,5 +42,5 @@ jobs: run: git --no-pager diff --exit-code - name: Test - run: NETBIRD_STORE_ENGINE=${{ matrix.store }} CI=true go test -tags=devcert -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -timeout 5m -p 1 $(go list ./... | grep -v /management) + run: NETBIRD_STORE_ENGINE=${{ matrix.store }} CI=true go test -tags=devcert -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -race -timeout 5m -p 1 $(go list ./... | grep -v /management) diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index cbce3e6e4..e1fe707fc 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -144,7 +144,7 @@ jobs: run: git --no-pager diff --exit-code - name: Test - run: CGO_ENABLED=1 GOARCH=${{ matrix.arch }} CI=true go test -tags devcert -exec 'sudo' -timeout 10m -p 1 $(go list ./... | grep -v -e /management -e /signal -e /relay) + run: CGO_ENABLED=1 GOARCH=${{ matrix.arch }} CI=true go test -tags devcert -exec 'sudo' -race -timeout 10m -p 1 $(go list ./... | grep -v -e /management -e /signal -e /relay) test_client_on_docker: name: "Client (Docker) / Unit" diff --git a/.github/workflows/golang-test-windows.yml b/.github/workflows/golang-test-windows.yml index d9ff0a84b..33624c6ab 100644 --- a/.github/workflows/golang-test-windows.yml +++ b/.github/workflows/golang-test-windows.yml @@ -66,7 +66,7 @@ jobs: - run: echo "files=$(go list ./... | ForEach-Object { $_ } | Where-Object { $_ -notmatch '/management' })" >> $env:GITHUB_ENV - name: test - run: PsExec64 -s -w ${{ github.workspace }} cmd.exe /c "C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe test -tags=devcert -timeout 10m -p 1 ${{ env.files }} > test-out.txt 2>&1" + run: PsExec64 -s -w ${{ github.workspace }} cmd.exe /c "C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe test -tags=devcert -race -timeout 10m -p 1 ${{ env.files }} > test-out.txt 2>&1" - name: test output if: ${{ always() }} run: Get-Content test-out.txt diff --git a/client/firewall/uspfilter/conntrack/udp.go b/client/firewall/uspfilter/conntrack/udp.go index 000eaa1b6..836169393 100644 --- a/client/firewall/uspfilter/conntrack/udp.go +++ b/client/firewall/uspfilter/conntrack/udp.go @@ -7,6 +7,7 @@ import ( "time" "github.com/google/uuid" + "golang.org/x/exp/maps" nblog "github.com/netbirdio/netbird/client/firewall/uspfilter/log" nftypes "github.com/netbirdio/netbird/client/internal/netflow/types" @@ -218,3 +219,11 @@ func (t *UDPTracker) sendEvent(typ nftypes.Type, conn *UDPConnTrack, ruleID []by TxBytes: conn.BytesTx.Load(), }) } + +func (t *UDPTracker) getConnections() map[ConnKey]*UDPConnTrack { + t.mutex.RLock() + defer t.mutex.RUnlock() + copyConn := make(map[ConnKey]*UDPConnTrack, len(t.connections)) + maps.Copy(copyConn, t.connections) + return copyConn +} diff --git a/client/firewall/uspfilter/conntrack/udp_test.go b/client/firewall/uspfilter/conntrack/udp_test.go index 7ad1e0e4b..9735498c2 100644 --- a/client/firewall/uspfilter/conntrack/udp_test.go +++ b/client/firewall/uspfilter/conntrack/udp_test.go @@ -202,13 +202,13 @@ func TestUDPTracker_Cleanup(t *testing.T) { } // Verify initial connections - assert.Len(t, tracker.connections, 2) + assert.Len(t, tracker.getConnections(), 2) // Wait for connection timeout and cleanup interval time.Sleep(timeout + 2*cleanupInterval) tracker.mutex.RLock() - connCount := len(tracker.connections) + connCount := len(tracker.getConnections()) tracker.mutex.RUnlock() // Verify connections were cleaned up diff --git a/client/internal/dns/upstream_test.go b/client/internal/dns/upstream_test.go index e440995d9..dbdedb7f2 100644 --- a/client/internal/dns/upstream_test.go +++ b/client/internal/dns/upstream_test.go @@ -4,6 +4,7 @@ import ( "context" "net/netip" "strings" + "sync" "testing" "time" @@ -135,20 +136,26 @@ func TestUpstreamResolver_DeactivationReactivation(t *testing.T) { responseWriter := &test.MockResponseWriter{ WriteMsgFunc: func(m *dns.Msg) error { return nil }, } - + lmux := sync.Mutex{} failed := false resolver.deactivate = func(error) { + lmux.Lock() failed = true + lmux.Unlock() } reactivated := false resolver.reactivate = func() { + lmux.Lock() reactivated = true + lmux.Unlock() } resolver.ServeDNS(responseWriter, new(dns.Msg).SetQuestion("one.one.one.one.", dns.TypeA)) - - if !failed { + lmux.Lock() + failedCheck := failed + lmux.Unlock() + if !failedCheck { t.Errorf("expected that resolving was deactivated") return } @@ -160,7 +167,10 @@ func TestUpstreamResolver_DeactivationReactivation(t *testing.T) { time.Sleep(time.Millisecond * 200) - if !reactivated { + lmux.Lock() + checkReactivated := reactivated + lmux.Unlock() + if !checkReactivated { t.Errorf("expected that resolving was reactivated") return } diff --git a/client/internal/engine.go b/client/internal/engine.go index e9772b359..90f5a36ea 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -836,7 +836,10 @@ func (e *Engine) updateSSH(sshConf *mgmProto.SSHConfig) error { } go func() { // blocking - err = e.sshServer.Start() + e.syncMsgMux.Lock() + sshServer := e.sshServer + e.syncMsgMux.Unlock() + err = sshServer.Start() if err != nil { // will throw error when we stop it even if it is a graceful stop log.Debugf("stopped SSH server with error %v", err) @@ -851,6 +854,8 @@ func (e *Engine) updateSSH(sshConf *mgmProto.SSHConfig) error { } } else if !isNil(e.sshServer) { // Disable SSH server request, so stop it if it was running + e.syncMsgMux.Lock() + defer e.syncMsgMux.Unlock() err := e.sshServer.Stop() if err != nil { log.Warnf("failed to stop SSH server %v", err) diff --git a/client/internal/lazyconn/activity/manager.go b/client/internal/lazyconn/activity/manager.go index 915fb9cb8..e2aba29cf 100644 --- a/client/internal/lazyconn/activity/manager.go +++ b/client/internal/lazyconn/activity/manager.go @@ -102,3 +102,11 @@ func (m *Manager) notify(peerConnID peerid.ConnID) { case m.OnActivityChan <- peerConnID: } } + +func (m *Manager) getPeerListener(peerConnID peerid.ConnID) (*Listener, bool) { + m.mu.Lock() + defer m.mu.Unlock() + + listener, ok := m.peers[peerConnID] + return listener, ok +} diff --git a/client/internal/lazyconn/activity/manager_test.go b/client/internal/lazyconn/activity/manager_test.go index c7c6c878a..69a9f3f57 100644 --- a/client/internal/lazyconn/activity/manager_test.go +++ b/client/internal/lazyconn/activity/manager_test.go @@ -50,8 +50,11 @@ func TestManager_MonitorPeerActivity(t *testing.T) { if err := mgr.MonitorPeerActivity(peerCfg1); err != nil { t.Fatalf("failed to monitor peer activity: %v", err) } - - if err := trigger(mgr.peers[peerCfg1.PeerConnID].conn.LocalAddr().String()); err != nil { + listener, ok := mgr.getPeerListener(peerCfg1.PeerConnID) + if !ok { + t.Fatalf("failed to get peer listener: %s", peerCfg1.PeerConnID) + } + if err := trigger(listener.conn.LocalAddr().String()); err != nil { t.Fatalf("failed to trigger activity: %v", err) } diff --git a/client/internal/peer/notifier_test.go b/client/internal/peer/notifier_test.go index bbdc00e13..8a2cd8d12 100644 --- a/client/internal/peer/notifier_test.go +++ b/client/internal/peer/notifier_test.go @@ -9,30 +9,54 @@ type mocListener struct { lastState int wg sync.WaitGroup peers int + mux sync.Mutex } func (l *mocListener) OnConnected() { + l.mux.Lock() + defer l.mux.Unlock() l.lastState = stateConnected l.wg.Done() } func (l *mocListener) OnDisconnected() { + l.mux.Lock() + defer l.mux.Unlock() l.lastState = stateDisconnected l.wg.Done() } func (l *mocListener) OnConnecting() { + l.mux.Lock() + defer l.mux.Unlock() l.lastState = stateConnecting l.wg.Done() } func (l *mocListener) OnDisconnecting() { + l.mux.Lock() + defer l.mux.Unlock() l.lastState = stateDisconnecting l.wg.Done() + +} + +func (l *mocListener) getLastState() int { + l.mux.Lock() + defer l.mux.Unlock() + return l.lastState } func (l *mocListener) OnAddressChanged(host, addr string) { } func (l *mocListener) OnPeersListChanged(size int) { + l.mux.Lock() l.peers = size + l.mux.Unlock() +} + +func (l *mocListener) getPeers() int { + l.mux.Lock() + defer l.mux.Unlock() + return l.peers } func (l *mocListener) setWaiter() { @@ -77,7 +101,7 @@ func Test_notifier_SetListener(t *testing.T) { n.lastNotification = stateConnecting n.setListener(listener) listener.wait() - if listener.lastState != n.lastNotification { + if listener.getLastState() != n.lastNotification { t.Errorf("invalid state: %d, expected: %d", listener.lastState, n.lastNotification) } } @@ -91,7 +115,7 @@ func Test_notifier_RemoveListener(t *testing.T) { n.removeListener() n.peerListChanged(1) - if listener.peers != 0 { + if listener.getPeers() != 0 { t.Errorf("invalid state: %d", listener.peers) } } diff --git a/go.mod b/go.mod index cf2a23758..8819d7a05 100644 --- a/go.mod +++ b/go.mod @@ -257,6 +257,6 @@ replace golang.zx2c4.com/wireguard => github.com/netbirdio/wireguard-go v0.0.0-2 replace github.com/cloudflare/circl => github.com/cunicu/circl v0.0.0-20230801113412-fec58fc7b5f6 -replace github.com/pion/ice/v3 => github.com/netbirdio/ice/v3 v3.0.0-20240315174635-e72a50fcb64e +replace github.com/pion/ice/v3 => github.com/netbirdio/ice/v3 v3.0.0-20250718163601-725c8ac53a31 replace github.com/libp2p/go-netroute => github.com/netbirdio/go-netroute v0.0.0-20240611143515-f59b0e1d3944 diff --git a/go.sum b/go.sum index 699a832dd..77713783d 100644 --- a/go.sum +++ b/go.sum @@ -501,8 +501,8 @@ github.com/neelance/astrewrite v0.0.0-20160511093645-99348263ae86/go.mod h1:kHJE github.com/neelance/sourcemap v0.0.0-20200213170602-2833bce08e4c/go.mod h1:Qr6/a/Q4r9LP1IltGz7tA7iOK1WonHEYhu1HRBA7ZiM= github.com/netbirdio/go-netroute v0.0.0-20240611143515-f59b0e1d3944 h1:TDtJKmM6Sf8uYFx/dMeqNOL90KUoRscdfpFZ3Im89uk= github.com/netbirdio/go-netroute v0.0.0-20240611143515-f59b0e1d3944/go.mod h1:sHA6TRxjQ6RLbnI+3R4DZo2Eseg/iKiPRfNmcuNySVQ= -github.com/netbirdio/ice/v3 v3.0.0-20240315174635-e72a50fcb64e h1:PURA50S8u4mF6RrkYYCAvvPCixhqqEiEy3Ej6avh04c= -github.com/netbirdio/ice/v3 v3.0.0-20240315174635-e72a50fcb64e/go.mod h1:YMLU7qbKfVjmEv7EoZPIVEI+kNYxWCdPK3VS0BU+U4Q= +github.com/netbirdio/ice/v3 v3.0.0-20250718163601-725c8ac53a31 h1:lr/CnQ9NnlHr4yjDaCqy3V1FW+y9DDpzqxu1+YXzXtc= +github.com/netbirdio/ice/v3 v3.0.0-20250718163601-725c8ac53a31/go.mod h1:YMLU7qbKfVjmEv7EoZPIVEI+kNYxWCdPK3VS0BU+U4Q= github.com/netbirdio/management-integrations/integrations v0.0.0-20250718071730-f4d133556ff5 h1:Zfn8d83OVyELCdxgprcyXR3D8uqoxHtXE9PUxVXDx/w= github.com/netbirdio/management-integrations/integrations v0.0.0-20250718071730-f4d133556ff5/go.mod h1:Gi9raplYzCCyh07Olw/DVfCJTFgpr1WCXJ/Q+8TSA9Q= github.com/netbirdio/service v0.0.0-20240911161631-f62744f42502 h1:3tHlFmhTdX9axERMVN63dqyFqnvuD+EMJHzM7mNGON8=