release-23.2.12-rc: server, util: fix failing TestServerController

pkg/server/server.go

@@ -1108,7 +1108,9 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf
 		node.storeCfg.KVFlowHandles,
 		node.storeCfg.KVFlowController,
 	)
-	cfg.CidrLookup.Start(ctx, stopper)
+	if err = cfg.CidrLookup.Start(ctx, stopper); err != nil {
+		return nil, err
+	}
 
 	// Instantiate the SQL server proper.
 	sqlServer, err := newSQLServer(ctx, sqlServerArgs{

pkg/server/server_controller_channel_orchestrator.go

@@ -182,7 +182,7 @@ func (o *channelOrchestrator) startControlledServer(
 	// the server. Suggested use is for logging. To synchronize on the
 	// server's state, use the resulting serverState instead.
 	startErrorFn func(ctx context.Context, tenantName roachpb.TenantName, err error),
-	// serverStartedFn is called when the server has started
+	// startCompleteFn is called when the server has started
 	// successfully and is accepting clients. Suggested use is for
 	// logging. To synchronize on the server's state, use the
 	// resulting serverState instead.

pkg/server/tenant.go

@@ -413,7 +413,9 @@ func newTenantServer(
 	// tenant separation model. For a small number of tenants this is OK, but if
 	// we have a large number of tenants in shared process mode this could be a
 	// problem from a memory and network perspective.
-	baseCfg.CidrLookup.Start(ctx, stopper)
+	if err = baseCfg.CidrLookup.Start(ctx, stopper); err != nil {
+		return nil, err
+	}
 
 	// Instantiate the SQL server proper.
 	sqlServer, err := newSQLServer(ctx, args)

pkg/util/cidr/cidr.go

@@ -113,7 +113,7 @@ func NewTestLookup() *Lookup {
 }
 
 // Start refreshes the lookup once and begins the CIDR lookup refresh task.
-func (c *Lookup) Start(ctx context.Context, stopper *stop.Stopper) (bool, *Lookup) {
+func (c *Lookup) Start(ctx context.Context, stopper *stop.Stopper) error {
 	getTickDuration := func() time.Duration {
 		tickDuration := cidrRefreshInterval.Get(c.st)
 		// If the tickDuration is 0, set to a year to avoid auto refreshing.
@@ -139,9 +139,10 @@ func (c *Lookup) Start(ctx context.Context, stopper *stop.Stopper) (bool, *Looku
 			}
 		}
 	}); err != nil {
-		log.Fatalf(ctx, "unable to start CIDR lookup refresh task: %v", err)
+		log.Errorf(ctx, "unable to start CIDR lookup refresh task: %v", err)
+		return err
 	}
-	return false, nil
+	return nil
 }
 
 // hexString returns a hex string representation of an IP address. The length of

pkg/util/cidr/cidr_test.go

@@ -106,7 +106,7 @@ func TestInvalidCIDR(t *testing.T) {
 func TestEmptyLookup(t *testing.T) {
 	settings := cluster.MakeClusterSettings()
 	c := NewLookup(&settings.SV)
-	c.Start(context.Background(), stop.NewStopper())
+	require.NoError(t, c.Start(context.Background(), stop.NewStopper()))
 	c.LookupIP(net.ParseIP("127.0.0.1"))
 }
 
@@ -144,7 +144,7 @@ func TestRefresh(t *testing.T) {
 
 	st := cluster.MakeClusterSettings()
 	c := NewLookup(&st.SV)
-	c.Start(context.Background(), stopper)
+	require.NoError(t, c.Start(context.Background(), stopper))
 	// We haven't set the URL yet, so it should return an empty string.
 	require.Equal(t, "", c.LookupIP(net.ParseIP("127.0.0.1")))