mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
fix: prevent activity bump for prebuilt workspaces (#19263)
## Description
This PR ensures that activity-based deadline extensions ("activity
bumping") are not applied to prebuilt workspaces. Prebuilds are managed
by the reconciliation loop and must not have `deadline` or
`max_deadline` values set or extended, as they are not part of the
regular lifecycle executor path.
## Changes
- Update `ActivityBumpWorkspace` SQL query to discard prebuilt
workspaces
- Update application layer to avoid calling activity bump logic on
prebuilt workspaces
Related with:
* Issue: https://github.com/coder/coder/issues/18898
* PR: https://github.com/coder/coder/pull/19252
This commit is contained in:
@@ -32,7 +32,7 @@ WITH latest AS (
|
||||
-- be as if the workspace auto started at the given time and the
|
||||
-- original TTL was applied.
|
||||
--
|
||||
-- Sadly we can't define ` + "`" + `activity_bump_interval` + "`" + ` above since
|
||||
-- Sadly we can't define 'activity_bump_interval' above since
|
||||
-- it won't be available for this CASE statement, so we have to
|
||||
-- copy the cast twice.
|
||||
WHEN NOW() + (templates.activity_bump / 1000 / 1000 / 1000 || ' seconds')::interval > $1 :: timestamptz
|
||||
@@ -62,7 +62,11 @@ WITH latest AS (
|
||||
ON workspaces.id = workspace_builds.workspace_id
|
||||
JOIN templates
|
||||
ON templates.id = workspaces.template_id
|
||||
WHERE workspace_builds.workspace_id = $2::uuid
|
||||
WHERE
|
||||
workspace_builds.workspace_id = $2::uuid
|
||||
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
|
||||
-- are managed by the reconciliation loop and not subject to activity bumping
|
||||
AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID
|
||||
ORDER BY workspace_builds.build_number DESC
|
||||
LIMIT 1
|
||||
)
|
||||
|
||||
@@ -22,7 +22,7 @@ WITH latest AS (
|
||||
-- be as if the workspace auto started at the given time and the
|
||||
-- original TTL was applied.
|
||||
--
|
||||
-- Sadly we can't define `activity_bump_interval` above since
|
||||
-- Sadly we can't define 'activity_bump_interval' above since
|
||||
-- it won't be available for this CASE statement, so we have to
|
||||
-- copy the cast twice.
|
||||
WHEN NOW() + (templates.activity_bump / 1000 / 1000 / 1000 || ' seconds')::interval > @next_autostart :: timestamptz
|
||||
@@ -52,7 +52,11 @@ WITH latest AS (
|
||||
ON workspaces.id = workspace_builds.workspace_id
|
||||
JOIN templates
|
||||
ON templates.id = workspaces.template_id
|
||||
WHERE workspace_builds.workspace_id = @workspace_id::uuid
|
||||
WHERE
|
||||
workspace_builds.workspace_id = @workspace_id::uuid
|
||||
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
|
||||
-- are managed by the reconciliation loop and not subject to activity bumping
|
||||
AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID
|
||||
ORDER BY workspace_builds.build_number DESC
|
||||
LIMIT 1
|
||||
)
|
||||
|
||||
@@ -149,33 +149,36 @@ func (r *Reporter) ReportAgentStats(ctx context.Context, now time.Time, workspac
|
||||
return nil
|
||||
}
|
||||
|
||||
// check next autostart
|
||||
var nextAutostart time.Time
|
||||
if workspace.AutostartSchedule.String != "" {
|
||||
templateSchedule, err := (*(r.opts.TemplateScheduleStore.Load())).Get(ctx, r.opts.Database, workspace.TemplateID)
|
||||
// If the template schedule fails to load, just default to bumping
|
||||
// without the next transition and log it.
|
||||
switch {
|
||||
case err == nil:
|
||||
next, allowed := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateSchedule)
|
||||
if allowed {
|
||||
nextAutostart = next
|
||||
// Prebuilds are not subject to activity-based deadline bumps
|
||||
if !workspace.IsPrebuild() {
|
||||
// check next autostart
|
||||
var nextAutostart time.Time
|
||||
if workspace.AutostartSchedule.String != "" {
|
||||
templateSchedule, err := (*(r.opts.TemplateScheduleStore.Load())).Get(ctx, r.opts.Database, workspace.TemplateID)
|
||||
// If the template schedule fails to load, just default to bumping
|
||||
// without the next transition and log it.
|
||||
switch {
|
||||
case err == nil:
|
||||
next, allowed := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateSchedule)
|
||||
if allowed {
|
||||
nextAutostart = next
|
||||
}
|
||||
case database.IsQueryCanceledError(err):
|
||||
r.opts.Logger.Debug(ctx, "query canceled while loading template schedule",
|
||||
slog.F("workspace_id", workspace.ID),
|
||||
slog.F("template_id", workspace.TemplateID))
|
||||
default:
|
||||
r.opts.Logger.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min",
|
||||
slog.F("workspace_id", workspace.ID),
|
||||
slog.F("template_id", workspace.TemplateID),
|
||||
slog.Error(err),
|
||||
)
|
||||
}
|
||||
case database.IsQueryCanceledError(err):
|
||||
r.opts.Logger.Debug(ctx, "query canceled while loading template schedule",
|
||||
slog.F("workspace_id", workspace.ID),
|
||||
slog.F("template_id", workspace.TemplateID))
|
||||
default:
|
||||
r.opts.Logger.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min",
|
||||
slog.F("workspace_id", workspace.ID),
|
||||
slog.F("template_id", workspace.TemplateID),
|
||||
slog.Error(err),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// bump workspace activity
|
||||
ActivityBumpWorkspace(ctx, r.opts.Logger.Named("activity_bump"), r.opts.Database, workspace.ID, nextAutostart)
|
||||
// bump workspace activity
|
||||
ActivityBumpWorkspace(ctx, r.opts.Logger.Named("activity_bump"), r.opts.Database, workspace.ID, nextAutostart)
|
||||
}
|
||||
|
||||
// bump workspace last_used_at
|
||||
r.opts.UsageTracker.Add(workspace.ID)
|
||||
|
||||
@@ -42,6 +42,7 @@ import (
|
||||
agplschedule "github.com/coder/coder/v2/coderd/schedule"
|
||||
"github.com/coder/coder/v2/coderd/schedule/cron"
|
||||
"github.com/coder/coder/v2/coderd/util/ptr"
|
||||
"github.com/coder/coder/v2/coderd/workspacestats"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
entaudit "github.com/coder/coder/v2/enterprise/audit"
|
||||
"github.com/coder/coder/v2/enterprise/audit/backends"
|
||||
@@ -2767,6 +2768,114 @@ func TestPrebuildUpdateLifecycleParams(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPrebuildActivityBump(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
clock := quartz.NewMock(t)
|
||||
clock.Set(dbtime.Now())
|
||||
|
||||
// Setup
|
||||
log := testutil.Logger(t)
|
||||
client, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{
|
||||
Options: &coderdtest.Options{
|
||||
IncludeProvisionerDaemon: true,
|
||||
Clock: clock,
|
||||
},
|
||||
LicenseOptions: &coderdenttest.LicenseOptions{
|
||||
Features: license.Features{
|
||||
codersdk.FeatureWorkspacePrebuilds: 1,
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
// Given: a template and a template version with preset and a prebuilt workspace
|
||||
presetID := uuid.New()
|
||||
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
|
||||
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
|
||||
// Configure activity bump on the template
|
||||
activityBump := time.Hour
|
||||
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
|
||||
ctr.ActivityBumpMillis = ptr.Ref[int64](activityBump.Milliseconds())
|
||||
})
|
||||
dbgen.Preset(t, db, database.InsertPresetParams{
|
||||
ID: presetID,
|
||||
TemplateVersionID: version.ID,
|
||||
DesiredInstances: sql.NullInt32{Int32: 1, Valid: true},
|
||||
})
|
||||
// Given: a prebuild with an expired Deadline
|
||||
deadline := clock.Now().Add(-30 * time.Minute)
|
||||
wb := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
|
||||
OwnerID: database.PrebuildsSystemUserID,
|
||||
TemplateID: template.ID,
|
||||
}).Seed(database.WorkspaceBuild{
|
||||
TemplateVersionID: version.ID,
|
||||
TemplateVersionPresetID: uuid.NullUUID{
|
||||
UUID: presetID,
|
||||
Valid: true,
|
||||
},
|
||||
Deadline: deadline,
|
||||
}).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
|
||||
ctx := dbauthz.AsSystemRestricted(testutil.Context(t, testutil.WaitLong))
|
||||
agent, err := db.GetWorkspaceAgentAndLatestBuildByAuthToken(ctx, uuid.MustParse(wb.AgentToken))
|
||||
require.NoError(t, err)
|
||||
err = db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{
|
||||
ID: agent.WorkspaceAgent.ID,
|
||||
LifecycleState: database.WorkspaceAgentLifecycleStateReady,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Given: a prebuilt workspace with a Deadline and an empty MaxDeadline
|
||||
prebuild := coderdtest.MustWorkspace(t, client, wb.Workspace.ID)
|
||||
require.Equal(t, deadline.UTC(), prebuild.LatestBuild.Deadline.Time.UTC())
|
||||
require.Zero(t, prebuild.LatestBuild.MaxDeadline)
|
||||
|
||||
// When: activity bump is applied to an unclaimed prebuild
|
||||
workspacestats.ActivityBumpWorkspace(ctx, log, db, prebuild.ID, clock.Now().Add(10*time.Hour))
|
||||
|
||||
// Then: prebuild Deadline/MaxDeadline remain unchanged
|
||||
prebuild = coderdtest.MustWorkspace(t, client, wb.Workspace.ID)
|
||||
require.Equal(t, deadline.UTC(), prebuild.LatestBuild.Deadline.Time.UTC())
|
||||
require.Zero(t, prebuild.LatestBuild.MaxDeadline)
|
||||
|
||||
// Given: the prebuilt workspace is claimed by a user
|
||||
user, err := client.User(ctx, "testUser")
|
||||
require.NoError(t, err)
|
||||
claimedWorkspace, err := client.CreateUserWorkspace(ctx, user.ID.String(), codersdk.CreateWorkspaceRequest{
|
||||
TemplateVersionID: version.ID,
|
||||
TemplateVersionPresetID: presetID,
|
||||
Name: coderdtest.RandomUsername(t),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, claimedWorkspace.LatestBuild.ID)
|
||||
workspace := coderdtest.MustWorkspace(t, client, claimedWorkspace.ID)
|
||||
require.Equal(t, prebuild.ID, workspace.ID)
|
||||
// Claimed workspaces have an empty Deadline and MaxDeadline
|
||||
require.Zero(t, workspace.LatestBuild.Deadline)
|
||||
require.Zero(t, workspace.LatestBuild.MaxDeadline)
|
||||
|
||||
// Given: the claimed workspace has an expired Deadline
|
||||
err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
|
||||
ID: workspace.LatestBuild.ID,
|
||||
Deadline: deadline,
|
||||
UpdatedAt: clock.Now(),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
workspace = coderdtest.MustWorkspace(t, client, claimedWorkspace.ID)
|
||||
|
||||
// When: activity bump is applied to a claimed prebuild
|
||||
workspacestats.ActivityBumpWorkspace(ctx, log, db, workspace.ID, clock.Now().Add(10*time.Hour))
|
||||
|
||||
// Then: Deadline is extended by the activity bump, MaxDeadline remains unset
|
||||
workspace = coderdtest.MustWorkspace(t, client, claimedWorkspace.ID)
|
||||
require.WithinDuration(t, clock.Now().Add(activityBump).UTC(), workspace.LatestBuild.Deadline.Time.UTC(), testutil.WaitMedium)
|
||||
require.Zero(t, workspace.LatestBuild.MaxDeadline)
|
||||
}
|
||||
|
||||
// TestWorkspaceTemplateParamsChange tests a workspace with a parameter that
|
||||
// validation changes on apply. The params used in create workspace are invalid
|
||||
// according to the static params on import.
|
||||
|
||||
Reference in New Issue
Block a user