From 7736647ba92bd3b69305d56762966553a6d5e1a4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 13:53:55 -0500 Subject: [PATCH 01/22] chore: provisioner job status as column --- coderd/autobuild/lifecycle_executor.go | 7 +- coderd/database/db2sdk/db2sdk.go | 24 -- coderd/database/db2sdk/db2sdk_test.go | 27 +- coderd/database/dbfake/dbfake.go | 48 +++- coderd/database/dump.sql | 33 ++- .../000160_provisioner_job_status.down.sql | 6 + .../000160_provisioner_job_status.up.sql | 34 +++ coderd/database/models.go | 76 ++++++ coderd/database/queries.sql.go | 247 +++++++++--------- coderd/prometheusmetrics/prometheusmetrics.go | 5 +- coderd/provisionerjobs.go | 5 +- coderd/unhanger/detector.go | 3 +- coderd/wsbuilder/wsbuilder.go | 4 +- codersdk/provisionerdaemons.go | 1 + enterprise/coderd/schedule/template.go | 3 +- 15 files changed, 349 insertions(+), 174 deletions(-) create mode 100644 coderd/database/migrations/000160_provisioner_job_status.down.sql create mode 100644 coderd/database/migrations/000160_provisioner_job_status.up.sql diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 294db0034447b..65e3e90afee49 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -13,7 +13,6 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" @@ -303,7 +302,7 @@ func getNextTransition( // isEligibleForAutostart returns true if the workspace should be autostarted. func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, templateSchedule schedule.TemplateScheduleOptions, currentTick time.Time) bool { // Don't attempt to autostart failed workspaces. - if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed { + if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed { return false } @@ -337,7 +336,7 @@ func isEligibleForAutostart(ws database.Workspace, build database.WorkspaceBuild // isEligibleForAutostart returns true if the workspace should be autostopped. func isEligibleForAutostop(ws database.Workspace, build database.WorkspaceBuild, job database.ProvisionerJob, currentTick time.Time) bool { - if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed { + if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed { return false } @@ -379,7 +378,7 @@ func isEligibleForFailedStop(build database.WorkspaceBuild, job database.Provisi // If the template has specified a failure TLL. return templateSchedule.FailureTTL > 0 && // And the job resulted in failure. - db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed && + codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed && build.Transition == database.WorkspaceTransitionStart && // And sufficient time has elapsed since the job has completed. job.CompletedAt.Valid && diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index d6a5bf4b69ef0..88701060d6a9c 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -71,30 +71,6 @@ func TemplateVersionParameter(param database.TemplateVersionParameter) (codersdk }, nil } -func ProvisionerJobStatus(provisionerJob database.ProvisionerJob) codersdk.ProvisionerJobStatus { - // The case where jobs are hung is handled by the unhang package. We can't - // just return Failed here when it's hung because that doesn't reflect in - // the database. - switch { - case provisionerJob.CanceledAt.Valid: - if !provisionerJob.CompletedAt.Valid { - return codersdk.ProvisionerJobCanceling - } - if provisionerJob.Error.String == "" { - return codersdk.ProvisionerJobCanceled - } - return codersdk.ProvisionerJobFailed - case !provisionerJob.StartedAt.Valid: - return codersdk.ProvisionerJobPending - case provisionerJob.CompletedAt.Valid: - if provisionerJob.Error.String == "" { - return codersdk.ProvisionerJobSucceeded - } - return codersdk.ProvisionerJobFailed - default: - return codersdk.ProvisionerJobRunning - } -} func User(user database.User, organizationIDs []uuid.UUID) codersdk.User { convertedUser := codersdk.User{ diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go index d1d360ad61cee..c6695019eff9b 100644 --- a/coderd/database/db2sdk/db2sdk_test.go +++ b/coderd/database/db2sdk/db2sdk_test.go @@ -7,10 +7,13 @@ import ( "testing" "time" + "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionersdk/proto" @@ -110,11 +113,33 @@ func TestProvisionerJobStatus(t *testing.T) { }, } + // Share db for all job inserts. + db, _ := dbtestutil.NewDB(t) + org := dbgen.Organization(t, db, database.Organization{}) + for _, tc := range cases { tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() - actual := db2sdk.ProvisionerJobStatus(tc.job) + // Populate standard fields + now := dbtime.Now().Round(time.Minute) + tc.job.ID = uuid.New() + tc.job.CreatedAt = now + tc.job.UpdatedAt = now + tc.job.InitiatorID = org.ID + tc.job.OrganizationID = org.ID + tc.job.Input = []byte("{}") + tc.job.Provisioner = database.ProvisionerTypeEcho + + inserted := dbgen.ProvisionerJob(t, db, nil, tc.job) + // Make sure the inserted job has the right values. + require.Equal(t, tc.job.StartedAt.Time.UTC(), inserted.StartedAt.Time.UTC(), "started at") + require.Equal(t, tc.job.CompletedAt.Time.UTC(), inserted.CompletedAt.Time.UTC(), "completed at") + require.Equal(t, tc.job.CanceledAt.Time.UTC(), inserted.CanceledAt.Time.UTC(), "cancelled at") + require.Equal(t, tc.job.Error, inserted.Error, "error") + require.Equal(t, tc.job.ErrorCode, inserted.ErrorCode, "error code") + + actual := codersdk.ProvisionerJobStatus(inserted.JobStatus) require.Equal(t, tc.status, actual) }) } diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 1e6e4d9d410e7..9c586fce93335 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -20,7 +20,6 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/rbac" @@ -604,16 +603,6 @@ func (q *FakeQuerier) getGroupByIDNoLock(_ context.Context, id uuid.UUID) (datab return database.Group{}, sql.ErrNoRows } -// isNull is only used in dbfake, so reflect is ok. Use this to make the logic -// look more similar to the postgres. -func isNull(v interface{}) bool { - return !isNotNull(v) -} - -func isNotNull(v interface{}) bool { - return reflect.ValueOf(v).FieldByName("Valid").Bool() -} - // ErrUnimplemented is returned by methods only used by the enterprise/tailnet.pgCoord. This coordinator explicitly // depends on postgres triggers that announce changes on the pubsub. Implementing support for this in the fake // 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 { return u } +func provisonerJobStatus(j database.ProvisionerJob) database.ProvisionerJobStatus { + if isNotNull(j.CompletedAt) { + if j.Error.String != "" { + return database.ProvisionerJobStatusFailed + } + if isNotNull(j.CanceledAt) { + return database.ProvisionerJobStatusCanceled + } + return database.ProvisionerJobStatusSucceeded + } + + if isNotNull(j.CanceledAt) { + return database.ProvisionerJobStatusCanceling + } + if isNull(j.StartedAt) { + return database.ProvisionerJobStatusPending + } + return database.ProvisionerJobStatusRunning +} + +// isNull is only used in dbfake, so reflect is ok. Use this to make the logic +// look more similar to the postgres. +func isNull(v interface{}) bool { + return !isNotNull(v) +} + +func isNotNull(v interface{}) bool { + return reflect.ValueOf(v).FieldByName("Valid").Bool() +} + func (*FakeQuerier) AcquireLock(_ context.Context, _ int64) error { return xerrors.New("AcquireLock must only be called within a transaction") } @@ -748,6 +767,7 @@ func (q *FakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.Acqu provisionerJob.StartedAt = arg.StartedAt provisionerJob.UpdatedAt = arg.StartedAt.Time provisionerJob.WorkerID = arg.WorkerID + provisionerJob.JobStatus = provisonerJobStatus(provisionerJob) q.provisionerJobs[index] = provisionerJob return provisionerJob, nil } @@ -4077,7 +4097,7 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no if err != nil { return nil, xerrors.Errorf("get provisioner job by ID: %w", err) } - if db2sdk.ProvisionerJobStatus(job) == codersdk.ProvisionerJobFailed { + if codersdk.ProvisionerJobStatus(job.JobStatus) == codersdk.ProvisionerJobFailed { workspaces = append(workspaces, workspace) continue } @@ -4464,6 +4484,7 @@ func (q *FakeQuerier) InsertProvisionerJob(_ context.Context, arg database.Inser Input: arg.Input, Tags: arg.Tags, } + job.JobStatus = provisonerJobStatus(job) q.provisionerJobs = append(q.provisionerJobs, job) return job, nil } @@ -5393,6 +5414,7 @@ func (q *FakeQuerier) UpdateProvisionerJobByID(_ context.Context, arg database.U continue } job.UpdatedAt = arg.UpdatedAt + job.JobStatus = provisonerJobStatus(job) q.provisionerJobs[index] = job return nil } @@ -5413,6 +5435,7 @@ func (q *FakeQuerier) UpdateProvisionerJobWithCancelByID(_ context.Context, arg } job.CanceledAt = arg.CanceledAt job.CompletedAt = arg.CompletedAt + job.JobStatus = provisonerJobStatus(job) q.provisionerJobs[index] = job return nil } @@ -5435,6 +5458,7 @@ func (q *FakeQuerier) UpdateProvisionerJobWithCompleteByID(_ context.Context, ar job.CompletedAt = arg.CompletedAt job.Error = arg.Error job.ErrorCode = arg.ErrorCode + job.JobStatus = provisonerJobStatus(job) q.provisionerJobs[index] = job return nil } diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 17bb9c539e741..2a1b311874c26 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -89,6 +89,18 @@ CREATE TYPE parameter_type_system AS ENUM ( 'hcl' ); +CREATE TYPE provisioner_job_status AS ENUM ( + 'pending', + 'running', + 'succeeded', + 'canceling', + 'canceled', + 'failed', + 'unknown' +); + +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.'; + CREATE TYPE provisioner_job_type AS ENUM ( 'template_version_import', 'workspace_build', @@ -500,8 +512,25 @@ CREATE TABLE provisioner_jobs ( file_id uuid NOT NULL, tags jsonb DEFAULT '{"scope": "organization"}'::jsonb NOT NULL, error_code text, - trace_metadata jsonb -); + trace_metadata jsonb, + job_status provisioner_job_status GENERATED ALWAYS AS ( +CASE + WHEN (completed_at IS NOT NULL) THEN + CASE + WHEN (error <> ''::text) THEN 'failed'::provisioner_job_status + WHEN (canceled_at IS NOT NULL) THEN 'canceled'::provisioner_job_status + ELSE 'succeeded'::provisioner_job_status + END + ELSE + CASE + WHEN (canceled_at IS NOT NULL) THEN 'canceling'::provisioner_job_status + WHEN (started_at IS NULL) THEN 'pending'::provisioner_job_status + ELSE 'running'::provisioner_job_status + END +END) STORED NOT NULL +); + +COMMENT ON COLUMN provisioner_jobs.job_status IS 'Computed column to track the status of the job.'; CREATE TABLE replicas ( id uuid NOT NULL, diff --git a/coderd/database/migrations/000160_provisioner_job_status.down.sql b/coderd/database/migrations/000160_provisioner_job_status.down.sql new file mode 100644 index 0000000000000..3f04c8dd11dfc --- /dev/null +++ b/coderd/database/migrations/000160_provisioner_job_status.down.sql @@ -0,0 +1,6 @@ +BEGIN; + +ALTER TABLE provisioner_jobs DROP COLUMN job_status; +DROP TYPE provisioner_job_status; + +COMMIT; diff --git a/coderd/database/migrations/000160_provisioner_job_status.up.sql b/coderd/database/migrations/000160_provisioner_job_status.up.sql new file mode 100644 index 0000000000000..5a0f73a097049 --- /dev/null +++ b/coderd/database/migrations/000160_provisioner_job_status.up.sql @@ -0,0 +1,34 @@ +BEGIN; + +CREATE TYPE provisioner_job_status AS ENUM ('pending', 'running', 'succeeded', 'canceling', 'canceled', 'failed', 'unknown'); +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.'; + +ALTER TABLE provisioner_jobs ADD COLUMN + job_status provisioner_job_status NOT NULL GENERATED ALWAYS AS ( + CASE + -- Completed means it is not in an "-ing" state + WHEN completed_at IS NOT NULL THEN + CASE + -- The order of these checks are important. + -- Check the error first, then cancelled, then completed. + WHEN error != '' THEN 'failed'::provisioner_job_status + WHEN canceled_at IS NOT NULL THEN 'canceled'::provisioner_job_status + ELSE 'succeeded'::provisioner_job_status + END + -- Not completed means it is in some "-ing" state + ELSE + CASE + WHEN canceled_at IS NOT NULL THEN 'canceling'::provisioner_job_status + -- Not done and not started means it is pending + WHEN started_at IS NULL THEN 'pending'::provisioner_job_status + -- Job is still running + ELSE 'running'::provisioner_job_status + END + END + -- Stored so we do not have to recompute it every time. + ) STORED; + + +COMMENT ON COLUMN provisioner_jobs.job_status IS 'Computed column to track the status of the job.'; + +COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index ab6d8861ee434..bc9ba550ef93f 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -837,6 +837,80 @@ func AllParameterTypeSystemValues() []ParameterTypeSystem { } } +// Computed status of a provisioner job. Jobs could be stuck in a hung state, these states do not guarantee any transition to another state. +type ProvisionerJobStatus string + +const ( + ProvisionerJobStatusPending ProvisionerJobStatus = "pending" + ProvisionerJobStatusRunning ProvisionerJobStatus = "running" + ProvisionerJobStatusSucceeded ProvisionerJobStatus = "succeeded" + ProvisionerJobStatusCanceling ProvisionerJobStatus = "canceling" + ProvisionerJobStatusCanceled ProvisionerJobStatus = "canceled" + ProvisionerJobStatusFailed ProvisionerJobStatus = "failed" + ProvisionerJobStatusUnknown ProvisionerJobStatus = "unknown" +) + +func (e *ProvisionerJobStatus) Scan(src interface{}) error { + switch s := src.(type) { + case []byte: + *e = ProvisionerJobStatus(s) + case string: + *e = ProvisionerJobStatus(s) + default: + return fmt.Errorf("unsupported scan type for ProvisionerJobStatus: %T", src) + } + return nil +} + +type NullProvisionerJobStatus struct { + ProvisionerJobStatus ProvisionerJobStatus `json:"provisioner_job_status"` + Valid bool `json:"valid"` // Valid is true if ProvisionerJobStatus is not NULL +} + +// Scan implements the Scanner interface. +func (ns *NullProvisionerJobStatus) Scan(value interface{}) error { + if value == nil { + ns.ProvisionerJobStatus, ns.Valid = "", false + return nil + } + ns.Valid = true + return ns.ProvisionerJobStatus.Scan(value) +} + +// Value implements the driver Valuer interface. +func (ns NullProvisionerJobStatus) Value() (driver.Value, error) { + if !ns.Valid { + return nil, nil + } + return string(ns.ProvisionerJobStatus), nil +} + +func (e ProvisionerJobStatus) Valid() bool { + switch e { + case ProvisionerJobStatusPending, + ProvisionerJobStatusRunning, + ProvisionerJobStatusSucceeded, + ProvisionerJobStatusCanceling, + ProvisionerJobStatusCanceled, + ProvisionerJobStatusFailed, + ProvisionerJobStatusUnknown: + return true + } + return false +} + +func AllProvisionerJobStatusValues() []ProvisionerJobStatus { + return []ProvisionerJobStatus{ + ProvisionerJobStatusPending, + ProvisionerJobStatusRunning, + ProvisionerJobStatusSucceeded, + ProvisionerJobStatusCanceling, + ProvisionerJobStatusCanceled, + ProvisionerJobStatusFailed, + ProvisionerJobStatusUnknown, + } +} + type ProvisionerJobType string const ( @@ -1671,6 +1745,8 @@ type ProvisionerJob struct { Tags StringMap `db:"tags" json:"tags"` ErrorCode sql.NullString `db:"error_code" json:"error_code"` TraceMetadata pqtype.NullRawMessage `db:"trace_metadata" json:"trace_metadata"` + // Computed column to track the status of the job. + JobStatus ProvisionerJobStatus `db:"job_status" json:"job_status"` } type ProvisionerJobLog struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 40b7ee7e72715..85a7f5dac7ba7 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2988,7 +2988,7 @@ WHERE SKIP LOCKED LIMIT 1 - ) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata + ) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status ` type AcquireProvisionerJobParams struct { @@ -3031,13 +3031,14 @@ func (q *sqlQuerier) AcquireProvisionerJob(ctx context.Context, arg AcquireProvi &i.Tags, &i.ErrorCode, &i.TraceMetadata, + &i.JobStatus, ) return i, err } const getHungProvisionerJobs = `-- name: GetHungProvisionerJobs :many SELECT - id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status FROM provisioner_jobs WHERE @@ -3074,6 +3075,7 @@ func (q *sqlQuerier) GetHungProvisionerJobs(ctx context.Context, updatedAt time. &i.Tags, &i.ErrorCode, &i.TraceMetadata, + &i.JobStatus, ); err != nil { return nil, err } @@ -3090,7 +3092,7 @@ func (q *sqlQuerier) GetHungProvisionerJobs(ctx context.Context, updatedAt time. const getProvisionerJobByID = `-- name: GetProvisionerJobByID :one SELECT - id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status FROM provisioner_jobs WHERE @@ -3119,13 +3121,14 @@ func (q *sqlQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (P &i.Tags, &i.ErrorCode, &i.TraceMetadata, + &i.JobStatus, ) return i, err } const getProvisionerJobsByIDs = `-- name: GetProvisionerJobsByIDs :many SELECT - id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata + id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status FROM provisioner_jobs WHERE @@ -3160,6 +3163,7 @@ func (q *sqlQuerier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUI &i.Tags, &i.ErrorCode, &i.TraceMetadata, + &i.JobStatus, ); err != nil { return nil, err } @@ -3194,7 +3198,7 @@ queue_size AS ( SELECT COUNT(*) as count FROM unstarted_jobs ) SELECT - pj.id, pj.created_at, pj.updated_at, pj.started_at, pj.canceled_at, pj.completed_at, pj.error, pj.organization_id, pj.initiator_id, pj.provisioner, pj.storage_method, pj.type, pj.input, pj.worker_id, pj.file_id, pj.tags, pj.error_code, pj.trace_metadata, + pj.id, pj.created_at, pj.updated_at, pj.started_at, pj.canceled_at, pj.completed_at, pj.error, pj.organization_id, pj.initiator_id, pj.provisioner, pj.storage_method, pj.type, pj.input, pj.worker_id, pj.file_id, pj.tags, pj.error_code, pj.trace_metadata, pj.job_status, COALESCE(qp.queue_position, 0) AS queue_position, COALESCE(qs.count, 0) AS queue_size FROM @@ -3241,6 +3245,7 @@ func (q *sqlQuerier) GetProvisionerJobsByIDsWithQueuePosition(ctx context.Contex &i.ProvisionerJob.Tags, &i.ProvisionerJob.ErrorCode, &i.ProvisionerJob.TraceMetadata, + &i.ProvisionerJob.JobStatus, &i.QueuePosition, &i.QueueSize, ); err != nil { @@ -3258,7 +3263,7 @@ func (q *sqlQuerier) GetProvisionerJobsByIDsWithQueuePosition(ctx context.Contex } const getProvisionerJobsCreatedAfter = `-- name: GetProvisionerJobsCreatedAfter :many -SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata FROM provisioner_jobs WHERE created_at > $1 +SELECT id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status FROM provisioner_jobs WHERE created_at > $1 ` func (q *sqlQuerier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]ProvisionerJob, error) { @@ -3289,6 +3294,7 @@ func (q *sqlQuerier) GetProvisionerJobsCreatedAfter(ctx context.Context, created &i.Tags, &i.ErrorCode, &i.TraceMetadata, + &i.JobStatus, ); err != nil { return nil, err } @@ -3320,7 +3326,7 @@ INSERT INTO trace_metadata ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) RETURNING id, created_at, updated_at, started_at, canceled_at, completed_at, error, organization_id, initiator_id, provisioner, storage_method, type, input, worker_id, file_id, tags, error_code, trace_metadata, job_status ` type InsertProvisionerJobParams struct { @@ -3373,6 +3379,7 @@ func (q *sqlQuerier) InsertProvisionerJob(ctx context.Context, arg InsertProvisi &i.Tags, &i.ErrorCode, &i.TraceMetadata, + &i.JobStatus, ) return i, err } @@ -9554,6 +9561,119 @@ func (q *sqlQuerier) InsertWorkspaceResourceMetadata(ctx context.Context, arg In return items, nil } +const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many +SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) +` + +func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many +INSERT INTO + workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) +SELECT + $1 :: uuid AS workspace_agent_id, + $2 :: timestamptz AS created_at, + unnest($3 :: uuid [ ]) AS log_source_id, + unnest($4 :: text [ ]) AS log_path, + unnest($5 :: text [ ]) AS script, + unnest($6 :: text [ ]) AS cron, + unnest($7 :: boolean [ ]) AS start_blocks_login, + unnest($8 :: boolean [ ]) AS run_on_start, + unnest($9 :: boolean [ ]) AS run_on_stop, + unnest($10 :: integer [ ]) AS timeout_seconds +RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds +` + +type InsertWorkspaceAgentScriptsParams struct { + WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` + LogPath []string `db:"log_path" json:"log_path"` + Script []string `db:"script" json:"script"` + Cron []string `db:"cron" json:"cron"` + StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` + RunOnStart []bool `db:"run_on_start" json:"run_on_start"` + RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` + TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` +} + +func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, + arg.WorkspaceAgentID, + arg.CreatedAt, + pq.Array(arg.LogSourceID), + pq.Array(arg.LogPath), + pq.Array(arg.Script), + pq.Array(arg.Cron), + pq.Array(arg.StartBlocksLogin), + pq.Array(arg.RunOnStart), + pq.Array(arg.RunOnStop), + pq.Array(arg.TimeoutSeconds), + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getDeploymentWorkspaceStats = `-- name: GetDeploymentWorkspaceStats :one WITH workspaces_with_jobs AS ( SELECT @@ -10502,116 +10622,3 @@ func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.C _, err := q.db.ExecContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) return err } - -const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many -SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) -` - -func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - -const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many -INSERT INTO - workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) -SELECT - $1 :: uuid AS workspace_agent_id, - $2 :: timestamptz AS created_at, - unnest($3 :: uuid [ ]) AS log_source_id, - unnest($4 :: text [ ]) AS log_path, - unnest($5 :: text [ ]) AS script, - unnest($6 :: text [ ]) AS cron, - unnest($7 :: boolean [ ]) AS start_blocks_login, - unnest($8 :: boolean [ ]) AS run_on_start, - unnest($9 :: boolean [ ]) AS run_on_stop, - unnest($10 :: integer [ ]) AS timeout_seconds -RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds -` - -type InsertWorkspaceAgentScriptsParams struct { - WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` - LogPath []string `db:"log_path" json:"log_path"` - Script []string `db:"script" json:"script"` - Cron []string `db:"cron" json:"cron"` - StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` - RunOnStart []bool `db:"run_on_start" json:"run_on_start"` - RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` - TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` -} - -func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, - arg.WorkspaceAgentID, - arg.CreatedAt, - pq.Array(arg.LogSourceID), - pq.Array(arg.LogPath), - pq.Array(arg.Script), - pq.Array(arg.Cron), - pq.Array(arg.StartBlocksLogin), - pq.Array(arg.RunOnStart), - pq.Array(arg.RunOnStop), - pq.Array(arg.TimeoutSeconds), - ) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} diff --git a/coderd/prometheusmetrics/prometheusmetrics.go b/coderd/prometheusmetrics/prometheusmetrics.go index b020912f9932b..7145c2afa3b39 100644 --- a/coderd/prometheusmetrics/prometheusmetrics.go +++ b/coderd/prometheusmetrics/prometheusmetrics.go @@ -10,13 +10,14 @@ import ( "sync/atomic" "time" + "github.com/coder/coder/v2/codersdk" + "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "tailscale.com/tailcfg" "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/tailnet" @@ -119,7 +120,7 @@ func Workspaces(ctx context.Context, registerer prometheus.Registerer, db databa gauge.Reset() for _, job := range jobs { - status := db2sdk.ProvisionerJobStatus(job) + status := codersdk.ProvisionerJobStatus(job.JobStatus) gauge.WithLabelValues(string(status)).Add(1) } } diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go index 5e3d27ee32800..315fe38593e6d 100644 --- a/coderd/provisionerjobs.go +++ b/coderd/provisionerjobs.go @@ -17,7 +17,6 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/httpapi" @@ -258,7 +257,7 @@ func convertProvisionerJob(pj database.GetProvisionerJobsByIDsWithQueuePositionR if provisionerJob.WorkerID.Valid { job.WorkerID = &provisionerJob.WorkerID.UUID } - job.Status = db2sdk.ProvisionerJobStatus(provisionerJob) + job.Status = codersdk.ProvisionerJobStatus(pj.ProvisionerJob.JobStatus) return job } @@ -282,7 +281,7 @@ func fetchAndWriteLogs(ctx context.Context, db database.Store, jobID uuid.UUID, } func jobIsComplete(logger slog.Logger, job database.ProvisionerJob) bool { - status := db2sdk.ProvisionerJobStatus(job) + status := codersdk.ProvisionerJobStatus(job.JobStatus) switch status { case codersdk.ProvisionerJobCanceled: return true diff --git a/coderd/unhanger/detector.go b/coderd/unhanger/detector.go index c250711b52457..9a3440f705ed7 100644 --- a/coderd/unhanger/detector.go +++ b/coderd/unhanger/detector.go @@ -14,7 +14,6 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/pubsub" @@ -240,7 +239,7 @@ func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubs } if job.CompletedAt.Valid { return jobInelligibleError{ - Err: xerrors.Errorf("job is completed (status %s)", db2sdk.ProvisionerJobStatus(job)), + Err: xerrors.Errorf("job is completed (status %s)", job.JobStatus), } } if job.UpdatedAt.After(time.Now().Add(-HungJobDuration)) { diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index f7c1699d29329..70fbf8f45479c 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -718,7 +718,7 @@ func (b *Builder) checkTemplateJobStatus() error { } } - templateVersionJobStatus := db2sdk.ProvisionerJobStatus(*templateVersionJob) + templateVersionJobStatus := codersdk.ProvisionerJobStatus((*templateVersionJob).JobStatus) switch templateVersionJobStatus { case codersdk.ProvisionerJobPending, codersdk.ProvisionerJobRunning: msg := fmt.Sprintf("The provided template version is %s. Wait for it to complete importing!", templateVersionJobStatus) @@ -755,7 +755,7 @@ func (b *Builder) checkRunningBuild() error { if err != nil { return BuildError{http.StatusInternalServerError, "failed to fetch prior build", err} } - if db2sdk.ProvisionerJobStatus(*job).Active() { + if codersdk.ProvisionerJobStatus((*job).JobStatus).Active() { msg := "A workspace build is already active." return BuildError{ http.StatusConflict, diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go index 4e787c6fe1a7b..ce2dd08758b8c 100644 --- a/codersdk/provisionerdaemons.go +++ b/codersdk/provisionerdaemons.go @@ -64,6 +64,7 @@ const ( ProvisionerJobCanceling ProvisionerJobStatus = "canceling" ProvisionerJobCanceled ProvisionerJobStatus = "canceled" ProvisionerJobFailed ProvisionerJobStatus = "failed" + ProvisionerJobUnknown ProvisionerJobStatus = "unknown" ) // JobErrorCode defines the error code returned by job runner. diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index b686bd7f9fb60..c78d9718762b6 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -11,7 +11,6 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" agpl "github.com/coder/coder/v2/coderd/schedule" @@ -242,7 +241,7 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte if err != nil { return xerrors.Errorf("get provisioner job %q: %w", build.JobID, err) } - if db2sdk.ProvisionerJobStatus(job) != codersdk.ProvisionerJobSucceeded { + if codersdk.ProvisionerJobStatus(job.JobStatus) != codersdk.ProvisionerJobSucceeded { // Only touch builds that are completed. return nil } From 16e17e8a5689f40c11f5d0e169def7633d4831f7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 14:05:43 -0500 Subject: [PATCH 02/22] use provisioner job status for workspace searching --- .../000160_provisioner_job_status.up.sql | 6 +- coderd/database/queries/workspaces.sql | 77 +++++++------------ 2 files changed, 34 insertions(+), 49 deletions(-) diff --git a/coderd/database/migrations/000160_provisioner_job_status.up.sql b/coderd/database/migrations/000160_provisioner_job_status.up.sql index 5a0f73a097049..015722a406658 100644 --- a/coderd/database/migrations/000160_provisioner_job_status.up.sql +++ b/coderd/database/migrations/000160_provisioner_job_status.up.sql @@ -18,10 +18,14 @@ ALTER TABLE provisioner_jobs ADD COLUMN -- Not completed means it is in some "-ing" state ELSE CASE + -- This should never happen because all errors set + -- should also set a completed_at timestamp. + -- But if there is an error, we should always return + -- a failed state. + WHEN error != '' THEN 'failed'::provisioner_job_status WHEN canceled_at IS NOT NULL THEN 'canceling'::provisioner_job_status -- Not done and not started means it is pending WHEN started_at IS NULL THEN 'pending'::provisioner_job_status - -- Job is still running ELSE 'running'::provisioner_job_status END END diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 0aa073301eb8f..7de4639f34911 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -96,7 +96,8 @@ LEFT JOIN LATERAL ( provisioner_jobs.updated_at, provisioner_jobs.canceled_at, provisioner_jobs.completed_at, - provisioner_jobs.error + provisioner_jobs.error, + provisioner_jobs.job_status FROM workspace_builds LEFT JOIN @@ -128,63 +129,43 @@ WHERE AND CASE WHEN @status :: text != '' THEN CASE - WHEN @status = 'pending' THEN - latest_build.started_at IS NULL + -- Some workspace specific status refer to the transition + -- type. By default, the standard provisioner job status + -- search strings are supported. + -- 'running' states WHEN @status = 'starting' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND + latest_build.job_status = 'running' AND + -- TODO: Should we drop the interval? Hung workspaces + -- should still be their last state? latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'start'::workspace_transition - - WHEN @status = 'running' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'start'::workspace_transition - WHEN @status = 'stopping' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND + latest_build.job_status = 'running' AND + -- TODO: Should we drop the interval? Hung workspaces + -- should still be their last state? latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'stop'::workspace_transition + WHEN @status = 'deleting' THEN + latest_build.job_status = 'running' AND + -- TODO: Should we drop the interval? Hung workspaces + -- should still be their last state? + latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND + latest_build.transition = 'delete'::workspace_transition + -- 'succeeded' states + WHEN @status = 'deleted' THEN + latest_build.job_status = 'succeeded' AND + latest_build.transition = 'delete'::workspace_transition WHEN @status = 'stopped' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND + latest_build.job_status = 'succeeded' AND latest_build.transition = 'stop'::workspace_transition + WHEN @status = 'started' THEN + latest_build.job_status = 'succeeded' AND + latest_build.transition = 'start'::workspace_transition - WHEN @status = 'failed' THEN - (latest_build.canceled_at IS NOT NULL AND - latest_build.error IS NOT NULL) OR - (latest_build.completed_at IS NOT NULL AND - latest_build.error IS NOT NULL) - - WHEN @status = 'canceling' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NULL - - WHEN @status = 'canceled' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NOT NULL - - WHEN @status = 'deleted' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NOT NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'delete'::workspace_transition AND - -- If the error field is not null, the status is 'failed' - latest_build.error IS NULL - - WHEN @status = 'deleting' THEN - latest_build.completed_at IS NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'delete'::workspace_transition - + WHEN @status != '' THEN + -- By default just match the job status exactly + latest_build.job_status = @status ELSE true END From a5d8268d8f412606802943abd372b319e2a29d23 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 14:41:15 -0500 Subject: [PATCH 03/22] Linting --- coderd/wsbuilder/wsbuilder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 70fbf8f45479c..008bc88ab72ab 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -718,7 +718,7 @@ func (b *Builder) checkTemplateJobStatus() error { } } - templateVersionJobStatus := codersdk.ProvisionerJobStatus((*templateVersionJob).JobStatus) + templateVersionJobStatus := codersdk.ProvisionerJobStatus(templateVersionJob.JobStatus) switch templateVersionJobStatus { case codersdk.ProvisionerJobPending, codersdk.ProvisionerJobRunning: msg := fmt.Sprintf("The provided template version is %s. Wait for it to complete importing!", templateVersionJobStatus) @@ -755,7 +755,7 @@ func (b *Builder) checkRunningBuild() error { if err != nil { return BuildError{http.StatusInternalServerError, "failed to fetch prior build", err} } - if codersdk.ProvisionerJobStatus((*job).JobStatus).Active() { + if codersdk.ProvisionerJobStatus(job.JobStatus).Active() { msg := "A workspace build is already active." return BuildError{ http.StatusConflict, From 9e2002499ec9b4529f73f826273b8947fa9b081a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 14:42:58 -0500 Subject: [PATCH 04/22] Remove intervals --- coderd/database/queries/workspaces.sql | 9 --------- 1 file changed, 9 deletions(-) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 7de4639f34911..946a29cef2b4e 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -135,21 +135,12 @@ WHERE -- 'running' states WHEN @status = 'starting' THEN latest_build.job_status = 'running' AND - -- TODO: Should we drop the interval? Hung workspaces - -- should still be their last state? - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'start'::workspace_transition WHEN @status = 'stopping' THEN latest_build.job_status = 'running' AND - -- TODO: Should we drop the interval? Hung workspaces - -- should still be their last state? - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'stop'::workspace_transition WHEN @status = 'deleting' THEN latest_build.job_status = 'running' AND - -- TODO: Should we drop the interval? Hung workspaces - -- should still be their last state? - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'delete'::workspace_transition -- 'succeeded' states From f21d58c94adc4441e33e35bab97cc90844ff5b8a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 15:20:35 -0500 Subject: [PATCH 05/22] make gen --- coderd/apidoc/docs.go | 6 ++- coderd/apidoc/swagger.json | 6 ++- coderd/database/db2sdk/db2sdk.go | 1 - coderd/database/dump.sql | 1 + coderd/database/queries.sql.go | 72 ++++++++++---------------------- docs/api/schemas.md | 1 + site/src/api/typesGenerated.ts | 4 +- 7 files changed, 35 insertions(+), 56 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index e523745f5e303..759671714f3a4 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -9124,7 +9124,8 @@ const docTemplate = `{ "succeeded", "canceling", "canceled", - "failed" + "failed", + "unknown" ], "x-enum-varnames": [ "ProvisionerJobPending", @@ -9132,7 +9133,8 @@ const docTemplate = `{ "ProvisionerJobSucceeded", "ProvisionerJobCanceling", "ProvisionerJobCanceled", - "ProvisionerJobFailed" + "ProvisionerJobFailed", + "ProvisionerJobUnknown" ] }, "codersdk.ProvisionerLogLevel": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index a97a865aaeb9d..90272bcbf8349 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -8211,7 +8211,8 @@ "succeeded", "canceling", "canceled", - "failed" + "failed", + "unknown" ], "x-enum-varnames": [ "ProvisionerJobPending", @@ -8219,7 +8220,8 @@ "ProvisionerJobSucceeded", "ProvisionerJobCanceling", "ProvisionerJobCanceled", - "ProvisionerJobFailed" + "ProvisionerJobFailed", + "ProvisionerJobUnknown" ] }, "codersdk.ProvisionerLogLevel": { diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 88701060d6a9c..a2b1d85ac89bb 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -71,7 +71,6 @@ func TemplateVersionParameter(param database.TemplateVersionParameter) (codersdk }, nil } - func User(user database.User, organizationIDs []uuid.UUID) codersdk.User { convertedUser := codersdk.User{ ID: user.ID, diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 2a1b311874c26..73b9cf66ad490 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -523,6 +523,7 @@ CASE END ELSE CASE + WHEN (error <> ''::text) THEN 'failed'::provisioner_job_status WHEN (canceled_at IS NOT NULL) THEN 'canceling'::provisioner_job_status WHEN (started_at IS NULL) THEN 'pending'::provisioner_job_status ELSE 'running'::provisioner_job_status diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 85a7f5dac7ba7..381dcb2102d93 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9961,7 +9961,8 @@ LEFT JOIN LATERAL ( provisioner_jobs.updated_at, provisioner_jobs.canceled_at, provisioner_jobs.completed_at, - provisioner_jobs.error + provisioner_jobs.error, + provisioner_jobs.job_status FROM workspace_builds LEFT JOIN @@ -9993,63 +9994,34 @@ WHERE AND CASE WHEN $2 :: text != '' THEN CASE - WHEN $2 = 'pending' THEN - latest_build.started_at IS NULL + -- Some workspace specific status refer to the transition + -- type. By default, the standard provisioner job status + -- search strings are supported. + -- 'running' states WHEN $2 = 'starting' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND + latest_build.job_status = 'running' AND latest_build.transition = 'start'::workspace_transition - - WHEN $2 = 'running' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'start'::workspace_transition - WHEN $2 = 'stopping' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND + latest_build.job_status = 'running' AND latest_build.transition = 'stop'::workspace_transition + WHEN $2 = 'deleting' THEN + latest_build.job_status = 'running' AND + latest_build.transition = 'delete'::workspace_transition + -- 'succeeded' states + WHEN $2 = 'deleted' THEN + latest_build.job_status = 'succeeded' AND + latest_build.transition = 'delete'::workspace_transition WHEN $2 = 'stopped' THEN - latest_build.completed_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND + latest_build.job_status = 'succeeded' AND latest_build.transition = 'stop'::workspace_transition + WHEN $2 = 'started' THEN + latest_build.job_status = 'succeeded' AND + latest_build.transition = 'start'::workspace_transition - WHEN $2 = 'failed' THEN - (latest_build.canceled_at IS NOT NULL AND - latest_build.error IS NOT NULL) OR - (latest_build.completed_at IS NOT NULL AND - latest_build.error IS NOT NULL) - - WHEN $2 = 'canceling' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NULL - - WHEN $2 = 'canceled' THEN - latest_build.canceled_at IS NOT NULL AND - latest_build.completed_at IS NOT NULL - - WHEN $2 = 'deleted' THEN - latest_build.started_at IS NOT NULL AND - latest_build.canceled_at IS NULL AND - latest_build.completed_at IS NOT NULL AND - latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND - latest_build.transition = 'delete'::workspace_transition AND - -- If the error field is not null, the status is 'failed' - latest_build.error IS NULL - - WHEN $2 = 'deleting' THEN - latest_build.completed_at IS NULL AND - latest_build.canceled_at IS NULL AND - latest_build.error IS NULL AND - latest_build.transition = 'delete'::workspace_transition - + WHEN $2 != '' THEN + -- By default just match the job status exactly + latest_build.job_status = $2 ELSE true END diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 7413bf4b4815e..e02b0f818d76b 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -3758,6 +3758,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `canceling` | | `canceled` | | `failed` | +| `unknown` | ## codersdk.ProvisionerLogLevel diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 3ce72f96dbfd4..6f9dc91527652 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1755,7 +1755,8 @@ export type ProvisionerJobStatus = | "failed" | "pending" | "running" - | "succeeded"; + | "succeeded" + | "unknown"; export const ProvisionerJobStatuses: ProvisionerJobStatus[] = [ "canceled", "canceling", @@ -1763,6 +1764,7 @@ export const ProvisionerJobStatuses: ProvisionerJobStatus[] = [ "pending", "running", "succeeded", + "unknown", ]; // From codersdk/workspaces.go From 60dd8dfe864408b1a2c87b2b80ab5ea92efa8672 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 15:44:51 -0500 Subject: [PATCH 06/22] Fix unit tests --- coderd/provisionerjobs_internal_test.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/coderd/provisionerjobs_internal_test.go b/coderd/provisionerjobs_internal_test.go index 6168ade5ff701..479da49b2c85d 100644 --- a/coderd/provisionerjobs_internal_test.go +++ b/coderd/provisionerjobs_internal_test.go @@ -44,8 +44,10 @@ func TestConvertProvisionerJob_Unit(t *testing.T) { expected codersdk.ProvisionerJob }{ { - name: "empty", - input: database.ProvisionerJob{}, + name: "empty", + input: database.ProvisionerJob{ + JobStatus: database.ProvisionerJobStatusPending, + }, expected: codersdk.ProvisionerJob{ Status: codersdk.ProvisionerJobPending, }, @@ -55,6 +57,7 @@ func TestConvertProvisionerJob_Unit(t *testing.T) { input: database.ProvisionerJob{ CanceledAt: validNullTimeMock, CompletedAt: invalidNullTimeMock, + JobStatus: database.ProvisionerJobStatusCanceling, }, expected: codersdk.ProvisionerJob{ CanceledAt: &validNullTimeMock.Time, @@ -67,6 +70,7 @@ func TestConvertProvisionerJob_Unit(t *testing.T) { CanceledAt: validNullTimeMock, CompletedAt: validNullTimeMock, Error: errorMock, + JobStatus: database.ProvisionerJobStatusFailed, }, expected: codersdk.ProvisionerJob{ CanceledAt: &validNullTimeMock.Time, @@ -80,6 +84,7 @@ func TestConvertProvisionerJob_Unit(t *testing.T) { input: database.ProvisionerJob{ CanceledAt: validNullTimeMock, CompletedAt: validNullTimeMock, + JobStatus: database.ProvisionerJobStatusCanceled, }, expected: codersdk.ProvisionerJob{ CanceledAt: &validNullTimeMock.Time, @@ -91,6 +96,7 @@ func TestConvertProvisionerJob_Unit(t *testing.T) { name: "job pending", input: database.ProvisionerJob{ StartedAt: invalidNullTimeMock, + JobStatus: database.ProvisionerJobStatusPending, }, expected: codersdk.ProvisionerJob{ Status: codersdk.ProvisionerJobPending, @@ -102,6 +108,7 @@ func TestConvertProvisionerJob_Unit(t *testing.T) { CompletedAt: validNullTimeMock, StartedAt: validNullTimeMock, Error: errorMock, + JobStatus: database.ProvisionerJobStatusFailed, }, expected: codersdk.ProvisionerJob{ CompletedAt: &validNullTimeMock.Time, @@ -115,6 +122,7 @@ func TestConvertProvisionerJob_Unit(t *testing.T) { input: database.ProvisionerJob{ CompletedAt: validNullTimeMock, StartedAt: validNullTimeMock, + JobStatus: database.ProvisionerJobStatusSucceeded, }, expected: codersdk.ProvisionerJob{ CompletedAt: &validNullTimeMock.Time, @@ -156,7 +164,8 @@ func Test_logFollower_completeBeforeFollow(t *testing.T) { Time: now.Add(-time.Second), Valid: true, }, - Error: sql.NullString{}, + Error: sql.NullString{}, + JobStatus: database.ProvisionerJobStatusSucceeded, } // we need an HTTP server to get a websocket @@ -217,6 +226,7 @@ func Test_logFollower_completeBeforeSubscribe(t *testing.T) { CanceledAt: sql.NullTime{}, CompletedAt: sql.NullTime{}, Error: sql.NullString{}, + JobStatus: database.ProvisionerJobStatusRunning, } // we need an HTTP server to get a websocket @@ -238,6 +248,7 @@ func Test_logFollower_completeBeforeSubscribe(t *testing.T) { Time: now.Add(-time.Millisecond), Valid: true, }, + JobStatus: database.ProvisionerJobStatusRunning, }, nil, ) @@ -293,6 +304,7 @@ func Test_logFollower_EndOfLogs(t *testing.T) { CanceledAt: sql.NullTime{}, CompletedAt: sql.NullTime{}, Error: sql.NullString{}, + JobStatus: database.ProvisionerJobStatusRunning, } // we need an HTTP server to get a websocket From 113bcb20f75d5ea5bb744539a9b04a3007e32e29 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 15:53:18 -0500 Subject: [PATCH 07/22] Handle unknown status --- site/src/utils/workspace.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/site/src/utils/workspace.tsx b/site/src/utils/workspace.tsx index 1cc36f45688b9..b53d2a39d26d3 100644 --- a/site/src/utils/workspace.tsx +++ b/site/src/utils/workspace.tsx @@ -57,6 +57,13 @@ export const getDisplayWorkspaceBuildStatus = ( color: theme.palette.text.secondary, status: DisplayWorkspaceBuildStatusLanguage.failed, } as const; + // Just handle unknown as failed + case "unknown": + return { + type: "error", + color: theme.palette.text.secondary, + status: DisplayWorkspaceBuildStatusLanguage.failed, + } as const; case "canceling": return { type: "warning", From 2ddb5441f3fa5265c2d9d2d474270f8155a02249 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 16:14:32 -0500 Subject: [PATCH 08/22] Fix unit test job tags --- coderd/database/db2sdk/db2sdk_test.go | 6 +++++- coderd/database/dbgen/dbgen.go | 11 ++++++++--- coderd/provisionerjobs_internal_test.go | 2 +- .../TemplateVersionStatusBadge.tsx | 1 + site/src/utils/workspace.tsx | 7 +------ 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/coderd/database/db2sdk/db2sdk_test.go b/coderd/database/db2sdk/db2sdk_test.go index c6695019eff9b..a25b44ad92fa2 100644 --- a/coderd/database/db2sdk/db2sdk_test.go +++ b/coderd/database/db2sdk/db2sdk_test.go @@ -4,6 +4,7 @@ import ( "crypto/rand" "database/sql" "encoding/json" + "fmt" "testing" "time" @@ -117,8 +118,9 @@ func TestProvisionerJobStatus(t *testing.T) { db, _ := dbtestutil.NewDB(t) org := dbgen.Organization(t, db, database.Organization{}) - for _, tc := range cases { + for i, tc := range cases { tc := tc + i := i t.Run(tc.name, func(t *testing.T) { t.Parallel() // Populate standard fields @@ -130,6 +132,8 @@ func TestProvisionerJobStatus(t *testing.T) { tc.job.OrganizationID = org.ID tc.job.Input = []byte("{}") tc.job.Provisioner = database.ProvisionerTypeEcho + // Unique tags for each job. + tc.job.Tags = map[string]string{fmt.Sprintf("%d", i): "true"} inserted := dbgen.ProvisionerJob(t, db, nil, tc.job) // Make sure the inserted job has the right values. diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index d460c3fbafff9..0d5f305c39139 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -328,16 +328,16 @@ func GroupMember(t testing.TB, db database.Store, orig database.GroupMember) dat // ProvisionerJob is a bit more involved to get the values such as "completedAt", "startedAt", "cancelledAt" set. ps // can be set to nil if you are SURE that you don't require a provisionerdaemon to acquire the job in your test. func ProvisionerJob(t testing.TB, db database.Store, ps pubsub.Pubsub, orig database.ProvisionerJob) database.ProvisionerJob { - id := takeFirst(orig.ID, uuid.New()) + jobID := takeFirst(orig.ID, uuid.New()) // Always set some tags to prevent Acquire from grabbing jobs it should not. if !orig.StartedAt.Time.IsZero() { if orig.Tags == nil { orig.Tags = make(database.StringMap) } // Make sure when we acquire the job, we only get this one. - orig.Tags[id.String()] = "true" + orig.Tags[jobID.String()] = "true" } - jobID := takeFirst(orig.ID, uuid.New()) + job, err := db.InsertProvisionerJob(genCtx, database.InsertProvisionerJobParams{ ID: jobID, CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()), @@ -365,6 +365,11 @@ func ProvisionerJob(t testing.TB, db database.Store, ps pubsub.Pubsub, orig data WorkerID: uuid.NullUUID{}, }) require.NoError(t, err) + if job.ID != jobID { + fmt.Println("wtf") + } + // There is no easy way to make sure we aquire the correct job. + require.Equal(t, jobID, job.ID, "acquired incorrect job") } if !orig.CompletedAt.Time.IsZero() || orig.Error.String != "" { diff --git a/coderd/provisionerjobs_internal_test.go b/coderd/provisionerjobs_internal_test.go index 479da49b2c85d..05fddb722b4b1 100644 --- a/coderd/provisionerjobs_internal_test.go +++ b/coderd/provisionerjobs_internal_test.go @@ -248,7 +248,7 @@ func Test_logFollower_completeBeforeSubscribe(t *testing.T) { Time: now.Add(-time.Millisecond), Valid: true, }, - JobStatus: database.ProvisionerJobStatusRunning, + JobStatus: database.ProvisionerJobStatusSucceeded, }, nil, ) diff --git a/site/src/pages/TemplateVersionEditorPage/TemplateVersionStatusBadge.tsx b/site/src/pages/TemplateVersionEditorPage/TemplateVersionStatusBadge.tsx index 776609b7a980b..b6697cce4aad4 100644 --- a/site/src/pages/TemplateVersionEditorPage/TemplateVersionStatusBadge.tsx +++ b/site/src/pages/TemplateVersionEditorPage/TemplateVersionStatusBadge.tsx @@ -56,6 +56,7 @@ export const getStatus = ( text: "Canceled", icon: , }; + case "unknown": case "failed": return { type: "error", diff --git a/site/src/utils/workspace.tsx b/site/src/utils/workspace.tsx index b53d2a39d26d3..8e4c6596e49a4 100644 --- a/site/src/utils/workspace.tsx +++ b/site/src/utils/workspace.tsx @@ -51,14 +51,9 @@ export const getDisplayWorkspaceBuildStatus = ( color: theme.palette.primary.main, status: DisplayWorkspaceBuildStatusLanguage.running, } as const; - case "failed": - return { - type: "error", - color: theme.palette.text.secondary, - status: DisplayWorkspaceBuildStatusLanguage.failed, - } as const; // Just handle unknown as failed case "unknown": + case "failed": return { type: "error", color: theme.palette.text.secondary, From afed3b60973fdb52b3cb69f40afa902b03716daf Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 16:20:26 -0500 Subject: [PATCH 09/22] Fix typo --- coderd/database/dbgen/dbgen.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 0d5f305c39139..85461a8a32486 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -368,7 +368,7 @@ func ProvisionerJob(t testing.TB, db database.Store, ps pubsub.Pubsub, orig data if job.ID != jobID { fmt.Println("wtf") } - // There is no easy way to make sure we aquire the correct job. + // There is no easy way to make sure we acquire the correct job. require.Equal(t, jobID, job.ID, "acquired incorrect job") } From 5c3c3590aba0e521a85bde3190b3a7cc6048f6b2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 21:30:55 +0000 Subject: [PATCH 10/22] Make gen --- coderd/database/queries.sql.go | 226 ++++++++++++++++----------------- 1 file changed, 113 insertions(+), 113 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 381dcb2102d93..58fc055cdec86 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9561,119 +9561,6 @@ func (q *sqlQuerier) InsertWorkspaceResourceMetadata(ctx context.Context, arg In return items, nil } -const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many -SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) -` - -func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - -const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many -INSERT INTO - workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) -SELECT - $1 :: uuid AS workspace_agent_id, - $2 :: timestamptz AS created_at, - unnest($3 :: uuid [ ]) AS log_source_id, - unnest($4 :: text [ ]) AS log_path, - unnest($5 :: text [ ]) AS script, - unnest($6 :: text [ ]) AS cron, - unnest($7 :: boolean [ ]) AS start_blocks_login, - unnest($8 :: boolean [ ]) AS run_on_start, - unnest($9 :: boolean [ ]) AS run_on_stop, - unnest($10 :: integer [ ]) AS timeout_seconds -RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds -` - -type InsertWorkspaceAgentScriptsParams struct { - WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` - LogPath []string `db:"log_path" json:"log_path"` - Script []string `db:"script" json:"script"` - Cron []string `db:"cron" json:"cron"` - StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` - RunOnStart []bool `db:"run_on_start" json:"run_on_start"` - RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` - TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` -} - -func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, - arg.WorkspaceAgentID, - arg.CreatedAt, - pq.Array(arg.LogSourceID), - pq.Array(arg.LogPath), - pq.Array(arg.Script), - pq.Array(arg.Cron), - pq.Array(arg.StartBlocksLogin), - pq.Array(arg.RunOnStart), - pq.Array(arg.RunOnStop), - pq.Array(arg.TimeoutSeconds), - ) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const getDeploymentWorkspaceStats = `-- name: GetDeploymentWorkspaceStats :one WITH workspaces_with_jobs AS ( SELECT @@ -10594,3 +10481,116 @@ func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.C _, err := q.db.ExecContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) return err } + +const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many +SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) +` + +func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many +INSERT INTO + workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) +SELECT + $1 :: uuid AS workspace_agent_id, + $2 :: timestamptz AS created_at, + unnest($3 :: uuid [ ]) AS log_source_id, + unnest($4 :: text [ ]) AS log_path, + unnest($5 :: text [ ]) AS script, + unnest($6 :: text [ ]) AS cron, + unnest($7 :: boolean [ ]) AS start_blocks_login, + unnest($8 :: boolean [ ]) AS run_on_start, + unnest($9 :: boolean [ ]) AS run_on_stop, + unnest($10 :: integer [ ]) AS timeout_seconds +RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds +` + +type InsertWorkspaceAgentScriptsParams struct { + WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` + LogPath []string `db:"log_path" json:"log_path"` + Script []string `db:"script" json:"script"` + Cron []string `db:"cron" json:"cron"` + StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` + RunOnStart []bool `db:"run_on_start" json:"run_on_start"` + RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` + TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` +} + +func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, + arg.WorkspaceAgentID, + arg.CreatedAt, + pq.Array(arg.LogSourceID), + pq.Array(arg.LogPath), + pq.Array(arg.Script), + pq.Array(arg.Cron), + pq.Array(arg.StartBlocksLogin), + pq.Array(arg.RunOnStart), + pq.Array(arg.RunOnStop), + pq.Array(arg.TimeoutSeconds), + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} From 7e879f9d508ded56bd67261c6b524c2447344ef8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 16:38:01 -0500 Subject: [PATCH 11/22] remove debug print --- coderd/database/dbgen/dbgen.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 85461a8a32486..5397ca1e3d6c5 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -365,9 +365,6 @@ func ProvisionerJob(t testing.TB, db database.Store, ps pubsub.Pubsub, orig data WorkerID: uuid.NullUUID{}, }) require.NoError(t, err) - if job.ID != jobID { - fmt.Println("wtf") - } // There is no easy way to make sure we acquire the correct job. require.Equal(t, jobID, job.ID, "acquired incorrect job") } From d028ed1f5206eac8cccbed86f32c9ea014c711e4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 16:44:10 -0500 Subject: [PATCH 12/22] sql spacing --- .../000160_provisioner_job_status.up.sql | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/coderd/database/migrations/000160_provisioner_job_status.up.sql b/coderd/database/migrations/000160_provisioner_job_status.up.sql index 015722a406658..9cfea7fbfb140 100644 --- a/coderd/database/migrations/000160_provisioner_job_status.up.sql +++ b/coderd/database/migrations/000160_provisioner_job_status.up.sql @@ -6,31 +6,31 @@ COMMENT ON TYPE provisioner_job_status IS 'Computed status of a provisioner job. ALTER TABLE provisioner_jobs ADD COLUMN job_status provisioner_job_status NOT NULL GENERATED ALWAYS AS ( CASE - -- Completed means it is not in an "-ing" state - WHEN completed_at IS NOT NULL THEN - CASE - -- The order of these checks are important. - -- Check the error first, then cancelled, then completed. - WHEN error != '' THEN 'failed'::provisioner_job_status - WHEN canceled_at IS NOT NULL THEN 'canceled'::provisioner_job_status - ELSE 'succeeded'::provisioner_job_status - END + -- Completed means it is not in an "-ing" state + WHEN completed_at IS NOT NULL THEN + CASE + -- The order of these checks are important. + -- Check the error first, then cancelled, then completed. + WHEN error != '' THEN 'failed'::provisioner_job_status + WHEN canceled_at IS NOT NULL THEN 'canceled'::provisioner_job_status + ELSE 'succeeded'::provisioner_job_status + END -- Not completed means it is in some "-ing" state - ELSE - CASE - -- This should never happen because all errors set - -- should also set a completed_at timestamp. - -- But if there is an error, we should always return - -- a failed state. - WHEN error != '' THEN 'failed'::provisioner_job_status - WHEN canceled_at IS NOT NULL THEN 'canceling'::provisioner_job_status - -- Not done and not started means it is pending - WHEN started_at IS NULL THEN 'pending'::provisioner_job_status - ELSE 'running'::provisioner_job_status - END - END + ELSE + CASE + -- This should never happen because all errors set + -- should also set a completed_at timestamp. + -- But if there is an error, we should always return + -- a failed state. + WHEN error != '' THEN 'failed'::provisioner_job_status + WHEN canceled_at IS NOT NULL THEN 'canceling'::provisioner_job_status + -- Not done and not started means it is pending + WHEN started_at IS NULL THEN 'pending'::provisioner_job_status + ELSE 'running'::provisioner_job_status + END + END -- Stored so we do not have to recompute it every time. - ) STORED; + ) STORED; COMMENT ON COLUMN provisioner_jobs.job_status IS 'Computed column to track the status of the job.'; From 4cc1822b7d82f7dfb63183150fd1153a0f3fdef8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 17:07:56 -0500 Subject: [PATCH 13/22] Fix sql comparisons --- coderd/database/queries/workspaces.sql | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 946a29cef2b4e..a8038873aee6e 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -127,17 +127,17 @@ WHERE -- Optionally include deleted workspaces workspaces.deleted = @deleted AND CASE - WHEN @status :: text != '' THEN + WHEN @statu :: text != '' THEN CASE -- Some workspace specific status refer to the transition -- type. By default, the standard provisioner job status -- search strings are supported. -- 'running' states WHEN @status = 'starting' THEN - latest_build.job_status = 'running' AND + latest_build.job_status = 'running'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition WHEN @status = 'stopping' THEN - latest_build.job_status = 'running' AND + latest_build.job_status = 'running'::provisioner_job_status AND latest_build.transition = 'stop'::workspace_transition WHEN @status = 'deleting' THEN latest_build.job_status = 'running' AND @@ -145,18 +145,18 @@ WHERE -- 'succeeded' states WHEN @status = 'deleted' THEN - latest_build.job_status = 'succeeded' AND + latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'delete'::workspace_transition WHEN @status = 'stopped' THEN - latest_build.job_status = 'succeeded' AND + latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'stop'::workspace_transition WHEN @status = 'started' THEN - latest_build.job_status = 'succeeded' AND + latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition WHEN @status != '' THEN -- By default just match the job status exactly - latest_build.job_status = @status + latest_build.job_status = @status::provisioner_job_status ELSE true END From e4c1c7d7e669dcfeca26aa0f4fdcb5a125326d92 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 17:10:44 -0500 Subject: [PATCH 14/22] Fix sqlc arg --- coderd/database/queries.sql.go | 238 ++++++++++++------------- coderd/database/queries/workspaces.sql | 2 +- 2 files changed, 120 insertions(+), 120 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 58fc055cdec86..0e8ac082ef55d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9561,6 +9561,119 @@ func (q *sqlQuerier) InsertWorkspaceResourceMetadata(ctx context.Context, arg In return items, nil } +const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many +SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) +` + +func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many +INSERT INTO + workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) +SELECT + $1 :: uuid AS workspace_agent_id, + $2 :: timestamptz AS created_at, + unnest($3 :: uuid [ ]) AS log_source_id, + unnest($4 :: text [ ]) AS log_path, + unnest($5 :: text [ ]) AS script, + unnest($6 :: text [ ]) AS cron, + unnest($7 :: boolean [ ]) AS start_blocks_login, + unnest($8 :: boolean [ ]) AS run_on_start, + unnest($9 :: boolean [ ]) AS run_on_stop, + unnest($10 :: integer [ ]) AS timeout_seconds +RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds +` + +type InsertWorkspaceAgentScriptsParams struct { + WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` + LogPath []string `db:"log_path" json:"log_path"` + Script []string `db:"script" json:"script"` + Cron []string `db:"cron" json:"cron"` + StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` + RunOnStart []bool `db:"run_on_start" json:"run_on_start"` + RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` + TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` +} + +func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, + arg.WorkspaceAgentID, + arg.CreatedAt, + pq.Array(arg.LogSourceID), + pq.Array(arg.LogPath), + pq.Array(arg.Script), + pq.Array(arg.Cron), + pq.Array(arg.StartBlocksLogin), + pq.Array(arg.RunOnStart), + pq.Array(arg.RunOnStop), + pq.Array(arg.TimeoutSeconds), + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getDeploymentWorkspaceStats = `-- name: GetDeploymentWorkspaceStats :one WITH workspaces_with_jobs AS ( SELECT @@ -9886,10 +9999,10 @@ WHERE -- search strings are supported. -- 'running' states WHEN $2 = 'starting' THEN - latest_build.job_status = 'running' AND + latest_build.job_status = 'running'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition WHEN $2 = 'stopping' THEN - latest_build.job_status = 'running' AND + latest_build.job_status = 'running'::provisioner_job_status AND latest_build.transition = 'stop'::workspace_transition WHEN $2 = 'deleting' THEN latest_build.job_status = 'running' AND @@ -9897,18 +10010,18 @@ WHERE -- 'succeeded' states WHEN $2 = 'deleted' THEN - latest_build.job_status = 'succeeded' AND + latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'delete'::workspace_transition WHEN $2 = 'stopped' THEN - latest_build.job_status = 'succeeded' AND + latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'stop'::workspace_transition WHEN $2 = 'started' THEN - latest_build.job_status = 'succeeded' AND + latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition WHEN $2 != '' THEN -- By default just match the job status exactly - latest_build.job_status = $2 + latest_build.job_status = $2::provisioner_job_status ELSE true END @@ -10481,116 +10594,3 @@ func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.C _, err := q.db.ExecContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) return err } - -const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many -SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) -` - -func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - -const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many -INSERT INTO - workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) -SELECT - $1 :: uuid AS workspace_agent_id, - $2 :: timestamptz AS created_at, - unnest($3 :: uuid [ ]) AS log_source_id, - unnest($4 :: text [ ]) AS log_path, - unnest($5 :: text [ ]) AS script, - unnest($6 :: text [ ]) AS cron, - unnest($7 :: boolean [ ]) AS start_blocks_login, - unnest($8 :: boolean [ ]) AS run_on_start, - unnest($9 :: boolean [ ]) AS run_on_stop, - unnest($10 :: integer [ ]) AS timeout_seconds -RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds -` - -type InsertWorkspaceAgentScriptsParams struct { - WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` - LogPath []string `db:"log_path" json:"log_path"` - Script []string `db:"script" json:"script"` - Cron []string `db:"cron" json:"cron"` - StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` - RunOnStart []bool `db:"run_on_start" json:"run_on_start"` - RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` - TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` -} - -func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, - arg.WorkspaceAgentID, - arg.CreatedAt, - pq.Array(arg.LogSourceID), - pq.Array(arg.LogPath), - pq.Array(arg.Script), - pq.Array(arg.Cron), - pq.Array(arg.StartBlocksLogin), - pq.Array(arg.RunOnStart), - pq.Array(arg.RunOnStop), - pq.Array(arg.TimeoutSeconds), - ) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index a8038873aee6e..8c9bb3dc87856 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -127,7 +127,7 @@ WHERE -- Optionally include deleted workspaces workspaces.deleted = @deleted AND CASE - WHEN @statu :: text != '' THEN + WHEN @status :: text != '' THEN CASE -- Some workspace specific status refer to the transition -- type. By default, the standard provisioner job status From 55e266a318a9c2aaa4d5e2a911241d14bfabc85d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Oct 2023 22:12:15 +0000 Subject: [PATCH 15/22] Make gen --- coderd/database/queries.sql.go | 226 ++++++++++++++++----------------- 1 file changed, 113 insertions(+), 113 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0e8ac082ef55d..326d8fe3018fb 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9561,119 +9561,6 @@ func (q *sqlQuerier) InsertWorkspaceResourceMetadata(ctx context.Context, arg In return items, nil } -const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many -SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) -` - -func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - -const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many -INSERT INTO - workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) -SELECT - $1 :: uuid AS workspace_agent_id, - $2 :: timestamptz AS created_at, - unnest($3 :: uuid [ ]) AS log_source_id, - unnest($4 :: text [ ]) AS log_path, - unnest($5 :: text [ ]) AS script, - unnest($6 :: text [ ]) AS cron, - unnest($7 :: boolean [ ]) AS start_blocks_login, - unnest($8 :: boolean [ ]) AS run_on_start, - unnest($9 :: boolean [ ]) AS run_on_stop, - unnest($10 :: integer [ ]) AS timeout_seconds -RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds -` - -type InsertWorkspaceAgentScriptsParams struct { - WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` - LogPath []string `db:"log_path" json:"log_path"` - Script []string `db:"script" json:"script"` - Cron []string `db:"cron" json:"cron"` - StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` - RunOnStart []bool `db:"run_on_start" json:"run_on_start"` - RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` - TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` -} - -func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, - arg.WorkspaceAgentID, - arg.CreatedAt, - pq.Array(arg.LogSourceID), - pq.Array(arg.LogPath), - pq.Array(arg.Script), - pq.Array(arg.Cron), - pq.Array(arg.StartBlocksLogin), - pq.Array(arg.RunOnStart), - pq.Array(arg.RunOnStop), - pq.Array(arg.TimeoutSeconds), - ) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const getDeploymentWorkspaceStats = `-- name: GetDeploymentWorkspaceStats :one WITH workspaces_with_jobs AS ( SELECT @@ -10594,3 +10481,116 @@ func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.C _, err := q.db.ExecContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) return err } + +const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many +SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) +` + +func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many +INSERT INTO + workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) +SELECT + $1 :: uuid AS workspace_agent_id, + $2 :: timestamptz AS created_at, + unnest($3 :: uuid [ ]) AS log_source_id, + unnest($4 :: text [ ]) AS log_path, + unnest($5 :: text [ ]) AS script, + unnest($6 :: text [ ]) AS cron, + unnest($7 :: boolean [ ]) AS start_blocks_login, + unnest($8 :: boolean [ ]) AS run_on_start, + unnest($9 :: boolean [ ]) AS run_on_stop, + unnest($10 :: integer [ ]) AS timeout_seconds +RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds +` + +type InsertWorkspaceAgentScriptsParams struct { + WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` + LogPath []string `db:"log_path" json:"log_path"` + Script []string `db:"script" json:"script"` + Cron []string `db:"cron" json:"cron"` + StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` + RunOnStart []bool `db:"run_on_start" json:"run_on_start"` + RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` + TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` +} + +func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, + arg.WorkspaceAgentID, + arg.CreatedAt, + pq.Array(arg.LogSourceID), + pq.Array(arg.LogPath), + pq.Array(arg.Script), + pq.Array(arg.Cron), + pq.Array(arg.StartBlocksLogin), + pq.Array(arg.RunOnStart), + pq.Array(arg.RunOnStop), + pq.Array(arg.TimeoutSeconds), + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} From 10a4a967d24dc442f4f2c6eea8ab967898551680 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Oct 2023 09:48:38 -0500 Subject: [PATCH 16/22] Workspace build status is flakey, need to rewrite test --- coderd/database/dbfake/dbfake.go | 66 ++++++++------------------------ coderd/workspaces_test.go | 46 ---------------------- 2 files changed, 16 insertions(+), 96 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 9c586fce93335..da1a2dfe89a71 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -6628,64 +6628,30 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. // This logic should match the logic in the workspace.sql file. var statusMatch bool switch database.WorkspaceStatus(arg.Status) { - case database.WorkspaceStatusPending: - statusMatch = isNull(job.StartedAt) case database.WorkspaceStatusStarting: - statusMatch = isNotNull(job.StartedAt) && - isNull(job.CanceledAt) && - isNull(job.CompletedAt) && - time.Since(job.UpdatedAt) < 30*time.Second && + statusMatch = job.JobStatus == database.ProvisionerJobStatusRunning && build.Transition == database.WorkspaceTransitionStart - - case database.WorkspaceStatusRunning: - statusMatch = isNotNull(job.CompletedAt) && - isNull(job.CanceledAt) && - isNull(job.Error) && - build.Transition == database.WorkspaceTransitionStart - case database.WorkspaceStatusStopping: - statusMatch = isNotNull(job.StartedAt) && - isNull(job.CanceledAt) && - isNull(job.CompletedAt) && - time.Since(job.UpdatedAt) < 30*time.Second && - build.Transition == database.WorkspaceTransitionStop - - case database.WorkspaceStatusStopped: - statusMatch = isNotNull(job.CompletedAt) && - isNull(job.CanceledAt) && - isNull(job.Error) && + statusMatch = job.JobStatus == database.ProvisionerJobStatusRunning && build.Transition == database.WorkspaceTransitionStop - case database.WorkspaceStatusFailed: - statusMatch = (isNotNull(job.CanceledAt) && isNotNull(job.Error)) || - (isNotNull(job.CompletedAt) && isNotNull(job.Error)) - - case database.WorkspaceStatusCanceling: - statusMatch = isNotNull(job.CanceledAt) && - isNull(job.CompletedAt) - - case database.WorkspaceStatusCanceled: - statusMatch = isNotNull(job.CanceledAt) && - isNotNull(job.CompletedAt) - - case database.WorkspaceStatusDeleted: - statusMatch = isNotNull(job.StartedAt) && - isNull(job.CanceledAt) && - isNotNull(job.CompletedAt) && - time.Since(job.UpdatedAt) < 30*time.Second && - build.Transition == database.WorkspaceTransitionDelete && - isNull(job.Error) - case database.WorkspaceStatusDeleting: - statusMatch = isNull(job.CompletedAt) && - isNull(job.CanceledAt) && - isNull(job.Error) && + statusMatch = job.JobStatus == database.ProvisionerJobStatusRunning && build.Transition == database.WorkspaceTransitionDelete + case "started": + statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded && + build.Transition == database.WorkspaceTransitionStart + case database.WorkspaceStatusDeleted: + statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded && + build.Transition == database.WorkspaceTransitionDelete + case database.WorkspaceStatusStopped: + statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded && + build.Transition == database.WorkspaceTransitionStop default: - return nil, xerrors.Errorf("unknown workspace status in filter: %q", arg.Status) - } - if !statusMatch { - continue + statusMatch = job.JobStatus == database.ProvisionerJobStatus(arg.Status) + if !statusMatch { + continue + } } } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 171ebb2a0dfd7..c8a39a550ab55 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -1220,52 +1220,6 @@ func TestWorkspaceFilterManual(t *testing.T) { require.Len(t, res.Workspaces, 1) require.Equal(t, workspace.ID, res.Workspaces[0].ID) }) - t.Run("Status", func(t *testing.T) { - t.Parallel() - - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) - user := coderdtest.CreateFirstUser(t, client) - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace1 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - workspace2 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - - // wait for workspaces to be "running" - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace1.LatestBuild.ID) - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace2.LatestBuild.ID) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - // filter finds both running workspaces - ws1, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) - require.NoError(t, err) - require.Len(t, ws1.Workspaces, 2) - - // stop workspace1 - build1 := coderdtest.CreateWorkspaceBuild(t, client, workspace1, database.WorkspaceTransitionStop) - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, build1.ID) - - // filter finds one running workspace - ws2, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ - Status: "running", - }) - require.NoError(t, err) - require.Len(t, ws2.Workspaces, 1) - require.Equal(t, workspace2.ID, ws2.Workspaces[0].ID) - - // stop workspace2 - build2 := coderdtest.CreateWorkspaceBuild(t, client, workspace2, database.WorkspaceTransitionStop) - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, build2.ID) - - // filter finds no running workspaces - ws3, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ - Status: "running", - }) - require.NoError(t, err) - require.Len(t, ws3.Workspaces, 0) - }) t.Run("FilterQuery", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) From 191ee82a2ee11d4353bfade83ada8bcdc393560e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Oct 2023 10:02:56 -0500 Subject: [PATCH 17/22] Put back test, fix running state --- coderd/database/dbfake/dbfake.go | 2 ++ coderd/database/queries/workspaces.sql | 5 +++ coderd/workspaces_test.go | 46 ++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index da1a2dfe89a71..19464d44f4bc0 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -6647,6 +6647,8 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. case database.WorkspaceStatusStopped: statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded && build.Transition == database.WorkspaceTransitionStop + case database.WorkspaceStatusRunning: + statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded default: statusMatch = job.JobStatus == database.ProvisionerJobStatus(arg.Status) if !statusMatch { diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 8c9bb3dc87856..ac1f6bf519c0a 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -154,6 +154,11 @@ WHERE latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition + -- Special case. A workspace status of "running" is a job status + -- of "succeeded". This is annoying... + WHEN @status = 'running' THEN + latest_build.job_status = 'succeeded'::provisioner_job_status + WHEN @status != '' THEN -- By default just match the job status exactly latest_build.job_status = @status::provisioner_job_status diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index ab87043001431..da0f1b8eb23a1 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -1220,6 +1220,52 @@ func TestWorkspaceFilterManual(t *testing.T) { require.Len(t, res.Workspaces, 1) require.Equal(t, workspace.ID, res.Workspaces[0].ID) }) + t.Run("Status", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace1 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + workspace2 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + + // wait for workspaces to be "running" + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace1.LatestBuild.ID) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace2.LatestBuild.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // filter finds both running workspaces + ws1, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) + require.NoError(t, err) + require.Len(t, ws1.Workspaces, 2) + + // stop workspace1 + build1 := coderdtest.CreateWorkspaceBuild(t, client, workspace1, database.WorkspaceTransitionStop) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build1.ID) + + // filter finds one running workspace + ws2, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + Status: "running", + }) + require.NoError(t, err) + require.Len(t, ws2.Workspaces, 1) + require.Equal(t, workspace2.ID, ws2.Workspaces[0].ID) + + // stop workspace2 + build2 := coderdtest.CreateWorkspaceBuild(t, client, workspace2, database.WorkspaceTransitionStop) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build2.ID) + + // filter finds no running workspaces + ws3, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{ + Status: "running", + }) + require.NoError(t, err) + require.Len(t, ws3.Workspaces, 0) + }) t.Run("FilterQuery", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) From 97d32a3c4e07503bc04382749bbd600a10222dd1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Oct 2023 15:07:27 +0000 Subject: [PATCH 18/22] make gen --- coderd/database/queries.sql.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 326d8fe3018fb..a7057903ef8c7 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9906,6 +9906,11 @@ WHERE latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition + -- Special case. A workspace status of "running" is a job status + -- of "succeeded". This is annoying... + WHEN $2 = 'running' THEN + latest_build.job_status = 'succeeded'::provisioner_job_status + WHEN $2 != '' THEN -- By default just match the job status exactly latest_build.job_status = $2::provisioner_job_status From 05ca60ec21c2fcb7f688f5178fcef5646decc5fc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Oct 2023 10:11:16 -0500 Subject: [PATCH 19/22] Fix spacing --- coderd/database/dbfake/dbfake.go | 3 ++- coderd/database/queries/workspaces.sql | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 19464d44f4bc0..0403154abea85 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -6648,7 +6648,8 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded && build.Transition == database.WorkspaceTransitionStop case database.WorkspaceStatusRunning: - statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded + statusMatch = job.JobStatus == database.ProvisionerJobStatusSucceeded && + build.Transition == database.WorkspaceTransitionStart default: statusMatch = job.JobStatus == database.ProvisionerJobStatus(arg.Status) if !statusMatch { diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index ac1f6bf519c0a..df5c46ff2a506 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -154,10 +154,13 @@ WHERE latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition - -- Special case. A workspace status of "running" is a job status - -- of "succeeded". This is annoying... + -- Special case where the provisioner status and workspace status + -- differ. A workspace is "running" if the job is "succeeded" and + -- the transition is "start". This is because a workspace starts + -- running when a job is complete. WHEN @status = 'running' THEN - latest_build.job_status = 'succeeded'::provisioner_job_status + latest_build.job_status = 'succeeded'::provisioner_job_status AND + latest_build.transition = 'start'::workspace_transition WHEN @status != '' THEN -- By default just match the job status exactly From b378708f90f58165e2189cc2a9fde9d343bd7448 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Oct 2023 15:13:29 +0000 Subject: [PATCH 20/22] Make gen --- coderd/database/queries.sql.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a7057903ef8c7..a2949bb542f3d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9906,10 +9906,13 @@ WHERE latest_build.job_status = 'succeeded'::provisioner_job_status AND latest_build.transition = 'start'::workspace_transition - -- Special case. A workspace status of "running" is a job status - -- of "succeeded". This is annoying... + -- Special case where the provisioner status and workspace status + -- differ. A workspace is "running" if the job is "succeeded" and + -- the transition is "start". This is because a workspace starts + -- running when a job is complete. WHEN $2 = 'running' THEN - latest_build.job_status = 'succeeded'::provisioner_job_status + latest_build.job_status = 'succeeded'::provisioner_job_status AND + latest_build.transition = 'start'::workspace_transition WHEN $2 != '' THEN -- By default just match the job status exactly From a95b70253728ac20a482c5c6111e4bbcf5d973d8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Oct 2023 10:31:24 -0500 Subject: [PATCH 21/22] Fix build --- coderd/workspaces_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index da0f1b8eb23a1..4488a3e7841bc 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -2877,10 +2877,10 @@ func TestWorkspaceDormant(t *testing.T) { client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) user = coderdtest.CreateFirstUser(t, client) version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) ) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) From 9ce0fa6411d1951aa434d64b5cd01265d7bee362 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Oct 2023 10:46:30 -0500 Subject: [PATCH 22/22] Merge issue broke test --- coderd/database/dbfake/dbfake.go | 6 +++--- coderd/workspaces_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 0403154abea85..1edfffb11c728 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -6652,9 +6652,9 @@ func (q *FakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. build.Transition == database.WorkspaceTransitionStart default: statusMatch = job.JobStatus == database.ProvisionerJobStatus(arg.Status) - if !statusMatch { - continue - } + } + if !statusMatch { + continue } } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 4488a3e7841bc..9ab6c0e9184a7 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -1226,7 +1226,7 @@ func TestWorkspaceFilterManual(t *testing.T) { client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, version.ID) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) workspace1 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) workspace2 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)