Skip to content

Commit 8567ecb

Browse files
authored
fix: set prebuilds lifecycle parameters on creation and claim (#19252)
## Description This PR ensures that prebuilt workspaces are properly excluded from the lifecycle executor and treated as a separate class of workspaces, fully managed by the prebuild reconciliation loop. It introduces two lifecycle guarantees: * When a prebuilt workspace is created (i.e., when the workspace build completes), all lifecycle-related fields are unset, ensuring the workspace does not participate in TTL, autostop, autostart, dormancy, or auto-deletion logic. * When a prebuilt workspace is claimed, it transitions into a regular user workspace. At this point, all lifecycle fields are correctly populated according to template-level configurations, allowing the workspace to be managed by the lifecycle executor as expected. ## Changes * Prebuilt workspaces now have all lifecycle-relevant fields unset during creation * When a prebuild is claimed: * Lifecycle fields are set based on template and workspace level configurations. This ensures a clean transition into the standard workspace lifecycle flow. * Updated lifecycle-related SQL update queries to explicitly exclude prebuilt workspaces. ## Relates Related issue: #18898 To reduce the scope of this PR and make the review process more manageable, the original implementation has been split into the following focused PRs: * #19259 * #19263 * #19264 * #19265 These PRs should be considered in conjunction with this one to understand the complete set of lifecycle separation changes for prebuilt workspaces.
1 parent f17ab92 commit 8567ecb

File tree

14 files changed

+485
-303
lines changed

14 files changed

+485
-303
lines changed

coderd/autobuild/lifecycle_executor_test.go

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,15 +1251,23 @@ func TestExecutorPrebuilds(t *testing.T) {
12511251
}()
12521252

12531253
// Then: the prebuilt workspace should remain in a start transition
1254-
prebuildStats := <-statsCh
1254+
prebuildStats := testutil.RequireReceive(ctx, t, statsCh)
12551255
require.Len(t, prebuildStats.Errors, 0)
12561256
require.Len(t, prebuildStats.Transitions, 0)
12571257
require.Equal(t, codersdk.WorkspaceTransitionStart, prebuild.LatestBuild.Transition)
12581258
prebuild = coderdtest.MustWorkspace(t, client, prebuild.ID)
12591259
require.Equal(t, codersdk.BuildReasonInitiator, prebuild.LatestBuild.Reason)
12601260

12611261
// Given: a user claims the prebuilt workspace
1262-
dbWorkspace := dbgen.ClaimPrebuild(t, db, user.ID, "claimedWorkspace-autostop", preset.ID)
1262+
dbWorkspace := dbgen.ClaimPrebuild(
1263+
t, db,
1264+
clock.Now(),
1265+
user.ID,
1266+
"claimedWorkspace-autostop",
1267+
preset.ID,
1268+
sql.NullString{},
1269+
sql.NullTime{},
1270+
sql.NullInt64{})
12631271
workspace := coderdtest.MustWorkspace(t, client, dbWorkspace.ID)
12641272

12651273
// When: the autobuild executor ticks *after* the deadline:
@@ -1269,7 +1277,7 @@ func TestExecutorPrebuilds(t *testing.T) {
12691277
}()
12701278

12711279
// Then: the workspace should be stopped
1272-
workspaceStats := <-statsCh
1280+
workspaceStats := testutil.RequireReceive(ctx, t, statsCh)
12731281
require.Len(t, workspaceStats.Errors, 0)
12741282
require.Len(t, workspaceStats.Transitions, 1)
12751283
require.Contains(t, workspaceStats.Transitions, workspace.ID)
@@ -1336,7 +1344,7 @@ func TestExecutorPrebuilds(t *testing.T) {
13361344
}()
13371345

13381346
// Then: the prebuilt workspace should remain in a stop transition
1339-
prebuildStats := <-statsCh
1347+
prebuildStats := testutil.RequireReceive(ctx, t, statsCh)
13401348
require.Len(t, prebuildStats.Errors, 0)
13411349
require.Len(t, prebuildStats.Transitions, 0)
13421350
require.Equal(t, codersdk.WorkspaceTransitionStop, prebuild.LatestBuild.Transition)
@@ -1353,7 +1361,15 @@ func TestExecutorPrebuilds(t *testing.T) {
13531361
database.WorkspaceTransitionStart)
13541362

13551363
// Given: a user claims the prebuilt workspace
1356-
dbWorkspace := dbgen.ClaimPrebuild(t, db, user.ID, "claimedWorkspace-autostart", preset.ID)
1364+
dbWorkspace := dbgen.ClaimPrebuild(
1365+
t, db,
1366+
clock.Now(),
1367+
user.ID,
1368+
"claimedWorkspace-autostart",
1369+
preset.ID,
1370+
autostartSched,
1371+
sql.NullTime{},
1372+
sql.NullInt64{})
13571373
workspace := coderdtest.MustWorkspace(t, client, dbWorkspace.ID)
13581374

13591375
// Given: the prebuilt workspace goes to a stop status
@@ -1374,7 +1390,7 @@ func TestExecutorPrebuilds(t *testing.T) {
13741390
}()
13751391

13761392
// Then: the workspace should eventually be started
1377-
workspaceStats := <-statsCh
1393+
workspaceStats := testutil.RequireReceive(ctx, t, statsCh)
13781394
require.Len(t, workspaceStats.Errors, 0)
13791395
require.Len(t, workspaceStats.Transitions, 1)
13801396
require.Contains(t, workspaceStats.Transitions, workspace.ID)
@@ -1486,8 +1502,8 @@ func setupTestDBWorkspaceBuild(
14861502
Architecture: "i386",
14871503
OperatingSystem: "linux",
14881504
LifecycleState: database.WorkspaceAgentLifecycleStateReady,
1489-
StartedAt: sql.NullTime{Time: time.Now().Add(time.Hour), Valid: true},
1490-
ReadyAt: sql.NullTime{Time: time.Now().Add(-1 * time.Hour), Valid: true},
1505+
StartedAt: sql.NullTime{Time: clock.Now().Add(time.Hour), Valid: true},
1506+
ReadyAt: sql.NullTime{Time: clock.Now().Add(-1 * time.Hour), Valid: true},
14911507
APIKeyScope: database.AgentKeyScopeEnumAll,
14921508
})
14931509

@@ -1524,8 +1540,9 @@ func setupTestDBPrebuiltWorkspace(
15241540
OrganizationID: orgID,
15251541
OwnerID: database.PrebuildsSystemUserID,
15261542
Deleted: false,
1527-
CreatedAt: time.Now().Add(-time.Hour * 2),
1543+
CreatedAt: clock.Now().Add(-time.Hour * 2),
15281544
AutostartSchedule: options.AutostartSchedule,
1545+
LastUsedAt: clock.Now(),
15291546
})
15301547
setupTestDBWorkspaceBuild(ctx, t, clock, db, ps, orgID, workspace.ID, templateVersionID, presetID, buildTransition)
15311548

coderd/database/dbgen/dbgen.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,11 +1436,25 @@ func UserSecret(t testing.TB, db database.Store, seed database.UserSecret) datab
14361436
return userSecret
14371437
}
14381438

1439-
func ClaimPrebuild(t testing.TB, db database.Store, newUserID uuid.UUID, newName string, presetID uuid.UUID) database.ClaimPrebuiltWorkspaceRow {
1439+
func ClaimPrebuild(
1440+
t testing.TB,
1441+
db database.Store,
1442+
now time.Time,
1443+
newUserID uuid.UUID,
1444+
newName string,
1445+
presetID uuid.UUID,
1446+
autostartSchedule sql.NullString,
1447+
nextStartAt sql.NullTime,
1448+
ttl sql.NullInt64,
1449+
) database.ClaimPrebuiltWorkspaceRow {
14401450
claimedWorkspace, err := db.ClaimPrebuiltWorkspace(genCtx, database.ClaimPrebuiltWorkspaceParams{
1441-
NewUserID: newUserID,
1442-
NewName: newName,
1443-
PresetID: presetID,
1451+
NewUserID: newUserID,
1452+
NewName: newName,
1453+
Now: now,
1454+
PresetID: presetID,
1455+
AutostartSchedule: autostartSchedule,
1456+
NextStartAt: nextStartAt,
1457+
WorkspaceTtl: ttl,
14441458
})
14451459
require.NoError(t, err, "claim prebuilt workspace")
14461460

coderd/database/queries.sql.go

Lines changed: 59 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries/prebuilds.sql

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,20 @@
22
UPDATE workspaces w
33
SET owner_id = @new_user_id::uuid,
44
name = @new_name::text,
5-
updated_at = NOW()
5+
updated_at = @now::timestamptz,
6+
-- Update autostart_schedule, next_start_at and ttl according to template and workspace-level
7+
-- configurations, allowing the workspace to be managed by the lifecycle executor as expected.
8+
autostart_schedule = @autostart_schedule,
9+
next_start_at = @next_start_at,
10+
ttl = @workspace_ttl,
11+
-- Update last_used_at during claim to ensure the claimed workspace is treated as recently used.
12+
-- This avoids unintended dormancy caused by prebuilds having stale usage timestamps.
13+
last_used_at = @now::timestamptz,
14+
-- Clear dormant and deletion timestamps as a safeguard to ensure a clean lifecycle state after claim.
15+
-- These fields should not be set on prebuilds, but we defensively reset them here to prevent
16+
-- accidental dormancy or deletion by the lifecycle executor.
17+
dormant_at = NULL,
18+
deleting_at = NULL
619
WHERE w.id IN (
720
SELECT p.id
821
FROM workspace_prebuilds p

coderd/database/queries/workspacebuilds.sql

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,15 @@ SET
127127
deadline = @deadline::timestamptz,
128128
max_deadline = @max_deadline::timestamptz,
129129
updated_at = @updated_at::timestamptz
130-
WHERE id = @id::uuid;
130+
FROM
131+
workspaces
132+
WHERE
133+
workspace_builds.id = @id::uuid
134+
AND workspace_builds.workspace_id = workspaces.id
135+
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
136+
-- are managed by the reconciliation loop, not the lifecycle executor which handles
137+
-- deadline and max_deadline
138+
AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID;
131139

132140
-- name: UpdateWorkspaceBuildProvisionerStateByID :exec
133141
UPDATE

coderd/database/queries/workspaces.sql

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -518,15 +518,23 @@ SET
518518
autostart_schedule = $2,
519519
next_start_at = $3
520520
WHERE
521-
id = $1;
521+
id = $1
522+
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
523+
-- are managed by the reconciliation loop, not the lifecycle executor which handles
524+
-- autostart_schedule and next_start_at
525+
AND owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID;
522526

523527
-- name: UpdateWorkspaceNextStartAt :exec
524528
UPDATE
525529
workspaces
526530
SET
527531
next_start_at = $2
528532
WHERE
529-
id = $1;
533+
id = $1
534+
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
535+
-- are managed by the reconciliation loop, not the lifecycle executor which handles
536+
-- next_start_at
537+
AND owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID;
530538

531539
-- name: BatchUpdateWorkspaceNextStartAt :exec
532540
UPDATE
@@ -550,15 +558,19 @@ UPDATE
550558
SET
551559
ttl = $2
552560
WHERE
553-
id = $1;
561+
id = $1
562+
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
563+
-- are managed by the reconciliation loop, not the lifecycle executor which handles
564+
-- ttl
565+
AND owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID;
554566

555567
-- name: UpdateWorkspacesTTLByTemplateID :exec
556568
UPDATE
557-
workspaces
569+
workspaces
558570
SET
559-
ttl = $2
571+
ttl = $2
560572
WHERE
561-
template_id = $1;
573+
template_id = $1;
562574

563575
-- name: UpdateWorkspaceLastUsedAt :exec
564576
UPDATE
@@ -791,6 +803,10 @@ FROM
791803
WHERE
792804
workspaces.id = $1
793805
AND templates.id = workspaces.template_id
806+
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
807+
-- are managed by the reconciliation loop, not the lifecycle executor which handles
808+
-- dormant_at and deleting_at
809+
AND owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID
794810
RETURNING
795811
workspaces.*;
796812

coderd/prebuilds/api.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package prebuilds
22

33
import (
44
"context"
5+
"database/sql"
6+
"time"
57

68
"github.com/google/uuid"
79
"golang.org/x/xerrors"
@@ -54,6 +56,15 @@ type StateSnapshotter interface {
5456
}
5557

5658
type Claimer interface {
57-
Claim(ctx context.Context, userID uuid.UUID, name string, presetID uuid.UUID) (*uuid.UUID, error)
59+
Claim(
60+
ctx context.Context,
61+
now time.Time,
62+
userID uuid.UUID,
63+
name string,
64+
presetID uuid.UUID,
65+
autostartSchedule sql.NullString,
66+
nextStartAt sql.NullTime,
67+
ttl sql.NullInt64,
68+
) (*uuid.UUID, error)
5869
Initiator() uuid.UUID
5970
}

0 commit comments

Comments
 (0)