Skip to content

Commit 3c1db17

Browse files
sreyasreya
authored
fix: use existing transaction to claim prebuild (coder#21862)
- Claiming a prebuild was happening outside a transaction
1 parent 5d24e17 commit 3c1db17

8 files changed

Lines changed: 19 additions & 21 deletions

File tree

‎coderd/prebuilds/api.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ type StateSnapshotter interface {
6565
type Claimer interface {
6666
Claim(
6767
ctx context.Context,
68+
store database.Store,
6869
now time.Time,
6970
userID uuid.UUID,
7071
name string,

‎coderd/prebuilds/noop.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ var DefaultReconciler ReconciliationOrchestrator = NoopReconciler{}
3434

3535
type NoopClaimer struct{}
3636

37-
func (NoopClaimer) Claim(context.Context, time.Time, uuid.UUID, string, uuid.UUID, sql.NullString, sql.NullTime, sql.NullInt64) (*uuid.UUID, error) {
37+
func (NoopClaimer) Claim(context.Context, database.Store, time.Time, uuid.UUID, string, uuid.UUID, sql.NullString, sql.NullTime, sql.NullInt64) (*uuid.UUID, error) {
3838
// Not entitled to claim prebuilds in AGPL version.
3939
return nil, ErrAGPLDoesNotSupportPrebuiltWorkspaces
4040
}

‎coderd/workspaces.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,7 @@ func claimPrebuild(
959959
nextStartAt sql.NullTime,
960960
ttl sql.NullInt64,
961961
) (*database.Workspace, error) {
962-
claimedID, err := claimer.Claim(ctx, now, owner.ID, name, templateVersionPresetID, autostartSchedule, nextStartAt, ttl)
962+
claimedID, err := claimer.Claim(ctx, db, now, owner.ID, name, templateVersionPresetID, autostartSchedule, nextStartAt, ttl)
963963
if err != nil {
964964
// TODO: enhance this by clarifying whether this *specific* prebuild failed or whether there are none to claim.
965965
return nil, xerrors.Errorf("claim prebuild: %w", err)

‎enterprise/cli/create_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func TestEnterpriseCreateWithPreset(t *testing.T) {
371371
noop.NewTracerProvider(),
372372
10,
373373
)
374-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
374+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
375375
api.AGPL.PrebuildsClaimer.Store(&claimer)
376376

377377
// 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) {
484484
noop.NewTracerProvider(),
485485
10,
486486
)
487-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
487+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
488488
api.AGPL.PrebuildsClaimer.Store(&claimer)
489489

490490
// Given: a template and a template version where the preset defines values for all required parameters,

‎enterprise/coderd/coderd.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1332,5 +1332,5 @@ func (api *API) setupPrebuilds(featureEnabled bool) (agplprebuilds.Reconciliatio
13321332
api.TracerProvider,
13331333
int(api.DeploymentValues.PostgresConnMaxOpen.Value()),
13341334
)
1335-
return reconciler, prebuilds.NewEnterpriseClaimer(api.Database)
1335+
return reconciler, prebuilds.NewEnterpriseClaimer()
13361336
}

‎enterprise/coderd/prebuilds/claim.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,15 @@ import (
1313
"github.com/coder/coder/v2/coderd/prebuilds"
1414
)
1515

16-
type EnterpriseClaimer struct {
17-
store database.Store
18-
}
16+
type EnterpriseClaimer struct{}
1917

20-
func NewEnterpriseClaimer(store database.Store) *EnterpriseClaimer {
21-
return &EnterpriseClaimer{
22-
store: store,
23-
}
18+
func NewEnterpriseClaimer() *EnterpriseClaimer {
19+
return &EnterpriseClaimer{}
2420
}
2521

26-
func (c EnterpriseClaimer) Claim(
22+
func (EnterpriseClaimer) Claim(
2723
ctx context.Context,
24+
store database.Store,
2825
now time.Time,
2926
userID uuid.UUID,
3027
name string,
@@ -33,7 +30,7 @@ func (c EnterpriseClaimer) Claim(
3330
nextStartAt sql.NullTime,
3431
ttl sql.NullInt64,
3532
) (*uuid.UUID, error) {
36-
result, err := c.store.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{
33+
result, err := store.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{
3734
NewUserID: userID,
3835
NewName: name,
3936
Now: now,

‎enterprise/coderd/prebuilds/claim_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func TestClaimPrebuild(t *testing.T) {
175175
noop.NewTracerProvider(),
176176
10,
177177
)
178-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(spy)
178+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
179179
api.AGPL.PrebuildsClaimer.Store(&claimer)
180180

181181
version := coderdtest.CreateTemplateVersion(t, client, orgID, templateWithAgentAndPresetsWithPrebuilds(desiredInstances))

‎enterprise/coderd/workspaces_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1992,7 +1992,7 @@ func TestPrebuildsAutobuild(t *testing.T) {
19921992
noop.NewTracerProvider(),
19931993
10,
19941994
)
1995-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
1995+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
19961996
api.AGPL.PrebuildsClaimer.Store(&claimer)
19971997

19981998
// Setup user, template and template version with a preset with 1 prebuild instance
@@ -2116,7 +2116,7 @@ func TestPrebuildsAutobuild(t *testing.T) {
21162116
noop.NewTracerProvider(),
21172117
10,
21182118
)
2119-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
2119+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
21202120
api.AGPL.PrebuildsClaimer.Store(&claimer)
21212121

21222122
// Setup user, template and template version with a preset with 1 prebuild instance
@@ -2240,7 +2240,7 @@ func TestPrebuildsAutobuild(t *testing.T) {
22402240
noop.NewTracerProvider(),
22412241
10,
22422242
)
2243-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
2243+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
22442244
api.AGPL.PrebuildsClaimer.Store(&claimer)
22452245

22462246
// Setup user, template and template version with a preset with 1 prebuild instance
@@ -2386,7 +2386,7 @@ func TestPrebuildsAutobuild(t *testing.T) {
23862386
noop.NewTracerProvider(),
23872387
10,
23882388
)
2389-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
2389+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
23902390
api.AGPL.PrebuildsClaimer.Store(&claimer)
23912391

23922392
// Setup user, template and template version with a preset with 1 prebuild instance
@@ -2533,7 +2533,7 @@ func TestPrebuildsAutobuild(t *testing.T) {
25332533
noop.NewTracerProvider(),
25342534
10,
25352535
)
2536-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
2536+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
25372537
api.AGPL.PrebuildsClaimer.Store(&claimer)
25382538

25392539
// Setup user, template and template version with a preset with 1 prebuild instance
@@ -2980,7 +2980,7 @@ func TestWorkspaceProvisionerdServerMetrics(t *testing.T) {
29802980
noop.NewTracerProvider(),
29812981
10,
29822982
)
2983-
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(db)
2983+
var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer()
29842984
api.AGPL.PrebuildsClaimer.Store(&claimer)
29852985

29862986
organizationName, err := client.Organization(ctx, owner.OrganizationID)

0 commit comments

Comments
 (0)