fix: prevent panic from duplicate metrics registration on license upload (#21832)

This commit is contained in:
Dean Sheather
2026-02-02 01:57:06 -08:00
committed by GitHub
parent edf97ce24a
commit 6954b73f8a
2 changed files with 48 additions and 1 deletions
+3 -1
View File
@@ -946,13 +946,15 @@ func (api *API) updateEntitlements(ctx context.Context) error {
} }
if initial, changed, enabled := featureChanged(codersdk.FeatureWorkspacePrebuilds); shouldUpdate(initial, changed, enabled) { 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 { 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")) stopCtx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, xerrors.New("gave up waiting for reconciler to stop"))
defer giveUp() defer giveUp()
(*current).Stop(stopCtx, xerrors.New("entitlements change")) (*current).Stop(stopCtx, xerrors.New("entitlements change"))
} }
reconciler, claimer := api.setupPrebuilds(enabled)
api.AGPL.PrebuildsReconciler.Store(&reconciler) api.AGPL.PrebuildsReconciler.Store(&reconciler)
// TODO: Should this context be the api.ctx context? To cancel when // TODO: Should this context be the api.ctx context? To cancel when
// the API (and entire app) is closed via shutdown? // the API (and entire app) is closed via shutdown?
+45
View File
@@ -115,6 +115,51 @@ func TestEntitlements(t *testing.T) {
assert.Nil(t, al.Actual) assert.Nil(t, al.Actual)
assert.Empty(t, res.Warnings) 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.Run("FullLicenseToNone", func(t *testing.T) {
t.Parallel() t.Parallel()
adminClient, adminUser := coderdenttest.New(t, &coderdenttest.Options{ adminClient, adminUser := coderdenttest.New(t, &coderdenttest.Options{