From 544f15523cf753fe75c61cb4920efbd1aa9164a1 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Wed, 8 Oct 2025 10:40:54 +0200 Subject: [PATCH] fix: adjust workspace claims to be initiated by users (#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. --- coderd/prebuilds/api.go | 1 - coderd/prebuilds/noop.go | 4 ---- coderd/workspaces.go | 1 - enterprise/coderd/prebuilds/claim.go | 4 ---- enterprise/coderd/prebuilds/claim_test.go | 15 +++++++++------ 5 files changed, 9 insertions(+), 16 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index 1bedeb1013..ed39f2a322 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -66,5 +66,4 @@ type Claimer interface { nextStartAt sql.NullTime, ttl sql.NullInt64, ) (*uuid.UUID, error) - Initiator() uuid.UUID } diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index ebb6d69642..170b0a12af 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -35,8 +35,4 @@ func (NoopClaimer) Claim(context.Context, time.Time, uuid.UUID, string, uuid.UUI return nil, ErrAGPLDoesNotSupportPrebuiltWorkspaces } -func (NoopClaimer) Initiator() uuid.UUID { - return uuid.Nil -} - var DefaultClaimer Claimer = NoopClaimer{} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 8f2317fc96..d67fa2ef4b 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -719,7 +719,6 @@ func createWorkspace( } else { // Prebuild found! workspaceID = claimedWorkspace.ID - initiatorID = prebuildsClaimer.Initiator() } // We have to refetch the workspace for the joined in fields. diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index daea281d38..743513cedb 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -55,8 +55,4 @@ func (c EnterpriseClaimer) Claim( return &result.ID, nil } -func (EnterpriseClaimer) Initiator() uuid.UUID { - return database.PrebuildsSystemUserID -} - var _ prebuilds.Claimer = &EnterpriseClaimer{} diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 9ed7e9ffd1..217a9ff096 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -86,10 +86,6 @@ func (m *storeSpy) ClaimPrebuiltWorkspace(ctx context.Context, arg database.Clai func TestClaimPrebuild(t *testing.T) { t.Parallel() - if !dbtestutil.WillUsePostgres() { - t.Skip("This test requires postgres") - } - const ( desiredInstances = 1 presetCount = 2 @@ -260,13 +256,15 @@ func TestClaimPrebuild(t *testing.T) { switch { case tc.claimingErr != nil && (isNoPrebuiltWorkspaces || isUnsupported): require.NoError(t, err) - build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID) - _ = build + coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID) // Then: the number of running prebuilds hasn't changed because claiming prebuild is failed and we fallback to creating new workspace. currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx) require.NoError(t, err) require.Equal(t, expectedPrebuildsCount, len(currentPrebuilds)) + // If there are no prebuilt workspaces to claim, a new workspace is created from scratch + // and the initiator is set as usual. + require.Equal(t, user.ID, userWorkspace.LatestBuild.Job.InitiatorID) return case tc.claimingErr != nil && errors.Is(tc.claimingErr, unexpectedClaimingError): @@ -278,6 +276,9 @@ func TestClaimPrebuild(t *testing.T) { currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx) require.NoError(t, err) require.Equal(t, expectedPrebuildsCount, len(currentPrebuilds)) + // If a prebuilt workspace claim fails for an unanticipated, erroneous reason, + // no workspace is created and therefore the initiator is not set. + require.Equal(t, uuid.Nil, userWorkspace.LatestBuild.Job.InitiatorID) return default: @@ -285,6 +286,8 @@ func TestClaimPrebuild(t *testing.T) { require.NoError(t, err) build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID) require.Equal(t, build.Job.Status, codersdk.ProvisionerJobSucceeded) + // Prebuild claims are initiated by the user who requested to create a workspace. + require.Equal(t, user.ID, userWorkspace.LatestBuild.Job.InitiatorID) } // at this point we know that tc.claimingErr is nil