From f751f81052f73c059c0bcd8488a7df8431c18658 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 21 Jul 2025 13:04:28 +0100 Subject: [PATCH] fix(coderd): fix flake in `TestAPI/ModifyAutostopWithRunningWorkspace` (#18932) Fixes https://github.com/coder/internal/issues/521 This happened due to a race condition present in how `AwaitWorkspaceBuildJobCompleted` works. `AwaitWorkspaceBuildJobCompleted` works by waiting until `/api/v2/workspacesbuilds/{workspacebuild}/` returns a workspace build with `.Job.CompletedAt != nil`. The issue here is that _sometimes_ the returned `codersdk.WorkspaceBuild` can contain a build from _before_ a provisioner 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-L1415 Inside of the `workspaceBuild` route handler, we call `workspaceBuildsData` https://github.com/coder/coder/blob/a3f64f74f794c733126ad21cd1feb0801caf67c4/coderd/workspacebuilds.go#L54 This then calls `GetProvisionerJobsByIDsWithQueuePosition` https://github.com/coder/coder/blob/a3f64f74f794c733126ad21cd1feb0801caf67c4/coderd/workspacebuilds.go#L852-L856 As these two calls happen _outside of a transaction_, the state of the world can change underneath. This can result in an in-progress workspace build having a completed provisioner job attached to it. --- coderd/workspaces_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index f99e3b9e3e..141c62ff3a 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -2875,13 +2875,18 @@ func TestWorkspaceUpdateTTL(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{ - TTLMillis: testCase.toTTL, - }) + // Re-fetch the workspace build. This is required because + // `AwaitWorkspaceBuildJobCompleted` can return stale data. + build, err := client.WorkspaceBuild(ctx, build.ID) require.NoError(t, err) deadlineBefore := build.Deadline + err = client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{ + TTLMillis: testCase.toTTL, + }) + require.NoError(t, err) + build, err = client.WorkspaceBuild(ctx, build.ID) require.NoError(t, err)