fix(services): HealthChecker.Register immediately populates cache when running

When Register() is called on an already-started HealthChecker, the new
service's health state now immediately reflects in IsReady() / IsHealthy()
without waiting for the next 15-second polling tick.

Root cause: the job spawner starts a service and then registers it with
the HealthChecker (spawner.go). Before this fix, the service would appear
absent from health reports for up to 15 seconds even though Ready() already
returned nil, because the HealthChecker cache was only refreshed by a
background ticker.

The servicesMu lock is released before calling IfStarted to avoid a
deadlock with Start(), which holds the StateMachine lock while calling
update() (which acquires servicesMu.RLock).

Author: Fletch153

pkg/services/health.go

@@ -218,14 +218,39 @@ func (c *HealthChecker) Register(service HealthReporter) error {
 		return fmt.Errorf("misconfigured check %#v for %T", name, service)
 	}
 
-	c.servicesMu.Lock()
-	defer c.servicesMu.Unlock()
-	if testing.Testing() {
-		if orig, ok := c.services[name]; ok {
-			panic(fmt.Errorf("duplicate name %q: service names must be unique: types %T & %T", name, service, orig))
+	func() {
+		c.servicesMu.Lock()
+		defer c.servicesMu.Unlock()
+		if testing.Testing() {
+			if orig, ok := c.services[name]; ok {
+				panic(fmt.Errorf("duplicate name %q: service names must be unique: types %T & %T", name, service, orig))
+			}
 		}
-	}
-	c.services[name] = service
+		c.services[name] = service
+	}()
+
+	// If already started, immediately populate the health state for this
+	// service so callers don't wait up to <interval> for the next tick.
+	// servicesMu must be released before calling IfStarted to avoid a
+	// deadlock with Start(), which holds the StateMachine lock while
+	// calling update() (which acquires servicesMu.RLock).
+	c.IfStarted(func() {
+		ctx := context.Background()
+		ready := service.Ready()
+
+		c.stateMu.Lock()
+		defer c.stateMu.Unlock()
+		c.ready[name] = ready
+		for n, err := range service.HealthReport() {
+			c.healthy[n] = err
+			value := 0
+			if err == nil {
+				value = 1
+			}
+			c.cfg.SetStatus(ctx, name, value)
+		}
+	})
+
 	return nil
 }
 

pkg/services/health_test.go

@@ -56,3 +56,65 @@ func TestContainsError(t *testing.T) {
 		})
 	}
 }
+
+// testHealthReporter is a minimal HealthReporter for use in tests.
+type testHealthReporter struct {
+	name    string
+	ready   error
+	healthy error
+}
+
+func (r *testHealthReporter) Name() string             { return r.name }
+func (r *testHealthReporter) Ready() error             { return r.ready }
+func (r *testHealthReporter) HealthReport() map[string]error {
+	return map[string]error{r.name: r.healthy}
+}
+
+// TestHealthChecker_Register_ImmediateUpdate verifies that Register() immediately
+// populates the health state when the HealthChecker is already running, so
+// callers don't wait up to 15 s for the next polling tick.
+func TestHealthChecker_Register_ImmediateUpdate(t *testing.T) {
+	hc := HealthCheckerConfig{Ver: "test", Sha: "abc1234"}.New()
+	if err := hc.Start(); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	t.Cleanup(func() { _ = hc.Close() })
+
+	svc := &testHealthReporter{name: "svc", ready: nil, healthy: nil}
+	if err := hc.Register(svc); err != nil {
+		t.Fatalf("Register: %v", err)
+	}
+
+	// No sleep needed — Register() must have synchronously populated the state.
+	ready, readyErrs := hc.IsReady()
+	assert.True(t, ready, "expected ready after Register, got errors: %v", readyErrs)
+	assert.Nil(t, readyErrs["svc"], "expected svc ready, got: %v", readyErrs["svc"])
+
+	healthy, healthyErrs := hc.IsHealthy()
+	assert.True(t, healthy, "expected healthy after Register, got errors: %v", healthyErrs)
+	assert.Nil(t, healthyErrs["svc"], "expected svc healthy, got: %v", healthyErrs["svc"])
+}
+
+// TestHealthChecker_Register_ImmediateUpdate_Unhealthy verifies that an unhealthy
+// service registered on a running HealthChecker is immediately visible.
+func TestHealthChecker_Register_ImmediateUpdate_Unhealthy(t *testing.T) {
+	hc := HealthCheckerConfig{Ver: "test", Sha: "abc1234"}.New()
+	if err := hc.Start(); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	t.Cleanup(func() { _ = hc.Close() })
+
+	notReady := errors.New("service is starting")
+	svc := &testHealthReporter{name: "svc2", ready: notReady, healthy: notReady}
+	if err := hc.Register(svc); err != nil {
+		t.Fatalf("Register: %v", err)
+	}
+
+	ready, readyErrs := hc.IsReady()
+	assert.False(t, ready, "expected not ready")
+	assert.Equal(t, notReady, readyErrs["svc2"])
+
+	healthy, healthyErrs := hc.IsHealthy()
+	assert.False(t, healthy, "expected not healthy")
+	assert.Equal(t, notReady, healthyErrs["svc2"])
+}