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()