Skip to content

Commit 7736647

Browse files
EmyrkEmyrk
committed
chore: provisioner job status as column
1 parent 70a4e56 commit 7736647

File tree

15 files changed

+349
-174
lines changed

15 files changed

+349
-174
lines changed

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"cdr.dev/slog"
1515
"github.com/coder/coder/v2/coderd/database"
16-
"github.com/coder/coder/v2/coderd/database/db2sdk"
1716
"github.com/coder/coder/v2/coderd/database/dbauthz"
1817
"github.com/coder/coder/v2/coderd/database/dbtime"
1918
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
@@ -303,7 +302,7 @@ func getNextTransition(
303302
// isEligibleForAutostart returns true if the workspace should be autostarted.
304303
func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) bool {
305304
// Don't attempt to autostart failed workspaces.
306-
if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed {
305+
if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed {
307306
return false
308307
}
309308

@@ -337,7 +336,7 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild
337336

338337
// isEligibleForAutostart returns true if the workspace should be autostopped.
339338
func isEligibleForAutostop(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, currentTick time.Time) bool {
340-
if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed {
339+
if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed {
341340
return false
342341
}
343342

@@ -379,7 +378,7 @@ func isEligibleForFailedStop(build database.WorkspaceBuild, job database.Provisi
379378
// If the template has specified a failure TLL.
380379
return templateSchedule.FailureTTL > 0 &&
381380
// And the job resulted in failure.
382-
db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed &&
381+
codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed &&
383382
build.Transition == database.WorkspaceTransitionStart &&
384383
// And sufficient time has elapsed since the job has completed.
385384
job.CompletedAt.Valid &&

‎coderd/database/db2sdk/db2sdk.go

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -71,30 +71,6 @@ func TemplateVersionParameter(param database.TemplateVersionParameter) (codersdk
7171
}, nil
7272
}
7373

74-
func ProvisionerJobStatus(provisionerJob database.ProvisionerJob) codersdk.ProvisionerJobStatus {
75-
// The case where jobs are hung is handled by the unhang package. We can't
76-
// just return Failed here when it's hung because that doesn't reflect in
77-
// the database.
78-
switch {
79-
case provisionerJob.CanceledAt.Valid:
80-
if !provisionerJob.CompletedAt.Valid {
81-
return codersdk.ProvisionerJobCanceling
82-
}
83-
if provisionerJob.Error.String == "" {
84-
return codersdk.ProvisionerJobCanceled
85-
}
86-
return codersdk.ProvisionerJobFailed
87-
case !provisionerJob.StartedAt.Valid:
88-
return codersdk.ProvisionerJobPending
89-
case provisionerJob.CompletedAt.Valid:
90-
if provisionerJob.Error.String == "" {
91-
return codersdk.ProvisionerJobSucceeded
92-
}
93-
return codersdk.ProvisionerJobFailed
94-
default:
95-
return codersdk.ProvisionerJobRunning
96-
}
97-
}
9874

9975
func User(user database.User, organizationIDs []uuid.UUID) codersdk.User {
10076
convertedUser := codersdk.User{

‎coderd/database/db2sdk/db2sdk_test.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,13 @@ import (
77
"testing"
88
"time"
99

10+
"github.com/google/uuid"
1011
"github.com/stretchr/testify/require"
1112

1213
"github.com/coder/coder/v2/coderd/database"
1314
"github.com/coder/coder/v2/coderd/database/db2sdk"
15+
"github.com/coder/coder/v2/coderd/database/dbgen"
16+
"github.com/coder/coder/v2/coderd/database/dbtestutil"
1417
"github.com/coder/coder/v2/coderd/database/dbtime"
1518
"github.com/coder/coder/v2/codersdk"
1619
"github.com/coder/coder/v2/provisionersdk/proto"
@@ -110,11 +113,33 @@ func TestProvisionerJobStatus(t *testing.T) {
110113
},
111114
}
112115

116+
// Share db for all job inserts.
117+
db, _ := dbtestutil.NewDB(t)
118+
org := dbgen.Organization(t, db, database.Organization{})
119+
113120
for _, tc := range cases {
114121
tc := tc
115122
t.Run(tc.name, func(t *testing.T) {
116123
t.Parallel()
117-
actual := db2sdk.ProvisionerJobStatus(tc.job)
124+
// Populate standard fields
125+
now := dbtime.Now().Round(time.Minute)
126+
tc.job.ID = uuid.New()
127+
tc.job.CreatedAt = now
128+
tc.job.UpdatedAt = now
129+
tc.job.InitiatorID = org.ID
130+
tc.job.OrganizationID = org.ID
131+
tc.job.Input = []byte("{}")
132+
tc.job.Provisioner = database.ProvisionerTypeEcho
133+
134+
inserted := dbgen.ProvisionerJob(t, db, nil, tc.job)
135+
// Make sure the inserted job has the right values.
136+
require.Equal(t, tc.job.StartedAt.Time.UTC(), inserted.StartedAt.Time.UTC(), "started at")
137+
require.Equal(t, tc.job.CompletedAt.Time.UTC(), inserted.CompletedAt.Time.UTC(), "completed at")
138+
require.Equal(t, tc.job.CanceledAt.Time.UTC(), inserted.CanceledAt.Time.UTC(), "cancelled at")
139+
require.Equal(t, tc.job.Error, inserted.Error, "error")
140+
require.Equal(t, tc.job.ErrorCode, inserted.ErrorCode, "error code")
141+
142+
actual := codersdk.ProvisionerJobStatus(inserted.JobStatus)
118143
require.Equal(t, tc.status, actual)
119144
})
120145
}

‎coderd/database/dbfake/dbfake.go

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"golang.org/x/xerrors"
2121

2222
"github.com/coder/coder/v2/coderd/database"
23-
"github.com/coder/coder/v2/coderd/database/db2sdk"
2423
"github.com/coder/coder/v2/coderd/database/dbtime"
2524
"github.com/coder/coder/v2/coderd/httpapi"
2625
"github.com/coder/coder/v2/coderd/rbac"
@@ -604,16 +603,6 @@ func (q *FakeQuerier) getGroupByIDNoLock(_ context.Context, id uuid.UUID) (datab
604603
return database.Group{}, sql.ErrNoRows
605604
}
606605

607-
// isNull is only used in dbfake, so reflect is ok. Use this to make the logic
608-
// look more similar to the postgres.
609-
func isNull(v interface{}) bool {
610-
return !isNotNull(v)
611-
}
612-
613-
func isNotNull(v interface{}) bool {
614-
return reflect.ValueOf(v).FieldByName("Valid").Bool()
615-
}
616-
617606
// ErrUnimplemented is returned by methods only used by the enterprise/tailnet.pgCoord. This coordinator explicitly
618607
// depends on postgres triggers that announce changes on the pubsub. Implementing support for this in the fake
619608
// database would strongly couple the FakeQuerier to the pubsub, which is undesirable. Furthermore, it makes little
@@ -695,6 +684,36 @@ func minTime(t, u time.Time) time.Time {
695684
return u
696685
}
697686

687+
func provisonerJobStatus(j database.ProvisionerJob) database.ProvisionerJobStatus {
688+
if isNotNull(j.CompletedAt) {
689+
if j.Error.String != "" {
690+
return database.ProvisionerJobStatusFailed
691+
}
692+
if isNotNull(j.CanceledAt) {
693+
return database.ProvisionerJobStatusCanceled
694+
}
695+
return database.ProvisionerJobStatusSucceeded
696+
}
697+
698+
if isNotNull(j.CanceledAt) {
699+
return database.ProvisionerJobStatusCanceling
700+
}
701+
if isNull(j.StartedAt) {
702+
return database.ProvisionerJobStatusPending
703+
}
704+
return database.ProvisionerJobStatusRunning
705+
}
706+
707+
// isNull is only used in dbfake, so reflect is ok. Use this to make the logic
708+
// look more similar to the postgres.
709+
func isNull(v interface{}) bool {
710+
return !isNotNull(v)
711+
}
712+
713+
func isNotNull(v interface{}) bool {
714+
return reflect.ValueOf(v).FieldByName("Valid").Bool()
715+
}
716+
698717
func (*FakeQuerier) AcquireLock(_ context.Context, _ int64) error {
699718
return xerrors.New("AcquireLock must only be called within a transaction")
700719
}
@@ -748,6 +767,7 @@ func (q *FakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.Acqu
748767
provisionerJob.StartedAt = arg.StartedAt
749768
provisionerJob.UpdatedAt = arg.StartedAt.Time
750769
provisionerJob.WorkerID = arg.WorkerID
770+
provisionerJob.JobStatus = provisonerJobStatus(provisionerJob)
751771
q.provisionerJobs[index] = provisionerJob
752772
return provisionerJob, nil
753773
}
@@ -4077,7 +4097,7 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no
40774097
if err != nil {
40784098
return nil, xerrors.Errorf("get provisioner job by ID: %w", err)
40794099
}
4080-
if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed {
4100+
if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed {
40814101
workspaces = append(workspaces, workspace)
40824102
continue
40834103
}
@@ -4464,6 +4484,7 @@ func (q *FakeQuerier) InsertProvisionerJob(_ context.Context, arg database.Inser
44644484
Input: arg.Input,
44654485
Tags: arg.Tags,
44664486
}
4487+
job.JobStatus = provisonerJobStatus(job)
44674488
q.provisionerJobs = append(q.provisionerJobs, job)
44684489
return job, nil
44694490
}
@@ -5393,6 +5414,7 @@ func (q *FakeQuerier) UpdateProvisionerJobByID(_ context.Context, arg database.U
53935414
continue
53945415
}
53955416
job.UpdatedAt = arg.UpdatedAt
5417+
job.JobStatus = provisonerJobStatus(job)
53965418
q.provisionerJobs[index] = job
53975419
return nil
53985420
}
@@ -5413,6 +5435,7 @@ func (q *FakeQuerier) UpdateProvisionerJobWithCancelByID(_ context.Context, arg
54135435
}
54145436
job.CanceledAt = arg.CanceledAt
54155437
job.CompletedAt = arg.CompletedAt
5438+
job.JobStatus = provisonerJobStatus(job)
54165439
q.provisionerJobs[index] = job
54175440
return nil
54185441
}
@@ -5435,6 +5458,7 @@ func (q *FakeQuerier) UpdateProvisionerJobWithCompleteByID(_ context.Context, ar
54355458
job.CompletedAt = arg.CompletedAt
54365459
job.Error = arg.Error
54375460
job.ErrorCode = arg.ErrorCode
5461+
job.JobStatus = provisonerJobStatus(job)
54385462
q.provisionerJobs[index] = job
54395463
return nil
54405464
}

‎coderd/database/dump.sql

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

‎coderd/database/migrations/000160_provisioner_job_status.down.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
BEGIN;
2+
3+
ALTER TABLE provisioner_jobs DROP COLUMN job_status;
4+
DROP TYPE provisioner_job_status;
5+
6+
COMMIT;

‎coderd/database/migrations/000160_provisioner_job_status.up.sql

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
BEGIN;
2+
3+
CREATE TYPE provisioner_job_status AS ENUM ('pending', 'running', 'succeeded', 'canceling', 'canceled', 'failed', 'unknown');
4+
COMMENT ON TYPE provisioner_job_status IS 'Computed status of a provisioner job. Jobs could be stuck in a hung state, these states do not guarantee any transition to another state.';
5+
6+
ALTER TABLE provisioner_jobs ADD COLUMN
7+
job_status provisioner_job_status NOT NULL GENERATED ALWAYS AS (
8+
CASE
9+
-- Completed means it is not in an "-ing" state
10+
WHEN completed_at IS NOT NULL THEN
11+
CASE
12+
-- The order of these checks are important.
13+
-- Check the error first, then cancelled, then completed.
14+
WHEN error != '' THEN 'failed'::provisioner_job_status
15+
WHEN canceled_at IS NOT NULL THEN 'canceled'::provisioner_job_status
16+
ELSE 'succeeded'::provisioner_job_status
17+
END
18+
-- Not completed means it is in some "-ing" state
19+
ELSE
20+
CASE
21+
WHEN canceled_at IS NOT NULL THEN 'canceling'::provisioner_job_status
22+
-- Not done and not started means it is pending
23+
WHEN started_at IS NULL THEN 'pending'::provisioner_job_status
24+
-- Job is still running
25+
ELSE 'running'::provisioner_job_status
26+
END
27+
END
28+
-- Stored so we do not have to recompute it every time.
29+
) STORED;
30+
31+
32+
COMMENT ON COLUMN provisioner_jobs.job_status IS 'Computed column to track the status of the job.';
33+
34+
COMMIT;

0 commit comments

Comments
 (0)