control/controlclient: fix canSkipStatus online conditions

concurrent netmaps that if the first is logged in, it is never skipped.
This should have been covered be the skip test case, but that case
wasn't updated to include level set state.

Updates #12639
Updates #17869

Signed-off-by: James Tucker <james@tailscale.com>
main
James Tucker 2 months ago committed by James Tucker
parent 0c5b17c1d3
commit de4a8dbcfc
  1. 15
      control/controlclient/auto.go
  2. 5
      control/controlclient/controlclient_test.go

@ -674,17 +674,16 @@ func canSkipStatus(s1, s2 *Status) bool {
// we can't skip it. // we can't skip it.
return false return false
} }
if s1.Err != nil || s1.URL != "" || s1.LoggedIn { if s1.Err != nil || s1.URL != "" {
// If s1 has an error, a URL, or LoginFinished set, we shouldn't skip it, // If s1 has an error or an URL, we shouldn't skip it, lest the error go
// lest the error go away in s2 or in-between. We want to make sure all // away in s2 or in-between. We want to make sure all the subsystems see
// the subsystems see it. Plus there aren't many of these, so not worth // it. Plus there aren't many of these, so not worth skipping.
// skipping.
return false return false
} }
if !s1.Persist.Equals(s2.Persist) || s1.LoggedIn != s2.LoggedIn || s1.InMapPoll != s2.InMapPoll || s1.URL != s2.URL { if !s1.Persist.Equals(s2.Persist) || s1.LoggedIn != s2.LoggedIn || s1.InMapPoll != s2.InMapPoll || s1.URL != s2.URL {
// If s1 has a different Persist, LoginFinished, Synced, or URL than s2, // If s1 has a different Persist, has changed login state, changed map
// don't skip it. We only care about skipping the typical // poll state, or has a new login URL, don't skip it. We only care about
// entries where the only difference is the NetMap. // skipping the typical entries where the only difference is the NetMap.
return false return false
} }
// If nothing above precludes it, and both s1 and s2 have NetMaps, then // If nothing above precludes it, and both s1 and s2 have NetMaps, then

@ -98,6 +98,7 @@ func TestCanSkipStatus(t *testing.T) {
nm1 := &netmap.NetworkMap{} nm1 := &netmap.NetworkMap{}
nm2 := &netmap.NetworkMap{} nm2 := &netmap.NetworkMap{}
commonPersist := new(persist.Persist).View()
tests := []struct { tests := []struct {
name string name string
s1, s2 *Status s1, s2 *Status
@ -165,8 +166,8 @@ func TestCanSkipStatus(t *testing.T) {
}, },
{ {
name: "skip", name: "skip",
s1: &Status{NetMap: nm1}, s1: &Status{NetMap: nm1, LoggedIn: true, InMapPoll: true, Persist: commonPersist},
s2: &Status{NetMap: nm2}, s2: &Status{NetMap: nm2, LoggedIn: true, InMapPoll: true, Persist: commonPersist},
want: true, want: true,
}, },
} }

Loading…
Cancel
Save