From 026b8a8b0481e355960b679ecb83343a221f88c8 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Mon, 19 Oct 2026 11:22:26 -0700 Subject: [PATCH 1/8] test(integration): verify script ordering against compatible Coder versions Add five cases that push templates using coder_script_order to a real Coder image. Four start a workspace agent inside the container and check, from a durable file and the build timings API, that startup scripts run in order, a dependent of a failed script is skipped, stop scripts run in order, and cancellation reports canceled outcomes. They also check that the stored dependencies match the template, so a renamed field on either side fails CI. The fifth proves an older Coder still builds the template. The agent cases are gated at v2.39.0 and skip on older images. The tests read the agent ID from the workspace because coderd generates its own on insert, and the compat template writes the agent ID to its output because Coder drops an agent that no resource references. --- README.md | 5 + integration/agent_test.go | 367 ++++++++++++++++++ integration/coder-script-order-cancel/main.tf | 103 +++++ integration/coder-script-order-compat/main.tf | 56 +++ integration/coder-script-order-skip/main.tf | 102 +++++ integration/coder-script-order-stop/main.tf | 88 +++++ integration/coder-script-order/main.tf | 104 +++++ integration/integration_test.go | 64 +++ 8 files changed, 889 insertions(+) create mode 100644 integration/agent_test.go create mode 100644 integration/coder-script-order-cancel/main.tf create mode 100644 integration/coder-script-order-compat/main.tf create mode 100644 integration/coder-script-order-skip/main.tf create mode 100644 integration/coder-script-order-stop/main.tf create mode 100644 integration/coder-script-order/main.tf diff --git a/README.md b/README.md index cff57558..31457029 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,11 @@ To run these integration tests locally: 1. Run `CODER_IMAGE=ghcr.io/coder/coder-preview CODER_VERSION=main-x.y.z-devel-abcd1234 make test-integration`. +The provider binary runs inside the Coder container, so on macOS build it for Linux first, for example `GOOS=linux GOARCH=arm64 CGO_ENABLED=0 go build .` on Apple silicon. +The `coder-script-order*` cases start a workspace agent inside the Coder container and read its results through the API. +While a run is in progress the agent's log is at `/tmp/-agent.log` inside the container. +These cases skip on Coder images older than v2.39, the first release that enforces script ordering. + > [!Note] > You can specify `CODER_IMAGE` if the Coder image you wish to test is hosted somewhere other than `ghcr.io/coder/coder`. > For example, `CODER_IMAGE=example.com/repo/coder CODER_VERSION=foobar make test-integration`. diff --git a/integration/agent_test.go b/integration/agent_test.go new file mode 100644 index 00000000..c0564237 --- /dev/null +++ b/integration/agent_test.go @@ -0,0 +1,367 @@ +package integration + +import ( + "context" + "encoding/json" + "fmt" + "slices" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// These mirror the fields of the Coder API that the script ordering cases +// read. The package does not import codersdk, see the note on TestIntegration. +type agentScriptDependency struct { + PrerequisiteResourceAddress string `json:"prerequisite_resource_address"` + Requirement string `json:"requirement"` +} + +type agentScript struct { + DisplayName string `json:"display_name"` + LogSourceID string `json:"log_source_id"` + ResourceAddress string `json:"resource_address"` + Dependencies []agentScriptDependency `json:"dependencies"` +} + +type workspaceAgent struct { + LifecycleState string `json:"lifecycle_state"` + Scripts []agentScript `json:"scripts"` +} + +type scriptTiming struct { + StartedAt time.Time `json:"started_at"` + EndedAt time.Time `json:"ended_at"` + ExitCode int32 `json:"exit_code"` + Stage string `json:"stage"` + Status string `json:"status"` + DisplayName string `json:"display_name"` +} + +type buildTimings struct { + AgentScriptTimings []scriptTiming `json:"agent_script_timings"` +} + +type agentLog struct { + Output string `json:"output"` + SourceID string `json:"source_id"` +} + +const agentWaitTimeout = 2 * time.Minute + +// startAgent runs coder agent in the background inside the Coder container +// for the workspace the named case created. +func startAgent(ctx context.Context, t *testing.T, ctrID, name string) { + t.Helper() + // The agent reads its token from the file the template wrote, so the + // token never appears in the command the harness logs. The subshell + // execs the agent so the PID file holds the agent's own PID. + cmd := fmt.Sprintf( + `mkdir -p /tmp/%[1]s-agent && `+ + `( exec env CODER_AGENT_URL=http://localhost:3000 `+ + `CODER_AGENT_TOKEN_FILE=/tmp/%[1]s.agent-token `+ + `CODER_AGENT_LOG_DIR=/tmp/%[1]s-agent `+ + `CODER_AGENT_SCRIPT_DATA_DIR=/tmp/%[1]s-agent `+ + `coder agent >/tmp/%[1]s-agent.log 2>&1 ) & `+ + `echo $! >/tmp/%[1]s-agent.pid`, + name) + _, rc := execContainer(ctx, t, ctrID, cmd) + require.Equal(t, 0, rc, "start agent for %s", name) +} + +// stopAgent sends SIGTERM to the case's agent so it runs its stop scripts. +func stopAgent(ctx context.Context, t *testing.T, ctrID, name string) { + t.Helper() + // Never signal the process by name: the server in the same container is + // the same binary. + _, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("kill -TERM $(cat /tmp/%s-agent.pid)", name)) + require.Equal(t, 0, rc, "stop agent for %s", name) +} + +// apiGet performs a GET against the Coder in the container as the workspace +// owner and decodes the JSON body into out. +func apiGet(ctx context.Context, t *testing.T, ctrID, name, path string, out any) error { + t.Helper() + // The session token is read by the container's shell at the moment of + // the call, so it never appears in the command the harness logs. + cmd := fmt.Sprintf( + `curl -s -w '\n%%{http_code}' -H "Coder-Session-Token: $(cat /tmp/%s.session-token)" http://localhost:3000%s`, + name, path) + body, rc := execContainer(ctx, t, ctrID, cmd) + if rc != 0 { + return fmt.Errorf("GET %s: exit %d: %s", path, rc, body) + } + i := strings.LastIndex(body, "\n") + if i < 0 { + return fmt.Errorf("GET %s: no status line in %q", path, body) + } + status, body := body[i+1:], body[:i] + if status != "200" { + return fmt.Errorf("GET %s: HTTP %s: %s", path, status, body) + } + if err := json.Unmarshal([]byte(body), out); err != nil { + return fmt.Errorf("decode GET %s: %w: %s", path, err, body) + } + return nil +} + +// agentIDFor returns the ID coderd assigned to the workspace's one agent. +func agentIDFor(ctx context.Context, t *testing.T, ctrID, name, workspaceID string) string { + t.Helper() + // coderd generates its own agent ID on insert, so the ID in the + // Terraform state is not the one the API knows. + var ws struct { + LatestBuild struct { + Resources []struct { + Agents []struct { + ID string `json:"id"` + } `json:"agents"` + } `json:"resources"` + } `json:"latest_build"` + } + require.NoError(t, apiGet(ctx, t, ctrID, name, "/api/v2/workspaces/"+workspaceID, &ws)) + var ids []string + for _, r := range ws.LatestBuild.Resources { + for _, a := range r.Agents { + ids = append(ids, a.ID) + } + } + require.Lenf(t, ids, 1, "expected one agent on workspace %s, got %v", workspaceID, ids) + return ids[0] +} + +// waitForAgent polls the agent until its lifecycle state is one of want and +// returns the last response. +func waitForAgent(ctx context.Context, t *testing.T, ctrID, name, agentID string, want ...string) workspaceAgent { + t.Helper() + deadline := time.Now().Add(agentWaitTimeout) + var agent workspaceAgent + var lastErr error + for time.Now().Before(deadline) { + lastErr = apiGet(ctx, t, ctrID, name, "/api/v2/workspaceagents/"+agentID, &agent) + if lastErr == nil && slices.Contains(want, agent.LifecycleState) { + return agent + } + time.Sleep(time.Second) + } + tail, _ := execContainer(ctx, t, ctrID, fmt.Sprintf("tail -n 50 /tmp/%s-agent.log", name)) + t.Fatalf("agent %s did not reach %v in time: last state %q, last error %v\nagent log tail:\n%s", + name, want, agent.LifecycleState, lastErr, tail) + return agent +} + +// orderLogLines returns the lines the case's scripts appended to the durable +// file, in the order they were written. +func orderLogLines(ctx context.Context, t *testing.T, ctrID, name string) []string { + t.Helper() + out, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log", name)) + require.Equal(t, 0, rc, "read order log for %s", name) + return strings.Split(strings.TrimSpace(out), "\n") +} + +// requireLineAfter asserts that later appears exactly once and after earlier. +func requireLineAfter(t *testing.T, lines []string, later, earlier string) { + t.Helper() + // Only relative order is checked. Scripts that start together may write + // in either order. + laterIdx := slices.Index(lines, later) + earlierIdx := slices.Index(lines, earlier) + require.NotEqualf(t, -1, laterIdx, "line %q missing from %q", later, lines) + require.NotEqualf(t, -1, earlierIdx, "line %q missing from %q", earlier, lines) + require.Equalf(t, 1, countOf(lines, later), "line %q appears more than once in %q", later, lines) + require.Greaterf(t, laterIdx, earlierIdx, "%q (line %d) should come after %q (line %d): %q", later, laterIdx, earlier, earlierIdx, lines) +} + +func countOf(lines []string, want string) int { + n := 0 + for _, l := range lines { + if l == want { + n++ + } + } + return n +} + +// latestBuildTimings fetches the timings of the workspace's latest build. +func latestBuildTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID string) buildTimings { + t.Helper() + var ws struct { + LatestBuild struct { + ID string `json:"id"` + } `json:"latest_build"` + } + require.NoError(t, apiGet(ctx, t, ctrID, name, "/api/v2/workspaces/"+workspaceID, &ws)) + var timings buildTimings + require.NoError(t, apiGet(ctx, t, ctrID, name, "/api/v2/workspacebuilds/"+ws.LatestBuild.ID+"/timings", &timings)) + return timings +} + +// timingFor returns the one timing with the given stage and display name. +func timingFor(t *testing.T, timings buildTimings, stage, displayName string) scriptTiming { + t.Helper() + var found []scriptTiming + for _, tm := range timings.AgentScriptTimings { + if tm.Stage == stage && tm.DisplayName == displayName { + found = append(found, tm) + } + } + require.Lenf(t, found, 1, "expected one %s timing for %q, got %+v", stage, displayName, found) + return found[0] +} + +// scriptFor returns the agent's script with the given display name. +func scriptFor(t *testing.T, agent workspaceAgent, displayName string) agentScript { + t.Helper() + for _, s := range agent.Scripts { + if s.DisplayName == displayName { + return s + } + } + t.Fatalf("no script named %q in %+v", displayName, agent.Scripts) + return agentScript{} +} + +// requireDependencies asserts a script's resource address and its stored +// dependencies. A field renamed on either side of the provider and Coder +// contract shows up here as a script with no dependencies. +func requireDependencies(t *testing.T, agent workspaceAgent, displayName, address string, want []agentScriptDependency) { + t.Helper() + s := scriptFor(t, agent, displayName) + require.Equalf(t, address, s.ResourceAddress, "%s resource_address", displayName) + require.Equalf(t, want, s.Dependencies, "%s dependencies", displayName) +} + +// agentLogLines returns the agent log lines for one script's log source. +func agentLogLines(ctx context.Context, t *testing.T, ctrID, name, agentID, sourceID string) []string { + t.Helper() + var logs []agentLog + require.NoError(t, apiGet(ctx, t, ctrID, name, "/api/v2/workspaceagents/"+agentID+"/logs", &logs)) + var lines []string + for _, l := range logs { + if l.SourceID == sourceID { + lines = append(lines, l.Output) + } + } + return lines +} + +func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { + const name = "coder-script-order" + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") + require.Equal(t, "ready", agent.LifecycleState) + + lines := orderLogLines(ctx, t, ctrID, name) + require.Len(t, lines, 8, "expected two lines per script: %q", lines) + requireLineAfter(t, lines, "install_tools start", "clone_repo end") + requireLineAfter(t, lines, "post_setup start", "clone_repo end") + requireLineAfter(t, lines, "clone_repo end", "start_code_server start") + + timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + clone := timingFor(t, timings, "start", "Clone repo") + install := timingFor(t, timings, "start", "Install tools") + post := timingFor(t, timings, "start", "Post setup") + code := timingFor(t, timings, "start", "code-server") + for _, tm := range []scriptTiming{clone, install, post, code} { + require.Equalf(t, "ok", tm.Status, "%s: %+v", tm.DisplayName, tm) + require.Equalf(t, int32(0), tm.ExitCode, "%s: %+v", tm.DisplayName, tm) + } + require.Falsef(t, install.StartedAt.Before(clone.EndedAt), "Install tools started %s before Clone repo ended %s", install.StartedAt, clone.EndedAt) + require.Falsef(t, post.StartedAt.Before(clone.EndedAt), "Post setup started %s before Clone repo ended %s", post.StartedAt, clone.EndedAt) + require.Truef(t, code.StartedAt.Before(clone.EndedAt), "code-server started %s after Clone repo ended %s", code.StartedAt, clone.EndedAt) + require.GreaterOrEqual(t, clone.EndedAt.Sub(clone.StartedAt), 3*time.Second) + + requireDependencies(t, agent, "Clone repo", "coder_script.clone_repo", nil) + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + requireDependencies(t, agent, "Post setup", "coder_script.post_setup", + []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) + requireDependencies(t, agent, "code-server", "coder_script.start_code_server", nil) +} + +func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { + const name = "coder-script-order-skip" + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") + require.Equal(t, "start_error", agent.LifecycleState) + + lines := orderLogLines(ctx, t, ctrID, name) + require.NotContains(t, lines, "install_tools start", "skipped script must not run: %q", lines) + requireLineAfter(t, lines, "post_setup start", "clone_repo end") + + timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + clone := timingFor(t, timings, "start", "Clone repo") + require.Equal(t, "exit_failure", clone.Status) + require.Equal(t, int32(1), clone.ExitCode) + install := timingFor(t, timings, "start", "Install tools") + require.Equal(t, "skipped", install.Status) + require.Truef(t, install.StartedAt.Equal(install.EndedAt), "skipped timing must be zero-length: %+v", install) + require.Equal(t, "ok", timingFor(t, timings, "start", "Post setup").Status) + require.Equal(t, "ok", timingFor(t, timings, "start", "code-server").Status) + + installLines := agentLogLines(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID) + require.Contains(t, installLines, `Skipping script: dependency "Clone repo" failed.`) + + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) +} + +func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { + const name = "coder-script-order-stop" + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + waitForAgent(ctx, t, ctrID, name, agentID, "ready") + stopAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") + require.Equal(t, "off", agent.LifecycleState) + + lines := orderLogLines(ctx, t, ctrID, name) + requireLineAfter(t, lines, "stop_db start", "save_state end") + + timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + save := timingFor(t, timings, "stop", "Save state") + stop := timingFor(t, timings, "stop", "Stop DB") + require.Equal(t, "ok", save.Status) + require.Equal(t, "ok", stop.Status) + require.Falsef(t, stop.StartedAt.Before(save.EndedAt), "Stop DB started %s before Save state ended %s", stop.StartedAt, save.EndedAt) + + requireDependencies(t, agent, "Stop DB", "coder_script.stop_db", + []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.save_state", Requirement: "success"}}) +} + +func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { + const name = "coder-script-order-cancel" + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + require.Eventually(t, func() bool { + o, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log 2>/dev/null", name)) + return rc == 0 && strings.Contains(o, "clone_repo start") + }, agentWaitTimeout, time.Second, "clone_repo never started") + stopAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") + + lines := orderLogLines(ctx, t, ctrID, name) + require.NotContains(t, lines, "clone_repo end") + require.NotContains(t, lines, "install_tools start") + require.NotContains(t, lines, "post_setup start") + + timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + clone := timingFor(t, timings, "start", "Clone repo") + require.Equal(t, "canceled", clone.Status) + require.Truef(t, clone.EndedAt.After(clone.StartedAt), "canceled running script must keep its real interval: %+v", clone) + for _, n := range []string{"Install tools", "Post setup"} { + tm := timingFor(t, timings, "start", n) + require.Equalf(t, "canceled", tm.Status, "%s: %+v", n, tm) + require.Truef(t, tm.StartedAt.Equal(tm.EndedAt), "%s timing must be zero-length: %+v", n, tm) + } + require.Equal(t, "ok", timingFor(t, timings, "start", "code-server").Status) + + installLines := agentLogLines(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID) + require.Contains(t, installLines, "Canceled before starting.") + cloneLines := agentLogLines(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Clone repo").LogSourceID) + require.Contains(t, cloneLines, "Canceled while running.") +} diff --git a/integration/coder-script-order-cancel/main.tf b/integration/coder-script-order-cancel/main.tf new file mode 100644 index 00000000..0354bde5 --- /dev/null +++ b/integration/coder-script-order-cancel/main.tf @@ -0,0 +1,103 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + local = { + source = "hashicorp/local" + } + } +} + +data "coder_provisioner" "me" {} +data "coder_workspace" "me" {} +data "coder_workspace_owner" "me" {} + +resource "coder_agent" "main" { + os = "linux" + arch = data.coder_provisioner.me.arch +} + +locals { + order_log = "/tmp/coder-script-order-cancel.log" +} + +# Runs long enough for the test to stop the agent while the dependents are +# still waiting. +resource "coder_script" "clone_repo" { + agent_id = coder_agent.main.id + display_name = "Clone repo" + run_on_start = true + timeout = 120 + script = <<-EOT + echo "clone_repo start" >> ${local.order_log} + sleep 60 + echo "clone_repo end" >> ${local.order_log} + EOT +} + +resource "coder_script" "install_tools" { + agent_id = coder_agent.main.id + display_name = "Install tools" + run_on_start = true + script = <<-EOT + echo "install_tools start" >> ${local.order_log} + echo "install_tools end" >> ${local.order_log} + EOT +} + +resource "coder_script" "post_setup" { + agent_id = coder_agent.main.id + display_name = "Post setup" + run_on_start = true + script = <<-EOT + echo "post_setup start" >> ${local.order_log} + echo "post_setup end" >> ${local.order_log} + EOT +} + +resource "coder_script" "start_code_server" { + agent_id = coder_agent.main.id + display_name = "code-server" + run_on_start = true + script = <<-EOT + echo "start_code_server start" >> ${local.order_log} + echo "start_code_server end" >> ${local.order_log} + EOT +} + +data "coder_script_order" "bootstrap" { + rule { + run = ["coder_script.install_tools"] + after = ["coder_script.clone_repo"] + } + rule { + run = ["coder_script.post_setup"] + after = ["coder_script.clone_repo"] + requires = "completion" + } +} + +variable "output_path" { + type = string +} + +resource "local_file" "output" { + filename = var.output_path + content = jsonencode({ + "workspace.id" = data.coder_workspace.me.id + "agent.id" = coder_agent.main.id + "script_order.id" = data.coder_script_order.bootstrap.id + "script_order.rule.#" = tostring(length(data.coder_script_order.bootstrap.rule)) + }) +} + +resource "local_sensitive_file" "agent_token" { + filename = "/tmp/coder-script-order-cancel.agent-token" + content = coder_agent.main.token +} + +resource "local_sensitive_file" "session_token" { + filename = "/tmp/coder-script-order-cancel.session-token" + content = data.coder_workspace_owner.me.session_token +} diff --git a/integration/coder-script-order-compat/main.tf b/integration/coder-script-order-compat/main.tf new file mode 100644 index 00000000..c044a58f --- /dev/null +++ b/integration/coder-script-order-compat/main.tf @@ -0,0 +1,56 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + local = { + source = "hashicorp/local" + } + } +} + +data "coder_provisioner" "me" {} + +resource "coder_agent" "main" { + os = "linux" + arch = data.coder_provisioner.me.arch +} + +resource "coder_script" "clone_repo" { + agent_id = coder_agent.main.id + display_name = "Clone repo" + run_on_start = true + script = "true" +} + +resource "coder_script" "install_tools" { + agent_id = coder_agent.main.id + display_name = "Install tools" + run_on_start = true + script = "true" +} + +data "coder_script_order" "bootstrap" { + rule { + run = ["coder_script.install_tools"] + after = ["coder_script.clone_repo"] + } +} + +variable "output_path" { + type = string +} + +# Coder drops an agent that no resource references. Writing the agent ID +# here makes this file that resource. +resource "local_file" "output" { + filename = var.output_path + content = jsonencode({ + "agent.id" = coder_agent.main.id + "script_order.id" = data.coder_script_order.bootstrap.id + "script_order.rule.0.run.0" = data.coder_script_order.bootstrap.rule[0].run[0] + "script_order.rule.0.after.0" = data.coder_script_order.bootstrap.rule[0].after[0] + "script_order.rule.0.requires" = data.coder_script_order.bootstrap.rule[0].requires + "script_order.rule.0.phase" = data.coder_script_order.bootstrap.rule[0].phase + }) +} diff --git a/integration/coder-script-order-skip/main.tf b/integration/coder-script-order-skip/main.tf new file mode 100644 index 00000000..40277c79 --- /dev/null +++ b/integration/coder-script-order-skip/main.tf @@ -0,0 +1,102 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + local = { + source = "hashicorp/local" + } + } +} + +data "coder_provisioner" "me" {} +data "coder_workspace" "me" {} +data "coder_workspace_owner" "me" {} + +resource "coder_agent" "main" { + os = "linux" + arch = data.coder_provisioner.me.arch +} + +locals { + order_log = "/tmp/coder-script-order-skip.log" +} + +# Fails on purpose so that install_tools is skipped and post_setup still runs. +resource "coder_script" "clone_repo" { + agent_id = coder_agent.main.id + display_name = "Clone repo" + run_on_start = true + timeout = 30 + script = <<-EOT + echo "clone_repo start" >> ${local.order_log} + echo "clone_repo end" >> ${local.order_log} + exit 1 + EOT +} + +resource "coder_script" "install_tools" { + agent_id = coder_agent.main.id + display_name = "Install tools" + run_on_start = true + script = <<-EOT + echo "install_tools start" >> ${local.order_log} + echo "install_tools end" >> ${local.order_log} + EOT +} + +resource "coder_script" "post_setup" { + agent_id = coder_agent.main.id + display_name = "Post setup" + run_on_start = true + script = <<-EOT + echo "post_setup start" >> ${local.order_log} + echo "post_setup end" >> ${local.order_log} + EOT +} + +resource "coder_script" "start_code_server" { + agent_id = coder_agent.main.id + display_name = "code-server" + run_on_start = true + script = <<-EOT + echo "start_code_server start" >> ${local.order_log} + echo "start_code_server end" >> ${local.order_log} + EOT +} + +data "coder_script_order" "bootstrap" { + rule { + run = ["coder_script.install_tools"] + after = ["coder_script.clone_repo"] + } + rule { + run = ["coder_script.post_setup"] + after = ["coder_script.clone_repo"] + requires = "completion" + } +} + +variable "output_path" { + type = string +} + +resource "local_file" "output" { + filename = var.output_path + content = jsonencode({ + "workspace.id" = data.coder_workspace.me.id + "agent.id" = coder_agent.main.id + "script_order.id" = data.coder_script_order.bootstrap.id + "script_order.rule.#" = tostring(length(data.coder_script_order.bootstrap.rule)) + }) +} + +resource "local_sensitive_file" "agent_token" { + filename = "/tmp/coder-script-order-skip.agent-token" + content = coder_agent.main.token +} + +resource "local_sensitive_file" "session_token" { + filename = "/tmp/coder-script-order-skip.session-token" + content = data.coder_workspace_owner.me.session_token +} diff --git a/integration/coder-script-order-stop/main.tf b/integration/coder-script-order-stop/main.tf new file mode 100644 index 00000000..b348e938 --- /dev/null +++ b/integration/coder-script-order-stop/main.tf @@ -0,0 +1,88 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + local = { + source = "hashicorp/local" + } + } +} + +data "coder_provisioner" "me" {} +data "coder_workspace" "me" {} +data "coder_workspace_owner" "me" {} + +resource "coder_agent" "main" { + os = "linux" + arch = data.coder_provisioner.me.arch +} + +locals { + order_log = "/tmp/coder-script-order-stop.log" +} + +# One startup script so the agent reaches the ready state before the test +# stops it. +resource "coder_script" "warm" { + agent_id = coder_agent.main.id + display_name = "Warm" + run_on_start = true + script = <<-EOT + echo "warm start" >> ${local.order_log} + echo "warm end" >> ${local.order_log} + EOT +} + +resource "coder_script" "save_state" { + agent_id = coder_agent.main.id + display_name = "Save state" + run_on_stop = true + script = <<-EOT + echo "save_state start" >> ${local.order_log} + sleep 2 + echo "save_state end" >> ${local.order_log} + EOT +} + +resource "coder_script" "stop_db" { + agent_id = coder_agent.main.id + display_name = "Stop DB" + run_on_stop = true + script = <<-EOT + echo "stop_db start" >> ${local.order_log} + echo "stop_db end" >> ${local.order_log} + EOT +} + +data "coder_script_order" "shutdown" { + rule { + run = ["coder_script.stop_db"] + after = ["coder_script.save_state"] + phase = "stop" + } +} + +variable "output_path" { + type = string +} + +resource "local_file" "output" { + filename = var.output_path + content = jsonencode({ + "workspace.id" = data.coder_workspace.me.id + "agent.id" = coder_agent.main.id + "script_order.id" = data.coder_script_order.shutdown.id + "script_order.rule.#" = tostring(length(data.coder_script_order.shutdown.rule)) + }) +} + +resource "local_sensitive_file" "agent_token" { + filename = "/tmp/coder-script-order-stop.agent-token" + content = coder_agent.main.token +} + +resource "local_sensitive_file" "session_token" { + filename = "/tmp/coder-script-order-stop.session-token" + content = data.coder_workspace_owner.me.session_token +} diff --git a/integration/coder-script-order/main.tf b/integration/coder-script-order/main.tf new file mode 100644 index 00000000..3f10990b --- /dev/null +++ b/integration/coder-script-order/main.tf @@ -0,0 +1,104 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + local = { + source = "hashicorp/local" + } + } +} + +data "coder_provisioner" "me" {} +data "coder_workspace" "me" {} +data "coder_workspace_owner" "me" {} + +resource "coder_agent" "main" { + os = "linux" + arch = data.coder_provisioner.me.arch +} + +locals { + order_log = "/tmp/coder-script-order.log" +} + +# The sleep gives the unordered script time to start before this one ends. +resource "coder_script" "clone_repo" { + agent_id = coder_agent.main.id + display_name = "Clone repo" + run_on_start = true + timeout = 30 + script = <<-EOT + echo "clone_repo start" >> ${local.order_log} + sleep 3 + echo "clone_repo end" >> ${local.order_log} + EOT +} + +resource "coder_script" "install_tools" { + agent_id = coder_agent.main.id + display_name = "Install tools" + run_on_start = true + script = <<-EOT + echo "install_tools start" >> ${local.order_log} + echo "install_tools end" >> ${local.order_log} + EOT +} + +resource "coder_script" "post_setup" { + agent_id = coder_agent.main.id + display_name = "Post setup" + run_on_start = true + script = <<-EOT + echo "post_setup start" >> ${local.order_log} + echo "post_setup end" >> ${local.order_log} + EOT +} + +resource "coder_script" "start_code_server" { + agent_id = coder_agent.main.id + display_name = "code-server" + run_on_start = true + script = <<-EOT + echo "start_code_server start" >> ${local.order_log} + echo "start_code_server end" >> ${local.order_log} + EOT +} + +data "coder_script_order" "bootstrap" { + rule { + run = ["coder_script.install_tools"] + after = ["coder_script.clone_repo"] + } + rule { + run = ["coder_script.post_setup"] + after = ["coder_script.clone_repo"] + requires = "completion" + } +} + +variable "output_path" { + type = string +} + +resource "local_file" "output" { + filename = var.output_path + content = jsonencode({ + "workspace.id" = data.coder_workspace.me.id + "agent.id" = coder_agent.main.id + "script_order.id" = data.coder_script_order.bootstrap.id + "script_order.rule.#" = tostring(length(data.coder_script_order.bootstrap.rule)) + }) +} + +# The test reads these files inside the container. Keeping the tokens out of +# the output JSON keeps them out of the test log. +resource "local_sensitive_file" "agent_token" { + filename = "/tmp/coder-script-order.agent-token" + content = coder_agent.main.token +} + +resource "local_sensitive_file" "session_token" { + filename = "/tmp/coder-script-order.session-token" + content = data.coder_workspace_owner.me.session_token +} diff --git a/integration/integration_test.go b/integration/integration_test.go index afe38e1e..ecab43ed 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -68,6 +68,9 @@ func TestIntegration(t *testing.T) { minVersion string // map of string to regex to be passed to assertOutput() expectedOutput map[string]string + // Optional. Runs after assertOutput for cases that need to inspect the + // workspace after provisioning. + verify func(ctx context.Context, t *testing.T, ctrID string, output map[string]string) }{ { name: "test-data-source", @@ -225,6 +228,64 @@ func TestIntegration(t *testing.T) { "task.enabled": "false", }, }, + { + name: "coder-script-order", + minVersion: "v2.39.0", + expectedOutput: map[string]string{ + "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "script_order.id": `.+`, + "script_order.rule.#": `^2$`, + }, + verify: verifyScriptOrder, + }, + { + name: "coder-script-order-skip", + minVersion: "v2.39.0", + expectedOutput: map[string]string{ + "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "script_order.id": `.+`, + "script_order.rule.#": `^2$`, + }, + verify: verifyScriptOrderSkip, + }, + { + name: "coder-script-order-stop", + minVersion: "v2.39.0", + expectedOutput: map[string]string{ + "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "script_order.id": `.+`, + "script_order.rule.#": `^1$`, + }, + verify: verifyScriptOrderStop, + }, + { + name: "coder-script-order-cancel", + minVersion: "v2.39.0", + expectedOutput: map[string]string{ + "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "script_order.id": `.+`, + "script_order.rule.#": `^2$`, + }, + verify: verifyScriptOrderCancel, + }, + { + // Runs on every Coder version. A Coder older than the data source + // ignores it and must still import and build the template. + name: "coder-script-order-compat", + minVersion: "v0.0.0", + expectedOutput: map[string]string{ + "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, + "script_order.id": `.+`, + "script_order.rule.0.run.0": `^coder_script\.install_tools$`, + "script_order.rule.0.after.0": `^coder_script\.clone_repo$`, + "script_order.rule.0.requires": `^success$`, + "script_order.rule.0.phase": `^$`, + }, + }, } { t.Run(tt.name, func(t *testing.T) { t.Parallel() @@ -263,6 +324,9 @@ func TestIntegration(t *testing.T) { actual := make(map[string]string) require.NoError(t, json.NewDecoder(strings.NewReader(out)).Decode(&actual)) assertOutput(t, tt.expectedOutput, actual) + if tt.verify != nil { + tt.verify(ctx, t, ctrID, actual) + } }) } } From 6e6399517c69364ca08f251261a308d8dc2a3e36 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Mon, 19 Oct 2026 12:11:44 -0700 Subject: [PATCH 2/8] test(integration): poll timings and logs after lifecycle state changes The agent reports ready or off before the goroutines that send script timings and logs have finished, so a single read right after the state change can come back empty. --- integration/agent_test.go | 72 ++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 24 deletions(-) diff --git a/integration/agent_test.go b/integration/agent_test.go index c0564237..6263b740 100644 --- a/integration/agent_test.go +++ b/integration/agent_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -49,7 +50,10 @@ type agentLog struct { SourceID string `json:"source_id"` } -const agentWaitTimeout = 2 * time.Minute +const ( + agentWaitTimeout = 2 * time.Minute + reportWaitTimeout = 30 * time.Second +) // startAgent runs coder agent in the background inside the Coder container // for the workspace the named case created. @@ -184,8 +188,9 @@ func countOf(lines []string, want string) int { return n } -// latestBuildTimings fetches the timings of the workspace's latest build. -func latestBuildTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID string) buildTimings { +// waitForTimings polls the latest build's timings until every named script +// has one timing for the stage. +func waitForTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID, stage string, displayNames ...string) buildTimings { t.Helper() var ws struct { LatestBuild struct { @@ -193,8 +198,25 @@ func latestBuildTimings(ctx context.Context, t *testing.T, ctrID, name, workspac } `json:"latest_build"` } require.NoError(t, apiGet(ctx, t, ctrID, name, "/api/v2/workspaces/"+workspaceID, &ws)) + // The agent reports its lifecycle state before the goroutines that send + // each timing have finished, so the first read after a state change can + // be empty. var timings buildTimings - require.NoError(t, apiGet(ctx, t, ctrID, name, "/api/v2/workspacebuilds/"+ws.LatestBuild.ID+"/timings", &timings)) + require.EventuallyWithT(t, func(c *assert.CollectT) { + timings = buildTimings{} + if !assert.NoError(c, apiGet(ctx, t, ctrID, name, "/api/v2/workspacebuilds/"+ws.LatestBuild.ID+"/timings", &timings)) { + return + } + for _, dn := range displayNames { + n := 0 + for _, tm := range timings.AgentScriptTimings { + if tm.Stage == stage && tm.DisplayName == dn { + n++ + } + } + assert.Equalf(c, 1, n, "expected one %s timing for %q, got %+v", stage, dn, timings.AgentScriptTimings) + } + }, reportWaitTimeout, time.Second) return timings } @@ -233,18 +255,23 @@ func requireDependencies(t *testing.T, agent workspaceAgent, displayName, addres require.Equalf(t, want, s.Dependencies, "%s dependencies", displayName) } -// agentLogLines returns the agent log lines for one script's log source. -func agentLogLines(ctx context.Context, t *testing.T, ctrID, name, agentID, sourceID string) []string { +// requireLogLine polls one script's agent log lines until want appears. +func requireLogLine(ctx context.Context, t *testing.T, ctrID, name, agentID, sourceID, want string) { t.Helper() - var logs []agentLog - require.NoError(t, apiGet(ctx, t, ctrID, name, "/api/v2/workspaceagents/"+agentID+"/logs", &logs)) - var lines []string - for _, l := range logs { - if l.SourceID == sourceID { - lines = append(lines, l.Output) + // Script logs are flushed after the agent reports its lifecycle state. + require.EventuallyWithT(t, func(c *assert.CollectT) { + var logs []agentLog + if !assert.NoError(c, apiGet(ctx, t, ctrID, name, "/api/v2/workspaceagents/"+agentID+"/logs", &logs)) { + return } - } - return lines + var lines []string + for _, l := range logs { + if l.SourceID == sourceID { + lines = append(lines, l.Output) + } + } + assert.Contains(c, lines, want) + }, reportWaitTimeout, time.Second) } func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { @@ -260,7 +287,7 @@ func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID string, out map[ requireLineAfter(t, lines, "post_setup start", "clone_repo end") requireLineAfter(t, lines, "clone_repo end", "start_code_server start") - timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") clone := timingFor(t, timings, "start", "Clone repo") install := timingFor(t, timings, "start", "Install tools") post := timingFor(t, timings, "start", "Post setup") @@ -293,7 +320,7 @@ func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID string, out require.NotContains(t, lines, "install_tools start", "skipped script must not run: %q", lines) requireLineAfter(t, lines, "post_setup start", "clone_repo end") - timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") clone := timingFor(t, timings, "start", "Clone repo") require.Equal(t, "exit_failure", clone.Status) require.Equal(t, int32(1), clone.ExitCode) @@ -303,8 +330,7 @@ func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID string, out require.Equal(t, "ok", timingFor(t, timings, "start", "Post setup").Status) require.Equal(t, "ok", timingFor(t, timings, "start", "code-server").Status) - installLines := agentLogLines(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID) - require.Contains(t, installLines, `Skipping script: dependency "Clone repo" failed.`) + requireLogLine(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID, `Skipping script: dependency "Clone repo" failed.`) requireDependencies(t, agent, "Install tools", "coder_script.install_tools", []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) @@ -322,7 +348,7 @@ func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID string, out lines := orderLogLines(ctx, t, ctrID, name) requireLineAfter(t, lines, "stop_db start", "save_state end") - timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "stop", "Save state", "Stop DB") save := timingFor(t, timings, "stop", "Save state") stop := timingFor(t, timings, "stop", "Stop DB") require.Equal(t, "ok", save.Status) @@ -349,7 +375,7 @@ func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID string, ou require.NotContains(t, lines, "install_tools start") require.NotContains(t, lines, "post_setup start") - timings := latestBuildTimings(ctx, t, ctrID, name, out["workspace.id"]) + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") clone := timingFor(t, timings, "start", "Clone repo") require.Equal(t, "canceled", clone.Status) require.Truef(t, clone.EndedAt.After(clone.StartedAt), "canceled running script must keep its real interval: %+v", clone) @@ -360,8 +386,6 @@ func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID string, ou } require.Equal(t, "ok", timingFor(t, timings, "start", "code-server").Status) - installLines := agentLogLines(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID) - require.Contains(t, installLines, "Canceled before starting.") - cloneLines := agentLogLines(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Clone repo").LogSourceID) - require.Contains(t, cloneLines, "Canceled while running.") + requireLogLine(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID, "Canceled before starting.") + requireLogLine(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Clone repo").LogSourceID, "Canceled while running.") } From e7fd688bc0629deb6824bd6e36705b57da8bfc92 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Mon, 19 Oct 2026 12:17:59 -0700 Subject: [PATCH 3/8] test(integration): wait for code-server to exit before canceling code-server starts in parallel with clone_repo, so a signal sent as soon as clone_repo has started can also cancel code-server and fail the ok assertion on its timing. --- integration/agent_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/integration/agent_test.go b/integration/agent_test.go index 6263b740..7ff20256 100644 --- a/integration/agent_test.go +++ b/integration/agent_test.go @@ -363,10 +363,12 @@ func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID string, ou const name = "coder-script-order-cancel" agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) startAgent(ctx, t, ctrID, name) + // code-server starts in parallel with clone_repo and must have exited + // before the signal, otherwise it is canceled too. require.Eventually(t, func() bool { o, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log 2>/dev/null", name)) - return rc == 0 && strings.Contains(o, "clone_repo start") - }, agentWaitTimeout, time.Second, "clone_repo never started") + return rc == 0 && strings.Contains(o, "clone_repo start") && strings.Contains(o, "start_code_server end") + }, agentWaitTimeout, time.Second, "clone_repo never started or code-server never finished") stopAgent(ctx, t, ctrID, name) agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") From 42b6e6ffab5b0b331c0760c3a7dfe87f5957a49d Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 20 Oct 2026 07:54:48 -0700 Subject: [PATCH 4/8] test(integration): name the failing script and drop log wording checks Check dependencies before order, print the agent log tail on a wrong lifecycle state, and add the script name to status assertions. The checks on Coder's skip and cancel log wording are removed so a wording change on Coder main cannot fail provider CI. --- integration/agent_test.go | 229 ++++++++++++++++++-------------- integration/integration_test.go | 35 +++-- 2 files changed, 156 insertions(+), 108 deletions(-) diff --git a/integration/agent_test.go b/integration/agent_test.go index 7ff20256..83058c31 100644 --- a/integration/agent_test.go +++ b/integration/agent_test.go @@ -13,26 +13,27 @@ import ( "github.com/stretchr/testify/require" ) -// These mirror the fields of the Coder API that the script ordering cases -// read. The package does not import codersdk, see the note on TestIntegration. -type agentScriptDependency struct { +// These mirror the codersdk types of the same name, keeping only the fields +// the script ordering cases read. The package does not import codersdk, see +// the note on TestIntegration. +type workspaceAgentScriptDependency struct { PrerequisiteResourceAddress string `json:"prerequisite_resource_address"` Requirement string `json:"requirement"` } -type agentScript struct { - DisplayName string `json:"display_name"` - LogSourceID string `json:"log_source_id"` - ResourceAddress string `json:"resource_address"` - Dependencies []agentScriptDependency `json:"dependencies"` +type workspaceAgentScript struct { + DisplayName string `json:"display_name"` + LogSourceID string `json:"log_source_id"` + ResourceAddress string `json:"resource_address"` + Dependencies []workspaceAgentScriptDependency `json:"dependencies"` } type workspaceAgent struct { - LifecycleState string `json:"lifecycle_state"` - Scripts []agentScript `json:"scripts"` + LifecycleState string `json:"lifecycle_state"` + Scripts []workspaceAgentScript `json:"scripts"` } -type scriptTiming struct { +type agentScriptTiming struct { StartedAt time.Time `json:"started_at"` EndedAt time.Time `json:"ended_at"` ExitCode int32 `json:"exit_code"` @@ -41,18 +42,15 @@ type scriptTiming struct { DisplayName string `json:"display_name"` } -type buildTimings struct { - AgentScriptTimings []scriptTiming `json:"agent_script_timings"` -} - -type agentLog struct { - Output string `json:"output"` - SourceID string `json:"source_id"` +type workspaceBuildTimings struct { + AgentScriptTimings []agentScriptTiming `json:"agent_script_timings"` } const ( agentWaitTimeout = 2 * time.Minute reportWaitTimeout = 30 * time.Second + // Matches the sleep in coder-script-order/main.tf. + cloneRepoSleep = 3 * time.Second ) // startAgent runs coder agent in the background inside the Coder container @@ -75,7 +73,7 @@ func startAgent(ctx context.Context, t *testing.T, ctrID, name string) { require.Equal(t, 0, rc, "start agent for %s", name) } -// stopAgent sends SIGTERM to the case's agent so it runs its stop scripts. +// stopAgent sends SIGTERM to the case's agent. func stopAgent(ctx context.Context, t *testing.T, ctrID, name string) { t.Helper() // Never signal the process by name: the server in the same container is @@ -84,6 +82,13 @@ func stopAgent(ctx context.Context, t *testing.T, ctrID, name string) { require.Equal(t, 0, rc, "stop agent for %s", name) } +// agentLogTail returns the last lines of the agent's own log file. +func agentLogTail(ctx context.Context, t *testing.T, ctrID, name string) string { + t.Helper() + tail, _ := execContainer(ctx, t, ctrID, fmt.Sprintf("tail -n 50 /tmp/%s-agent.log", name)) + return tail +} + // apiGet performs a GET against the Coder in the container as the workspace // owner and decodes the JSON body into out. func apiGet(ctx context.Context, t *testing.T, ctrID, name, path string, out any) error { @@ -150,14 +155,47 @@ func waitForAgent(ctx context.Context, t *testing.T, ctrID, name, agentID string } time.Sleep(time.Second) } - tail, _ := execContainer(ctx, t, ctrID, fmt.Sprintf("tail -n 50 /tmp/%s-agent.log", name)) t.Fatalf("agent %s did not reach %v in time: last state %q, last error %v\nagent log tail:\n%s", - name, want, agent.LifecycleState, lastErr, tail) + name, want, agent.LifecycleState, lastErr, agentLogTail(ctx, t, ctrID, name)) return agent } -// orderLogLines returns the lines the case's scripts appended to the durable -// file, in the order they were written. +// requireLifecycle fails with the agent log tail when the agent ended in a +// different state, since the log names the script that caused it. +func requireLifecycle(ctx context.Context, t *testing.T, ctrID, name string, agent workspaceAgent, want string) { + t.Helper() + if agent.LifecycleState != want { + t.Fatalf("agent %s ended in %q, want %q\nagent log tail:\n%s", + name, agent.LifecycleState, want, agentLogTail(ctx, t, ctrID, name)) + } +} + +// waitForOrderLog polls the case's order log until it contains every line in +// want. +func waitForOrderLog(ctx context.Context, t *testing.T, ctrID, name string, want ...string) { + t.Helper() + ok := assert.Eventually(t, func() bool { + // The condition runs on testify's goroutine, where a failed require + // would stall the poll instead of ending the test. + o, _, err := tryExecContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log 2>/dev/null", name)) + if err != nil { + return false + } + for _, w := range want { + if !strings.Contains(o, w) { + return false + } + } + return true + }, agentWaitTimeout, time.Second) + if !ok { + t.Fatalf("order log for %s never contained all of %q\nagent log tail:\n%s", + name, want, agentLogTail(ctx, t, ctrID, name)) + } +} + +// orderLogLines returns the lines the case's scripts appended to the order +// log, in the order they were written. func orderLogLines(ctx context.Context, t *testing.T, ctrID, name string) []string { t.Helper() out, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log", name)) @@ -190,7 +228,7 @@ func countOf(lines []string, want string) int { // waitForTimings polls the latest build's timings until every named script // has one timing for the stage. -func waitForTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID, stage string, displayNames ...string) buildTimings { +func waitForTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID, stage string, displayNames ...string) workspaceBuildTimings { t.Helper() var ws struct { LatestBuild struct { @@ -201,9 +239,9 @@ func waitForTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID, // The agent reports its lifecycle state before the goroutines that send // each timing have finished, so the first read after a state change can // be empty. - var timings buildTimings + var timings workspaceBuildTimings require.EventuallyWithT(t, func(c *assert.CollectT) { - timings = buildTimings{} + timings = workspaceBuildTimings{} if !assert.NoError(c, apiGet(ctx, t, ctrID, name, "/api/v2/workspacebuilds/"+ws.LatestBuild.ID+"/timings", &timings)) { return } @@ -221,9 +259,9 @@ func waitForTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID, } // timingFor returns the one timing with the given stage and display name. -func timingFor(t *testing.T, timings buildTimings, stage, displayName string) scriptTiming { +func timingFor(t *testing.T, timings workspaceBuildTimings, stage, displayName string) agentScriptTiming { t.Helper() - var found []scriptTiming + var found []agentScriptTiming for _, tm := range timings.AgentScriptTimings { if tm.Stage == stage && tm.DisplayName == displayName { found = append(found, tm) @@ -233,8 +271,18 @@ func timingFor(t *testing.T, timings buildTimings, stage, displayName string) sc return found[0] } -// scriptFor returns the agent's script with the given display name. -func scriptFor(t *testing.T, agent workspaceAgent, displayName string) agentScript { +func requireStatus(t *testing.T, tm agentScriptTiming, want string) { + t.Helper() + require.Equalf(t, want, tm.Status, "%s: %+v", tm.DisplayName, tm) +} + +func requireExitCode(t *testing.T, tm agentScriptTiming, want int32) { + t.Helper() + require.Equalf(t, want, tm.ExitCode, "%s: %+v", tm.DisplayName, tm) +} + +// scriptFor fails the test if no script has displayName. +func scriptFor(t *testing.T, agent workspaceAgent, displayName string) workspaceAgentScript { t.Helper() for _, s := range agent.Scripts { if s.DisplayName == displayName { @@ -242,44 +290,34 @@ func scriptFor(t *testing.T, agent workspaceAgent, displayName string) agentScri } } t.Fatalf("no script named %q in %+v", displayName, agent.Scripts) - return agentScript{} + return workspaceAgentScript{} } // requireDependencies asserts a script's resource address and its stored // dependencies. A field renamed on either side of the provider and Coder // contract shows up here as a script with no dependencies. -func requireDependencies(t *testing.T, agent workspaceAgent, displayName, address string, want []agentScriptDependency) { +func requireDependencies(t *testing.T, agent workspaceAgent, displayName, address string, want []workspaceAgentScriptDependency) { t.Helper() s := scriptFor(t, agent, displayName) require.Equalf(t, address, s.ResourceAddress, "%s resource_address", displayName) require.Equalf(t, want, s.Dependencies, "%s dependencies", displayName) } -// requireLogLine polls one script's agent log lines until want appears. -func requireLogLine(ctx context.Context, t *testing.T, ctrID, name, agentID, sourceID, want string) { - t.Helper() - // Script logs are flushed after the agent reports its lifecycle state. - require.EventuallyWithT(t, func(c *assert.CollectT) { - var logs []agentLog - if !assert.NoError(c, apiGet(ctx, t, ctrID, name, "/api/v2/workspaceagents/"+agentID+"/logs", &logs)) { - return - } - var lines []string - for _, l := range logs { - if l.SourceID == sourceID { - lines = append(lines, l.Output) - } - } - assert.Contains(c, lines, want) - }, reportWaitTimeout, time.Second) -} - -func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { - const name = "coder-script-order" +func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) startAgent(ctx, t, ctrID, name) agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") - require.Equal(t, "ready", agent.LifecycleState) + + // Checked before the order so a contract break is named as such rather + // than as a script running too early. + requireDependencies(t, agent, "Clone repo", "coder_script.clone_repo", nil) + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + requireDependencies(t, agent, "Post setup", "coder_script.post_setup", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) + requireDependencies(t, agent, "code-server", "coder_script.start_code_server", nil) + + requireLifecycle(ctx, t, ctrID, name, agent, "ready") lines := orderLogLines(ctx, t, ctrID, name) require.Len(t, lines, 8, "expected two lines per script: %q", lines) @@ -292,29 +330,25 @@ func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID string, out map[ install := timingFor(t, timings, "start", "Install tools") post := timingFor(t, timings, "start", "Post setup") code := timingFor(t, timings, "start", "code-server") - for _, tm := range []scriptTiming{clone, install, post, code} { - require.Equalf(t, "ok", tm.Status, "%s: %+v", tm.DisplayName, tm) - require.Equalf(t, int32(0), tm.ExitCode, "%s: %+v", tm.DisplayName, tm) + for _, tm := range []agentScriptTiming{clone, install, post, code} { + requireStatus(t, tm, "ok") + requireExitCode(t, tm, 0) } require.Falsef(t, install.StartedAt.Before(clone.EndedAt), "Install tools started %s before Clone repo ended %s", install.StartedAt, clone.EndedAt) require.Falsef(t, post.StartedAt.Before(clone.EndedAt), "Post setup started %s before Clone repo ended %s", post.StartedAt, clone.EndedAt) require.Truef(t, code.StartedAt.Before(clone.EndedAt), "code-server started %s after Clone repo ended %s", code.StartedAt, clone.EndedAt) - require.GreaterOrEqual(t, clone.EndedAt.Sub(clone.StartedAt), 3*time.Second) - - requireDependencies(t, agent, "Clone repo", "coder_script.clone_repo", nil) - requireDependencies(t, agent, "Install tools", "coder_script.install_tools", - []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) - requireDependencies(t, agent, "Post setup", "coder_script.post_setup", - []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) - requireDependencies(t, agent, "code-server", "coder_script.start_code_server", nil) + require.GreaterOrEqualf(t, clone.EndedAt.Sub(clone.StartedAt), cloneRepoSleep, "Clone repo duration: %+v", clone) } -func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { - const name = "coder-script-order-skip" +func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) startAgent(ctx, t, ctrID, name) agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") - require.Equal(t, "start_error", agent.LifecycleState) + + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + + requireLifecycle(ctx, t, ctrID, name, agent, "start_error") lines := orderLogLines(ctx, t, ctrID, name) require.NotContains(t, lines, "install_tools start", "skipped script must not run: %q", lines) @@ -322,28 +356,26 @@ func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID string, out timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") clone := timingFor(t, timings, "start", "Clone repo") - require.Equal(t, "exit_failure", clone.Status) - require.Equal(t, int32(1), clone.ExitCode) + requireStatus(t, clone, "exit_failure") + requireExitCode(t, clone, 1) install := timingFor(t, timings, "start", "Install tools") - require.Equal(t, "skipped", install.Status) + requireStatus(t, install, "skipped") require.Truef(t, install.StartedAt.Equal(install.EndedAt), "skipped timing must be zero-length: %+v", install) - require.Equal(t, "ok", timingFor(t, timings, "start", "Post setup").Status) - require.Equal(t, "ok", timingFor(t, timings, "start", "code-server").Status) - - requireLogLine(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID, `Skipping script: dependency "Clone repo" failed.`) - - requireDependencies(t, agent, "Install tools", "coder_script.install_tools", - []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + requireStatus(t, timingFor(t, timings, "start", "Post setup"), "ok") + requireStatus(t, timingFor(t, timings, "start", "code-server"), "ok") } -func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { - const name = "coder-script-order-stop" +func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) startAgent(ctx, t, ctrID, name) - waitForAgent(ctx, t, ctrID, name, agentID, "ready") + agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready") + + requireDependencies(t, agent, "Stop DB", "coder_script.stop_db", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.save_state", Requirement: "success"}}) + stopAgent(ctx, t, ctrID, name) - agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") - require.Equal(t, "off", agent.LifecycleState) + agent = waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") + requireLifecycle(ctx, t, ctrID, name, agent, "off") lines := orderLogLines(ctx, t, ctrID, name) requireLineAfter(t, lines, "stop_db start", "save_state end") @@ -351,27 +383,27 @@ func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID string, out timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "stop", "Save state", "Stop DB") save := timingFor(t, timings, "stop", "Save state") stop := timingFor(t, timings, "stop", "Stop DB") - require.Equal(t, "ok", save.Status) - require.Equal(t, "ok", stop.Status) + requireStatus(t, save, "ok") + requireStatus(t, stop, "ok") require.Falsef(t, stop.StartedAt.Before(save.EndedAt), "Stop DB started %s before Save state ended %s", stop.StartedAt, save.EndedAt) - - requireDependencies(t, agent, "Stop DB", "coder_script.stop_db", - []agentScriptDependency{{PrerequisiteResourceAddress: "coder_script.save_state", Requirement: "success"}}) } -func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID string, out map[string]string) { - const name = "coder-script-order-cancel" +func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) startAgent(ctx, t, ctrID, name) // code-server starts in parallel with clone_repo and must have exited // before the signal, otherwise it is canceled too. - require.Eventually(t, func() bool { - o, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log 2>/dev/null", name)) - return rc == 0 && strings.Contains(o, "clone_repo start") && strings.Contains(o, "start_code_server end") - }, agentWaitTimeout, time.Second, "clone_repo never started or code-server never finished") + waitForOrderLog(ctx, t, ctrID, name, "clone_repo start", "start_code_server end") stopAgent(ctx, t, ctrID, name) agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + requireDependencies(t, agent, "Post setup", "coder_script.post_setup", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) + + requireLifecycle(ctx, t, ctrID, name, agent, "off") + lines := orderLogLines(ctx, t, ctrID, name) require.NotContains(t, lines, "clone_repo end") require.NotContains(t, lines, "install_tools start") @@ -379,15 +411,12 @@ func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID string, ou timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") clone := timingFor(t, timings, "start", "Clone repo") - require.Equal(t, "canceled", clone.Status) + requireStatus(t, clone, "canceled") require.Truef(t, clone.EndedAt.After(clone.StartedAt), "canceled running script must keep its real interval: %+v", clone) for _, n := range []string{"Install tools", "Post setup"} { tm := timingFor(t, timings, "start", n) - require.Equalf(t, "canceled", tm.Status, "%s: %+v", n, tm) + requireStatus(t, tm, "canceled") require.Truef(t, tm.StartedAt.Equal(tm.EndedAt), "%s timing must be zero-length: %+v", n, tm) } - require.Equal(t, "ok", timingFor(t, timings, "start", "code-server").Status) - - requireLogLine(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Install tools").LogSourceID, "Canceled before starting.") - requireLogLine(ctx, t, ctrID, name, agentID, scriptFor(t, agent, "Clone repo").LogSourceID, "Canceled while running.") + requireStatus(t, timingFor(t, timings, "start", "code-server"), "ok") } diff --git a/integration/integration_test.go b/integration/integration_test.go index ecab43ed..75fcc188 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -70,7 +70,7 @@ func TestIntegration(t *testing.T) { expectedOutput map[string]string // Optional. Runs after assertOutput for cases that need to inspect the // workspace after provisioning. - verify func(ctx context.Context, t *testing.T, ctrID string, output map[string]string) + verify func(ctx context.Context, t *testing.T, ctrID, name string, output map[string]string) }{ { name: "test-data-source", @@ -325,7 +325,7 @@ func TestIntegration(t *testing.T) { require.NoError(t, json.NewDecoder(strings.NewReader(out)).Decode(&actual)) assertOutput(t, tt.expectedOutput, actual) if tt.verify != nil { - tt.verify(ctx, t, ctrID, actual) + tt.verify(ctx, t, ctrID, tt.name, actual) } }) } @@ -459,10 +459,21 @@ func ensureImage(ctx context.Context, t *testing.T, cli *client.Client, ref stri // execContainer executes the given command in the given container and returns // the output and the exit code of the command. func execContainer(ctx context.Context, t *testing.T, containerID, command string) (string, int) { + t.Helper() + out, rc, err := tryExecContainer(ctx, t, containerID, command) + require.NoError(t, err) + return out, rc +} + +// tryExecContainer is execContainer for callers that cannot fail the test, +// such as a poll condition running on another goroutine. +func tryExecContainer(ctx context.Context, t *testing.T, containerID, command string) (string, int, error) { t.Helper() t.Logf("exec container cmd: %q", command) cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) - require.NoError(t, err, "connect to docker") + if err != nil { + return "", 0, fmt.Errorf("connect to docker: %w", err) + } defer cli.Close() execConfig := types.ExecConfig{ AttachStdout: true, @@ -470,18 +481,26 @@ func execContainer(ctx context.Context, t *testing.T, containerID, command strin Cmd: []string{"/bin/sh", "-c", command}, } ex, err := cli.ContainerExecCreate(ctx, containerID, execConfig) - require.NoError(t, err, "create container exec") + if err != nil { + return "", 0, fmt.Errorf("create container exec: %w", err) + } resp, err := cli.ContainerExecAttach(ctx, ex.ID, types.ExecStartCheck{}) - require.NoError(t, err, "attach to container exec") + if err != nil { + return "", 0, fmt.Errorf("attach to container exec: %w", err) + } defer resp.Close() var buf bytes.Buffer _, err = stdcopy.StdCopy(&buf, &buf, resp.Reader) - require.NoError(t, err, "read stdout") + if err != nil { + return "", 0, fmt.Errorf("read stdout: %w", err) + } out := buf.String() t.Log("exec container output:\n" + out) execResp, err := cli.ContainerExecInspect(ctx, ex.ID) - require.NoError(t, err, "get exec exit code") - return out, execResp.ExitCode + if err != nil { + return "", 0, fmt.Errorf("get exec exit code: %w", err) + } + return out, execResp.ExitCode, nil } // assertOutput asserts that, for each key-value pair in expected: From 144bfaaaa8cec565bc44608703f1c667a088e6e9 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 20 Oct 2026 07:54:55 -0700 Subject: [PATCH 5/8] test(integration): derive case paths from output_path and remove dead values Templates name their files after var.output_path so a copied template cannot share one. clone_repo waits for the unordered script instead of a sleep. Unused agent.id output, warm script, timeout and x/exp import removed. --- go.mod | 1 - go.sum | 2 -- integration/coder-script-order-cancel/main.tf | 18 ++++++------ integration/coder-script-order-compat/main.tf | 4 +-- integration/coder-script-order-skip/main.tf | 19 +++++++------ integration/coder-script-order-stop/main.tf | 28 ++++++------------- integration/coder-script-order/main.tf | 22 +++++++++------ integration/integration_test.go | 24 ++++++++-------- 8 files changed, 55 insertions(+), 63 deletions(-) diff --git a/go.mod b/go.mod index 3b175959..a3d84d3d 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,6 @@ require ( github.com/mitchellh/mapstructure v1.5.0 github.com/robfig/cron/v3 v3.0.1 github.com/stretchr/testify v1.11.1 - golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8 golang.org/x/mod v0.38.0 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 ) diff --git a/go.sum b/go.sum index be5dbdd5..52a811ad 100644 --- a/go.sum +++ b/go.sum @@ -230,8 +230,6 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= -golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8 h1:yixxcjnhBmY0nkL253HFVIm0JsFHwrHdT3Yh6szTnfY= -golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8/go.mod h1:jj3sYF3dwk5D+ghuXyeI3r5MFf+NT2An6/9dOA95KSI= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= diff --git a/integration/coder-script-order-cancel/main.tf b/integration/coder-script-order-cancel/main.tf index 0354bde5..a1ec5cc7 100644 --- a/integration/coder-script-order-cancel/main.tf +++ b/integration/coder-script-order-cancel/main.tf @@ -18,8 +18,15 @@ resource "coder_agent" "main" { arch = data.coder_provisioner.me.arch } +variable "output_path" { + type = string +} + +# Every file the test reads is named after the output path, so a copied +# template cannot share a file with another case. locals { - order_log = "/tmp/coder-script-order-cancel.log" + case = trimsuffix(var.output_path, ".json") + order_log = "${local.case}.log" } # Runs long enough for the test to stop the agent while the dependents are @@ -78,26 +85,21 @@ data "coder_script_order" "bootstrap" { } } -variable "output_path" { - type = string -} - resource "local_file" "output" { filename = var.output_path content = jsonencode({ "workspace.id" = data.coder_workspace.me.id - "agent.id" = coder_agent.main.id "script_order.id" = data.coder_script_order.bootstrap.id "script_order.rule.#" = tostring(length(data.coder_script_order.bootstrap.rule)) }) } resource "local_sensitive_file" "agent_token" { - filename = "/tmp/coder-script-order-cancel.agent-token" + filename = "${local.case}.agent-token" content = coder_agent.main.token } resource "local_sensitive_file" "session_token" { - filename = "/tmp/coder-script-order-cancel.session-token" + filename = "${local.case}.session-token" content = data.coder_workspace_owner.me.session_token } diff --git a/integration/coder-script-order-compat/main.tf b/integration/coder-script-order-compat/main.tf index c044a58f..6d5a6aef 100644 --- a/integration/coder-script-order-compat/main.tf +++ b/integration/coder-script-order-compat/main.tf @@ -41,8 +41,8 @@ variable "output_path" { type = string } -# Coder drops an agent that no resource references. Writing the agent ID -# here makes this file that resource. +# Coder does not count coder_script as a resource that references the +# agent, so without the agent ID here the agent is dropped. resource "local_file" "output" { filename = var.output_path content = jsonencode({ diff --git a/integration/coder-script-order-skip/main.tf b/integration/coder-script-order-skip/main.tf index 40277c79..d76d7ebc 100644 --- a/integration/coder-script-order-skip/main.tf +++ b/integration/coder-script-order-skip/main.tf @@ -18,8 +18,15 @@ resource "coder_agent" "main" { arch = data.coder_provisioner.me.arch } +variable "output_path" { + type = string +} + +# Every file the test reads is named after the output path, so a copied +# template cannot share a file with another case. locals { - order_log = "/tmp/coder-script-order-skip.log" + case = trimsuffix(var.output_path, ".json") + order_log = "${local.case}.log" } # Fails on purpose so that install_tools is skipped and post_setup still runs. @@ -27,7 +34,6 @@ resource "coder_script" "clone_repo" { agent_id = coder_agent.main.id display_name = "Clone repo" run_on_start = true - timeout = 30 script = <<-EOT echo "clone_repo start" >> ${local.order_log} echo "clone_repo end" >> ${local.order_log} @@ -77,26 +83,21 @@ data "coder_script_order" "bootstrap" { } } -variable "output_path" { - type = string -} - resource "local_file" "output" { filename = var.output_path content = jsonencode({ "workspace.id" = data.coder_workspace.me.id - "agent.id" = coder_agent.main.id "script_order.id" = data.coder_script_order.bootstrap.id "script_order.rule.#" = tostring(length(data.coder_script_order.bootstrap.rule)) }) } resource "local_sensitive_file" "agent_token" { - filename = "/tmp/coder-script-order-skip.agent-token" + filename = "${local.case}.agent-token" content = coder_agent.main.token } resource "local_sensitive_file" "session_token" { - filename = "/tmp/coder-script-order-skip.session-token" + filename = "${local.case}.session-token" content = data.coder_workspace_owner.me.session_token } diff --git a/integration/coder-script-order-stop/main.tf b/integration/coder-script-order-stop/main.tf index b348e938..c45def2d 100644 --- a/integration/coder-script-order-stop/main.tf +++ b/integration/coder-script-order-stop/main.tf @@ -18,20 +18,15 @@ resource "coder_agent" "main" { arch = data.coder_provisioner.me.arch } -locals { - order_log = "/tmp/coder-script-order-stop.log" +variable "output_path" { + type = string } -# One startup script so the agent reaches the ready state before the test -# stops it. -resource "coder_script" "warm" { - agent_id = coder_agent.main.id - display_name = "Warm" - run_on_start = true - script = <<-EOT - echo "warm start" >> ${local.order_log} - echo "warm end" >> ${local.order_log} - EOT +# Every file the test reads is named after the output path, so a copied +# template cannot share a file with another case. +locals { + case = trimsuffix(var.output_path, ".json") + order_log = "${local.case}.log" } resource "coder_script" "save_state" { @@ -63,26 +58,21 @@ data "coder_script_order" "shutdown" { } } -variable "output_path" { - type = string -} - resource "local_file" "output" { filename = var.output_path content = jsonencode({ "workspace.id" = data.coder_workspace.me.id - "agent.id" = coder_agent.main.id "script_order.id" = data.coder_script_order.shutdown.id "script_order.rule.#" = tostring(length(data.coder_script_order.shutdown.rule)) }) } resource "local_sensitive_file" "agent_token" { - filename = "/tmp/coder-script-order-stop.agent-token" + filename = "${local.case}.agent-token" content = coder_agent.main.token } resource "local_sensitive_file" "session_token" { - filename = "/tmp/coder-script-order-stop.session-token" + filename = "${local.case}.session-token" content = data.coder_workspace_owner.me.session_token } diff --git a/integration/coder-script-order/main.tf b/integration/coder-script-order/main.tf index 3f10990b..f925db64 100644 --- a/integration/coder-script-order/main.tf +++ b/integration/coder-script-order/main.tf @@ -18,11 +18,19 @@ resource "coder_agent" "main" { arch = data.coder_provisioner.me.arch } +variable "output_path" { + type = string +} + +# Every file the test reads is named after the output path, so a copied +# template cannot share a file with another case. locals { - order_log = "/tmp/coder-script-order.log" + case = trimsuffix(var.output_path, ".json") + order_log = "${local.case}.log" } -# The sleep gives the unordered script time to start before this one ends. +# Waits for the unordered script so the test can assert it started before +# this one ended. The sleep keeps the timing interval measurable. resource "coder_script" "clone_repo" { agent_id = coder_agent.main.id display_name = "Clone repo" @@ -30,6 +38,7 @@ resource "coder_script" "clone_repo" { timeout = 30 script = <<-EOT echo "clone_repo start" >> ${local.order_log} + until grep -q "start_code_server start" ${local.order_log}; do sleep 0.1; done sleep 3 echo "clone_repo end" >> ${local.order_log} EOT @@ -77,15 +86,10 @@ data "coder_script_order" "bootstrap" { } } -variable "output_path" { - type = string -} - resource "local_file" "output" { filename = var.output_path content = jsonencode({ "workspace.id" = data.coder_workspace.me.id - "agent.id" = coder_agent.main.id "script_order.id" = data.coder_script_order.bootstrap.id "script_order.rule.#" = tostring(length(data.coder_script_order.bootstrap.rule)) }) @@ -94,11 +98,11 @@ resource "local_file" "output" { # The test reads these files inside the container. Keeping the tokens out of # the output JSON keeps them out of the test log. resource "local_sensitive_file" "agent_token" { - filename = "/tmp/coder-script-order.agent-token" + filename = "${local.case}.agent-token" content = coder_agent.main.token } resource "local_sensitive_file" "session_token" { - filename = "/tmp/coder-script-order.session-token" + filename = "${local.case}.session-token" content = data.coder_workspace_owner.me.session_token } diff --git a/integration/integration_test.go b/integration/integration_test.go index 75fcc188..3dff8f70 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -10,6 +10,7 @@ import ( "os/signal" "path/filepath" "runtime" + "slices" "strconv" "strings" "sync" @@ -23,7 +24,6 @@ import ( "github.com/docker/docker/pkg/stdcopy" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/exp/slices" "golang.org/x/mod/semver" ) @@ -61,6 +61,9 @@ func TestIntegration(t *testing.T) { t.Cleanup(cancel) ctrID := setup(ctx, t, t.Name(), coderImg, coderVersion) + // Anticipated version, update as required. + const scriptOrderMinVersion = "v2.39.0" + for _, tt := range []struct { // Name of the folder under `integration/` containing a test template name string @@ -68,8 +71,7 @@ func TestIntegration(t *testing.T) { minVersion string // map of string to regex to be passed to assertOutput() expectedOutput map[string]string - // Optional. Runs after assertOutput for cases that need to inspect the - // workspace after provisioning. + // Optional. Runs after assertOutput. verify func(ctx context.Context, t *testing.T, ctrID, name string, output map[string]string) }{ { @@ -230,10 +232,9 @@ func TestIntegration(t *testing.T) { }, { name: "coder-script-order", - minVersion: "v2.39.0", + minVersion: scriptOrderMinVersion, expectedOutput: map[string]string{ "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, - "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, "script_order.id": `.+`, "script_order.rule.#": `^2$`, }, @@ -241,10 +242,9 @@ func TestIntegration(t *testing.T) { }, { name: "coder-script-order-skip", - minVersion: "v2.39.0", + minVersion: scriptOrderMinVersion, expectedOutput: map[string]string{ "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, - "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, "script_order.id": `.+`, "script_order.rule.#": `^2$`, }, @@ -252,10 +252,9 @@ func TestIntegration(t *testing.T) { }, { name: "coder-script-order-stop", - minVersion: "v2.39.0", + minVersion: scriptOrderMinVersion, expectedOutput: map[string]string{ "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, - "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, "script_order.id": `.+`, "script_order.rule.#": `^1$`, }, @@ -263,18 +262,17 @@ func TestIntegration(t *testing.T) { }, { name: "coder-script-order-cancel", - minVersion: "v2.39.0", + minVersion: scriptOrderMinVersion, expectedOutput: map[string]string{ "workspace.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, - "agent.id": `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, "script_order.id": `.+`, "script_order.rule.#": `^2$`, }, verify: verifyScriptOrderCancel, }, { - // Runs on every Coder version. A Coder older than the data source - // ignores it and must still import and build the template. + // A Coder older than the data source ignores it and must still + // import and build the template. name: "coder-script-order-compat", minVersion: "v0.0.0", expectedOutput: map[string]string{ From bc88ac83a1d75f5aa525d34636ac129b56b31e4f Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 20 Oct 2026 07:54:55 -0700 Subject: [PATCH 6/8] docs: say which integration cases skip and when --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 31457029..81464d2e 100644 --- a/README.md +++ b/README.md @@ -80,12 +80,14 @@ To run these integration tests locally: docker pull ghcr.io/coder/coder-preview:main-x.y.z-devel-abcd1234 ``` +1. On macOS, build the provider for Linux, since the binary runs inside the Coder container. For example `GOOS=linux GOARCH=arm64 CGO_ENABLED=0 go build .` on Apple silicon. Repeat this after editing provider sources, otherwise `make` rebuilds the binary for macOS. + 1. Run `CODER_IMAGE=ghcr.io/coder/coder-preview CODER_VERSION=main-x.y.z-devel-abcd1234 make test-integration`. -The provider binary runs inside the Coder container, so on macOS build it for Linux first, for example `GOOS=linux GOARCH=arm64 CGO_ENABLED=0 go build .` on Apple silicon. -The `coder-script-order*` cases start a workspace agent inside the Coder container and read its results through the API. +The four agent cases (`coder-script-order`, `-skip`, `-stop` and `-cancel`) start a workspace agent inside the Coder container and read its results through the API. While a run is in progress the agent's log is at `/tmp/-agent.log` inside the container. -These cases skip on Coder images older than v2.39, the first release that enforces script ordering. +They run only when `CODER_VERSION` is `latest` or a release tag at or above `v2.39.0`, the first release that enforces script ordering. A preview tag like the one above does not compare as a release and skips them, so use `CODER_VERSION=latest` to run them against the newest preview image. +The `coder-script-order-compat` case runs on every image. > [!Note] > You can specify `CODER_IMAGE` if the Coder image you wish to test is hosted somewhere other than `ghcr.io/coder/coder`. From f3c3c19dabd4a990b534fff5f5dd2c621bb81189 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 20 Oct 2026 13:28:24 -0700 Subject: [PATCH 7/8] test(integration): split script ordering cases from the agent helpers agent_test.go keeps the helpers any case with a live agent can reuse: starting and stopping the agent, reading the API, waiting for a lifecycle state and reading build timings. script_order_test.go holds the order log helpers, the dependency assertion and the four verify functions, which only make sense for the script ordering templates. No code changed, only its location. --- integration/agent_test.go | 186 ---------------------------- integration/script_order_test.go | 200 +++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 186 deletions(-) create mode 100644 integration/script_order_test.go diff --git a/integration/agent_test.go b/integration/agent_test.go index 83058c31..7888b279 100644 --- a/integration/agent_test.go +++ b/integration/agent_test.go @@ -49,8 +49,6 @@ type workspaceBuildTimings struct { const ( agentWaitTimeout = 2 * time.Minute reportWaitTimeout = 30 * time.Second - // Matches the sleep in coder-script-order/main.tf. - cloneRepoSleep = 3 * time.Second ) // startAgent runs coder agent in the background inside the Coder container @@ -170,62 +168,6 @@ func requireLifecycle(ctx context.Context, t *testing.T, ctrID, name string, age } } -// waitForOrderLog polls the case's order log until it contains every line in -// want. -func waitForOrderLog(ctx context.Context, t *testing.T, ctrID, name string, want ...string) { - t.Helper() - ok := assert.Eventually(t, func() bool { - // The condition runs on testify's goroutine, where a failed require - // would stall the poll instead of ending the test. - o, _, err := tryExecContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log 2>/dev/null", name)) - if err != nil { - return false - } - for _, w := range want { - if !strings.Contains(o, w) { - return false - } - } - return true - }, agentWaitTimeout, time.Second) - if !ok { - t.Fatalf("order log for %s never contained all of %q\nagent log tail:\n%s", - name, want, agentLogTail(ctx, t, ctrID, name)) - } -} - -// orderLogLines returns the lines the case's scripts appended to the order -// log, in the order they were written. -func orderLogLines(ctx context.Context, t *testing.T, ctrID, name string) []string { - t.Helper() - out, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log", name)) - require.Equal(t, 0, rc, "read order log for %s", name) - return strings.Split(strings.TrimSpace(out), "\n") -} - -// requireLineAfter asserts that later appears exactly once and after earlier. -func requireLineAfter(t *testing.T, lines []string, later, earlier string) { - t.Helper() - // Only relative order is checked. Scripts that start together may write - // in either order. - laterIdx := slices.Index(lines, later) - earlierIdx := slices.Index(lines, earlier) - require.NotEqualf(t, -1, laterIdx, "line %q missing from %q", later, lines) - require.NotEqualf(t, -1, earlierIdx, "line %q missing from %q", earlier, lines) - require.Equalf(t, 1, countOf(lines, later), "line %q appears more than once in %q", later, lines) - require.Greaterf(t, laterIdx, earlierIdx, "%q (line %d) should come after %q (line %d): %q", later, laterIdx, earlier, earlierIdx, lines) -} - -func countOf(lines []string, want string) int { - n := 0 - for _, l := range lines { - if l == want { - n++ - } - } - return n -} - // waitForTimings polls the latest build's timings until every named script // has one timing for the stage. func waitForTimings(ctx context.Context, t *testing.T, ctrID, name, workspaceID, stage string, displayNames ...string) workspaceBuildTimings { @@ -292,131 +234,3 @@ func scriptFor(t *testing.T, agent workspaceAgent, displayName string) workspace t.Fatalf("no script named %q in %+v", displayName, agent.Scripts) return workspaceAgentScript{} } - -// requireDependencies asserts a script's resource address and its stored -// dependencies. A field renamed on either side of the provider and Coder -// contract shows up here as a script with no dependencies. -func requireDependencies(t *testing.T, agent workspaceAgent, displayName, address string, want []workspaceAgentScriptDependency) { - t.Helper() - s := scriptFor(t, agent, displayName) - require.Equalf(t, address, s.ResourceAddress, "%s resource_address", displayName) - require.Equalf(t, want, s.Dependencies, "%s dependencies", displayName) -} - -func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { - agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) - startAgent(ctx, t, ctrID, name) - agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") - - // Checked before the order so a contract break is named as such rather - // than as a script running too early. - requireDependencies(t, agent, "Clone repo", "coder_script.clone_repo", nil) - requireDependencies(t, agent, "Install tools", "coder_script.install_tools", - []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) - requireDependencies(t, agent, "Post setup", "coder_script.post_setup", - []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) - requireDependencies(t, agent, "code-server", "coder_script.start_code_server", nil) - - requireLifecycle(ctx, t, ctrID, name, agent, "ready") - - lines := orderLogLines(ctx, t, ctrID, name) - require.Len(t, lines, 8, "expected two lines per script: %q", lines) - requireLineAfter(t, lines, "install_tools start", "clone_repo end") - requireLineAfter(t, lines, "post_setup start", "clone_repo end") - requireLineAfter(t, lines, "clone_repo end", "start_code_server start") - - timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") - clone := timingFor(t, timings, "start", "Clone repo") - install := timingFor(t, timings, "start", "Install tools") - post := timingFor(t, timings, "start", "Post setup") - code := timingFor(t, timings, "start", "code-server") - for _, tm := range []agentScriptTiming{clone, install, post, code} { - requireStatus(t, tm, "ok") - requireExitCode(t, tm, 0) - } - require.Falsef(t, install.StartedAt.Before(clone.EndedAt), "Install tools started %s before Clone repo ended %s", install.StartedAt, clone.EndedAt) - require.Falsef(t, post.StartedAt.Before(clone.EndedAt), "Post setup started %s before Clone repo ended %s", post.StartedAt, clone.EndedAt) - require.Truef(t, code.StartedAt.Before(clone.EndedAt), "code-server started %s after Clone repo ended %s", code.StartedAt, clone.EndedAt) - require.GreaterOrEqualf(t, clone.EndedAt.Sub(clone.StartedAt), cloneRepoSleep, "Clone repo duration: %+v", clone) -} - -func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { - agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) - startAgent(ctx, t, ctrID, name) - agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") - - requireDependencies(t, agent, "Install tools", "coder_script.install_tools", - []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) - - requireLifecycle(ctx, t, ctrID, name, agent, "start_error") - - lines := orderLogLines(ctx, t, ctrID, name) - require.NotContains(t, lines, "install_tools start", "skipped script must not run: %q", lines) - requireLineAfter(t, lines, "post_setup start", "clone_repo end") - - timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") - clone := timingFor(t, timings, "start", "Clone repo") - requireStatus(t, clone, "exit_failure") - requireExitCode(t, clone, 1) - install := timingFor(t, timings, "start", "Install tools") - requireStatus(t, install, "skipped") - require.Truef(t, install.StartedAt.Equal(install.EndedAt), "skipped timing must be zero-length: %+v", install) - requireStatus(t, timingFor(t, timings, "start", "Post setup"), "ok") - requireStatus(t, timingFor(t, timings, "start", "code-server"), "ok") -} - -func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { - agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) - startAgent(ctx, t, ctrID, name) - agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready") - - requireDependencies(t, agent, "Stop DB", "coder_script.stop_db", - []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.save_state", Requirement: "success"}}) - - stopAgent(ctx, t, ctrID, name) - agent = waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") - requireLifecycle(ctx, t, ctrID, name, agent, "off") - - lines := orderLogLines(ctx, t, ctrID, name) - requireLineAfter(t, lines, "stop_db start", "save_state end") - - timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "stop", "Save state", "Stop DB") - save := timingFor(t, timings, "stop", "Save state") - stop := timingFor(t, timings, "stop", "Stop DB") - requireStatus(t, save, "ok") - requireStatus(t, stop, "ok") - require.Falsef(t, stop.StartedAt.Before(save.EndedAt), "Stop DB started %s before Save state ended %s", stop.StartedAt, save.EndedAt) -} - -func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { - agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) - startAgent(ctx, t, ctrID, name) - // code-server starts in parallel with clone_repo and must have exited - // before the signal, otherwise it is canceled too. - waitForOrderLog(ctx, t, ctrID, name, "clone_repo start", "start_code_server end") - stopAgent(ctx, t, ctrID, name) - agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") - - requireDependencies(t, agent, "Install tools", "coder_script.install_tools", - []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) - requireDependencies(t, agent, "Post setup", "coder_script.post_setup", - []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) - - requireLifecycle(ctx, t, ctrID, name, agent, "off") - - lines := orderLogLines(ctx, t, ctrID, name) - require.NotContains(t, lines, "clone_repo end") - require.NotContains(t, lines, "install_tools start") - require.NotContains(t, lines, "post_setup start") - - timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") - clone := timingFor(t, timings, "start", "Clone repo") - requireStatus(t, clone, "canceled") - require.Truef(t, clone.EndedAt.After(clone.StartedAt), "canceled running script must keep its real interval: %+v", clone) - for _, n := range []string{"Install tools", "Post setup"} { - tm := timingFor(t, timings, "start", n) - requireStatus(t, tm, "canceled") - require.Truef(t, tm.StartedAt.Equal(tm.EndedAt), "%s timing must be zero-length: %+v", n, tm) - } - requireStatus(t, timingFor(t, timings, "start", "code-server"), "ok") -} diff --git a/integration/script_order_test.go b/integration/script_order_test.go new file mode 100644 index 00000000..66f874be --- /dev/null +++ b/integration/script_order_test.go @@ -0,0 +1,200 @@ +package integration + +import ( + "context" + "fmt" + "slices" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Matches the sleep in coder-script-order/main.tf. +const cloneRepoSleep = 3 * time.Second + +// waitForOrderLog polls the case's order log until it contains every line in +// want. +func waitForOrderLog(ctx context.Context, t *testing.T, ctrID, name string, want ...string) { + t.Helper() + ok := assert.Eventually(t, func() bool { + // The condition runs on testify's goroutine, where a failed require + // would stall the poll instead of ending the test. + o, _, err := tryExecContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log 2>/dev/null", name)) + if err != nil { + return false + } + for _, w := range want { + if !strings.Contains(o, w) { + return false + } + } + return true + }, agentWaitTimeout, time.Second) + if !ok { + t.Fatalf("order log for %s never contained all of %q\nagent log tail:\n%s", + name, want, agentLogTail(ctx, t, ctrID, name)) + } +} + +// orderLogLines returns the lines the case's scripts appended to the order +// log, in the order they were written. +func orderLogLines(ctx context.Context, t *testing.T, ctrID, name string) []string { + t.Helper() + out, rc := execContainer(ctx, t, ctrID, fmt.Sprintf("cat /tmp/%s.log", name)) + require.Equal(t, 0, rc, "read order log for %s", name) + return strings.Split(strings.TrimSpace(out), "\n") +} + +// requireLineAfter asserts that later appears exactly once and after earlier. +func requireLineAfter(t *testing.T, lines []string, later, earlier string) { + t.Helper() + // Only relative order is checked. Scripts that start together may write + // in either order. + laterIdx := slices.Index(lines, later) + earlierIdx := slices.Index(lines, earlier) + require.NotEqualf(t, -1, laterIdx, "line %q missing from %q", later, lines) + require.NotEqualf(t, -1, earlierIdx, "line %q missing from %q", earlier, lines) + require.Equalf(t, 1, countOf(lines, later), "line %q appears more than once in %q", later, lines) + require.Greaterf(t, laterIdx, earlierIdx, "%q (line %d) should come after %q (line %d): %q", later, laterIdx, earlier, earlierIdx, lines) +} + +func countOf(lines []string, want string) int { + n := 0 + for _, l := range lines { + if l == want { + n++ + } + } + return n +} + +// requireDependencies asserts a script's resource address and its stored +// dependencies. A field renamed on either side of the provider and Coder +// contract shows up here as a script with no dependencies. +func requireDependencies(t *testing.T, agent workspaceAgent, displayName, address string, want []workspaceAgentScriptDependency) { + t.Helper() + s := scriptFor(t, agent, displayName) + require.Equalf(t, address, s.ResourceAddress, "%s resource_address", displayName) + require.Equalf(t, want, s.Dependencies, "%s dependencies", displayName) +} + +func verifyScriptOrder(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") + + // Checked before the order so a contract break is named as such rather + // than as a script running too early. + requireDependencies(t, agent, "Clone repo", "coder_script.clone_repo", nil) + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + requireDependencies(t, agent, "Post setup", "coder_script.post_setup", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) + requireDependencies(t, agent, "code-server", "coder_script.start_code_server", nil) + + requireLifecycle(ctx, t, ctrID, name, agent, "ready") + + lines := orderLogLines(ctx, t, ctrID, name) + require.Len(t, lines, 8, "expected two lines per script: %q", lines) + requireLineAfter(t, lines, "install_tools start", "clone_repo end") + requireLineAfter(t, lines, "post_setup start", "clone_repo end") + requireLineAfter(t, lines, "clone_repo end", "start_code_server start") + + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") + clone := timingFor(t, timings, "start", "Clone repo") + install := timingFor(t, timings, "start", "Install tools") + post := timingFor(t, timings, "start", "Post setup") + code := timingFor(t, timings, "start", "code-server") + for _, tm := range []agentScriptTiming{clone, install, post, code} { + requireStatus(t, tm, "ok") + requireExitCode(t, tm, 0) + } + require.Falsef(t, install.StartedAt.Before(clone.EndedAt), "Install tools started %s before Clone repo ended %s", install.StartedAt, clone.EndedAt) + require.Falsef(t, post.StartedAt.Before(clone.EndedAt), "Post setup started %s before Clone repo ended %s", post.StartedAt, clone.EndedAt) + require.Truef(t, code.StartedAt.Before(clone.EndedAt), "code-server started %s after Clone repo ended %s", code.StartedAt, clone.EndedAt) + require.GreaterOrEqualf(t, clone.EndedAt.Sub(clone.StartedAt), cloneRepoSleep, "Clone repo duration: %+v", clone) +} + +func verifyScriptOrderSkip(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready", "start_error") + + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + + requireLifecycle(ctx, t, ctrID, name, agent, "start_error") + + lines := orderLogLines(ctx, t, ctrID, name) + require.NotContains(t, lines, "install_tools start", "skipped script must not run: %q", lines) + requireLineAfter(t, lines, "post_setup start", "clone_repo end") + + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") + clone := timingFor(t, timings, "start", "Clone repo") + requireStatus(t, clone, "exit_failure") + requireExitCode(t, clone, 1) + install := timingFor(t, timings, "start", "Install tools") + requireStatus(t, install, "skipped") + require.Truef(t, install.StartedAt.Equal(install.EndedAt), "skipped timing must be zero-length: %+v", install) + requireStatus(t, timingFor(t, timings, "start", "Post setup"), "ok") + requireStatus(t, timingFor(t, timings, "start", "code-server"), "ok") +} + +func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "ready") + + requireDependencies(t, agent, "Stop DB", "coder_script.stop_db", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.save_state", Requirement: "success"}}) + + stopAgent(ctx, t, ctrID, name) + agent = waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") + requireLifecycle(ctx, t, ctrID, name, agent, "off") + + lines := orderLogLines(ctx, t, ctrID, name) + requireLineAfter(t, lines, "stop_db start", "save_state end") + + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "stop", "Save state", "Stop DB") + save := timingFor(t, timings, "stop", "Save state") + stop := timingFor(t, timings, "stop", "Stop DB") + requireStatus(t, save, "ok") + requireStatus(t, stop, "ok") + require.Falsef(t, stop.StartedAt.Before(save.EndedAt), "Stop DB started %s before Save state ended %s", stop.StartedAt, save.EndedAt) +} + +func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { + agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) + startAgent(ctx, t, ctrID, name) + // code-server starts in parallel with clone_repo and must have exited + // before the signal, otherwise it is canceled too. + waitForOrderLog(ctx, t, ctrID, name, "clone_repo start", "start_code_server end") + stopAgent(ctx, t, ctrID, name) + agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout") + + requireDependencies(t, agent, "Install tools", "coder_script.install_tools", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "success"}}) + requireDependencies(t, agent, "Post setup", "coder_script.post_setup", + []workspaceAgentScriptDependency{{PrerequisiteResourceAddress: "coder_script.clone_repo", Requirement: "completion"}}) + + requireLifecycle(ctx, t, ctrID, name, agent, "off") + + lines := orderLogLines(ctx, t, ctrID, name) + require.NotContains(t, lines, "clone_repo end") + require.NotContains(t, lines, "install_tools start") + require.NotContains(t, lines, "post_setup start") + + timings := waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "Clone repo", "Install tools", "Post setup", "code-server") + clone := timingFor(t, timings, "start", "Clone repo") + requireStatus(t, clone, "canceled") + require.Truef(t, clone.EndedAt.After(clone.StartedAt), "canceled running script must keep its real interval: %+v", clone) + for _, n := range []string{"Install tools", "Post setup"} { + tm := timingFor(t, timings, "start", n) + requireStatus(t, tm, "canceled") + require.Truef(t, tm.StartedAt.Equal(tm.EndedAt), "%s timing must be zero-length: %+v", n, tm) + } + requireStatus(t, timingFor(t, timings, "start", "code-server"), "ok") +} From 44a754aa99ba5ea90607791196582c0be20e2375 Mon Sep 17 00:00:00 2001 From: Bobby Ho Date: Tue, 20 Oct 2026 13:44:10 -0700 Subject: [PATCH 8/8] test(integration): close two edge cases in the agent helpers apiGet now returns a Docker error instead of calling require. It is polled from testify's goroutine in waitForTimings, where a failed require ends the poll early and leaves the test to fail later on empty timings. The cancel case waits for the code-server start timing before sending the signal. The order log only shows the script's last line ran; the timing shows the agent saw the process exit and marked it ok, so the signal cannot cancel it as well. --- integration/agent_test.go | 7 ++++++- integration/script_order_test.go | 7 ++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/integration/agent_test.go b/integration/agent_test.go index 7888b279..74d324a1 100644 --- a/integration/agent_test.go +++ b/integration/agent_test.go @@ -96,7 +96,12 @@ func apiGet(ctx context.Context, t *testing.T, ctrID, name, path string, out any cmd := fmt.Sprintf( `curl -s -w '\n%%{http_code}' -H "Coder-Session-Token: $(cat /tmp/%s.session-token)" http://localhost:3000%s`, name, path) - body, rc := execContainer(ctx, t, ctrID, cmd) + // Callers poll from testify's goroutine, where a failed require would + // end the poll early instead of the test. + body, rc, err := tryExecContainer(ctx, t, ctrID, cmd) + if err != nil { + return fmt.Errorf("GET %s: %w", path, err) + } if rc != 0 { return fmt.Errorf("GET %s: exit %d: %s", path, rc, body) } diff --git a/integration/script_order_test.go b/integration/script_order_test.go index 66f874be..78018de8 100644 --- a/integration/script_order_test.go +++ b/integration/script_order_test.go @@ -169,9 +169,10 @@ func verifyScriptOrderStop(ctx context.Context, t *testing.T, ctrID, name string func verifyScriptOrderCancel(ctx context.Context, t *testing.T, ctrID, name string, out map[string]string) { agentID := agentIDFor(ctx, t, ctrID, name, out["workspace.id"]) startAgent(ctx, t, ctrID, name) - // code-server starts in parallel with clone_repo and must have exited - // before the signal, otherwise it is canceled too. - waitForOrderLog(ctx, t, ctrID, name, "clone_repo start", "start_code_server end") + // code-server starts in parallel with clone_repo. Its timing proves the + // agent saw it exit, so the signal cannot cancel it as well. + waitForOrderLog(ctx, t, ctrID, name, "clone_repo start") + waitForTimings(ctx, t, ctrID, name, out["workspace.id"], "start", "code-server") stopAgent(ctx, t, ctrID, name) agent := waitForAgent(ctx, t, ctrID, name, agentID, "off", "shutdown_error", "shutdown_timeout")