From 3c1db173614797dc4be0d36cb1dfe168358983c7 Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Mon, 2 Feb 2026 23:57:59 +0000 Subject: [PATCH] fix: use existing transaction to claim prebuild (#21862) - Claiming a prebuild was happening outside a transaction --- coderd/prebuilds/api.go | 1 + coderd/prebuilds/noop.go | 2 +- coderd/workspaces.go | 2 +- enterprise/cli/create_test.go | 4 ++-- enterprise/coderd/coderd.go | 2 +- enterprise/coderd/prebuilds/claim.go | 17 +++++++---------- enterprise/coderd/prebuilds/claim_test.go | 2 +- enterprise/coderd/workspaces_test.go | 12 ++++++------ 8 files changed, 20 insertions(+), 22 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index cf29e29535..d4032aadfc 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -65,6 +65,7 @@ type StateSnapshotter interface { type Claimer interface { Claim( ctx context.Context, + store database.Store, now time.Time, userID uuid.UUID, name string, diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index 0859d428b4..1dda74c1dd 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -34,7 +34,7 @@ var DefaultReconciler ReconciliationOrchestrator = NoopReconciler{} type NoopClaimer struct{} -func (NoopClaimer) Claim(context.Context, time.Time, uuid.UUID, string, uuid.UUID, sql.NullString, sql.NullTime, sql.NullInt64) (*uuid.UUID, error) { +func (NoopClaimer) Claim(context.Context, database.Store, time.Time, uuid.UUID, string, uuid.UUID, sql.NullString, sql.NullTime, sql.NullInt64) (*uuid.UUID, error) { // Not entitled to claim prebuilds in AGPL version. return nil, ErrAGPLDoesNotSupportPrebuiltWorkspaces } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 2afbfcbff2..3adbb6b0e8 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -959,7 +959,7 @@ func claimPrebuild( nextStartAt sql.NullTime, ttl sql.NullInt64, ) (*database.Workspace, error) { - claimedID, err := claimer.Claim(ctx, now, owner.ID, name, templateVersionPresetID, autostartSchedule, nextStartAt, ttl) + claimedID, err := claimer.Claim(ctx, db, now, owner.ID, name, templateVersionPresetID, autostartSchedule, nextStartAt, ttl) if err != nil { // TODO: enhance this by clarifying whether this *specific* prebuild failed or whether there are none to claim. return nil, xerrors.Errorf("claim prebuild: %w", err) diff --git a/enterprise/cli/create_test.go b/enterprise/cli/create_test.go index be841dc8ae..f14e901e45 100644 --- a/enterprise/cli/create_test.go +++ b/enterprise/cli/create_test.go @@ -371,7 +371,7 @@ func TestEnterpriseCreateWithPreset(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) // Given: a template and a template version where the preset defines values for all required parameters, @@ -484,7 +484,7 @@ func TestEnterpriseCreateWithPreset(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) // Given: a template and a template version where the preset defines values for all required parameters, diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index e2b2134ef8..f6e386a76e 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -1332,5 +1332,5 @@ func (api *API) setupPrebuilds(featureEnabled bool) (agplprebuilds.Reconciliatio api.TracerProvider, int(api.DeploymentValues.PostgresConnMaxOpen.Value()), ) - return reconciler, prebuilds.NewEnterpriseClaimer(api.Database) + return reconciler, prebuilds.NewEnterpriseClaimer() } diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index 743513cedb..e057fb03d6 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -13,18 +13,15 @@ import ( "github.com/coder/coder/v2/coderd/prebuilds" ) -type EnterpriseClaimer struct { - store database.Store +type EnterpriseClaimer struct{} + +func NewEnterpriseClaimer() *EnterpriseClaimer { + return &EnterpriseClaimer{} } -func NewEnterpriseClaimer(store database.Store) *EnterpriseClaimer { - return &EnterpriseClaimer{ - store: store, - } -} - -func (c EnterpriseClaimer) Claim( +func (EnterpriseClaimer) Claim( ctx context.Context, + store database.Store, now time.Time, userID uuid.UUID, name string, @@ -33,7 +30,7 @@ func (c EnterpriseClaimer) Claim( nextStartAt sql.NullTime, ttl sql.NullInt64, ) (*uuid.UUID, error) { - result, err := c.store.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{ + result, err := store.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{ NewUserID: userID, NewName: name, Now: now, diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 5657072f12..aa711f14e0 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -175,7 +175,7 @@ func TestClaimPrebuild(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(spy) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) version := coderdtest.CreateTemplateVersion(t, client, orgID, templateWithAgentAndPresetsWithPrebuilds(desiredInstances)) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 2ab511470f..59b390fb1b 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -1992,7 +1992,7 @@ func TestPrebuildsAutobuild(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) // Setup user, template and template version with a preset with 1 prebuild instance @@ -2116,7 +2116,7 @@ func TestPrebuildsAutobuild(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) // Setup user, template and template version with a preset with 1 prebuild instance @@ -2240,7 +2240,7 @@ func TestPrebuildsAutobuild(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) // Setup user, template and template version with a preset with 1 prebuild instance @@ -2386,7 +2386,7 @@ func TestPrebuildsAutobuild(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) // Setup user, template and template version with a preset with 1 prebuild instance @@ -2533,7 +2533,7 @@ func TestPrebuildsAutobuild(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) // Setup user, template and template version with a preset with 1 prebuild instance @@ -2980,7 +2980,7 @@ func TestWorkspaceProvisionerdServerMetrics(t *testing.T) { noop.NewTracerProvider(), 10, ) - var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db) + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer() api.AGPL.PrebuildsClaimer.Store(&claimer) organizationName, err := client.Organization(ctx, owner.OrganizationID)