Skip to content

Commit 544f155

Browse files
authored
fix: adjust workspace claims to be initiated by users (coder#20179)
The prebuilds user never initiates a workspace claim autonomously. A claim can only happen when a user attempts to create a workspace. When listing prebuild provisioner jobs, it would not make sense to see jobs related to users who are creating workspaces and have gotten a prebuilt workspace. When cleaning up an overwhelmed provisioner queue, we should not delete claims as they have humans waiting for them and are not part of the thundering herd. Therefore, this PR ensures that provisioner jobs that claim workspaces are considered to be initiated by the user, not the prebuilds system.
1 parent 037e6f0 commit 544f155

File tree

5 files changed

+9
-16
lines changed

5 files changed

+9
-16
lines changed

coderd/prebuilds/api.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,4 @@ type Claimer interface {
6666
nextStartAt sql.NullTime,
6767
ttl sql.NullInt64,
6868
) (*uuid.UUID, error)
69-
Initiator() uuid.UUID
7069
}

coderd/prebuilds/noop.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,4 @@ func (NoopClaimer) Claim(context.Context, time.Time, uuid.UUID, string, uuid.UUI
3535
return nil, ErrAGPLDoesNotSupportPrebuiltWorkspaces
3636
}
3737

38-
func (NoopClaimer) Initiator() uuid.UUID {
39-
return uuid.Nil
40-
}
41-
4238
var DefaultClaimer Claimer = NoopClaimer{}

coderd/workspaces.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,6 @@ func createWorkspace(
719719
} else {
720720
// Prebuild found!
721721
workspaceID = claimedWorkspace.ID
722-
initiatorID = prebuildsClaimer.Initiator()
723722
}
724723

725724
// We have to refetch the workspace for the joined in fields.

enterprise/coderd/prebuilds/claim.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,4 @@ func (c EnterpriseClaimer) Claim(
5555
return &result.ID, nil
5656
}
5757

58-
func (EnterpriseClaimer) Initiator() uuid.UUID {
59-
return database.PrebuildsSystemUserID
60-
}
61-
6258
var _ prebuilds.Claimer = &EnterpriseClaimer{}

enterprise/coderd/prebuilds/claim_test.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,6 @@ func (m *storeSpy) ClaimPrebuiltWorkspace(ctx context.Context, arg database.Clai
8686
func TestClaimPrebuild(t *testing.T) {
8787
t.Parallel()
8888

89-
if !dbtestutil.WillUsePostgres() {
90-
t.Skip("This test requires postgres")
91-
}
92-
9389
const (
9490
desiredInstances = 1
9591
presetCount = 2
@@ -260,13 +256,15 @@ func TestClaimPrebuild(t *testing.T) {
260256
switch {
261257
case tc.claimingErr != nil && (isNoPrebuiltWorkspaces || isUnsupported):
262258
require.NoError(t, err)
263-
build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID)
264-
_ = build
259+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID)
265260

266261
// Then: the number of running prebuilds hasn't changed because claiming prebuild is failed and we fallback to creating new workspace.
267262
currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx)
268263
require.NoError(t, err)
269264
require.Equal(t, expectedPrebuildsCount, len(currentPrebuilds))
265+
// If there are no prebuilt workspaces to claim, a new workspace is created from scratch
266+
// and the initiator is set as usual.
267+
require.Equal(t, user.ID, userWorkspace.LatestBuild.Job.InitiatorID)
270268
return
271269

272270
case tc.claimingErr != nil && errors.Is(tc.claimingErr, unexpectedClaimingError):
@@ -278,13 +276,18 @@ func TestClaimPrebuild(t *testing.T) {
278276
currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx)
279277
require.NoError(t, err)
280278
require.Equal(t, expectedPrebuildsCount, len(currentPrebuilds))
279+
// If a prebuilt workspace claim fails for an unanticipated, erroneous reason,
280+
// no workspace is created and therefore the initiator is not set.
281+
require.Equal(t, uuid.Nil, userWorkspace.LatestBuild.Job.InitiatorID)
281282
return
282283

283284
default:
284285
// tc.claimingErr is nil scenario
285286
require.NoError(t, err)
286287
build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID)
287288
require.Equal(t, build.Job.Status, codersdk.ProvisionerJobSucceeded)
289+
// Prebuild claims are initiated by the user who requested to create a workspace.
290+
require.Equal(t, user.ID, userWorkspace.LatestBuild.Job.InitiatorID)
288291
}
289292

290293
// at this point we know that tc.claimingErr is nil

0 commit comments

Comments
 (0)