From f5858c8a18c1aaf877139602323a53fd27dc7057 Mon Sep 17 00:00:00 2001
From: Susana Ferreira <susana@coder.com>
Date: Fri, 23 Jan 2026 14:45:27 +0000
Subject: [PATCH] fix: unregister metrics on reconciler stop to prevent panic
 on restart (#21647)

## Description

Fixes a panic that occurs when the prebuilds feature is toggled by
adding/removing a license. The `StoreReconciler` was not unregistering
the `reconciliationDuration` histogram, causing a "duplicate metrics
collector registration attempted" panic when a new reconciler was
created.

## Changes

* Unregister the `reconciliationDuration` histogram in `Stop()`
alongside the existing metrics collector
* Change log level when stopping the reconciler with a cause, since
"entitlements change" is not an error condition
* Add `TestReconcilerLifecycle` to verify the reconciler can be stopped
and recreated with the same prometheus registry

Related to internal slack thread:
https://codercom.slack.com/archives/C07GRNNRW03/p1769116582171379
---
 enterprise/coderd/prebuilds/reconcile.go      | 11 +++--
 enterprise/coderd/prebuilds/reconcile_test.go | 44 +++++++++++++++++++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/enterprise/coderd/prebuilds/reconcile.go b/enterprise/coderd/prebuilds/reconcile.go
index f09e1998b5..6816ce1799 100644
--- a/enterprise/coderd/prebuilds/reconcile.go
+++ b/enterprise/coderd/prebuilds/reconcile.go
@@ -260,9 +260,9 @@ func (c *StoreReconciler) Stop(ctx context.Context, cause error) {
 	defer c.running.Store(false)
 
 	if cause != nil {
-		c.logger.Error(context.Background(), "stopping reconciler due to an error", slog.Error(cause))
+		c.logger.Info(context.Background(), "stopping reconciler", slog.F("cause", cause.Error()))
 	} else {
-		c.logger.Info(context.Background(), "gracefully stopping reconciler")
+		c.logger.Info(context.Background(), "stopping reconciler")
 	}
 
 	// If previously stopped (Swap returns previous value), then short-circuit.
@@ -272,7 +272,7 @@ func (c *StoreReconciler) Stop(ctx context.Context, cause error) {
 		return
 	}
 
-	// Unregister the metrics collector.
+	// Unregister prebuilds state and operational metrics.
 	if c.metrics != nil && c.registerer != nil {
 		if !c.registerer.Unregister(c.metrics) {
 			// The API doesn't allow us to know why the de-registration failed, but it's not very consequential.
@@ -281,6 +281,11 @@ func (c *StoreReconciler) Stop(ctx context.Context, cause error) {
 			// feature again. If the metrics cannot be registered, it'll log an error from NewStoreReconciler.
 			c.logger.Warn(context.Background(), "failed to unregister metrics collector")
 		}
+		if c.reconciliationDuration != nil {
+			if !c.registerer.Unregister(c.reconciliationDuration) {
+				c.logger.Warn(context.Background(), "failed to unregister reconciliation duration histogram")
+			}
+		}
 	}
 
 	// If the reconciler is not running, there's nothing else to do.
diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go
index e1200e6385..f896cf6b8f 100644
--- a/enterprise/coderd/prebuilds/reconcile_test.go
+++ b/enterprise/coderd/prebuilds/reconcile_test.go
@@ -1401,6 +1401,50 @@ func TestRunLoop(t *testing.T) {
 	reconciler.Stop(ctx, nil)
 }
 
+// TestReconcilerLifecycle tests that a StoreReconciler can be stopped and a new one
+// created to simulate the prebuilds feature being disabled and re-enabled.
+func TestReconcilerLifecycle(t *testing.T) {
+	t.Parallel()
+
+	ctx := testutil.Context(t, testutil.WaitLong)
+	logger := testutil.Logger(t)
+	db, ps := dbtestutil.NewDB(t)
+	cfg := codersdk.PrebuildsConfig{
+		ReconciliationInterval: serpent.Duration(testutil.WaitLong),
+	}
+	registry := prometheus.NewRegistry()
+	cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
+
+	// Given: a running reconciler (simulating the prebuilds feature being enabled)
+	reconciler := prebuilds.NewStoreReconciler(
+		db, ps, cache, cfg, logger,
+		quartz.NewMock(t),
+		registry,
+		newNoopEnqueuer(),
+		newNoopUsageCheckerPtr(),
+		noop.NewTracerProvider(),
+		10,
+	)
+
+	// When: the reconciler is stopped (simulating the prebuilds feature being disabled)
+	reconciler.Stop(ctx, xerrors.New("entitlements change"))
+
+	// Then: a new reconciler can be created without error
+	// (simulating the prebuilds feature being re-enabled)
+	reconciler = prebuilds.NewStoreReconciler(
+		db, ps, cache, cfg, logger,
+		quartz.NewMock(t),
+		registry,
+		newNoopEnqueuer(),
+		newNoopUsageCheckerPtr(),
+		noop.NewTracerProvider(),
+		10,
+	)
+
+	// Gracefully stop the reconciliation loop
+	reconciler.Stop(ctx, nil)
+}
+
 func TestFailedBuildBackoff(t *testing.T) {
 	t.Parallel()