Skip to content

Commit dc59885

Browse files
deansheatherdeansheather
authored
chore: improve build deadline code (#19203)
- Adds/improves a lot of comments to make the autostop calculation code clearer - Changes the behavior of the enterprise template schedule store to match the behavior of the workspace TTL endpoint when the new TTL is zero - Fixes a bug in the workspace TTL endpoint where it could unset the build deadline, even though a max_deadline was specified - Adds a new constraint to the workspace_builds table that enforces the deadline is non-zero and below the max_deadline if it is set - Adds CHECK constraint enum generation to scripts/dbgen, used for testing the above constraint - Adds Dean and Danielle as CODEOWNERS for the autostop calculation code
1 parent ec3b8ce commit dc59885

16 files changed

+658
-256
lines changed

‎CODEOWNERS

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,8 @@ site/src/api/countriesGenerated.ts
2929
site/src/api/rbacresourcesGenerated.ts
3030
site/src/api/typesGenerated.ts
3131
site/CLAUDE.md
32+
33+
# The blood and guts of the autostop algorithm, which is quite complex and
34+
# requires elite ball knowledge of most of the scheduling code to make changes
35+
# without inadvertently affecting other parts of the codebase.
36+
coderd/schedule/autostop.go @deansheather @DanielleMaywood

‎coderd/database/check_constraint.go

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

‎coderd/database/dump.sql

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/errors.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,28 @@ func IsForeignKeyViolation(err error, foreignKeyConstraints ...ForeignKeyConstra
5959
return false
6060
}
6161

62+
// IsCheckViolation checks if the error is due to a check violation. If one or
63+
// more specific check constraints are given as arguments, the error must be
64+
// caused by one of them. If no constraints are given, this function returns
65+
// true for any check violation.
66+
func IsCheckViolation(err error, checkConstraints ...CheckConstraint) bool {
67+
var pqErr *pq.Error
68+
if errors.As(err, &pqErr) {
69+
if pqErr.Code.Name() == "check_violation" {
70+
if len(checkConstraints) == 0 {
71+
return true
72+
}
73+
for _, cc := range checkConstraints {
74+
if pqErr.Constraint == string(cc) {
75+
return true
76+
}
77+
}
78+
}
79+
}
80+
81+
return false
82+
}
83+
6284
// IsQueryCanceledError checks if the error is due to a query being canceled.
6385
func IsQueryCanceledError(err error) bool {
6486
var pqErr *pq.Error

‎coderd/database/migrations/000356_enforce_deadline_below_max_deadline.down.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE workspace_builds
2+
DROP CONSTRAINT workspace_builds_deadline_below_max_deadline;

‎coderd/database/migrations/000356_enforce_deadline_below_max_deadline.up.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
-- New constraint: (deadline IS NOT zero AND deadline <= max_deadline) UNLESS max_deadline is zero.
2+
-- Unfortunately, "zero" here means `time.Time{}`...
3+
4+
-- Update previous builds that would fail this new constraint. This matches the
5+
-- intended behaviour of the autostop algorithm.
6+
UPDATE
7+
workspace_builds
8+
SET
9+
deadline = max_deadline
10+
WHERE
11+
deadline > max_deadline
12+
AND max_deadline != '0001-01-01 00:00:00+00';
13+
14+
-- Add the new constraint.
15+
ALTER TABLE workspace_builds
16+
ADD CONSTRAINT workspace_builds_deadline_below_max_deadline
17+
CHECK (
18+
(deadline != '0001-01-01 00:00:00+00'::timestamptz AND deadline <= max_deadline)
19+
OR max_deadline = '0001-01-01 00:00:00+00'::timestamptz
20+
);

‎coderd/database/querier_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6003,3 +6003,102 @@ func TestGetRunningPrebuiltWorkspaces(t *testing.T) {
60036003
require.Len(t, runningPrebuilds, 1, "expected only one running prebuilt workspace")
60046004
require.Equal(t, runningPrebuild.ID, runningPrebuilds[0].ID, "expected the running prebuilt workspace to be returned")
60056005
}
6006+
6007+
func TestWorkspaceBuildDeadlineConstraint(t *testing.T) {
6008+
t.Parallel()
6009+
6010+
ctx := testutil.Context(t, testutil.WaitLong)
6011+
6012+
db, _ := dbtestutil.NewDB(t)
6013+
org := dbgen.Organization(t, db, database.Organization{})
6014+
user := dbgen.User(t, db, database.User{})
6015+
template := dbgen.Template(t, db, database.Template{
6016+
CreatedBy: user.ID,
6017+
OrganizationID: org.ID,
6018+
})
6019+
templateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{
6020+
TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true},
6021+
OrganizationID: org.ID,
6022+
CreatedBy: user.ID,
6023+
})
6024+
workspace := dbgen.Workspace(t, db, database.WorkspaceTable{
6025+
OwnerID: user.ID,
6026+
TemplateID: template.ID,
6027+
Name: "test-workspace",
6028+
Deleted: false,
6029+
})
6030+
job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{
6031+
OrganizationID: org.ID,
6032+
InitiatorID: database.PrebuildsSystemUserID,
6033+
Provisioner: database.ProvisionerTypeEcho,
6034+
Type: database.ProvisionerJobTypeWorkspaceBuild,
6035+
StartedAt: sql.NullTime{Time: time.Now().Add(-time.Minute), Valid: true},
6036+
CompletedAt: sql.NullTime{Time: time.Now(), Valid: true},
6037+
})
6038+
workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{
6039+
WorkspaceID: workspace.ID,
6040+
TemplateVersionID: templateVersion.ID,
6041+
JobID: job.ID,
6042+
BuildNumber: 1,
6043+
})
6044+
6045+
cases := []struct {
6046+
name string
6047+
deadline time.Time
6048+
maxDeadline time.Time
6049+
expectOK bool
6050+
}{
6051+
{
6052+
name: "no deadline or max_deadline",
6053+
deadline: time.Time{},
6054+
maxDeadline: time.Time{},
6055+
expectOK: true,
6056+
},
6057+
{
6058+
name: "deadline set when max_deadline is not set",
6059+
deadline: time.Now().Add(time.Hour),
6060+
maxDeadline: time.Time{},
6061+
expectOK: true,
6062+
},
6063+
{
6064+
name: "deadline before max_deadline",
6065+
deadline: time.Now().Add(-time.Hour),
6066+
maxDeadline: time.Now().Add(time.Hour),
6067+
expectOK: true,
6068+
},
6069+
{
6070+
name: "deadline is max_deadline",
6071+
deadline: time.Now().Add(time.Hour),
6072+
maxDeadline: time.Now().Add(time.Hour),
6073+
expectOK: true,
6074+
},
6075+
6076+
{
6077+
name: "deadline after max_deadline",
6078+
deadline: time.Now().Add(time.Hour),
6079+
maxDeadline: time.Now().Add(-time.Hour),
6080+
expectOK: false,
6081+
},
6082+
{
6083+
name: "deadline is not set when max_deadline is set",
6084+
deadline: time.Time{},
6085+
maxDeadline: time.Now().Add(time.Hour),
6086+
expectOK: false,
6087+
},
6088+
}
6089+
6090+
for _, c := range cases {
6091+
err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
6092+
ID: workspaceBuild.ID,
6093+
Deadline: c.deadline,
6094+
MaxDeadline: c.maxDeadline,
6095+
UpdatedAt: time.Now(),
6096+
})
6097+
if c.expectOK {
6098+
require.NoError(t, err)
6099+
} else {
6100+
require.Error(t, err)
6101+
require.True(t, database.IsCheckViolation(err, database.CheckWorkspaceBuildsDeadlineBelowMaxDeadline))
6102+
}
6103+
}
6104+
}

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1866,8 +1866,9 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
18661866
Database: db,
18671867
TemplateScheduleStore: templateScheduleStore,
18681868
UserQuietHoursScheduleStore: *s.UserQuietHoursScheduleStore.Load(),
1869-
Now: now,
1870-
Workspace: workspace.WorkspaceTable(),
1869+
// `now` is used below to set the build completion time.
1870+
WorkspaceBuildCompletedAt: now,
1871+
Workspace: workspace.WorkspaceTable(),
18711872
// Allowed to be the empty string.
18721873
WorkspaceAutostart: workspace.AutostartSchedule.String,
18731874
})

0 commit comments

Comments
 (0)