From 6954b73f8a5b11c190e06f860ae2eebde36f108a Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Mon, 2 Feb 2026 01:57:06 -0800 Subject: [PATCH] fix: prevent panic from duplicate metrics registration on license upload (#21832) --- enterprise/coderd/coderd.go | 4 ++- enterprise/coderd/coderd_test.go | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 83c2402451..e2b2134ef8 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -946,13 +946,15 @@ func (api *API) updateEntitlements(ctx context.Context) error { } if initial, changed, enabled := featureChanged(codersdk.FeatureWorkspacePrebuilds); shouldUpdate(initial, changed, enabled) { - reconciler, claimer := api.setupPrebuilds(enabled) + // Stop the old reconciler first to unregister its metrics before + // creating a new one. This prevents duplicate metric registration panics. if current := api.AGPL.PrebuildsReconciler.Load(); current != nil { stopCtx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, xerrors.New("gave up waiting for reconciler to stop")) defer giveUp() (*current).Stop(stopCtx, xerrors.New("entitlements change")) } + reconciler, claimer := api.setupPrebuilds(enabled) api.AGPL.PrebuildsReconciler.Store(&reconciler) // TODO: Should this context be the api.ctx context? To cancel when // the API (and entire app) is closed via shutdown? diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 3ee0ae60ae..fe4306e2b8 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -115,6 +115,51 @@ func TestEntitlements(t *testing.T) { assert.Nil(t, al.Actual) assert.Empty(t, res.Warnings) }) + + // TestEntitlements/MultiplePrebuildsLicenseUpdates verifies that uploading + // multiple licenses with prebuilds enabled doesn't cause a panic from + // duplicate Prometheus metric registration. This was a bug where the new + // reconciler's metrics were registered before the old reconciler was stopped. + t.Run("MultiplePrebuildsLicenseUpdates", func(t *testing.T) { + t.Parallel() + adminClient, _, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + DontAddLicense: true, + }) + + // Add first license with prebuilds to initialize the reconciler + features := license.Features{ + codersdk.FeatureUserLimit: 100, + codersdk.FeatureWorkspacePrebuilds: 1, + } + license1 := coderdenttest.AddLicense(t, adminClient, coderdenttest.LicenseOptions{ + Features: features, + }) + res, err := adminClient.Entitlements(context.Background()) + require.NoError(t, err) + require.True(t, res.HasLicense) + require.Equal(t, codersdk.EntitlementEntitled, res.Features[codersdk.FeatureWorkspacePrebuilds].Entitlement) + + // Verify the reconciler was set up + reconciler1 := api.AGPL.PrebuildsReconciler.Load() + require.NotNil(t, reconciler1) + + // Delete the license to disable prebuilds, then add a new one. + // This tests the enabled -> disabled -> enabled transition. + err = adminClient.DeleteLicense(context.Background(), license1.ID) + require.NoError(t, err) + + coderdenttest.AddLicense(t, adminClient, coderdenttest.LicenseOptions{ + Features: features, + }) + res, err = adminClient.Entitlements(context.Background()) + require.NoError(t, err) + require.True(t, res.HasLicense) + require.Equal(t, codersdk.EntitlementEntitled, res.Features[codersdk.FeatureWorkspacePrebuilds].Entitlement) + + // Verify a new reconciler was created + reconciler2 := api.AGPL.PrebuildsReconciler.Load() + require.NotNil(t, reconciler2) + }) t.Run("FullLicenseToNone", func(t *testing.T) { t.Parallel() adminClient, adminUser := coderdenttest.New(t, &coderdenttest.Options{