Skip to content

Commit

Permalink
ipn/ipnlocal: fix integration test failure in tkaSyncIfNeeded
Browse files Browse the repository at this point in the history
In main, some of the prefs handling was reworked and some of those
changes were cherry picked to 1.32. This prevents failures for the
internal integration test for TKA that was failing due to an
uninitialized prefs.Persist.

Signed-off-by: Adrian Dewhurst <adrian@tailscale.com>
  • Loading branch information
sailorfrag committed Nov 18, 2022
1 parent 39d73f9 commit 5e65717
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 7 deletions.
2 changes: 1 addition & 1 deletion ipn/ipnlocal/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
}
if st.NetMap != nil {
b.mu.Unlock() // respect locking rules for tkaSyncIfNeeded
if err := b.tkaSyncIfNeeded(st.NetMap); err != nil {
if err := b.tkaSyncIfNeeded(st.NetMap, prefs.View()); err != nil {
b.logf("[v1] TKA sync error: %v", err)
}
b.mu.Lock()
Expand Down
5 changes: 3 additions & 2 deletions ipn/ipnlocal/network-lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"time"

"tailscale.com/envknob"
"tailscale.com/ipn"
"tailscale.com/ipn/ipnstate"
"tailscale.com/tailcfg"
"tailscale.com/tka"
Expand Down Expand Up @@ -89,7 +90,7 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
//
// tkaSyncIfNeeded immediately takes b.takeSyncLock which is held throughout,
// and may take b.mu as required.
func (b *LocalBackend) tkaSyncIfNeeded(nm *netmap.NetworkMap) error {
func (b *LocalBackend) tkaSyncIfNeeded(nm *netmap.NetworkMap, prefs ipn.PrefsView) error {
if !envknob.UseWIPCode() {
// If the feature flag is not enabled, pretend we don't exist.
return nil
Expand All @@ -100,7 +101,7 @@ func (b *LocalBackend) tkaSyncIfNeeded(nm *netmap.NetworkMap) error {
b.mu.Lock() // take mu to protect access to synchronized fields.
defer b.mu.Unlock()

ourNodeKey := b.prefs.Persist().PublicNodeKey()
ourNodeKey := prefs.Persist().PublicNodeKey()

isEnabled := b.tka != nil
wantEnabled := nm.TKAEnabled
Expand Down
8 changes: 4 additions & 4 deletions ipn/ipnlocal/network-lock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestTKAEnablementFlow(t *testing.T) {
err = b.tkaSyncIfNeeded(&netmap.NetworkMap{
TKAEnabled: true,
TKAHead: a1.Head(),
})
}, b.prefs)
if err != nil {
t.Errorf("tkaSyncIfNeededLocked() failed: %v", err)
}
Expand Down Expand Up @@ -229,7 +229,7 @@ func TestTKADisablementFlow(t *testing.T) {
err = b.tkaSyncIfNeeded(&netmap.NetworkMap{
TKAEnabled: false,
TKAHead: authority.Head(),
})
}, b.prefs)
if err != nil {
t.Errorf("tkaSyncIfNeededLocked() failed: %v", err)
}
Expand All @@ -242,7 +242,7 @@ func TestTKADisablementFlow(t *testing.T) {
err = b.tkaSyncIfNeeded(&netmap.NetworkMap{
TKAEnabled: false,
TKAHead: authority.Head(),
})
}, b.prefs)
if err != nil {
t.Errorf("tkaSyncIfNeededLocked() failed: %v", err)
}
Expand Down Expand Up @@ -465,7 +465,7 @@ func TestTKASync(t *testing.T) {
err = b.tkaSyncIfNeeded(&netmap.NetworkMap{
TKAEnabled: true,
TKAHead: controlAuthority.Head(),
})
}, b.prefs)
if err != nil {
t.Errorf("tkaSyncIfNeededLocked() failed: %v", err)
}
Expand Down

0 comments on commit 5e65717

Please sign in to comment.