From 1dea00dd0441bc1c8143dfb3f5910a966fd6bc83 Mon Sep 17 00:00:00 2001 From: "J. Scott Miller" Date: Mon, 29 Jun 2026 09:56:42 -0500 Subject: [PATCH] fix: deflake TestWorkspaceTagsTerraform with context-aware build waits (#26315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TestWorkspaceTagsTerraform` runs a real terraform provisioner but waited on builds with `coderdtest` helpers whose deadlines are sized for the echo provisioner used by most tests, which replays canned responses and completes in well under a second. On Windows runners, where terraform providers are not cached and every `terraform init` downloads from the registry, template imports exceeded the 25s budget in `AwaitTemplateVersionJobCompleted` and workspace builds exceeded the 10s context in `AwaitWorkspaceBuildJobCompleted`, even though the test intends a 120s budget. Add `AwaitTemplateVersionJobCompletedWithTimeout` and `AwaitWorkspaceBuildJobCompletedWithTimeout`, which take a caller-provided wait bound, and use them in the test with `2*testutil.WaitSuperLong` (120s). Also fix `AwaitWorkspaceBuildJobCompleted` creating a `WaitShort` (10s) context while polling for `WaitMedium` (15s), which guaranteed `context deadline exceeded` errors for the final five seconds of polling. `TestWorkspaceTemplateParamsChange` has the same shape (real terraform provisioner, 120s test context, plain await helpers) and the same latent bug, so it gets the same fix. Closes https://github.com/coder/internal/issues/1470 (Linear: PLAT-176)
Root cause analysis Two CI failures, same mechanism: - 2026-04-16 (run 24493089585, windows-2022): `overrides_with_dynamic_option_from_var/dynamic` failed at `coderdtest.AwaitTemplateVersionJobCompleted` with `Condition never satisfied ... make sure you set IncludeProvisionerDaemon!`. The template import job (real terraform init/plan, with network provider download) did not complete within `WaitLong` (25s). - 2026-05-27 (run 26492817796, windows-2022): `tag_param/dynamic` failed at `coderdtest.AwaitWorkspaceBuildJobCompleted` with `failed to get workspace build ...: context deadline exceeded`. The helper's internal context was `WaitShort` (10s) while its polling window was `WaitMedium` (15s), so after 10s every poll could only fail. The logged `terraform apply: exit status 1` and the `TempDir RemoveAll ... Access is denied` cleanup error are consequences of test teardown canceling the in-flight job while the provider exe was still file-locked. The test declares a 120s budget (`2*testutil.WaitSuperLong`, commented "This can take a while"), but the await helpers ignored it and applied their own 10-25s budgets. `testutil.CacheTFProviders` is a no-op on Windows, so real builds are much slower there. This change raises the ceiling for the tests rather than making terraform faster; both observed failure signatures are eliminated. The default helper budgets are unchanged for the ~880 existing call sites. One small behavior change: `AwaitTemplateVersionJobCompleted` previously marked the test failed on any transient poll error via `assert.NoError`; it now logs and keeps polling, matching the workspace build helper, and still fails on timeout. `TestWorkspaceTemplateParamsChange` is the sibling real-terraform test in the same file (also covered by the original provider-caching work in #20603). It runs three sequential real builds with the plain await helpers under a 120s context, so it is exposed to the same Windows slowness even though it has not produced its own issue yet. Its context is raised to `6*testutil.WaitSuperLong` to outlast three sequential await budgets. API note: a context-taking variant was considered first, but a `time.Duration` parameter avoids an implicit "context must have a deadline" contract and matches how the existing helpers manage their own wait budgets.
--- 🤖 This PR was generated by Coder Agents on behalf of @jscottmiller. --- coderd/coderdtest/coderdtest.go | 49 +++++++++++++++++++++------- enterprise/coderd/workspaces_test.go | 27 ++++++++++----- 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 0babe14dfa..8ff4334828 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1223,35 +1223,59 @@ func AwaitTemplateVersionJobRunning(t testing.TB, client *codersdk.Client, versi } // AwaitTemplateVersionJobCompleted waits for the build to be completed. This may result -// from cancelation, an error, or from completing successfully. +// from cancelation, an error, or from completing successfully. The wait is bounded by +// testutil.WaitLong; use AwaitTemplateVersionJobCompletedWithTimeout to wait longer. func AwaitTemplateVersionJobCompleted(t testing.TB, client *codersdk.Client, version uuid.UUID) codersdk.TemplateVersion { t.Helper() + return AwaitTemplateVersionJobCompletedWithTimeout(t, client, version, testutil.WaitLong) +} - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() +// AwaitTemplateVersionJobCompletedWithTimeout waits up to timeout for the template +// version build job to complete, polling at testutil.IntervalFast. Transient API errors +// are logged and retried. Fails the test if the job does not complete in time. +func AwaitTemplateVersionJobCompletedWithTimeout(t testing.TB, client *codersdk.Client, version uuid.UUID, timeout time.Duration) codersdk.TemplateVersion { + t.Helper() + + ctx := testutil.Context(t, timeout) t.Logf("waiting for template version %s build job to complete", version) var templateVersion codersdk.TemplateVersion - require.Eventually(t, func() bool { + completed := testutil.Eventually(ctx, t, func(ctx context.Context) bool { var err error templateVersion, err = client.TemplateVersion(ctx, version) + if err != nil { + t.Logf("failed to get template version %s: %v", version, err) + return false + } t.Logf("template version job status: %s", templateVersion.Job.Status) - return assert.NoError(t, err) && templateVersion.Job.CompletedAt != nil - }, testutil.WaitLong, testutil.IntervalFast, "make sure you set `IncludeProvisionerDaemon`!") + return templateVersion.Job.CompletedAt != nil + }, testutil.IntervalFast, "make sure you set `IncludeProvisionerDaemon`!") + if !completed { + t.FailNow() + } t.Logf("template version %s job has completed", version) return templateVersion } -// AwaitWorkspaceBuildJobCompleted waits for a workspace provision job to reach completed status. +// AwaitWorkspaceBuildJobCompleted waits for a workspace provision job to reach completed +// status. The wait is bounded by testutil.WaitMedium; use +// AwaitWorkspaceBuildJobCompletedWithTimeout to wait longer. func AwaitWorkspaceBuildJobCompleted(t testing.TB, client *codersdk.Client, build uuid.UUID) codersdk.WorkspaceBuild { t.Helper() + return AwaitWorkspaceBuildJobCompletedWithTimeout(t, client, build, testutil.WaitMedium) +} - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) - defer cancel() +// AwaitWorkspaceBuildJobCompletedWithTimeout waits up to timeout for a workspace +// provision job to reach completed status, polling at testutil.IntervalFast. Transient +// API errors are logged and retried. Fails the test if the job does not complete in time. +func AwaitWorkspaceBuildJobCompletedWithTimeout(t testing.TB, client *codersdk.Client, build uuid.UUID, timeout time.Duration) codersdk.WorkspaceBuild { + t.Helper() + + ctx := testutil.Context(t, timeout) t.Logf("waiting for workspace build job %s", build) var workspaceBuild codersdk.WorkspaceBuild - require.Eventually(t, func() bool { + completed := testutil.Eventually(ctx, t, func(ctx context.Context) bool { var err error workspaceBuild, err = client.WorkspaceBuild(ctx, build) if err != nil { @@ -1263,7 +1287,10 @@ func AwaitWorkspaceBuildJobCompleted(t testing.TB, client *codersdk.Client, buil return false } return true - }, testutil.WaitMedium, testutil.IntervalFast) + }, testutil.IntervalFast, "workspace build %s did not complete", build) + if !completed { + t.FailNow() + } t.Logf("got workspace build job %s (status: %s)", build, workspaceBuild.Job.Status) return workspaceBuild } diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 117deff7a6..b4913a7d2c 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -3233,8 +3233,9 @@ func TestWorkspaceTemplateParamsChange(t *testing.T) { _ = coderdenttest.NewExternalProvisionerDaemonTerraform(t, client, owner.OrganizationID, nil) - // This can take a while, so set a relatively long timeout. - ctx := testutil.Context(t, 2*testutil.WaitSuperLong) + // This can take a while, so set a long timeout that outlasts the three + // build awaits below. + ctx := testutil.Context(t, 6*testutil.WaitSuperLong) // Creating a template as a template admin must succeed templateFiles := map[string]string{"main.tf": mainTfTemplate} @@ -3250,7 +3251,9 @@ func TestWorkspaceTemplateParamsChange(t *testing.T) { UserVariableValues: []codersdk.VariableValue{}, }) require.NoError(t, err, "failed to create template version") - coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, tv.ID) + // Uncached Windows runners make real terraform builds much slower than + // the default await timeout. + coderdtest.AwaitTemplateVersionJobCompletedWithTimeout(t, templateAdmin, tv.ID, 2*testutil.WaitSuperLong) tpl := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, tv.ID) // Set to dynamic params @@ -3281,7 +3284,8 @@ func TestWorkspaceTemplateParamsChange(t *testing.T) { // Then: the build should succeed. The updated value of param_min should be // used to validate param instead of the value defined in the temp require.NoError(t, err, "failed to create workspace") - createBuild := coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID) + // Same timeout reason as above. + createBuild := coderdtest.AwaitWorkspaceBuildJobCompletedWithTimeout(t, member, ws.LatestBuild.ID, 2*testutil.WaitSuperLong) require.Equal(t, createBuild.Status, codersdk.WorkspaceStatusRunning) // File should exist @@ -3293,7 +3297,8 @@ func TestWorkspaceTemplateParamsChange(t *testing.T) { Transition: codersdk.WorkspaceTransitionDelete, }) require.NoError(t, err) - build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, build.ID) + // Same timeout reason as above. + build = coderdtest.AwaitWorkspaceBuildJobCompletedWithTimeout(t, member, build.ID, 2*testutil.WaitSuperLong) require.Equal(t, codersdk.WorkspaceStatusDeleted, build.Status) logsCh, closeLogs, err := member.WorkspaceBuildLogsAfter(ctx, build.ID, 0) @@ -3527,8 +3532,9 @@ func workspaceTagsTerraform(t *testing.T, tc testWorkspaceTagsTerraformCase, dyn templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin()) member, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) - // This can take a while, so set a relatively long timeout. - ctx := testutil.Context(t, 2*testutil.WaitSuperLong) + // This can take a while, so set a long timeout that outlasts both build + // awaits below. + ctx := testutil.Context(t, 4*testutil.WaitSuperLong) emptyTar := testutil.CreateTar(t, map[string]string{"main.tf": ""}) emptyFi, err := templateAdmin.Upload(ctx, "application/x-tar", bytes.NewReader(emptyTar)) @@ -3566,7 +3572,9 @@ func workspaceTagsTerraform(t *testing.T, tc testWorkspaceTagsTerraformCase, dyn TemplateID: tpl.ID, }) require.NoError(t, err, "failed to create template version") - coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, tv.ID) + // Uncached Windows runners make real terraform builds much slower than + // the default await timeout. + coderdtest.AwaitTemplateVersionJobCompletedWithTimeout(t, templateAdmin, tv.ID, 2*testutil.WaitSuperLong) err = templateAdmin.UpdateActiveTemplateVersion(ctx, tpl.ID, codersdk.UpdateActiveTemplateVersion{ ID: tv.ID, @@ -3583,7 +3591,8 @@ func workspaceTagsTerraform(t *testing.T, tc testWorkspaceTagsTerraformCase, dyn require.NoError(t, err, "failed to create workspace") tagJSON, _ := json.Marshal(ws.LatestBuild.Job.Tags) t.Logf("Created workspace build [%s] with tags: %s", ws.LatestBuild.Job.Type, tagJSON) - coderdtest.AwaitWorkspaceBuildJobCompleted(t, member, ws.LatestBuild.ID) + // Same timeout reason as above. + coderdtest.AwaitWorkspaceBuildJobCompletedWithTimeout(t, member, ws.LatestBuild.ID, 2*testutil.WaitSuperLong) } }