diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 6c217c052f0ff..04744a5ea4b86 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -277,15 +277,24 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte } // If max deadline is before now()+2h, then set it to that. + // This is intended to give ample warning to this workspace about an upcoming auto-stop. + // If we were to omit this "grace" period, then this workspace could be set to be stopped "now". + // The "2 hours" was an arbitrary decision for this window. now := s.now() - if autostop.MaxDeadline.Before(now.Add(2 * time.Hour)) { + if !autostop.MaxDeadline.IsZero() && autostop.MaxDeadline.Before(now.Add(2*time.Hour)) { autostop.MaxDeadline = now.Add(time.Hour * 2) } // If the current deadline on the build is after the new max_deadline, then // set it to the max_deadline. autostop.Deadline = build.Deadline - if autostop.Deadline.After(autostop.MaxDeadline) { + if !autostop.MaxDeadline.IsZero() && autostop.Deadline.After(autostop.MaxDeadline) { + autostop.Deadline = autostop.MaxDeadline + } + + // If there's a max_deadline but the deadline is 0, then set the deadline to + // the max_deadline. + if !autostop.MaxDeadline.IsZero() && autostop.Deadline.IsZero() { autostop.Deadline = autostop.MaxDeadline } diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index ac158a795b3a3..4df05c026496f 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" agplschedule "github.com/coder/coder/v2/coderd/schedule" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/cryptorand" "github.com/coder/coder/v2/enterprise/coderd/schedule" "github.com/coder/coder/v2/testutil" @@ -27,30 +28,35 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { db, _ := dbtestutil.NewDB(t) var ( - org = dbgen.Organization(t, db, database.Organization{}) - user = dbgen.User(t, db, database.User{}) + org = dbgen.Organization(t, db, database.Organization{}) + quietUser = dbgen.User(t, db, database.User{ + Username: "quiet", + }) + noQuietUser = dbgen.User(t, db, database.User{ + Username: "no-quiet", + }) file = dbgen.File(t, db, database.File{ - CreatedBy: user.ID, + CreatedBy: quietUser.ID, }) templateJob = dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ OrganizationID: org.ID, FileID: file.ID, - InitiatorID: user.ID, + InitiatorID: quietUser.ID, Tags: database.StringMap{ "foo": "bar", }, }) templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{ OrganizationID: org.ID, - CreatedBy: user.ID, + CreatedBy: quietUser.ID, JobID: templateJob.ID, }) ) const userQuietHoursSchedule = "CRON_TZ=UTC 0 0 * * *" // midnight UTC ctx := testutil.Context(t, testutil.WaitLong) - user, err := db.UpdateUserQuietHoursSchedule(ctx, database.UpdateUserQuietHoursScheduleParams{ - ID: user.ID, + quietUser, err := db.UpdateUserQuietHoursSchedule(ctx, database.UpdateUserQuietHoursScheduleParams{ + ID: quietUser.ID, QuietHoursSchedule: userQuietHoursSchedule, }) require.NoError(t, err) @@ -62,12 +68,15 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { // Workspace old max_deadline too soon cases := []struct { - name string - now time.Time - deadline time.Time - maxDeadline time.Time - newDeadline time.Time // 0 for no change + name string + now time.Time + deadline time.Time + maxDeadline time.Time + // Set to nil for no change. + newDeadline *time.Time newMaxDeadline time.Time + noQuietHours bool + autostopReq *agplschedule.TemplateAutostopRequirement }{ { name: "SkippedWorkspaceMaxDeadlineTooSoon", @@ -75,7 +84,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { deadline: buildTime, maxDeadline: buildTime.Add(1 * time.Hour), // Unchanged since the max deadline is too soon. - newDeadline: time.Time{}, + newDeadline: nil, newMaxDeadline: buildTime.Add(1 * time.Hour), }, { @@ -85,7 +94,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { deadline: buildTime, // Far into the future... maxDeadline: nextQuietHours.Add(24 * time.Hour), - newDeadline: time.Time{}, + newDeadline: nil, // We will use now() + 2 hours if the newly calculated max deadline // from the workspace build time is before now. newMaxDeadline: nextQuietHours.Add(8 * time.Hour), @@ -97,7 +106,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { deadline: buildTime, // Far into the future... maxDeadline: nextQuietHours.Add(24 * time.Hour), - newDeadline: time.Time{}, + newDeadline: nil, // We will use now() + 2 hours if the newly calculated max deadline // from the workspace build time is within the next 2 hours. newMaxDeadline: nextQuietHours.Add(1 * time.Hour), @@ -109,7 +118,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { deadline: buildTime, // Far into the future... maxDeadline: nextQuietHours.Add(24 * time.Hour), - newDeadline: time.Time{}, + newDeadline: nil, newMaxDeadline: nextQuietHours, }, { @@ -120,7 +129,56 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { deadline: nextQuietHours.Add(24 * time.Hour), maxDeadline: nextQuietHours.Add(24 * time.Hour), // The deadline should match since it is after the new max deadline. - newDeadline: nextQuietHours, + newDeadline: ptr.Ref(nextQuietHours), + newMaxDeadline: nextQuietHours, + }, + { + // There was a bug if a user has no quiet hours set, and autostop + // req is not turned on, then the max deadline is set to `time.Time{}`. + // This zero value was "in the past", so the workspace deadline would + // be set to "now" + 2 hours. + // This is a mistake because the max deadline being zero means + // there is no max deadline. + name: "MaxDeadlineShouldBeUnset", + now: buildTime, + deadline: buildTime.Add(time.Hour * 8), + maxDeadline: time.Time{}, // No max set + // Should be unchanged + newDeadline: ptr.Ref(buildTime.Add(time.Hour * 8)), + newMaxDeadline: time.Time{}, + noQuietHours: true, + autostopReq: &agplschedule.TemplateAutostopRequirement{ + DaysOfWeek: 0, + Weeks: 0, + }, + }, + { + // A bug existed where MaxDeadline could be set, but deadline was + // `time.Time{}`. This is a logical inconsistency because the "max" + // deadline was ignored. + name: "NoDeadline", + now: buildTime, + deadline: time.Time{}, + maxDeadline: time.Time{}, // No max set + // Should be unchanged + newDeadline: ptr.Ref(time.Time{}), + newMaxDeadline: time.Time{}, + noQuietHours: true, + autostopReq: &agplschedule.TemplateAutostopRequirement{ + DaysOfWeek: 0, + Weeks: 0, + }, + }, + + { + // Similar to 'NoDeadline' test. This has a MaxDeadline set, so + // the deadline of the workspace should now be set. + name: "WorkspaceDeadlineNowSet", + now: nextQuietHours.Add(-6 * time.Hour), + // Start with unset times + deadline: time.Time{}, + maxDeadline: time.Time{}, + newDeadline: ptr.Ref(nextQuietHours), newMaxDeadline: nextQuietHours, }, } @@ -131,6 +189,11 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { t.Run(c.name, func(t *testing.T) { t.Parallel() + user := quietUser + if c.noQuietHours { + user = noQuietUser + } + t.Log("buildTime", buildTime) t.Log("nextQuietHours", nextQuietHours) t.Log("now", c.now) @@ -217,17 +280,22 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { templateScheduleStore.TimeNowFn = func() time.Time { return c.now } + + autostopReq := agplschedule.TemplateAutostopRequirement{ + // Every day + DaysOfWeek: 0b01111111, + Weeks: 0, + } + if c.autostopReq != nil { + autostopReq = *c.autostopReq + } _, err = templateScheduleStore.Set(ctx, db, template, agplschedule.TemplateScheduleOptions{ - UserAutostartEnabled: false, - UserAutostopEnabled: false, - DefaultTTL: 0, - MaxTTL: 0, - UseMaxTTL: false, - AutostopRequirement: agplschedule.TemplateAutostopRequirement{ - // Every day - DaysOfWeek: 0b01111111, - Weeks: 0, - }, + UserAutostartEnabled: false, + UserAutostopEnabled: false, + DefaultTTL: 0, + MaxTTL: 0, + UseMaxTTL: false, + AutostopRequirement: autostopReq, FailureTTL: 0, TimeTilDormant: 0, TimeTilDormantAutoDelete: 0, @@ -238,10 +306,10 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { newBuild, err := db.GetWorkspaceBuildByID(ctx, wsBuild.ID) require.NoError(t, err) - if c.newDeadline.IsZero() { - c.newDeadline = wsBuild.Deadline + if c.newDeadline == nil { + c.newDeadline = &wsBuild.Deadline } - require.WithinDuration(t, c.newDeadline, newBuild.Deadline, time.Second) + require.WithinDuration(t, *c.newDeadline, newBuild.Deadline, time.Second) require.WithinDuration(t, c.newMaxDeadline, newBuild.MaxDeadline, time.Second) // Check that the new build has the same state as before.