Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: prevent activity bump for prebuilt workspaces#19263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ssncferreira merged 9 commits intomainfromssncferreira/fix-activity-bump-prebuilds
Aug 20, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
9 commits
Select commitHold shift + click to select a range
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletionscoderd/database/queries.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

8 changes: 6 additions & 2 deletionscoderd/database/queries/activitybump.sql
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
)
Expand Down
51 changes: 27 additions & 24 deletionscoderd/workspacestats/reporter.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

suggestion: invert the logic and continue / return early

Copy link
ContributorAuthor

@ssncferreirassncferreiraAug 19, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Initially, that was my approach, but this function seems to do a lot of things:

  • Update agent stats
  • Update prometheus metrics (for observability)
  • Activity bump (for lifecycle management)
  • Bump workspacelast_used_at (for usage tracking)
  • Sendstats_update notify event (AFAIU this is related with real-time updates on the UI regarding the workspace).

Therefore, I kept all these updates except the activity bump since it was the only related with lifecycle parameters. For the case of prebuilds I believe it should be all or nothing, either this approach or we check if it is a prebuild in the beginning of the function and return early. Not sure what is the workflow that makes more sense here for prebuilds.

johnstcn reacted with thumbs up emoji
// 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)
Expand Down
109 changes: 109 additions & 0 deletionsenterprise/coderd/workspaces_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"
Expand DownExpand Up@@ -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.
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp