Skip to content

Commit 5762d8a

Browse files
DanielleMaywoodDanielleMaywood
authored
fix: return only the first workspace agent script timing per script (coder#16203)
Fixes coder#16124 If a workspace agent crashes, it is possible for any startup scripts to be ran again. This PR makes it so that the `GetWorkspaceAgentScriptTimingsByBuildID` query only returns the first timing recorded per-script.
1 parent 56cf0d8 commit 5762d8a

File tree

6 files changed

+77
-10
lines changed

6 files changed

+77
-10
lines changed

‎coderd/database/dbgen/dbgen.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,17 @@ func WorkspaceAgentScript(t testing.TB, db database.Store, orig database.Workspa
211211
return scripts[0]
212212
}
213213

214-
func WorkspaceAgentScriptTimings(t testing.TB, db database.Store, script database.WorkspaceAgentScript, count int) []database.WorkspaceAgentScriptTiming {
215-
timings := make([]database.WorkspaceAgentScriptTiming, count)
216-
for i := range count {
214+
func WorkspaceAgentScripts(t testing.TB, db database.Store, count int, orig database.WorkspaceAgentScript) []database.WorkspaceAgentScript {
215+
scripts := make([]database.WorkspaceAgentScript, 0, count)
216+
for range count {
217+
scripts = append(scripts, WorkspaceAgentScript(t, db, orig))
218+
}
219+
return scripts
220+
}
221+
222+
func WorkspaceAgentScriptTimings(t testing.TB, db database.Store, scripts []database.WorkspaceAgentScript) []database.WorkspaceAgentScriptTiming {
223+
timings := make([]database.WorkspaceAgentScriptTiming, len(scripts))
224+
for i, script := range scripts {
217225
timings[i] = WorkspaceAgentScriptTiming(t, db, database.WorkspaceAgentScriptTiming{
218226
ScriptID: script.ID,
219227
})

‎coderd/database/dbmem/dbmem.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6388,6 +6388,15 @@ func (q *FakeQuerier) GetWorkspaceAgentScriptTimingsByBuildID(ctx context.Contex
63886388
WorkspaceAgentName: agent.Name,
63896389
})
63906390
}
6391+
6392+
// We want to only return the first script run for each Script ID.
6393+
slices.SortFunc(rows, func(a, b database.GetWorkspaceAgentScriptTimingsByBuildIDRow) int {
6394+
return a.StartedAt.Compare(b.StartedAt)
6395+
})
6396+
rows = slices.CompactFunc(rows, func(e1, e2 database.GetWorkspaceAgentScriptTimingsByBuildIDRow) bool {
6397+
return e1.ScriptID == e2.ScriptID
6398+
})
6399+
63916400
return rows, nil
63926401
}
63936402

‎coderd/database/queries.sql.go

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaceagents.sql

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ RETURNING workspace_agent_script_timings.*;
304304

305305
-- name: GetWorkspaceAgentScriptTimingsByBuildID :many
306306
SELECT
307-
workspace_agent_script_timings.*,
307+
DISTINCT ON (workspace_agent_script_timings.script_id) workspace_agent_script_timings.*,
308308
workspace_agent_scripts.display_name,
309309
workspace_agents.id as workspace_agent_id,
310310
workspace_agents.name as workspace_agent_name
@@ -313,4 +313,5 @@ INNER JOIN workspace_agent_scripts ON workspace_agent_scripts.id = workspace_age
313313
INNER JOIN workspace_agents ON workspace_agents.id = workspace_agent_scripts.workspace_agent_id
314314
INNER JOIN workspace_resources ON workspace_resources.id = workspace_agents.resource_id
315315
INNER JOIN workspace_builds ON workspace_builds.job_id = workspace_resources.job_id
316-
WHERE workspace_builds.id = $1;
316+
WHERE workspace_builds.id = $1
317+
ORDER BY workspace_agent_script_timings.script_id, workspace_agent_script_timings.started_at;

‎coderd/workspacebuilds_test.go

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/stretchr/testify/require"
1515
"go.opentelemetry.io/otel"
1616
"go.opentelemetry.io/otel/propagation"
17+
"golang.org/x/exp/slices"
1718
"golang.org/x/xerrors"
1819

1920
"cdr.dev/slog"
@@ -1547,6 +1548,47 @@ func TestWorkspaceBuildTimings(t *testing.T) {
15471548
}
15481549
})
15491550

1551+
t.Run("MultipleTimingsForSameAgentScript", func(t *testing.T) {
1552+
t.Parallel()
1553+
1554+
// Given: a build with multiple timings for the same script
1555+
build := makeBuild(t)
1556+
resource := dbgen.WorkspaceResource(t, db, database.WorkspaceResource{
1557+
JobID: build.JobID,
1558+
})
1559+
agent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
1560+
ResourceID: resource.ID,
1561+
})
1562+
script := dbgen.WorkspaceAgentScript(t, db, database.WorkspaceAgentScript{
1563+
WorkspaceAgentID: agent.ID,
1564+
})
1565+
timings := make([]database.WorkspaceAgentScriptTiming, 3)
1566+
scriptStartedAt := dbtime.Now()
1567+
for i := range timings {
1568+
timings[i] = dbgen.WorkspaceAgentScriptTiming(t, db, database.WorkspaceAgentScriptTiming{
1569+
StartedAt: scriptStartedAt,
1570+
EndedAt: scriptStartedAt.Add(1 * time.Minute),
1571+
ScriptID: script.ID,
1572+
})
1573+
1574+
// Add an hour to the previous "started at" so we can
1575+
// reliably differentiate the scripts from each other.
1576+
scriptStartedAt = scriptStartedAt.Add(1 * time.Hour)
1577+
}
1578+
1579+
// When: fetching timings for the build
1580+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
1581+
t.Cleanup(cancel)
1582+
res, err := client.WorkspaceBuildTimings(ctx, build.ID)
1583+
require.NoError(t, err)
1584+
1585+
// Then: return a response with the first agent script timing
1586+
require.Len(t, res.AgentScriptTimings, 1)
1587+
1588+
require.Equal(t, timings[0].StartedAt.UnixMilli(), res.AgentScriptTimings[0].StartedAt.UnixMilli())
1589+
require.Equal(t, timings[0].EndedAt.UnixMilli(), res.AgentScriptTimings[0].EndedAt.UnixMilli())
1590+
})
1591+
15501592
t.Run("AgentScriptTimings", func(t *testing.T) {
15511593
t.Parallel()
15521594

@@ -1558,10 +1600,10 @@ func TestWorkspaceBuildTimings(t *testing.T) {
15581600
agent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
15591601
ResourceID: resource.ID,
15601602
})
1561-
script := dbgen.WorkspaceAgentScript(t, db, database.WorkspaceAgentScript{
1603+
scripts := dbgen.WorkspaceAgentScripts(t, db, 5, database.WorkspaceAgentScript{
15621604
WorkspaceAgentID: agent.ID,
15631605
})
1564-
agentScriptTimings := dbgen.WorkspaceAgentScriptTimings(t, db, script, 5)
1606+
agentScriptTimings := dbgen.WorkspaceAgentScriptTimings(t, db, scripts)
15651607

15661608
// When: fetching timings for the build
15671609
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
@@ -1571,6 +1613,12 @@ func TestWorkspaceBuildTimings(t *testing.T) {
15711613

15721614
// Then: return a response with the expected timings
15731615
require.Len(t, res.AgentScriptTimings, 5)
1616+
slices.SortFunc(res.AgentScriptTimings, func(a, b codersdk.AgentScriptTiming) int {
1617+
return a.StartedAt.Compare(b.StartedAt)
1618+
})
1619+
slices.SortFunc(agentScriptTimings, func(a, b database.WorkspaceAgentScriptTiming) int {
1620+
return a.StartedAt.Compare(b.StartedAt)
1621+
})
15741622
for i := range res.AgentScriptTimings {
15751623
timingRes := res.AgentScriptTimings[i]
15761624
genTiming := agentScriptTimings[i]

‎coderd/workspaces_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3913,10 +3913,10 @@ func TestWorkspaceTimings(t *testing.T) {
39133913
agent := dbgen.WorkspaceAgent(t, db, database.WorkspaceAgent{
39143914
ResourceID: resource.ID,
39153915
})
3916-
script := dbgen.WorkspaceAgentScript(t, db, database.WorkspaceAgentScript{
3916+
scripts := dbgen.WorkspaceAgentScripts(t, db, 3, database.WorkspaceAgentScript{
39173917
WorkspaceAgentID: agent.ID,
39183918
})
3919-
dbgen.WorkspaceAgentScriptTimings(t, db, script, 3)
3919+
dbgen.WorkspaceAgentScriptTimings(t, db, scripts)
39203920

39213921
// When: fetching the timings
39223922
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)

0 commit comments

Comments
 (0)