fix: Test flake in `TestWorkspaceStatus` (#4333)

This also changes the status to be on the workspace build, since
that's where the true value is calculated. This exposed a bug where
jobs could never enter the canceled state unless fetched by a
provisioner daemon, which was nice to fix!

See: https://github.com/coder/coder/actions/runs/3175304200/jobs/5173479506
This commit is contained in:
Kyle Carberry 2022-10-03 11:43:11 -05:00 committed by GitHub
parent d11d83cc98
commit df2649ed2a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 148 additions and 143 deletions

View File

@ -2238,6 +2238,7 @@ func (q *fakeQuerier) UpdateProvisionerJobWithCancelByID(_ context.Context, arg
continue
}
job.CanceledAt = arg.CanceledAt
job.CompletedAt = arg.CompletedAt
q.provisionerJobs[index] = job
return nil
}

View File

@ -2125,18 +2125,20 @@ const updateProvisionerJobWithCancelByID = `-- name: UpdateProvisionerJobWithCan
UPDATE
provisioner_jobs
SET
canceled_at = $2
canceled_at = $2,
completed_at = $3
WHERE
id = $1
`
type UpdateProvisionerJobWithCancelByIDParams struct {
ID uuid.UUID `db:"id" json:"id"`
CanceledAt sql.NullTime `db:"canceled_at" json:"canceled_at"`
ID uuid.UUID `db:"id" json:"id"`
CanceledAt sql.NullTime `db:"canceled_at" json:"canceled_at"`
CompletedAt sql.NullTime `db:"completed_at" json:"completed_at"`
}
func (q *sqlQuerier) UpdateProvisionerJobWithCancelByID(ctx context.Context, arg UpdateProvisionerJobWithCancelByIDParams) error {
_, err := q.db.ExecContext(ctx, updateProvisionerJobWithCancelByID, arg.ID, arg.CanceledAt)
_, err := q.db.ExecContext(ctx, updateProvisionerJobWithCancelByID, arg.ID, arg.CanceledAt, arg.CompletedAt)
return err
}

View File

@ -78,7 +78,8 @@ WHERE
UPDATE
provisioner_jobs
SET
canceled_at = $2
canceled_at = $2,
completed_at = $3
WHERE
id = $1;

View File

@ -85,6 +85,11 @@ func (api *API) patchCancelTemplateVersion(rw http.ResponseWriter, r *http.Reque
Time: database.Now(),
Valid: true,
},
CompletedAt: sql.NullTime{
Time: database.Now(),
// If the job is running, don't mark it completed!
Valid: !job.WorkerID.Valid,
},
})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@ -339,6 +344,11 @@ func (api *API) patchTemplateVersionDryRunCancel(rw http.ResponseWriter, r *http
Time: database.Now(),
Valid: true,
},
CompletedAt: sql.NullTime{
Time: database.Now(),
// If the job is running, don't mark it completed!
Valid: !job.WorkerID.Valid,
},
})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{

View File

@ -715,29 +715,12 @@ func TestTemplateVersionDryRun(t *testing.T) {
ParameterValues: []codersdk.CreateParameterRequest{},
})
require.NoError(t, err)
require.Eventually(t, func() bool {
job, err := client.TemplateVersionDryRun(ctx, version.ID, job.ID)
if !assert.NoError(t, err) {
return false
}
t.Logf("Status: %s", job.Status)
return job.Status == codersdk.ProvisionerJobPending
}, testutil.WaitShort, testutil.IntervalFast)
require.Equal(t, codersdk.ProvisionerJobPending, job.Status)
err = client.CancelTemplateVersionDryRun(ctx, version.ID, job.ID)
require.NoError(t, err)
require.Eventually(t, func() bool {
job, err := client.TemplateVersionDryRun(ctx, version.ID, job.ID)
if !assert.NoError(t, err) {
return false
}
t.Logf("Status: %s", job.Status)
return job.Status == codersdk.ProvisionerJobCanceling
}, testutil.WaitShort, testutil.IntervalFast)
job, err = client.TemplateVersionDryRun(ctx, version.ID, job.ID)
require.NoError(t, err)
require.Equal(t, codersdk.ProvisionerJobCanceled, job.Status)
})
t.Run("AlreadyCompleted", func(t *testing.T) {

View File

@ -552,6 +552,11 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques
Time: database.Now(),
Valid: true,
},
CompletedAt: sql.NullTime{
Time: database.Now(),
// If the job is running, don't mark it completed!
Valid: !job.WorkerID.Valid,
},
})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@ -835,7 +840,8 @@ func (api *API) convertWorkspaceBuild(
metadata := append(make([]database.WorkspaceResourceMetadatum, 0), metadataByResourceID[resource.ID]...)
apiResources = append(apiResources, convertWorkspaceResource(resource, apiAgents, metadata))
}
apiJob := convertProvisionerJob(job)
transition := codersdk.WorkspaceTransition(build.Transition)
return codersdk.WorkspaceBuild{
ID: build.ID,
CreatedAt: build.CreatedAt,
@ -846,13 +852,14 @@ func (api *API) convertWorkspaceBuild(
WorkspaceName: workspace.Name,
TemplateVersionID: build.TemplateVersionID,
BuildNumber: build.BuildNumber,
Transition: codersdk.WorkspaceTransition(build.Transition),
Transition: transition,
InitiatorID: build.InitiatorID,
InitiatorUsername: initiator.Username,
Job: convertProvisionerJob(job),
Job: apiJob,
Deadline: codersdk.NewNullTime(build.Deadline, !build.Deadline.IsZero()),
Reason: codersdk.BuildReason(build.Reason),
Resources: apiResources,
Status: convertWorkspaceStatus(apiJob.Status, transition),
}, nil
}
@ -898,3 +905,37 @@ func convertWorkspaceResource(resource database.WorkspaceResource, agents []code
Metadata: convertedMetadata,
}
}
func convertWorkspaceStatus(jobStatus codersdk.ProvisionerJobStatus, transition codersdk.WorkspaceTransition) codersdk.WorkspaceStatus {
switch jobStatus {
case codersdk.ProvisionerJobPending:
return codersdk.WorkspaceStatusPending
case codersdk.ProvisionerJobRunning:
switch transition {
case codersdk.WorkspaceTransitionStart:
return codersdk.WorkspaceStatusStarting
case codersdk.WorkspaceTransitionStop:
return codersdk.WorkspaceStatusStopping
case codersdk.WorkspaceTransitionDelete:
return codersdk.WorkspaceStatusDeleting
}
case codersdk.ProvisionerJobSucceeded:
switch transition {
case codersdk.WorkspaceTransitionStart:
return codersdk.WorkspaceStatusRunning
case codersdk.WorkspaceTransitionStop:
return codersdk.WorkspaceStatusStopped
case codersdk.WorkspaceTransitionDelete:
return codersdk.WorkspaceStatusDeleted
}
case codersdk.ProvisionerJobCanceling:
return codersdk.WorkspaceStatusCanceling
case codersdk.ProvisionerJobCanceled:
return codersdk.WorkspaceStatusCanceled
case codersdk.ProvisionerJobFailed:
return codersdk.WorkspaceStatusFailed
}
// return error status since we should never get here
return codersdk.WorkspaceStatusFailed
}

View File

@ -485,3 +485,50 @@ func TestWorkspaceBuildState(t *testing.T) {
require.NoError(t, err)
require.Equal(t, wantState, gotState)
}
func TestWorkspaceBuildStatus(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
client, closeDaemon, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
closeDaemon.Close()
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
// initial returned state is "pending"
require.EqualValues(t, codersdk.WorkspaceStatusPending, workspace.LatestBuild.Status)
closeDaemon = coderdtest.NewProvisionerDaemon(t, api)
// after successful build is "running"
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
workspace, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusRunning, workspace.LatestBuild.Status)
// after successful stop is "stopped"
build := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
workspace, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusStopped, workspace.LatestBuild.Status)
_ = closeDaemon.Close()
// after successful cancel is "canceled"
build = coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart)
err = client.CancelWorkspaceBuild(ctx, build.ID)
require.NoError(t, err)
workspace, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusCanceled, workspace.LatestBuild.Status)
_ = coderdtest.NewProvisionerDaemon(t, api)
// after successful delete is "deleted"
build = coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionDelete)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
workspace, err = client.DeletedWorkspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusDeleted, workspace.LatestBuild.Status)
}

View File

@ -996,44 +996,9 @@ func convertWorkspace(
AutostartSchedule: autostartSchedule,
TTLMillis: ttlMillis,
LastUsedAt: workspace.LastUsedAt,
Status: convertStatus(workspaceBuild),
}
}
func convertStatus(build codersdk.WorkspaceBuild) codersdk.WorkspaceStatus {
switch build.Job.Status {
case codersdk.ProvisionerJobPending:
return codersdk.WorkspaceStatusPending
case codersdk.ProvisionerJobRunning:
switch build.Transition {
case codersdk.WorkspaceTransitionStart:
return codersdk.WorkspaceStatusStarting
case codersdk.WorkspaceTransitionStop:
return codersdk.WorkspaceStatusStopping
case codersdk.WorkspaceTransitionDelete:
return codersdk.WorkspaceStatusDeleting
}
case codersdk.ProvisionerJobSucceeded:
switch build.Transition {
case codersdk.WorkspaceTransitionStart:
return codersdk.WorkspaceStatusRunning
case codersdk.WorkspaceTransitionStop:
return codersdk.WorkspaceStatusStopped
case codersdk.WorkspaceTransitionDelete:
return codersdk.WorkspaceStatusDeleted
}
case codersdk.ProvisionerJobCanceling:
return codersdk.WorkspaceStatusCanceling
case codersdk.ProvisionerJobCanceled:
return codersdk.WorkspaceStatusCanceled
case codersdk.ProvisionerJobFailed:
return codersdk.WorkspaceStatusFailed
}
// return error status since we should never get here
return codersdk.WorkspaceStatusFailed
}
func convertWorkspaceTTLMillis(i sql.NullInt64) *int64 {
if !i.Valid {
return nil

View File

@ -1256,52 +1256,6 @@ func TestWorkspaceWatcher(t *testing.T) {
require.EqualValues(t, codersdk.Workspace{}, <-wc)
}
func TestWorkspaceStatus(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
var (
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)
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
)
// initial returned state is "pending"
require.EqualValues(t, codersdk.WorkspaceStatusPending, workspace.Status)
// after successful build is "running"
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
workspace, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusRunning, workspace.Status)
// after successful stop is "stopped"
build := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
workspace, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusStopped, workspace.Status)
// after successful cancel is "canceled"
build = coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart)
err = client.CancelWorkspaceBuild(ctx, build.ID)
require.NoError(t, err)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
workspace, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusCanceled, workspace.Status)
// after successful delete is "deleted"
build = coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionDelete)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
workspace, err = client.DeletedWorkspace(ctx, workspace.ID)
require.NoError(t, err)
require.EqualValues(t, codersdk.WorkspaceStatusDeleted, workspace.Status)
}
func mustLocation(t *testing.T, location string) *time.Location {
t.Helper()
loc, err := time.LoadLocation(location)

View File

@ -19,6 +19,21 @@ const (
WorkspaceTransitionDelete WorkspaceTransition = "delete"
)
type WorkspaceStatus string
const (
WorkspaceStatusPending WorkspaceStatus = "pending"
WorkspaceStatusStarting WorkspaceStatus = "starting"
WorkspaceStatusRunning WorkspaceStatus = "running"
WorkspaceStatusStopping WorkspaceStatus = "stopping"
WorkspaceStatusStopped WorkspaceStatus = "stopped"
WorkspaceStatusFailed WorkspaceStatus = "failed"
WorkspaceStatusCanceling WorkspaceStatus = "canceling"
WorkspaceStatusCanceled WorkspaceStatus = "canceled"
WorkspaceStatusDeleting WorkspaceStatus = "deleting"
WorkspaceStatusDeleted WorkspaceStatus = "deleted"
)
type BuildReason string
const (
@ -52,6 +67,7 @@ type WorkspaceBuild struct {
Reason BuildReason `db:"reason" json:"reason"`
Resources []WorkspaceResource `json:"resources"`
Deadline NullTime `json:"deadline,omitempty"`
Status WorkspaceStatus `json:"status"`
}
// WorkspaceBuild returns a single workspace build for a workspace.

View File

@ -15,38 +15,22 @@ import (
// Workspace is a deployment of a template. It references a specific
// version and can be updated.
type Workspace struct {
ID uuid.UUID `json:"id"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
OwnerID uuid.UUID `json:"owner_id"`
OwnerName string `json:"owner_name"`
TemplateID uuid.UUID `json:"template_id"`
TemplateName string `json:"template_name"`
TemplateIcon string `json:"template_icon"`
LatestBuild WorkspaceBuild `json:"latest_build"`
Outdated bool `json:"outdated"`
Name string `json:"name"`
AutostartSchedule *string `json:"autostart_schedule,omitempty"`
TTLMillis *int64 `json:"ttl_ms,omitempty"`
LastUsedAt time.Time `json:"last_used_at"`
Status WorkspaceStatus `json:"status"`
ID uuid.UUID `json:"id"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
OwnerID uuid.UUID `json:"owner_id"`
OwnerName string `json:"owner_name"`
TemplateID uuid.UUID `json:"template_id"`
TemplateName string `json:"template_name"`
TemplateIcon string `json:"template_icon"`
LatestBuild WorkspaceBuild `json:"latest_build"`
Outdated bool `json:"outdated"`
Name string `json:"name"`
AutostartSchedule *string `json:"autostart_schedule,omitempty"`
TTLMillis *int64 `json:"ttl_ms,omitempty"`
LastUsedAt time.Time `json:"last_used_at"`
}
type WorkspaceStatus string
const (
WorkspaceStatusPending WorkspaceStatus = "pending"
WorkspaceStatusStarting WorkspaceStatus = "starting"
WorkspaceStatusRunning WorkspaceStatus = "running"
WorkspaceStatusStopping WorkspaceStatus = "stopping"
WorkspaceStatusStopped WorkspaceStatus = "stopped"
WorkspaceStatusFailed WorkspaceStatus = "failed"
WorkspaceStatusCanceling WorkspaceStatus = "canceling"
WorkspaceStatusCanceled WorkspaceStatus = "canceled"
WorkspaceStatusDeleting WorkspaceStatus = "deleting"
WorkspaceStatusDeleted WorkspaceStatus = "deleted"
)
// CreateWorkspaceBuildRequest provides options to update the latest workspace build.
type CreateWorkspaceBuildRequest struct {
TemplateVersionID uuid.UUID `json:"template_version_id,omitempty"`

View File

@ -550,7 +550,6 @@ export interface Workspace {
readonly autostart_schedule?: string
readonly ttl_ms?: number
readonly last_used_at: string
readonly status: WorkspaceStatus
}
// From codersdk/workspaceresources.go
@ -625,6 +624,7 @@ export interface WorkspaceBuild {
readonly reason: BuildReason
readonly resources: WorkspaceResource[]
readonly deadline?: string
readonly status: WorkspaceStatus
}
// From codersdk/workspaces.go
@ -736,7 +736,7 @@ export type WorkspaceAgentStatus = "connected" | "connecting" | "disconnected"
// From codersdk/workspaceapps.go
export type WorkspaceAppHealth = "disabled" | "healthy" | "initializing" | "unhealthy"
// From codersdk/workspaces.go
// From codersdk/workspacebuilds.go
export type WorkspaceStatus =
| "canceled"
| "canceling"

View File

@ -216,6 +216,7 @@ export const MockWorkspaceBuild: TypesGen.WorkspaceBuild = {
deadline: "2022-05-17T23:39:00.00Z",
reason: "initiator",
resources: [],
status: "running",
}
export const MockFailedWorkspaceBuild = (
@ -237,6 +238,7 @@ export const MockFailedWorkspaceBuild = (
deadline: "2022-05-17T23:39:00.00Z",
reason: "initiator",
resources: [],
status: "running",
})
export const MockWorkspaceBuildStop: TypesGen.WorkspaceBuild = {
@ -268,7 +270,6 @@ export const MockWorkspace: TypesGen.Workspace = {
ttl_ms: 2 * 60 * 60 * 1000, // 2 hours as milliseconds
latest_build: MockWorkspaceBuild,
last_used_at: "",
status: "running",
}
export const MockStoppedWorkspace: TypesGen.Workspace = {