Skip to content

Commit ef0d740

Browse files
bradfitzbradfitz
committed
control/controlclient: remove Client.SetStatusFunc
It can't change at runtime. Make it an option. Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
1 parent 70a2797 commit ef0d740

File tree

5 files changed

+34
-37
lines changed

5 files changed

+34
-37
lines changed

‎control/controlclient/auto.go

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package controlclient
66

77
import (
88
"context"
9+
"errors"
910
"fmt"
1011
"net/http"
1112
"sync"
@@ -46,17 +47,17 @@ var _ Client = (*Auto)(nil)
4647
// Auto connects to a tailcontrol server for a node.
4748
// It's a concrete implementation of the Client interface.
4849
type Auto struct {
49-
direct *Direct // our interface to the server APIs
50-
timeNow func() time.Time
51-
logf logger.Logf
52-
expiry *time.Time
53-
closed bool
54-
newMapCh chan struct{} // readable when we must restart a map request
50+
direct *Direct // our interface to the server APIs
51+
timeNow func() time.Time
52+
logf logger.Logf
53+
expiry *time.Time
54+
closed bool
55+
newMapCh chan struct{} // readable when we must restart a map request
56+
statusFunc func(Status) // called to update Client status; always non-nil
5557

5658
unregisterHealthWatch func()
5759

58-
mu sync.Mutex // mutex guards the following fields
59-
statusFunc func(Status) // called to update Client status
60+
mu sync.Mutex // mutex guards the following fields
6061

6162
paused bool // whether we should stop making HTTP requests
6263
unpauseWaiters []chan struct{}
@@ -92,20 +93,24 @@ func NewNoStart(opts Options) (*Auto, error) {
9293
if err != nil {
9394
return nil, err
9495
}
96+
if opts.Status == nil {
97+
return nil, errors.New("missing required Options.Status")
98+
}
9599
if opts.Logf == nil {
96100
opts.Logf = func(fmt string, args ...any) {}
97101
}
98102
if opts.TimeNow == nil {
99103
opts.TimeNow = time.Now
100104
}
101105
c := &Auto{
102-
direct: direct,
103-
timeNow: opts.TimeNow,
104-
logf: opts.Logf,
105-
newMapCh: make(chan struct{}, 1),
106-
quit: make(chan struct{}),
107-
authDone: make(chan struct{}),
108-
mapDone: make(chan struct{}),
106+
direct: direct,
107+
timeNow: opts.TimeNow,
108+
logf: opts.Logf,
109+
newMapCh: make(chan struct{}, 1),
110+
quit: make(chan struct{}),
111+
authDone: make(chan struct{}),
112+
mapDone: make(chan struct{}),
113+
statusFunc: opts.Status,
109114
}
110115
c.authCtx, c.authCancel = context.WithCancel(context.Background())
111116
c.mapCtx, c.mapCancel = context.WithCancel(context.Background())
@@ -533,13 +538,6 @@ func (c *Auto) AuthCantContinue() bool {
533538
return !c.loggedIn && (c.loginGoal == nil || c.loginGoal.url != "")
534539
}
535540

536-
// SetStatusFunc sets fn as the callback to run on any status change.
537-
func (c *Auto) SetStatusFunc(fn func(Status)) {
538-
c.mu.Lock()
539-
c.statusFunc = fn
540-
c.mu.Unlock()
541-
}
542-
543541
func (c *Auto) SetHostinfo(hi *tailcfg.Hostinfo) {
544542
if hi == nil {
545543
panic("nil Hostinfo")
@@ -567,10 +565,13 @@ func (c *Auto) SetNetInfo(ni *tailcfg.NetInfo) {
567565

568566
func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkMap) {
569567
c.mu.Lock()
568+
if c.closed {
569+
c.mu.Unlock()
570+
return
571+
}
570572
state := c.state
571573
loggedIn := c.loggedIn
572574
synced := c.synced
573-
statusFunc := c.statusFunc
574575
c.inSendStatus++
575576
c.mu.Unlock()
576577

@@ -601,9 +602,7 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
601602
State: state,
602603
Err: err,
603604
}
604-
if statusFunc != nil {
605-
statusFunc(new)
606-
}
605+
c.statusFunc(new)
607606

608607
c.mu.Lock()
609608
c.inSendStatus--
@@ -684,7 +683,6 @@ func (c *Auto) Shutdown() {
684683
direct := c.direct
685684
if !closed {
686685
c.closed = true
687-
c.statusFunc = nil
688686
}
689687
c.mu.Unlock()
690688

‎control/controlclient/client.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ const (
2828
// Currently this is done through a pair of polling https requests in
2929
// the Auto client, but that might change eventually.
3030
type Client interface {
31-
// SetStatusFunc provides a callback to call when control sends us
32-
// a message.
33-
SetStatusFunc(func(Status))
3431
// Shutdown closes this session, which should not be used any further
3532
// afterwards.
3633
Shutdown()

‎control/controlclient/direct.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ type Options struct {
108108
PopBrowserURL func(url string) // optional func to open browser
109109
Dialer *tsdial.Dialer // non-nil
110110

111+
// Status is called when there's a change in status.
112+
Status func(Status)
113+
111114
// KeepSharerAndUserSplit controls whether the client
112115
// understands Node.Sharer. If false, the Sharer is mapped to the User.
113116
KeepSharerAndUserSplit bool

‎ipn/ipnlocal/local.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
10561056
Pinger: b,
10571057
PopBrowserURL: b.tellClientToBrowseToURL,
10581058
Dialer: b.Dialer(),
1059+
Status: b.setClientStatus,
10591060

10601061
// Don't warn about broken Linux IP forwarding when
10611062
// netstack is being used.
@@ -1075,7 +1076,6 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
10751076
cc.UpdateEndpoints(endpoints)
10761077
}
10771078

1078-
cc.SetStatusFunc(b.setClientStatus)
10791079
b.e.SetNetInfoCallback(b.setNetInfo)
10801080

10811081
b.mu.Lock()

‎ipn/ipnlocal/state_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,6 @@ func (cc *mockControl) logf(format string, args ...any) {
114114
cc.logfActual(format, args...)
115115
}
116116

117-
func (cc *mockControl) SetStatusFunc(fn func(controlclient.Status)) {
118-
cc.statusFunc = fn
119-
}
120-
121117
func (cc *mockControl) populateKeys() (newKeys bool) {
122118
cc.mu.Lock()
123119
defer cc.mu.Unlock()
@@ -295,12 +291,15 @@ func TestStateMachine(t *testing.T) {
295291
}
296292
t.Cleanup(e.Close)
297293

298-
cc := newMockControl(t)
299-
t.Cleanup(func() { cc.preventLog.Set(true) }) // hacky way to pacify issue 3020
300294
b, err := NewLocalBackend(logf, "logid", store, nil, e, 0)
301295
if err != nil {
302296
t.Fatalf("NewLocalBackend: %v", err)
303297
}
298+
299+
cc := newMockControl(t)
300+
cc.statusFunc = b.setClientStatus
301+
t.Cleanup(func() { cc.preventLog.Set(true) }) // hacky way to pacify issue 3020
302+
304303
b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) {
305304
cc.mu.Lock()
306305
cc.opts = opts

0 commit comments

Comments
 (0)