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

Commit586586e

Browse files
fix: do not set max deadline for workspaces on template update (#12446)
* fix: do not set max deadline for workspaces on template updateWhen templates are updated and schedule data is changed, we update allrunning workspaces to have up-to-date scheduling information that sticksto the new policy.When updating the max_deadline for existing running workspaces, if themax_deadline was before now()+2h we would set the max_deadline tonow()+2h.Builds that don't/shouldn't have a max_deadline have it set to 0, whichis always before now()+2h, and thus would always have the max_deadlineupdated.* test: add unit test to excercise template schedule bug---------Co-authored-by: Steven Masley <stevenmasley@gmail.com>
1 parent17caf58 commit586586e

File tree

2 files changed

+109
-32
lines changed

2 files changed

+109
-32
lines changed

‎enterprise/coderd/schedule/template.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,15 +277,24 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte
277277
}
278278

279279
// If max deadline is before now()+2h, then set it to that.
280+
// This is intended to give ample warning to this workspace about an upcoming auto-stop.
281+
// If we were to omit this "grace" period, then this workspace could be set to be stopped "now".
282+
// The "2 hours" was an arbitrary decision for this window.
280283
now:=s.now()
281-
ifautostop.MaxDeadline.Before(now.Add(2*time.Hour)) {
284+
if!autostop.MaxDeadline.IsZero()&&autostop.MaxDeadline.Before(now.Add(2*time.Hour)) {
282285
autostop.MaxDeadline=now.Add(time.Hour*2)
283286
}
284287

285288
// If the current deadline on the build is after the new max_deadline, then
286289
// set it to the max_deadline.
287290
autostop.Deadline=build.Deadline
288-
ifautostop.Deadline.After(autostop.MaxDeadline) {
291+
if!autostop.MaxDeadline.IsZero()&&autostop.Deadline.After(autostop.MaxDeadline) {
292+
autostop.Deadline=autostop.MaxDeadline
293+
}
294+
295+
// If there's a max_deadline but the deadline is 0, then set the deadline to
296+
// the max_deadline.
297+
if!autostop.MaxDeadline.IsZero()&&autostop.Deadline.IsZero() {
289298
autostop.Deadline=autostop.MaxDeadline
290299
}
291300

‎enterprise/coderd/schedule/template_test.go

Lines changed: 98 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/coder/coder/v2/coderd/database/dbgen"
1717
"github.com/coder/coder/v2/coderd/database/dbtestutil"
1818
agplschedule"github.com/coder/coder/v2/coderd/schedule"
19+
"github.com/coder/coder/v2/coderd/util/ptr"
1920
"github.com/coder/coder/v2/cryptorand"
2021
"github.com/coder/coder/v2/enterprise/coderd/schedule"
2122
"github.com/coder/coder/v2/testutil"
@@ -27,30 +28,35 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
2728
db,_:=dbtestutil.NewDB(t)
2829

2930
var (
30-
org=dbgen.Organization(t,db, database.Organization{})
31-
user=dbgen.User(t,db, database.User{})
31+
org=dbgen.Organization(t,db, database.Organization{})
32+
quietUser=dbgen.User(t,db, database.User{
33+
Username:"quiet",
34+
})
35+
noQuietUser=dbgen.User(t,db, database.User{
36+
Username:"no-quiet",
37+
})
3238
file=dbgen.File(t,db, database.File{
33-
CreatedBy:user.ID,
39+
CreatedBy:quietUser.ID,
3440
})
3541
templateJob=dbgen.ProvisionerJob(t,db,nil, database.ProvisionerJob{
3642
OrganizationID:org.ID,
3743
FileID:file.ID,
38-
InitiatorID:user.ID,
44+
InitiatorID:quietUser.ID,
3945
Tags: database.StringMap{
4046
"foo":"bar",
4147
},
4248
})
4349
templateVersion=dbgen.TemplateVersion(t,db, database.TemplateVersion{
4450
OrganizationID:org.ID,
45-
CreatedBy:user.ID,
51+
CreatedBy:quietUser.ID,
4652
JobID:templateJob.ID,
4753
})
4854
)
4955

5056
constuserQuietHoursSchedule="CRON_TZ=UTC 0 0 * * *"// midnight UTC
5157
ctx:=testutil.Context(t,testutil.WaitLong)
52-
user,err:=db.UpdateUserQuietHoursSchedule(ctx, database.UpdateUserQuietHoursScheduleParams{
53-
ID:user.ID,
58+
quietUser,err:=db.UpdateUserQuietHoursSchedule(ctx, database.UpdateUserQuietHoursScheduleParams{
59+
ID:quietUser.ID,
5460
QuietHoursSchedule:userQuietHoursSchedule,
5561
})
5662
require.NoError(t,err)
@@ -62,20 +68,23 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
6268

6369
// Workspace old max_deadline too soon
6470
cases:= []struct {
65-
namestring
66-
now time.Time
67-
deadline time.Time
68-
maxDeadline time.Time
69-
newDeadline time.Time// 0 for no change
71+
namestring
72+
now time.Time
73+
deadline time.Time
74+
maxDeadline time.Time
75+
// Set to nil for no change.
76+
newDeadline*time.Time
7077
newMaxDeadline time.Time
78+
noQuietHoursbool
79+
autostopReq*agplschedule.TemplateAutostopRequirement
7180
}{
7281
{
7382
name:"SkippedWorkspaceMaxDeadlineTooSoon",
7483
now:buildTime,
7584
deadline:buildTime,
7685
maxDeadline:buildTime.Add(1*time.Hour),
7786
// Unchanged since the max deadline is too soon.
78-
newDeadline:time.Time{},
87+
newDeadline:nil,
7988
newMaxDeadline:buildTime.Add(1*time.Hour),
8089
},
8190
{
@@ -85,7 +94,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
8594
deadline:buildTime,
8695
// Far into the future...
8796
maxDeadline:nextQuietHours.Add(24*time.Hour),
88-
newDeadline:time.Time{},
97+
newDeadline:nil,
8998
// We will use now() + 2 hours if the newly calculated max deadline
9099
// from the workspace build time is before now.
91100
newMaxDeadline:nextQuietHours.Add(8*time.Hour),
@@ -97,7 +106,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
97106
deadline:buildTime,
98107
// Far into the future...
99108
maxDeadline:nextQuietHours.Add(24*time.Hour),
100-
newDeadline:time.Time{},
109+
newDeadline:nil,
101110
// We will use now() + 2 hours if the newly calculated max deadline
102111
// from the workspace build time is within the next 2 hours.
103112
newMaxDeadline:nextQuietHours.Add(1*time.Hour),
@@ -109,7 +118,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
109118
deadline:buildTime,
110119
// Far into the future...
111120
maxDeadline:nextQuietHours.Add(24*time.Hour),
112-
newDeadline:time.Time{},
121+
newDeadline:nil,
113122
newMaxDeadline:nextQuietHours,
114123
},
115124
{
@@ -120,7 +129,56 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
120129
deadline:nextQuietHours.Add(24*time.Hour),
121130
maxDeadline:nextQuietHours.Add(24*time.Hour),
122131
// The deadline should match since it is after the new max deadline.
123-
newDeadline:nextQuietHours,
132+
newDeadline:ptr.Ref(nextQuietHours),
133+
newMaxDeadline:nextQuietHours,
134+
},
135+
{
136+
// There was a bug if a user has no quiet hours set, and autostop
137+
// req is not turned on, then the max deadline is set to `time.Time{}`.
138+
// This zero value was "in the past", so the workspace deadline would
139+
// be set to "now" + 2 hours.
140+
// This is a mistake because the max deadline being zero means
141+
// there is no max deadline.
142+
name:"MaxDeadlineShouldBeUnset",
143+
now:buildTime,
144+
deadline:buildTime.Add(time.Hour*8),
145+
maxDeadline: time.Time{},// No max set
146+
// Should be unchanged
147+
newDeadline:ptr.Ref(buildTime.Add(time.Hour*8)),
148+
newMaxDeadline: time.Time{},
149+
noQuietHours:true,
150+
autostopReq:&agplschedule.TemplateAutostopRequirement{
151+
DaysOfWeek:0,
152+
Weeks:0,
153+
},
154+
},
155+
{
156+
// A bug existed where MaxDeadline could be set, but deadline was
157+
// `time.Time{}`. This is a logical inconsistency because the "max"
158+
// deadline was ignored.
159+
name:"NoDeadline",
160+
now:buildTime,
161+
deadline: time.Time{},
162+
maxDeadline: time.Time{},// No max set
163+
// Should be unchanged
164+
newDeadline:ptr.Ref(time.Time{}),
165+
newMaxDeadline: time.Time{},
166+
noQuietHours:true,
167+
autostopReq:&agplschedule.TemplateAutostopRequirement{
168+
DaysOfWeek:0,
169+
Weeks:0,
170+
},
171+
},
172+
173+
{
174+
// Similar to 'NoDeadline' test. This has a MaxDeadline set, so
175+
// the deadline of the workspace should now be set.
176+
name:"WorkspaceDeadlineNowSet",
177+
now:nextQuietHours.Add(-6*time.Hour),
178+
// Start with unset times
179+
deadline: time.Time{},
180+
maxDeadline: time.Time{},
181+
newDeadline:ptr.Ref(nextQuietHours),
124182
newMaxDeadline:nextQuietHours,
125183
},
126184
}
@@ -131,6 +189,11 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
131189
t.Run(c.name,func(t*testing.T) {
132190
t.Parallel()
133191

192+
user:=quietUser
193+
ifc.noQuietHours {
194+
user=noQuietUser
195+
}
196+
134197
t.Log("buildTime",buildTime)
135198
t.Log("nextQuietHours",nextQuietHours)
136199
t.Log("now",c.now)
@@ -217,17 +280,22 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
217280
templateScheduleStore.TimeNowFn=func() time.Time {
218281
returnc.now
219282
}
283+
284+
autostopReq:= agplschedule.TemplateAutostopRequirement{
285+
// Every day
286+
DaysOfWeek:0b01111111,
287+
Weeks:0,
288+
}
289+
ifc.autostopReq!=nil {
290+
autostopReq=*c.autostopReq
291+
}
220292
_,err=templateScheduleStore.Set(ctx,db,template, agplschedule.TemplateScheduleOptions{
221-
UserAutostartEnabled:false,
222-
UserAutostopEnabled:false,
223-
DefaultTTL:0,
224-
MaxTTL:0,
225-
UseMaxTTL:false,
226-
AutostopRequirement: agplschedule.TemplateAutostopRequirement{
227-
// Every day
228-
DaysOfWeek:0b01111111,
229-
Weeks:0,
230-
},
293+
UserAutostartEnabled:false,
294+
UserAutostopEnabled:false,
295+
DefaultTTL:0,
296+
MaxTTL:0,
297+
UseMaxTTL:false,
298+
AutostopRequirement:autostopReq,
231299
FailureTTL:0,
232300
TimeTilDormant:0,
233301
TimeTilDormantAutoDelete:0,
@@ -238,10 +306,10 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) {
238306
newBuild,err:=db.GetWorkspaceBuildByID(ctx,wsBuild.ID)
239307
require.NoError(t,err)
240308

241-
ifc.newDeadline.IsZero() {
242-
c.newDeadline=wsBuild.Deadline
309+
ifc.newDeadline==nil {
310+
c.newDeadline=&wsBuild.Deadline
243311
}
244-
require.WithinDuration(t,c.newDeadline,newBuild.Deadline,time.Second)
312+
require.WithinDuration(t,*c.newDeadline,newBuild.Deadline,time.Second)
245313
require.WithinDuration(t,c.newMaxDeadline,newBuild.MaxDeadline,time.Second)
246314

247315
// Check that the new build has the same state as before.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp