control/controlclient: replace a status func with Observer interface

For now the method has only one interface (the same as the func it's
replacing) but it will grow, eventually with the goal to remove the
controlclient.Status type for most purposes.

Updates #1909

Change-Id: I715c8bf95e3f5943055a94e76af98d988558a2f2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

control/controlclient/auto.go

@@ -138,14 +138,14 @@ type updateGen int64
 // Auto connects to a tailcontrol server for a node.
 // It's a concrete implementation of the Client interface.
 type Auto struct {
-	direct     *Direct // our interface to the server APIs
+	direct   *Direct // our interface to the server APIs
-	clock      tstime.Clock
+	clock    tstime.Clock
-	logf       logger.Logf
+	logf     logger.Logf
-	expiry     *time.Time
+	expiry   *time.Time
-	closed     bool
+	closed   bool
-	updateCh   chan struct{} // readable when we should inform the server of a change
+	updateCh chan struct{} // readable when we should inform the server of a change
-	newMapCh   chan struct{} // readable when we must restart a map request
+	newMapCh chan struct{} // readable when we must restart a map request
-	statusFunc func(Status)  // called to update Client status; always non-nil
+	observer Observer      // called to update Client status; always non-nil
 
 	unregisterHealthWatch func()
 
@@ -194,8 +194,8 @@ func NewNoStart(opts Options) (_ *Auto, err error) {
 		}
 	}()
 
-	if opts.Status == nil {
+	if opts.Observer == nil {
-		return nil, errors.New("missing required Options.Status")
+		return nil, errors.New("missing required Options.Observer")
 	}
 	if opts.Logf == nil {
 		opts.Logf = func(fmt string, args ...any) {}
@@ -213,7 +213,7 @@ func NewNoStart(opts Options) (_ *Auto, err error) {
 		authDone:   make(chan struct{}),
 		mapDone:    make(chan struct{}),
 		updateDone: make(chan struct{}),
-		statusFunc: opts.Status,
+		observer:   opts.Observer,
 	}
 	c.authCtx, c.authCancel = context.WithCancel(context.Background())
 	c.authCtx = sockstats.WithSockStats(c.authCtx, sockstats.LabelControlClientAuto, opts.Logf)
@@ -669,7 +669,7 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM
 		State:          state,
 		Err:            err,
 	}
-	c.statusFunc(new)
+	c.observer.SetControlClientStatus(new)
 
 	c.mu.Lock()
 	c.inSendStatus--

control/controlclient/direct.go

@@ -101,6 +101,12 @@ type Direct struct {
 	lastPingURL  string // last PingRequest.URL received, for dup suppression
 }
 
+// Observer is implemented by users of the control client (such as LocalBackend)
+// to get notified of changes in the control client's status.
+type Observer interface {
+	SetControlClientStatus(Status)
+}
+
 type Options struct {
 	Persist              persist.Persist                    // initial persistent data
 	GetMachinePrivateKey func() (key.MachinePrivate, error) // returns the machine key to use
@@ -122,8 +128,9 @@ type Options struct {
 	Dialer               *tsdial.Dialer               // non-nil
 	C2NHandler           http.Handler                 // or nil
 
-	// Status is called when there's a change in status.
+	// Observer is called when there's a change in status to report
-	Status func(Status)
+	// from the control client.
+	Observer Observer
 
 	// SkipIPForwardingCheck declares that the host's IP
 	// forwarding works and should not be double-checked by the

ipn/ipnlocal/local.go

@@ -888,9 +888,9 @@ func (b *LocalBackend) SetDecompressor(fn func() (controlclient.Decompressor, er
 	b.newDecompressor = fn
 }
 
-// setClientStatus is the callback invoked by the control client whenever it posts a new status.
+// SetControlClientStatus is the callback invoked by the control client whenever it posts a new status.
 // Among other things, this is where we update the netmap, packet filters, DNS and DERP maps.
-func (b *LocalBackend) setClientStatus(st controlclient.Status) {
+func (b *LocalBackend) SetControlClientStatus(st controlclient.Status) {
 	// The following do not depend on any data for which we need to lock b.
 	if st.Err != nil {
 		// TODO(crawshaw): display in the UI.
@@ -947,7 +947,7 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 				// Call ourselves with the current status again; the logic in
 				// setClientStatus will take care of updating the expired field
 				// of peers in the netmap.
-				b.setClientStatus(st)
+				b.SetControlClientStatus(st)
 			})
 		}
 	}
@@ -1457,7 +1457,7 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 		OnClientVersion:      b.onClientVersion,
 		OnControlTime:        b.em.onControlTime,
 		Dialer:               b.Dialer(),
-		Status:               b.setClientStatus,
+		Observer:             b,
 		C2NHandler:           http.HandlerFunc(b.handleC2N),
 		DialPlan:             &b.dialPlan, // pointer because it can't be copied
 

ipn/ipnlocal/network-lock_test.go

@@ -29,6 +29,12 @@ import (
 	"tailscale.com/util/must"
 )
 
+type observerFunc func(controlclient.Status)
+
+func (f observerFunc) SetControlClientStatus(s controlclient.Status) {
+	f(s)
+}
+
 func fakeControlClient(t *testing.T, c *http.Client) *controlclient.Auto {
 	hi := hostinfo.New()
 	ni := tailcfg.NetInfo{LinkType: "wired"}
@@ -43,7 +49,7 @@ func fakeControlClient(t *testing.T, c *http.Client) *controlclient.Auto {
 		},
 		HTTPTestClient:  c,
 		NoiseTestClient: c,
-		Status:          func(controlclient.Status) {},
+		Observer:        observerFunc(func(controlclient.Status) {}),
 	}
 
 	cc, err := controlclient.NewNoStart(opts)

ipn/ipnlocal/state_test.go

@@ -162,7 +162,7 @@ func (cc *mockControl) send(err error, url string, loginFinished bool, nm *netma
 		cc.authBlocked = false
 		cc.mu.Unlock()
 	}
-	if cc.opts.Status != nil {
+	if cc.opts.Observer != nil {
 		pv := cc.persist.View()
 		s := controlclient.Status{
 			URL:     url,
@@ -175,7 +175,7 @@ func (cc *mockControl) send(err error, url string, loginFinished bool, nm *netma
 		} else if url == "" && err == nil && nm == nil {
 			s.LogoutFinished = &empty.Message{}
 		}
-		cc.opts.Status(s)
+		cc.opts.Observer.SetControlClientStatus(s)
 	}
 }