control/controlclient: remove some public API, move to Options & test-only

Includes adding StartPaused, which will be used in a future change to
enable netmap caching testing.

Updates #12639

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

control/controlclient/auto.go

@@ -23,6 +23,7 @@ import (
 	"tailscale.com/util/backoff"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/execqueue"
+	"tailscale.com/util/testenv"
 )
 
 type LoginGoal struct {
@@ -123,6 +124,7 @@ type Auto struct {
 
 	mu sync.Mutex // mutex guards the following fields
 
+	started      bool   // whether [Auto.Start] has been called
 	wantLoggedIn bool   // whether the user wants to be logged in per last method call
 	urlToVisit   string // the last url we were told to visit
 	expiry       time.Time
@@ -150,15 +152,21 @@ type Auto struct {
 
 // New creates and starts a new Auto.
 func New(opts Options) (*Auto, error) {
-	c, err := NewNoStart(opts)
+	c, err := newNoStart(opts)
-	if c != nil {
+	if err != nil {
-		c.Start()
+		return nil, err
+	}
+	if opts.StartPaused {
+		c.SetPaused(true)
+	}
+	if !opts.SkipStartForTests {
+		c.start()
 	}
 	return c, err
 }
 
-// NewNoStart creates a new Auto, but without calling Start on it.
+// newNoStart creates a new Auto, but without calling Start on it.
-func NewNoStart(opts Options) (_ *Auto, err error) {
+func newNoStart(opts Options) (_ *Auto, err error) {
 	direct, err := NewDirect(opts)
 	if err != nil {
 		return nil, err
@@ -218,10 +226,21 @@ func (c *Auto) SetPaused(paused bool) {
 	c.unpauseWaiters = nil
 }
 
-// Start starts the client's goroutines.
+// StartForTest starts the client's goroutines.
 //
-// It should only be called for clients created by NewNoStart.
+// It should only be called for clients created with [Options.SkipStartForTests].
-func (c *Auto) Start() {
+func (c *Auto) StartForTest() {
+	testenv.AssertInTest()
+	c.start()
+}
+
+func (c *Auto) start() {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if c.started {
+		return
+	}
+	c.started = true
 	go c.authRoutine()
 	go c.mapRoutine()
 	go c.updateRoutine()

control/controlclient/direct.go

@@ -146,6 +146,14 @@ type Options struct {
 	ControlKnobs         *controlknobs.Knobs // or nil to ignore
 	Bus                  *eventbus.Bus       // non-nil, for setting up publishers
 
+	SkipStartForTests bool // if true, don't call [Auto.Start] to avoid any background goroutines (for tests only)
+
+	// StartPaused indicates whether the client should start in a paused state
+	// where it doesn't do network requests. This primarily exists for testing
+	// but not necessarily "go test" tests, so it isn't restricted to only
+	// being used in tests.
+	StartPaused bool
+
 	// Observer is called when there's a change in status to report
 	// from the control client.
 	// If nil, no status updates are reported.

ipn/ipnlocal/network-lock_test.go

@@ -60,9 +60,11 @@ func fakeControlClient(t *testing.T, c *http.Client) (*controlclient.Auto, *even
 		NoiseTestClient: c,
 		Dialer:          dialer,
 		Bus:             bus,
+
+		SkipStartForTests: true,
 	}
 
-	cc, err := controlclient.NewNoStart(opts)
+	cc, err := controlclient.New(opts)
 	if err != nil {
 		t.Fatal(err)
 	}