Skip to content

Commit 7654838

Browse files
ParkreinerParkreiner
committed
Merge branch 'main' of https://github.com/coder/coder 
2 parents f7561e1 + bd17290 commit 7654838

21 files changed

+374
-131
lines changed

‎coderd/activitybump.go

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,39 @@ import (
1212
)
1313

1414
// activityBumpWorkspace automatically bumps the workspace's auto-off timer
15-
// if it is set to expire soon.
16-
func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID) {
15+
// if it is set to expire soon. The deadline will be bumped by 1 hour*.
16+
// If the bump crosses over an autostart time, the workspace will be
17+
// bumped by the workspace ttl instead.
18+
//
19+
// If nextAutostart is the zero value or in the past, the workspace
20+
// will be bumped by 1 hour.
21+
// It handles the edge case in the example:
22+
// 1. Autostart is set to 9am.
23+
// 2. User works all day, and leaves a terminal open to the workspace overnight.
24+
// 3. The open terminal continually bumps the workspace deadline.
25+
// 4. 9am the next day, the activity bump pushes to 10am.
26+
// 5. If the user goes inactive for 1 hour during the day, the workspace will
27+
// now stop, because it has been extended by 1 hour durations. Despite the TTL
28+
// being set to 8hrs from the autostart time.
29+
//
30+
// So the issue is that when the workspace is bumped across an autostart
31+
// deadline, we should treat the workspace as being "started" again and
32+
// extend the deadline by the autostart time + workspace ttl instead.
33+
//
34+
// The issue still remains with build_max_deadline. We need to respect the original
35+
// maximum deadline, so that will need to be handled separately.
36+
// A way to avoid this is to configure the max deadline to something that will not
37+
// span more than 1 day. This will force the workspace to restart and reset the deadline
38+
// each morning when it autostarts.
39+
func activityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID, nextAutostart time.Time) {
1740
// We set a short timeout so if the app is under load, these
1841
// low priority operations fail first.
1942
ctx, cancel := context.WithTimeout(ctx, time.Second*15)
2043
defer cancel()
21-
if err := db.ActivityBumpWorkspace(ctx, workspaceID); err != nil {
44+
if err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{
45+
NextAutostart: nextAutostart.UTC(),
46+
WorkspaceID: workspaceID,
47+
}); err != nil {
2248
if !xerrors.Is(err, context.Canceled) && !database.IsQueryCanceledError(err) {
2349
// Bump will fail if the context is canceled, but this is ok.
2450
log.Error(ctx, "bump failed", slog.Error(err),

‎coderd/activitybump_internal_test.go

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
4242
templateTTL time.Duration
4343
templateDisallowsUserAutostop bool
4444
expectedBump time.Duration
45+
nextAutostart time.Time
4546
}{
4647
{
4748
name: "NotFinishedYet",
@@ -66,22 +67,41 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
6667
workspaceTTL: 8 * time.Hour,
6768
expectedBump: 0,
6869
},
70+
{
71+
// Expected bump is 0 because the original deadline is more than 1 hour
72+
// out, so a bump would decrease the deadline.
73+
name: "BumpLessThanDeadline",
74+
transition: database.WorkspaceTransitionStart,
75+
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)},
76+
buildDeadlineOffset: ptr.Ref(8*time.Hour - 30*time.Minute),
77+
workspaceTTL: 8 * time.Hour,
78+
expectedBump: 0,
79+
},
6980
{
7081
name: "TimeToBump",
7182
transition: database.WorkspaceTransitionStart,
72-
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
73-
buildDeadlineOffset: ptr.Ref(8*time.Hour - 24*time.Minute),
83+
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)},
84+
buildDeadlineOffset: ptr.Ref(-30 * time.Minute),
85+
workspaceTTL: 8 * time.Hour,
86+
expectedBump: time.Hour,
87+
},
88+
{
89+
name: "TimeToBumpNextAutostart",
90+
transition: database.WorkspaceTransitionStart,
91+
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)},
92+
buildDeadlineOffset: ptr.Ref(-30 * time.Minute),
7493
workspaceTTL: 8 * time.Hour,
75-
expectedBump: 8 * time.Hour,
94+
expectedBump: 8*time.Hour + 30*time.Minute,
95+
nextAutostart: time.Now().Add(time.Minute * 30),
7696
},
7797
{
7898
name: "MaxDeadline",
7999
transition: database.WorkspaceTransitionStart,
80100
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
81101
buildDeadlineOffset: ptr.Ref(time.Minute), // last chance to bump!
82-
maxDeadlineOffset: ptr.Ref(time.Hour),
102+
maxDeadlineOffset: ptr.Ref(time.Minute * 30),
83103
workspaceTTL: 8 * time.Hour,
84-
expectedBump: 1 * time.Hour,
104+
expectedBump: time.Minute * 30,
85105
},
86106
{
87107
// A workspace that is still running, has passed its deadline, but has not
@@ -91,7 +111,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
91111
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
92112
buildDeadlineOffset: ptr.Ref(-time.Minute),
93113
workspaceTTL: 8 * time.Hour,
94-
expectedBump: 8 * time.Hour,
114+
expectedBump: time.Hour,
95115
},
96116
{
97117
// A stopped workspace should never bump.
@@ -106,12 +126,13 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
106126
// by the template TTL instead.
107127
name: "TemplateDisallowsUserAutostop",
108128
transition: database.WorkspaceTransitionStart,
109-
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)},
110-
buildDeadlineOffset: ptr.Ref(8*time.Hour - 24*time.Minute),
111-
workspaceTTL: 6 * time.Hour,
112-
templateTTL: 8 * time.Hour,
129+
jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-7 * time.Hour)},
130+
buildDeadlineOffset: ptr.Ref(-30 * time.Minute),
131+
workspaceTTL: 2 * time.Hour,
132+
templateTTL: 10 * time.Hour,
113133
templateDisallowsUserAutostop: true,
114-
expectedBump: 8 * time.Hour,
134+
expectedBump: 10*time.Hour + (time.Minute * 30),
135+
nextAutostart: time.Now().Add(time.Minute * 30),
115136
},
116137
} {
117138
tt := tt
@@ -215,7 +236,7 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
215236

216237
// Bump duration is measured from the time of the bump, so we measure from here.
217238
start := dbtime.Now()
218-
activityBumpWorkspace(ctx, log, db, bld.WorkspaceID)
239+
activityBumpWorkspace(ctx, log, db, bld.WorkspaceID, tt.nextAutostart)
219240
end := dbtime.Now()
220241

221242
// Validate our state after bump
@@ -233,9 +254,9 @@ func Test_ActivityBumpWorkspace(t *testing.T) {
233254
return
234255
}
235256

236-
// Assert that the bump occurred between start and end.
237-
expectedDeadlineStart := start.Add(tt.expectedBump)
238-
expectedDeadlineEnd := end.Add(tt.expectedBump)
257+
// Assert that the bump occurred between start and end. 1min buffer on either side.
258+
expectedDeadlineStart := start.Add(tt.expectedBump).Add(time.Minute * -1)
259+
expectedDeadlineEnd := end.Add(tt.expectedBump).Add(time.Minute)
239260
require.GreaterOrEqual(t, updatedBuild.Deadline, expectedDeadlineStart, "new deadline should be greater than or equal to start")
240261
require.LessOrEqual(t, updatedBuild.Deadline, expectedDeadlineEnd, "new deadline should be lesser than or equal to end")
241262
})

‎coderd/activitybump_test.go

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
3030
// max_deadline on the build directly in the database.
3131
setupActivityTest := func(t *testing.T, deadline ...time.Duration) (client *codersdk.Client, workspace codersdk.Workspace, assertBumped func(want bool)) {
3232
t.Helper()
33-
const ttl = time.Minute
33+
const ttl = time.Hour
3434
maxTTL := time.Duration(0)
3535
if len(deadline) > 0 {
3636
maxTTL = deadline[0]
@@ -71,28 +71,29 @@ func TestWorkspaceActivityBump(t *testing.T) {
7171
})
7272
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
7373

74+
var maxDeadline time.Time
7475
// Update the max deadline.
7576
if maxTTL != 0 {
76-
dbBuild, err := db.GetWorkspaceBuildByID(ctx, workspace.LatestBuild.ID)
77-
require.NoError(t, err)
78-
79-
err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
80-
ID: workspace.LatestBuild.ID,
81-
UpdatedAt: dbtime.Now(),
82-
Deadline: dbBuild.Deadline,
83-
MaxDeadline: dbtime.Now().Add(maxTTL),
84-
})
85-
require.NoError(t, err)
77+
maxDeadline = dbtime.Now().Add(maxTTL)
8678
}
8779

80+
err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
81+
ID: workspace.LatestBuild.ID,
82+
UpdatedAt: dbtime.Now(),
83+
// Make the deadline really close so it needs to be bumped immediately.
84+
Deadline: dbtime.Now().Add(time.Minute),
85+
MaxDeadline: maxDeadline,
86+
})
87+
require.NoError(t, err)
88+
8889
_ = agenttest.New(t, client.URL, agentToken)
8990
coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
9091

91-
// Sanity-check that deadline is near.
92-
workspace, err := client.Workspace(ctx, workspace.ID)
92+
// Sanity-check that deadline is nearing requiring a bump.
93+
workspace, err = client.Workspace(ctx, workspace.ID)
9394
require.NoError(t, err)
9495
require.WithinDuration(t,
95-
time.Now().Add(time.Duration(ttlMillis)*time.Millisecond),
96+
time.Now().Add(time.Minute),
9697
workspace.LatestBuild.Deadline.Time,
9798
testutil.WaitMedium,
9899
)
@@ -133,7 +134,7 @@ func TestWorkspaceActivityBump(t *testing.T) {
133134
workspace, err = client.Workspace(ctx, workspace.ID)
134135
require.NoError(t, err)
135136
updatedAfter = dbtime.Now()
136-
if workspace.LatestBuild.Deadline.Time == firstDeadline {
137+
if workspace.LatestBuild.Deadline.Time.Equal(firstDeadline) {
137138
updatedAfter = time.Now()
138139
return false
139140
}
@@ -151,6 +152,13 @@ func TestWorkspaceActivityBump(t *testing.T) {
151152
require.LessOrEqual(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time)
152153
return
153154
}
155+
now := dbtime.Now()
156+
zone, offset := time.Now().Zone()
157+
t.Logf("[Zone=%s %d] originDeadline: %s, deadline: %s, now %s, (now-deadline)=%s",
158+
zone, offset,
159+
firstDeadline, workspace.LatestBuild.Deadline.Time, now,
160+
now.Sub(workspace.LatestBuild.Deadline.Time),
161+
)
154162
require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, testutil.WaitShort)
155163
}
156164
}
@@ -192,9 +200,9 @@ func TestWorkspaceActivityBump(t *testing.T) {
192200
t.Run("NotExceedMaxDeadline", func(t *testing.T) {
193201
t.Parallel()
194202

195-
// Set the max deadline to be in 61 seconds. We bump by 1 minute, so we
203+
// Set the max deadline to be in 30min. We bump by 1 hour, so we
196204
// should expect the deadline to match the max deadline exactly.
197-
client, workspace, assertBumped := setupActivityTest(t, 61*time.Second)
205+
client, workspace, assertBumped := setupActivityTest(t, time.Minute*30)
198206

199207
// Bump by dialing the workspace and sending traffic.
200208
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)

‎coderd/apidoc/docs.go

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

‎coderd/apidoc/swagger.json

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

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -357,26 +357,36 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild
357357
return false
358358
}
359359

360-
sched, err := cron.Weekly(ws.AutostartSchedule.String)
361-
if err != nil {
360+
nextTransition, allowed := NextAutostartSchedule(build.CreatedAt, ws.AutostartSchedule.String, templateSchedule)
361+
if !allowed {
362362
return false
363363
}
364+
365+
// Must use '.Before' vs '.After' so equal times are considered "valid for autostart".
366+
return !currentTick.Before(nextTransition)
367+
}
368+
369+
// NextAutostartSchedule takes the workspace and template schedule and returns the next autostart schedule
370+
// after "at". The boolean returned is if the autostart should be allowed to start based on the template
371+
// schedule.
372+
func NextAutostartSchedule(at time.Time, wsSchedule string, templateSchedule schedule.TemplateScheduleOptions) (time.Time, bool) {
373+
sched, err := cron.Weekly(wsSchedule)
374+
if err != nil {
375+
return time.Time{}, false
376+
}
377+
364378
// Round down to the nearest minute, as this is the finest granularity cron supports.
365379
// Truncate is probably not necessary here, but doing it anyway to be sure.
366-
nextTransition := sched.Next(build.CreatedAt).Truncate(time.Minute)
380+
nextTransition := sched.Next(at).Truncate(time.Minute)
367381

368382
// The nextTransition is when the auto start should kick off. If it lands on a
369383
// forbidden day, do not allow the auto start. We use the time location of the
370384
// schedule to determine the weekday. So if "Saturday" is disallowed, the
371385
// definition of "Saturday" depends on the location of the schedule.
372386
zonedTransition := nextTransition.In(sched.Location())
373387
allowed := templateSchedule.AutostartRequirement.DaysMap()[zonedTransition.Weekday()]
374-
if !allowed {
375-
return false
376-
}
377388

378-
// Must used '.Before' vs '.After' so equal times are considered "valid for autostart".
379-
return !currentTick.Before(nextTransition)
389+
return zonedTransition, allowed
380390
}
381391

382392
// isEligibleForAutostart returns true if the workspace should be autostopped.

‎coderd/autobuild/lifecycle_executor_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,6 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
387387
t.Parallel()
388388

389389
var (
390-
ctx = context.Background()
391390
tickCh = make(chan time.Time)
392391
statsCh = make(chan autobuild.Stats)
393392
client = coderdtest.New(t, &coderdtest.Options{
@@ -396,26 +395,23 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
396395
AutobuildStats: statsCh,
397396
})
398397
// Given: we have a user with a workspace
399-
workspace = mustProvisionWorkspace(t, client)
398+
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
399+
cwr.TTLMillis = nil
400+
})
400401
)
401402

402403
// Given: workspace has no TTL set
403-
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil})
404-
require.NoError(t, err)
405-
workspace, err = client.Workspace(ctx, workspace.ID)
406-
require.NoError(t, err)
404+
workspace = coderdtest.MustWorkspace(t, client, workspace.ID)
407405
require.Nil(t, workspace.TTLMillis)
408-
409-
// TODO(cian): need to stop and start the workspace as we do not update the deadline. See: #2229
410-
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
411-
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart)
406+
require.Zero(t, workspace.LatestBuild.Deadline)
407+
require.NotZero(t, workspace.LatestBuild.Job.CompletedAt)
412408

413409
// Given: workspace is running
414410
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)
415411

416-
// When: the autobuild executor ticks past the TTL
412+
// When: the autobuild executor ticks a year in the future
417413
go func() {
418-
tickCh <- workspace.LatestBuild.Deadline.Time.Add(time.Minute)
414+
tickCh <- workspace.LatestBuild.Job.CompletedAt.AddDate(1, 0, 0)
419415
close(tickCh)
420416
}()
421417

@@ -505,6 +501,10 @@ func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) {
505501
workspace = mustProvisionWorkspace(t, client)
506502
)
507503

504+
// Given: workspace is running and has a non-zero deadline
505+
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)
506+
require.NotZero(t, workspace.LatestBuild.Deadline)
507+
508508
// When: the autobuild executor ticks before the TTL
509509
go func() {
510510
tickCh <- workspace.LatestBuild.Deadline.Time.Add(-1 * time.Minute)

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -660,9 +660,9 @@ func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.Acquir
660660
return q.db.AcquireProvisionerJob(ctx, arg)
661661
}
662662

663-
func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg uuid.UUID) error {
664-
fetch := func(ctx context.Context, arg uuid.UUID) (database.Workspace, error) {
665-
return q.db.GetWorkspaceByID(ctx, arg)
663+
func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error {
664+
fetch := func(ctx context.Context, arg database.ActivityBumpWorkspaceParams) (database.Workspace, error) {
665+
return q.db.GetWorkspaceByID(ctx, arg.WorkspaceID)
666666
}
667667
return update(q.log, q.auth, fetch, q.db.ActivityBumpWorkspace)(ctx, arg)
668668
}

0 commit comments

Comments
 (0)