mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
test(testutil): retry terraform provider cache population on transient failures (#26196)
## Problem
Tests that run real Terraform (e.g.
`enterprise/coderd.TestWorkspaceTemplateParamsChange`,
`provisioner/terraform.TestProvision`) intermittently fail when
populating the shared provider cache. On a cache miss,
`DownloadTFProviders` shells out to `terraform init` and `terraform
providers mirror` against the live registry, which periodically returns
transient 5xx errors from the registry/GitHub (504, 500). The
cache-population helper had no application-level retry, so a single
transient failure failed the whole test. Terraform's own registry client
only retries each request once ("the request failed after 2 attempts"),
which is insufficient for these bursts.
## Fix
`runCmd` now retries on any non-zero exit using
`github.com/coder/retry`, logging each failed attempt and preserving the
original failure message format. Retry-all is safe here because
`terraform init` and `terraform providers mirror` are idempotent: each
run reconciles the existing state in the working directory.
The backoff window is deliberately wide: 5 attempts with a 5s floor and
30s ceiling. `coder/retry` grows the delay by phi (~1.618) from the
floor and caps it at the ceiling, so attempts start at roughly t=0s, 8s,
21s, 42s, and 72s (waits of ~8.1s, ~13.1s, ~21.2s, and 30s capped, plus
command runtime). Registry/GitHub incidents typically last seconds to
minutes rather than a single unlucky request, so a narrow window would
only survive an isolated blip, while the early second attempt (~8s)
still recovers quickly from brief ones. This is affordable because the
network path runs only on a cache miss, not on every test: a populated
cache short-circuits via `os.Stat` and is reused within and across runs
(persisted by `.github/actions/test-cache`). The wait is therefore
rarely incurred and is negligible against the 20m per-package test
timeout. The only downside is a slower failure on a genuinely doomed
run.
This only affects the test provider-cache helper. Production provisioner
code, the Windows no-op path, and the CI cache strategy are unchanged.
Refs https://github.com/coder/internal/issues/1201
<details>
<summary>Investigation notes</summary>
The CI cache (`~/.cache/coderv2-test`, via `.github/actions/test-cache`)
is persisted across runs and keyed by a hash of a stable caller-supplied
label + template file contents, so cache hits avoid the network
entirely. The flake only surfaces on a cache miss (provider version
bump, monthly cache reset, or new label/template), where the populating
`terraform init` was the sole unprotected network call. This change
closes that gap without weakening the "use real Terraform" intent of the
tests.
</details>
---
🤖 Generated with Coder Agents on behalf of @jscottmiller.
This commit is contained in:
@@ -13,8 +13,11 @@ import (
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/coder/retry"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -85,17 +88,51 @@ func WriteTFCliConfig(t *testing.T, dir string) string {
|
||||
return cliConfigPath
|
||||
}
|
||||
|
||||
const (
|
||||
runCmdMaxAttempts = 5
|
||||
runCmdRetryFloor = 5 * time.Second
|
||||
runCmdRetryCeil = 30 * time.Second
|
||||
)
|
||||
|
||||
// runCmd runs the given command, retrying on any non-zero exit. The provider
|
||||
// cache population commands hit the network and intermittently fail with
|
||||
// transient registry/GitHub 5xx errors; retrying is safe because `terraform
|
||||
// init` and `terraform providers mirror` are idempotent. The backoff window is
|
||||
// wide because registry incidents last seconds to minutes, and the wait is
|
||||
// only incurred on a cache miss (see DownloadTFProviders).
|
||||
func runCmd(t *testing.T, dir string, args ...string) {
|
||||
t.Helper()
|
||||
|
||||
stdout, stderr := bytes.NewBuffer(nil), bytes.NewBuffer(nil)
|
||||
cmd := exec.Command(args[0], args[1:]...) //#nosec
|
||||
cmd.Dir = dir
|
||||
cmd.Stdout = stdout
|
||||
cmd.Stderr = stderr
|
||||
if err := cmd.Run(); err != nil {
|
||||
t.Fatalf("failed to run %s: %s\nstdout: %s\nstderr: %s", strings.Join(args, " "), err, stdout.String(), stderr.String())
|
||||
ctx := t.Context()
|
||||
var (
|
||||
attempt int
|
||||
lastErr error
|
||||
lastStdout, lastStderr string
|
||||
)
|
||||
for r := retry.New(runCmdRetryFloor, runCmdRetryCeil); attempt < runCmdMaxAttempts && r.Wait(ctx); attempt++ {
|
||||
stdout, stderr := bytes.NewBuffer(nil), bytes.NewBuffer(nil)
|
||||
// #nosec G204 - args are test-controlled (the terraform binary plus fixed
|
||||
// subcommands and a cache dir path), never external input.
|
||||
cmd := exec.CommandContext(ctx, args[0], args[1:]...)
|
||||
cmd.Dir = dir
|
||||
cmd.Stdout = stdout
|
||||
cmd.Stderr = stderr
|
||||
err := cmd.Run()
|
||||
if err == nil {
|
||||
return
|
||||
}
|
||||
lastErr = err
|
||||
lastStdout, lastStderr = stdout.String(), stderr.String()
|
||||
t.Logf("attempt %d/%d to run %s failed: %s\nstdout: %s\nstderr: %s",
|
||||
attempt+1, runCmdMaxAttempts, strings.Join(args, " "), err, lastStdout, lastStderr)
|
||||
}
|
||||
if lastErr == nil {
|
||||
// The loop exited without running a command, which means r.Wait saw the
|
||||
// context canceled before the first attempt. Report that, not a nil error.
|
||||
t.Fatalf("failed to run %s: %v", strings.Join(args, " "), ctx.Err())
|
||||
}
|
||||
t.Fatalf("failed to run %s after %d attempts: %s\nstdout: %s\nstderr: %s",
|
||||
strings.Join(args, " "), attempt, lastErr, lastStdout, lastStderr)
|
||||
}
|
||||
|
||||
// GetTestTFCacheDir returns a unique cache directory path based on the test name and template files.
|
||||
|
||||
Reference in New Issue
Block a user