Commitf751f81

authored

fix(coderd): fix flake inTestAPI/ModifyAutostopWithRunningWorkspace (#18932)

Fixescoder/internal#521This happened due to a race condition present in how`AwaitWorkspaceBuildJobCompleted` works.`AwaitWorkspaceBuildJobCompleted` works by waiting until`/api/v2/workspacesbuilds/{workspacebuild}/` returns a workspace buildwith `.Job.CompletedAt != nil`. The issue here is that _sometimes_ thereturned `codersdk.WorkspaceBuild` can contain a build from _before_ aprovisioner job completed, but contain the provisioner job from _after_it completed.Let me demonstrate:Here we query the database for `database.WorkspaceBuild`.https://github.com/coder/coder/blob/a3f64f74f794c733126ad21cd1feb0801caf67c4/coderd/coderd.go#L1409-L1415Inside of the `workspaceBuild` route handler, we call`workspaceBuildsData`https://github.com/coder/coder/blob/a3f64f74f794c733126ad21cd1feb0801caf67c4/coderd/workspacebuilds.go#L54This then calls `GetProvisionerJobsByIDsWithQueuePosition`https://github.com/coder/coder/blob/a3f64f74f794c733126ad21cd1feb0801caf67c4/coderd/workspacebuilds.go#L852-L856As these two calls happen _outside of a transaction_, the state of theworld can change underneath. This can result in an in-progress workspacebuild having a completed provisioner job attached to it.

1 parent0d3b770 commitf751f81Copy full SHA for f751f81

File tree

-3

lines changed

-3

lines changed

Lines changed: 8 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -2875,13 +2875,18 @@ func TestWorkspaceUpdateTTL(t *testing.T) {`
`2875`	`2875`	`ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)`
`2876`	`2876`	`defercancel()`
`2877`	`2877`
`2878`		`-err:=client.UpdateWorkspaceTTL(ctx,workspace.ID, codersdk.UpdateWorkspaceTTLRequest{`
`2879`		`-TTLMillis:testCase.toTTL,`
`2880`		`-})`
	`2878`	`+// Re-fetch the workspace build. This is required because`
	`2879`	+// `AwaitWorkspaceBuildJobCompleted` can return stale data.
	`2880`	`+build,err:=client.WorkspaceBuild(ctx,build.ID)`
`2881`	`2881`	`require.NoError(t,err)`
`2882`	`2882`
`2883`	`2883`	`deadlineBefore:=build.Deadline`
`2884`	`2884`
	`2885`	`+err=client.UpdateWorkspaceTTL(ctx,workspace.ID, codersdk.UpdateWorkspaceTTLRequest{`
	`2886`	`+TTLMillis:testCase.toTTL,`
	`2887`	`+})`
	`2888`	`+require.NoError(t,err)`
	`2889`	`+`
`2885`	`2890`	`build,err=client.WorkspaceBuild(ctx,build.ID)`
`2886`	`2891`	`require.NoError(t,err)`
`2887`	`2892`

Comments

(0)