diff --git a/client/internal/peer/status.go b/client/internal/peer/status.go index 69e333bf1..40956e68e 100644 --- a/client/internal/peer/status.go +++ b/client/internal/peer/status.go @@ -289,11 +289,7 @@ func (d *Status) UpdatePeerState(receivedState State) error { return errors.New("peer doesn't exist") } - if receivedState.IP != "" { - peerState.IP = receivedState.IP - } - - skipNotification := shouldSkipNotify(receivedState.ConnStatus, peerState) + oldState := peerState.ConnStatus if receivedState.ConnStatus != peerState.ConnStatus { peerState.ConnStatus = receivedState.ConnStatus @@ -309,11 +305,15 @@ func (d *Status) UpdatePeerState(receivedState State) error { d.peers[receivedState.PubKey] = peerState - if skipNotification { - return nil + if hasConnStatusChanged(oldState, receivedState.ConnStatus) { + d.notifyPeerListChanged() } - d.notifyPeerListChanged() + // when we close the connection we will not notify the router manager + if receivedState.ConnStatus == StatusIdle { + d.notifyPeerStateChangeListeners(receivedState.PubKey) + + } return nil } @@ -380,11 +380,8 @@ func (d *Status) UpdatePeerICEState(receivedState State) error { return errors.New("peer doesn't exist") } - if receivedState.IP != "" { - peerState.IP = receivedState.IP - } - - skipNotification := shouldSkipNotify(receivedState.ConnStatus, peerState) + oldState := peerState.ConnStatus + oldIsRelayed := peerState.Relayed peerState.ConnStatus = receivedState.ConnStatus peerState.ConnStatusUpdate = receivedState.ConnStatusUpdate @@ -397,12 +394,13 @@ func (d *Status) UpdatePeerICEState(receivedState State) error { d.peers[receivedState.PubKey] = peerState - if skipNotification { - return nil + if hasConnStatusChanged(oldState, receivedState.ConnStatus) { + d.notifyPeerListChanged() } - d.notifyPeerStateChangeListeners(receivedState.PubKey) - d.notifyPeerListChanged() + if hasStatusOrRelayedChange(oldState, receivedState.ConnStatus, oldIsRelayed, receivedState.Relayed) { + d.notifyPeerStateChangeListeners(receivedState.PubKey) + } return nil } @@ -415,7 +413,8 @@ func (d *Status) UpdatePeerRelayedState(receivedState State) error { return errors.New("peer doesn't exist") } - skipNotification := shouldSkipNotify(receivedState.ConnStatus, peerState) + oldState := peerState.ConnStatus + oldIsRelayed := peerState.Relayed peerState.ConnStatus = receivedState.ConnStatus peerState.ConnStatusUpdate = receivedState.ConnStatusUpdate @@ -425,12 +424,13 @@ func (d *Status) UpdatePeerRelayedState(receivedState State) error { d.peers[receivedState.PubKey] = peerState - if skipNotification { - return nil + if hasConnStatusChanged(oldState, receivedState.ConnStatus) { + d.notifyPeerListChanged() } - d.notifyPeerStateChangeListeners(receivedState.PubKey) - d.notifyPeerListChanged() + if hasStatusOrRelayedChange(oldState, receivedState.ConnStatus, oldIsRelayed, receivedState.Relayed) { + d.notifyPeerStateChangeListeners(receivedState.PubKey) + } return nil } @@ -443,7 +443,8 @@ func (d *Status) UpdatePeerRelayedStateToDisconnected(receivedState State) error return errors.New("peer doesn't exist") } - skipNotification := shouldSkipNotify(receivedState.ConnStatus, peerState) + oldState := peerState.ConnStatus + oldIsRelayed := peerState.Relayed peerState.ConnStatus = receivedState.ConnStatus peerState.Relayed = receivedState.Relayed @@ -452,12 +453,13 @@ func (d *Status) UpdatePeerRelayedStateToDisconnected(receivedState State) error d.peers[receivedState.PubKey] = peerState - if skipNotification { - return nil + if hasConnStatusChanged(oldState, receivedState.ConnStatus) { + d.notifyPeerListChanged() } - d.notifyPeerStateChangeListeners(receivedState.PubKey) - d.notifyPeerListChanged() + if hasStatusOrRelayedChange(oldState, receivedState.ConnStatus, oldIsRelayed, receivedState.Relayed) { + d.notifyPeerStateChangeListeners(receivedState.PubKey) + } return nil } @@ -470,7 +472,8 @@ func (d *Status) UpdatePeerICEStateToDisconnected(receivedState State) error { return errors.New("peer doesn't exist") } - skipNotification := shouldSkipNotify(receivedState.ConnStatus, peerState) + oldState := peerState.ConnStatus + oldIsRelayed := peerState.Relayed peerState.ConnStatus = receivedState.ConnStatus peerState.Relayed = receivedState.Relayed @@ -482,12 +485,13 @@ func (d *Status) UpdatePeerICEStateToDisconnected(receivedState State) error { d.peers[receivedState.PubKey] = peerState - if skipNotification { - return nil + if hasConnStatusChanged(oldState, receivedState.ConnStatus) { + d.notifyPeerListChanged() } - d.notifyPeerStateChangeListeners(receivedState.PubKey) - d.notifyPeerListChanged() + if hasStatusOrRelayedChange(oldState, receivedState.ConnStatus, oldIsRelayed, receivedState.Relayed) { + d.notifyPeerStateChangeListeners(receivedState.PubKey) + } return nil } @@ -510,17 +514,12 @@ func (d *Status) UpdateWireGuardPeerState(pubKey string, wgStats configurer.WGSt return nil } -func shouldSkipNotify(receivedConnStatus ConnStatus, curr State) bool { - switch { - case receivedConnStatus == StatusConnecting: - return true - case receivedConnStatus == StatusIdle && curr.ConnStatus == StatusConnecting: - return true - case receivedConnStatus == StatusIdle && curr.ConnStatus == StatusIdle: - return curr.IP != "" - default: - return false - } +func hasStatusOrRelayedChange(oldConnStatus, newConnStatus ConnStatus, oldRelayed, newRelayed bool) bool { + return oldRelayed != newRelayed || hasConnStatusChanged(newConnStatus, oldConnStatus) +} + +func hasConnStatusChanged(oldStatus, newStatus ConnStatus) bool { + return newStatus != oldStatus } // UpdatePeerFQDN update peer's state fqdn only diff --git a/client/internal/peer/status_test.go b/client/internal/peer/status_test.go index bdf8f087a..8f28a9862 100644 --- a/client/internal/peer/status_test.go +++ b/client/internal/peer/status_test.go @@ -4,6 +4,7 @@ import ( "errors" "sync" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -42,16 +43,16 @@ func TestGetPeer(t *testing.T) { func TestUpdatePeerState(t *testing.T) { key := "abc" ip := "10.10.10.10" + fqdn := "peer-a.netbird.local" status := NewRecorder("https://mgm") + _ = status.AddPeer(key, fqdn, ip) + peerState := State{ - PubKey: key, - Mux: new(sync.RWMutex), + PubKey: key, + ConnStatusUpdate: time.Now(), + ConnStatus: StatusConnecting, } - status.peers[key] = peerState - - peerState.IP = ip - err := status.UpdatePeerState(peerState) assert.NoError(t, err, "shouldn't return error") @@ -83,17 +84,17 @@ func TestGetPeerStateChangeNotifierLogic(t *testing.T) { key := "abc" ip := "10.10.10.10" status := NewRecorder("https://mgm") - peerState := State{ - PubKey: key, - Mux: new(sync.RWMutex), - } - - status.peers[key] = peerState + _ = status.AddPeer(key, "abc.netbird", ip) ch := status.GetPeerStateChangeNotifier(key) assert.NotNil(t, ch, "channel shouldn't be nil") - peerState.IP = ip + peerState := State{ + PubKey: key, + ConnStatus: StatusConnecting, + Relayed: false, + ConnStatusUpdate: time.Now(), + } err := status.UpdatePeerRelayedStateToDisconnected(peerState) assert.NoError(t, err, "shouldn't return error") diff --git a/client/internal/routemanager/client.go b/client/internal/routemanager/client.go index 847949a53..137e00d31 100644 --- a/client/internal/routemanager/client.go +++ b/client/internal/routemanager/client.go @@ -232,7 +232,7 @@ func (c *clientNetwork) watchPeerStatusChanges(ctx context.Context, peerKey stri return case <-c.statusRecorder.GetPeerStateChangeNotifier(peerKey): state, err := c.statusRecorder.GetPeer(peerKey) - if err != nil || state.ConnStatus == peer.StatusConnecting { + if err != nil { continue } peerStateUpdate <- struct{}{}