Skip to content

Commit 5021e23

Browse files
EmyrkEmyrk
authored
chore: compute job status as column (#10024)
* chore: provisioner job status as column * use provisioner job status for workspace searching
1 parent d504044 commit 5021e23

24 files changed

+355
-219
lines changed

‎coderd/apidoc/docs.go

+4-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+4-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/autobuild/lifecycle_executor.go

+3-4
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

-25
Original file line numberDiff line numberDiff line change
@@ -71,31 +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-
}
98-
9974
func User(user database.User, organizationIDs []uuid.UUID) codersdk.User {
10075
convertedUser := codersdk.User{
10176
ID: user.ID,

‎coderd/database/db2sdk/db2sdk_test.go

+31-2
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,17 @@ import (
44
"crypto/rand"
55
"database/sql"
66
"encoding/json"
7+
"fmt"
78
"testing"
89
"time"
910

11+
"github.com/google/uuid"
1012
"github.com/stretchr/testify/require"
1113

1214
"github.com/coder/coder/v2/coderd/database"
1315
"github.com/coder/coder/v2/coderd/database/db2sdk"
16+
"github.com/coder/coder/v2/coderd/database/dbgen"
17+
"github.com/coder/coder/v2/coderd/database/dbtestutil"
1418
"github.com/coder/coder/v2/coderd/database/dbtime"
1519
"github.com/coder/coder/v2/codersdk"
1620
"github.com/coder/coder/v2/provisionersdk/proto"
@@ -110,11 +114,36 @@ func TestProvisionerJobStatus(t *testing.T) {
110114
},
111115
}
112116

113-
for _, tc := range cases {
117+
// Share db for all job inserts.
118+
db, _ := dbtestutil.NewDB(t)
119+
org := dbgen.Organization(t, db, database.Organization{})
120+
121+
for i, tc := range cases {
114122
tc := tc
123+
i := i
115124
t.Run(tc.name, func(t *testing.T) {
116125
t.Parallel()
117-
actual := db2sdk.ProvisionerJobStatus(tc.job)
126+
// Populate standard fields
127+
now := dbtime.Now().Round(time.Minute)
128+
tc.job.ID = uuid.New()
129+
tc.job.CreatedAt = now
130+
tc.job.UpdatedAt = now
131+
tc.job.InitiatorID = org.ID
132+
tc.job.OrganizationID = org.ID
133+
tc.job.Input = []byte("{}")
134+
tc.job.Provisioner = database.ProvisionerTypeEcho
135+
// Unique tags for each job.
136+
tc.job.Tags = map[string]string{fmt.Sprintf("%d", i): "true"}
137+
138+
inserted := dbgen.ProvisionerJob(t, db, nil, tc.job)
139+
// Make sure the inserted job has the right values.
140+
require.Equal(t, tc.job.StartedAt.Time.UTC(), inserted.StartedAt.Time.UTC(), "started at")
141+
require.Equal(t, tc.job.CompletedAt.Time.UTC(), inserted.CompletedAt.Time.UTC(), "completed at")
142+
require.Equal(t, tc.job.CanceledAt.Time.UTC(), inserted.CanceledAt.Time.UTC(), "cancelled at")
143+
require.Equal(t, tc.job.Error, inserted.Error, "error")
144+
require.Equal(t, tc.job.ErrorCode, inserted.ErrorCode, "error code")
145+
146+
actual := codersdk.ProvisionerJobStatus(inserted.JobStatus)
118147
require.Equal(t, tc.status, actual)
119148
})
120149
}

‎coderd/database/dbfake/dbfake.go

+52-59
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
}
@@ -6604,61 +6628,30 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database.
66046628
// This logic should match the logic in the workspace.sql file.
66056629
var statusMatch bool
66066630
switch database.WorkspaceStatus(arg.Status) {
6607-
case database.WorkspaceStatusPending:
6608-
statusMatch = isNull(job.StartedAt)
66096631
case database.WorkspaceStatusStarting:
6610-
statusMatch = isNotNull(job.StartedAt) &&
6611-
isNull(job.CanceledAt) &&
6612-
isNull(job.CompletedAt) &&
6613-
time.Since(job.UpdatedAt) < 30*time.Second &&
6614-
build.Transition == database.WorkspaceTransitionStart
6615-
6616-
case database.WorkspaceStatusRunning:
6617-
statusMatch = isNotNull(job.CompletedAt) &&
6618-
isNull(job.CanceledAt) &&
6619-
isNull(job.Error) &&
6632+
statusMatch = job.JobStatus == database.ProvisionerJobStatusRunning &&
66206633
build.Transition == database.WorkspaceTransitionStart
6621-
66226634
case database.WorkspaceStatusStopping:
6623-
statusMatch = isNotNull(job.StartedAt) &&
6624-
isNull(job.CanceledAt) &&
6625-
isNull(job.CompletedAt) &&
6626-
time.Since(job.UpdatedAt) < 30*time.Second &&
6627-
build.Transition == database.WorkspaceTransitionStop
6628-
6629-
case database.WorkspaceStatusStopped:
6630-
statusMatch = isNotNull(job.CompletedAt) &&
6631-
isNull(job.CanceledAt) &&
6632-
isNull(job.Error) &&
6635+
statusMatch = job.JobStatus == database.ProvisionerJobStatusRunning &&
66336636
build.Transition == database.WorkspaceTransitionStop
6634-
case database.WorkspaceStatusFailed:
6635-
statusMatch = (isNotNull(job.CanceledAt) && isNotNull(job.Error)) ||
6636-
(isNotNull(job.CompletedAt) && isNotNull(job.Error))
6637-
6638-
case database.WorkspaceStatusCanceling:
6639-
statusMatch = isNotNull(job.CanceledAt) &&
6640-
isNull(job.CompletedAt)
6641-
6642-
case database.WorkspaceStatusCanceled:
6643-
statusMatch = isNotNull(job.CanceledAt) &&
6644-
isNotNull(job.CompletedAt)
6645-
6646-
case database.WorkspaceStatusDeleted:
6647-
statusMatch = isNotNull(job.StartedAt) &&
6648-
isNull(job.CanceledAt) &&
6649-
isNotNull(job.CompletedAt) &&
6650-
time.Since(job.UpdatedAt) < 30*time.Second &&
6651-
build.Transition == database.WorkspaceTransitionDelete &&
6652-
isNull(job.Error)
6653-
66546637
case database.WorkspaceStatusDeleting:
6655-
statusMatch = isNull(job.CompletedAt) &&
6656-
isNull(job.CanceledAt) &&
6657-
isNull(job.Error) &&
6638+
statusMatch = job.JobStatus == database.ProvisionerJobStatusRunning &&
66586639
build.Transition == database.WorkspaceTransitionDelete
66596640

6641+
case "started":
6642+
statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded &&
6643+
build.Transition == database.WorkspaceTransitionStart
6644+
case database.WorkspaceStatusDeleted:
6645+
statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded &&
6646+
build.Transition == database.WorkspaceTransitionDelete
6647+
case database.WorkspaceStatusStopped:
6648+
statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded &&
6649+
build.Transition == database.WorkspaceTransitionStop
6650+
case database.WorkspaceStatusRunning:
6651+
statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded &&
6652+
build.Transition == database.WorkspaceTransitionStart
66606653
default:
6661-
return nil, xerrors.Errorf("unknown workspace status in filter: %q", arg.Status)
6654+
statusMatch = job.JobStatus == database.ProvisionerJobStatus(arg.Status)
66626655
}
66636656
if !statusMatch {
66646657
continue

‎coderd/database/dbgen/dbgen.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -328,16 +328,16 @@ func GroupMember(t testing.TB, db database.Store, orig database.GroupMember) dat
328328
// ProvisionerJob is a bit more involved to get the values such as "completedAt", "startedAt", "cancelledAt" set. ps
329329
// can be set to nil if you are SURE that you don't require a provisionerdaemon to acquire the job in your test.
330330
func ProvisionerJob(t testing.TB, db database.Store, ps pubsub.Pubsub, orig database.ProvisionerJob) database.ProvisionerJob {
331-
id := takeFirst(orig.ID, uuid.New())
331+
jobID := takeFirst(orig.ID, uuid.New())
332332
// Always set some tags to prevent Acquire from grabbing jobs it should not.
333333
if !orig.StartedAt.Time.IsZero() {
334334
if orig.Tags == nil {
335335
orig.Tags = make(database.StringMap)
336336
}
337337
// Make sure when we acquire the job, we only get this one.
338-
orig.Tags[id.String()] = "true"
338+
orig.Tags[jobID.String()] = "true"
339339
}
340-
jobID := takeFirst(orig.ID, uuid.New())
340+
341341
job, err := db.InsertProvisionerJob(genCtx, database.InsertProvisionerJobParams{
342342
ID: jobID,
343343
CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()),
@@ -365,6 +365,8 @@ func ProvisionerJob(t testing.TB, db database.Store, ps pubsub.Pubsub, orig data
365365
WorkerID: uuid.NullUUID{},
366366
})
367367
require.NoError(t, err)
368+
// There is no easy way to make sure we acquire the correct job.
369+
require.Equal(t, jobID, job.ID, "acquired incorrect job")
368370
}
369371

370372
if !orig.CompletedAt.Time.IsZero() || orig.Error.String != "" {

0 commit comments

Comments
 (0)