From d79a7797c23a65138b4917f80c927be1a87fc854 Mon Sep 17 00:00:00 2001 From: Susana Ferreira Date: Tue, 19 Aug 2025 13:08:01 +0100 Subject: [PATCH] fix: exclude prebuilt workspaces from template-level lifecycle updates (#19265) ## Description This PR ensures that lifecycle-related changes made via template schedule updates do **not affect prebuilt workspaces**. Since prebuilds are managed by the reconciliation loop and do not participate in the regular lifecycle executor flow, they must be excluded from any updates triggered by template configuration changes. This includes changes to TTL, dormant-deletion scheduling, deadline and autostart scheduling. ## Changes - Updated SQL query `UpdateWorkspacesTTLByTemplateID` to exclude prebuilt workspaces - Updated SQL query `UpdateWorkspacesDormantDeletingAtByTemplateID` to exclude prebuilt workspaces - Updated application-layer logic to skip any updates to lifecycle parameters if a workspace is a prebuild - Preserved all existing update behavior for regular user workspaces This change guarantees that only lifecycle-managed workspaces are affected when template-level configurations are modified, preserving strict boundaries between prebuild and user workspace lifecycles. Related with: * Issue: https://github.com/coder/coder/issues/18898 * PR: https://github.com/coder/coder/pull/19252 --- coderd/database/queries.sql.go | 13 +- coderd/database/queries/workspaces.sql | 15 +- enterprise/coderd/schedule/template.go | 11 +- enterprise/coderd/schedule/template_test.go | 251 ++++++++++++++++++++ 4 files changed, 282 insertions(+), 8 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8fc4a94a8a..1b63e7c1e9 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -21552,14 +21552,17 @@ UPDATE workspaces SET deleting_at = CASE WHEN $1::bigint = 0 THEN NULL - WHEN $2::timestamptz > '0001-01-01 00:00:00+00'::timestamptz THEN ($2::timestamptz) + interval '1 milliseconds' * $1::bigint + WHEN $2::timestamptz > '0001-01-01 00:00:00+00'::timestamptz THEN ($2::timestamptz) + interval '1 milliseconds' * $1::bigint ELSE dormant_at + interval '1 milliseconds' * $1::bigint END, dormant_at = CASE WHEN $2::timestamptz > '0001-01-01 00:00:00+00'::timestamptz THEN $2::timestamptz ELSE dormant_at END WHERE template_id = $3 -AND - dormant_at IS NOT NULL + AND dormant_at IS NOT NULL + -- Prebuilt workspaces (identified by having the prebuilds system user as owner_id) + -- should not have their dormant or deleting at set, as these are handled by the + -- prebuilds reconciliation loop. + AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite, next_start_at, group_acl, user_acl ` @@ -21618,6 +21621,10 @@ SET ttl = $2 WHERE template_id = $1 + -- Prebuilt workspaces (identified by having the prebuilds system user as owner_id) + -- should not have their TTL updated, as they are handled by the prebuilds + -- reconciliation loop. + AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID ` type UpdateWorkspacesTTLByTemplateIDParams struct { diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 34461b5040..a3deda6863 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -579,7 +579,11 @@ UPDATE SET ttl = $2 WHERE - template_id = $1; + template_id = $1 + -- Prebuilt workspaces (identified by having the prebuilds system user as owner_id) + -- should not have their TTL updated, as they are handled by the prebuilds + -- reconciliation loop. + AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID; -- name: UpdateWorkspaceLastUsedAt :exec UPDATE @@ -824,14 +828,17 @@ UPDATE workspaces SET deleting_at = CASE WHEN @time_til_dormant_autodelete_ms::bigint = 0 THEN NULL - WHEN @dormant_at::timestamptz > '0001-01-01 00:00:00+00'::timestamptz THEN (@dormant_at::timestamptz) + interval '1 milliseconds' * @time_til_dormant_autodelete_ms::bigint + WHEN @dormant_at::timestamptz > '0001-01-01 00:00:00+00'::timestamptz THEN (@dormant_at::timestamptz) + interval '1 milliseconds' * @time_til_dormant_autodelete_ms::bigint ELSE dormant_at + interval '1 milliseconds' * @time_til_dormant_autodelete_ms::bigint END, dormant_at = CASE WHEN @dormant_at::timestamptz > '0001-01-01 00:00:00+00'::timestamptz THEN @dormant_at::timestamptz ELSE dormant_at END WHERE template_id = @template_id -AND - dormant_at IS NOT NULL + AND dormant_at IS NOT NULL + -- Prebuilt workspaces (identified by having the prebuilds system user as owner_id) + -- should not have their dormant or deleting at set, as these are handled by the + -- prebuilds reconciliation loop. + AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID RETURNING *; -- name: UpdateTemplateWorkspacesLastUsedAt :exec diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 203de46db4..ed21b8160e 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -242,6 +242,10 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S nextStartAts := []time.Time{} for _, workspace := range workspaces { + // Skip prebuilt workspaces + if workspace.IsPrebuild() { + continue + } nextStartAt := time.Time{} if workspace.AutostartSchedule.Valid { next, err := agpl.NextAllowedAutostart(s.now(), workspace.AutostartSchedule.String, templateSchedule) @@ -254,7 +258,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S nextStartAts = append(nextStartAts, nextStartAt) } - //nolint:gocritic // We need to be able to update information about all workspaces. + //nolint:gocritic // We need to be able to update information about regular user workspaces. if err := db.BatchUpdateWorkspaceNextStartAt(dbauthz.AsSystemRestricted(ctx), database.BatchUpdateWorkspaceNextStartAtParams{ IDs: workspaceIDs, NextStartAts: nextStartAts, @@ -334,6 +338,11 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte return xerrors.Errorf("get workspace %q: %w", build.WorkspaceID, err) } + // Skip lifecycle updates for prebuilt workspaces + if workspace.IsPrebuild() { + return nil + } + job, err := db.GetProvisionerJobByID(ctx, build.JobID) if err != nil { return xerrors.Errorf("get provisioner job %q: %w", build.JobID, err) diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index 2eb13b4eb3..70dc308489 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -1,6 +1,7 @@ package schedule_test import ( + "context" "database/sql" "encoding/json" "fmt" @@ -17,14 +18,18 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/notifications/notificationstest" agplschedule "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/schedule/cron" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" "github.com/coder/coder/v2/enterprise/coderd/schedule" + "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" "github.com/coder/quartz" ) @@ -979,6 +984,252 @@ func TestTemplateTTL(t *testing.T) { }) } +func TestTemplateUpdatePrebuilds(t *testing.T) { + t.Parallel() + + // Dormant auto-delete configured to 10 hours + dormantAutoDelete := 10 * time.Hour + + // TTL configured to 8 hours + ttl := 8 * time.Hour + + // Autostop configuration set to everyday at midnight + autostopWeekdays, err := codersdk.WeekdaysToBitmap(codersdk.AllDaysOfWeek) + require.NoError(t, err) + + // Autostart configuration set to everyday at midnight + autostartSchedule, err := cron.Weekly("CRON_TZ=UTC 0 0 * * *") + require.NoError(t, err) + autostartWeekdays, err := codersdk.WeekdaysToBitmap(codersdk.AllDaysOfWeek) + require.NoError(t, err) + + cases := []struct { + name string + templateSchedule agplschedule.TemplateScheduleOptions + workspaceUpdate func(*testing.T, context.Context, database.Store, time.Time, database.ClaimPrebuiltWorkspaceRow) + assertWorkspace func(*testing.T, context.Context, database.Store, time.Time, bool, database.Workspace) + }{ + { + name: "TemplateDormantAutoDeleteUpdatePrebuildAfterClaim", + templateSchedule: agplschedule.TemplateScheduleOptions{ + // Template level TimeTilDormantAutodelete set to 10 hours + TimeTilDormantAutoDelete: dormantAutoDelete, + }, + workspaceUpdate: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, + workspace database.ClaimPrebuiltWorkspaceRow, + ) { + // When: the workspace is marked dormant + dormantWorkspace, err := db.UpdateWorkspaceDormantDeletingAt(ctx, database.UpdateWorkspaceDormantDeletingAtParams{ + ID: workspace.ID, + DormantAt: sql.NullTime{ + Time: now, + Valid: true, + }, + }) + require.NoError(t, err) + require.NotNil(t, dormantWorkspace.DormantAt) + }, + assertWorkspace: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, + isPrebuild bool, workspace database.Workspace, + ) { + if isPrebuild { + // The unclaimed prebuild should have an empty DormantAt and DeletingAt + require.True(t, workspace.DormantAt.Time.IsZero()) + require.True(t, workspace.DeletingAt.Time.IsZero()) + } else { + // The claimed workspace should have its DormantAt and DeletingAt updated + require.False(t, workspace.DormantAt.Time.IsZero()) + require.False(t, workspace.DeletingAt.Time.IsZero()) + require.WithinDuration(t, now.UTC(), workspace.DormantAt.Time.UTC(), time.Second) + require.WithinDuration(t, now.Add(dormantAutoDelete).UTC(), workspace.DeletingAt.Time.UTC(), time.Second) + } + }, + }, + { + name: "TemplateTTLUpdatePrebuildAfterClaim", + templateSchedule: agplschedule.TemplateScheduleOptions{ + // Template level TTL can only be set if autostop is disabled for users + DefaultTTL: ttl, + UserAutostopEnabled: false, + }, + workspaceUpdate: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, + workspace database.ClaimPrebuiltWorkspaceRow) { + }, + assertWorkspace: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, + isPrebuild bool, workspace database.Workspace, + ) { + if isPrebuild { + // The unclaimed prebuild should have an empty TTL + require.Equal(t, sql.NullInt64{}, workspace.Ttl) + } else { + // The claimed workspace should have its TTL updated + require.Equal(t, sql.NullInt64{Int64: int64(ttl), Valid: true}, workspace.Ttl) + } + }, + }, + { + name: "TemplateAutostopUpdatePrebuildAfterClaim", + templateSchedule: agplschedule.TemplateScheduleOptions{ + // Template level Autostop set for everyday + AutostopRequirement: agplschedule.TemplateAutostopRequirement{ + DaysOfWeek: autostopWeekdays, + Weeks: 0, + }, + }, + workspaceUpdate: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, + workspace database.ClaimPrebuiltWorkspaceRow) { + }, + assertWorkspace: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, isPrebuild bool, workspace database.Workspace) { + if isPrebuild { + // The unclaimed prebuild should have an empty MaxDeadline + prebuildBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspace.ID) + require.NoError(t, err) + require.True(t, prebuildBuild.MaxDeadline.IsZero()) + } else { + // The claimed workspace should have its MaxDeadline updated + workspaceBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspace.ID) + require.NoError(t, err) + require.False(t, workspaceBuild.MaxDeadline.IsZero()) + } + }, + }, + { + name: "TemplateAutostartUpdatePrebuildAfterClaim", + templateSchedule: agplschedule.TemplateScheduleOptions{ + // Template level Autostart set for everyday + UserAutostartEnabled: true, + AutostartRequirement: agplschedule.TemplateAutostartRequirement{ + DaysOfWeek: autostartWeekdays, + }, + }, + workspaceUpdate: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, workspace database.ClaimPrebuiltWorkspaceRow) { + // To compute NextStartAt, the workspace must have a valid autostart schedule + err = db.UpdateWorkspaceAutostart(ctx, database.UpdateWorkspaceAutostartParams{ + ID: workspace.ID, + AutostartSchedule: sql.NullString{ + String: autostartSchedule.String(), + Valid: true, + }, + }) + require.NoError(t, err) + }, + assertWorkspace: func(t *testing.T, ctx context.Context, db database.Store, now time.Time, isPrebuild bool, workspace database.Workspace) { + if isPrebuild { + // The unclaimed prebuild should have an empty NextStartAt + require.True(t, workspace.NextStartAt.Time.IsZero()) + } else { + // The claimed workspace should have its NextStartAt updated + require.False(t, workspace.NextStartAt.Time.IsZero()) + } + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + clock := quartz.NewMock(t) + clock.Set(dbtime.Now()) + + // Setup + var ( + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + db, _ = dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure()) + ctx = testutil.Context(t, testutil.WaitLong) + user = dbgen.User(t, db, database.User{}) + ) + + // Setup the template schedule store + notifyEnq := notifications.NewNoopEnqueuer() + const userQuietHoursSchedule = "CRON_TZ=UTC 0 0 * * *" // midnight UTC + userQuietHoursStore, err := schedule.NewEnterpriseUserQuietHoursScheduleStore(userQuietHoursSchedule, true) + require.NoError(t, err) + userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} + userQuietHoursStorePtr.Store(&userQuietHoursStore) + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifyEnq, logger, clock) + + // Given: a template and a template version with preset and a prebuilt workspace + presetID := uuid.New() + org := dbfake.Organization(t, db).Do() + tv := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{ + OrganizationID: org.Org.ID, + CreatedBy: user.ID, + }).Preset(database.TemplateVersionPreset{ + ID: presetID, + DesiredInstances: sql.NullInt32{ + Int32: 1, + Valid: true, + }, + }).Do() + workspaceBuild := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + TemplateID: tv.Template.ID, + OrganizationID: tv.Template.OrganizationID, + }).Seed(database.WorkspaceBuild{ + TemplateVersionID: tv.TemplateVersion.ID, + TemplateVersionPresetID: uuid.NullUUID{ + UUID: presetID, + Valid: true, + }, + }).WithAgent(func(agent []*proto.Agent) []*proto.Agent { + return agent + }).Do() + + // Mark the prebuilt workspace's agent as ready so the prebuild can be claimed + // nolint:gocritic + agentCtx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitLong)) + agent, err := db.GetWorkspaceAgentAndLatestBuildByAuthToken(agentCtx, uuid.MustParse(workspaceBuild.AgentToken)) + require.NoError(t, err) + err = db.UpdateWorkspaceAgentLifecycleStateByID(agentCtx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ + ID: agent.WorkspaceAgent.ID, + LifecycleState: database.WorkspaceAgentLifecycleStateReady, + }) + require.NoError(t, err) + + // Given: a prebuilt workspace + prebuild, err := db.GetWorkspaceByID(ctx, workspaceBuild.Workspace.ID) + require.NoError(t, err) + tc.assertWorkspace(t, ctx, db, clock.Now(), true, prebuild) + + // When: the template schedule is updated + _, err = templateScheduleStore.Set(ctx, db, tv.Template, tc.templateSchedule) + require.NoError(t, err) + + // Then: lifecycle parameters must remain unset while the prebuild is unclaimed + prebuild, err = db.GetWorkspaceByID(ctx, workspaceBuild.Workspace.ID) + require.NoError(t, err) + tc.assertWorkspace(t, ctx, db, clock.Now(), true, prebuild) + + // Given: the prebuilt workspace is claimed by a user + claimedWorkspace := dbgen.ClaimPrebuild( + t, db, + clock.Now(), + user.ID, + "claimedWorkspace-autostop", + presetID, + sql.NullString{}, + sql.NullTime{}, + sql.NullInt64{}) + require.Equal(t, prebuild.ID, claimedWorkspace.ID) + + // Given: the workspace level configurations are properly set in order to ensure the + // lifecycle parameters are updated + tc.workspaceUpdate(t, ctx, db, clock.Now(), claimedWorkspace) + + // When: the template schedule is updated + _, err = templateScheduleStore.Set(ctx, db, tv.Template, tc.templateSchedule) + require.NoError(t, err) + + // Then: the workspace should have its lifecycle parameters updated + workspace, err := db.GetWorkspaceByID(ctx, claimedWorkspace.ID) + require.NoError(t, err) + tc.assertWorkspace(t, ctx, db, clock.Now(), false, workspace) + }) + } +} + func must[V any](v V, err error) V { if err != nil { panic(err)