more fixes!

cmd/nebula-service/service.go

@@ -45,13 +45,14 @@ func (p *program) Start(s service.Service) error {
 		return err
 	}
 
-	if err := p.control.Start(); err != nil {
+	wait, err := p.control.Start()
+	if err != nil {
 		return err
 	}
 
 	// Nebula can stop itself on a fatal packet reader error, make sure to log it if it happens.
 	go func() {
-		if err := p.control.Wait(); err != nil {
+		if err := wait(); err != nil {
 			logger.Error(fmt.Sprintf("Nebula stopped due to fatal error: %v", err))
 			os.Exit(2)
 		}

cmd/nebula/main.go

@@ -84,7 +84,8 @@ func main() {
 	}
 
 	if !*configTest {
-		if err := ctrl.Start(); err != nil {
+		wait, err := ctrl.Start()
+		if err != nil {
 			util.LogWithContextIfNeeded("Error while running", err, l)
 			os.Exit(1)
 		}
@@ -92,7 +93,7 @@ func main() {
 		go ctrl.ShutdownBlock()
 		notifyReady(l)
 
-		if err := ctrl.Wait(); err != nil {
+		if err := wait(); err != nil {
 			l.Error("Nebula stopped due to fatal error", "error", err)
 			os.Exit(2)
 		}

control.go

@@ -69,19 +69,21 @@ type ControlHostInfo struct {
 }
 
 // Start actually runs nebula, this is a nonblocking call.
-// Use Wait to block until nebula has fully stopped and to learn whether a fatal reader error caused the shutdown.
-func (c *Control) Start() error {
+// The returned function blocks until nebula has fully stopped and reports the
+// fatal reader error (nil on a clean shutdown). Calling it is equivalent to
+// calling Wait.
+func (c *Control) Start() (func() error, error) {
 	c.stateLock.Lock()
 	defer c.stateLock.Unlock()
 	switch c.state {
 	case StateReady:
 		//yay!
 	case StateStopped, StateStopping:
-		return ErrAlreadyStopped
+		return nil, ErrAlreadyStopped
 	case StateStarted:
-		return ErrAlreadyStarted
+		return nil, ErrAlreadyStarted
 	default:
-		return ErrUnknownState
+		return nil, ErrUnknownState
 	}
 
 	// Activate the interface
@@ -91,7 +93,7 @@ func (c *Control) Start() error {
 		c.cancel()
 		_ = c.f.Close()
 		c.state = StateStopped
-		return err
+		return nil, err
 	}
 
 	// Call all the delayed funcs that waited patiently for the interface to be created.
@@ -116,7 +118,7 @@ func (c *Control) Start() error {
 	// Start reading packets.
 	c.f.run()
 	c.state = StateStarted
-	return nil
+	return c.Wait, nil
 }
 
 func (c *Control) State() RunState {

control_lifecycle_test.go

@@ -114,7 +114,8 @@ func TestControl_StopBeforeStart(t *testing.T) {
 	require.NoError(t, c.Wait())
 
 	// A stopped control can never be started
-	require.ErrorIs(t, c.Start(), ErrAlreadyStopped)
+	_, err := c.Start()
+	require.ErrorIs(t, err, ErrAlreadyStopped)
 
 	// A second Stop is a harmless no-op
 	c.Stop()
@@ -190,7 +191,8 @@ func TestControl_StartMultiqueueFailureReleases(t *testing.T) {
 	}
 
 	// The second reader fails to open, everything must be released
-	require.Error(t, c.Start())
+	_, err := c.Start()
+	require.Error(t, err)
 	assert.Equal(t, StateStopped, c.State())
 	assert.True(t, dev.closed, "the tun device should have been closed")
 	assert.True(t, conn.closed, "the udp socket should have been closed")
@@ -247,7 +249,7 @@ func TestControl_ConcurrentStopAndStart(t *testing.T) {
 	for i := 0; i < 2; i++ {
 		wg.Go(func() { c.Stop() })
 	}
-	wg.Go(func() { _ = c.Start() })
+	wg.Go(func() { _, _ = c.Start() })
 	wg.Go(func() {
 		_ = c.Wait()
 		// A returned Wait must always observe the final state, no matter how
@@ -260,15 +262,19 @@ func TestControl_ConcurrentStopAndStart(t *testing.T) {
 	// panic and Wait must observe the final state
 	require.NoError(t, c.Wait())
 	assert.Equal(t, StateStopped, c.State())
-	require.ErrorIs(t, c.Start(), ErrAlreadyStopped)
+	_, err := c.Start()
+	require.ErrorIs(t, err, ErrAlreadyStopped)
 }
 
 func TestControl_StartStopLifecycle(t *testing.T) {
 	c, dev, conn := newReadyControl(t)
 
-	require.NoError(t, c.Start())
+	wait, err := c.Start()
+	require.NoError(t, err)
+	require.NotNil(t, wait, "a successful Start must return a block function")
 	assert.Equal(t, StateStarted, c.State())
-	require.ErrorIs(t, c.Start(), ErrAlreadyStarted)
+	_, err = c.Start()
+	require.ErrorIs(t, err, ErrAlreadyStarted)
 
 	// Stop must unpark the reader blocked in the device and release everything
 	c.Stop()
@@ -279,7 +285,8 @@ func TestControl_StartStopLifecycle(t *testing.T) {
 
 	// The reader drained off a closed device, that is not a fatal error
 	require.NoError(t, c.Wait())
-	require.ErrorIs(t, c.Start(), ErrAlreadyStopped)
+	_, err = c.Start()
+	require.ErrorIs(t, err, ErrAlreadyStopped)
 }
 
 func TestControl_RebindIsGatedByState(t *testing.T) {
@@ -289,7 +296,9 @@ func TestControl_RebindIsGatedByState(t *testing.T) {
 	c.RebindUDPServer()
 	assert.Equal(t, 0, conn.rebinds, "rebind before start must be a no-op")
 
-	require.NoError(t, c.Start())
+	wait, err := c.Start()
+	require.NoError(t, err)
+	require.NotNil(t, wait, "a successful Start must return a block function")
 	c.RebindUDPServer()
 	assert.Equal(t, 1, conn.rebinds, "rebind while started must reach the conn")
 

service/service.go

@@ -50,7 +50,7 @@ func New(control *nebula.Control) (_ *Service, reterr error) {
 		return nil, errors.New("must be using user device")
 	}
 
-	err := control.Start()
+	wait, err := control.Start()
 	if err != nil {
 		return nil, err
 	}
@@ -155,7 +155,7 @@ func New(control *nebula.Control) (_ *Service, reterr error) {
 	// Add the nebula wait function to the group so a fatal reader error
 	// propagates out through errgroup.Wait().
 	eg.Go(func() error {
-		return control.Wait()
+		return wait()
 	})
 
 	return &s, nil