mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
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
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user