Skip to content

Commit ba41ae8

Browse files
kacpersawkacpersaw
committed
add support for canceling workspace builds with expect_state param (locking)
1 parent f89e057 commit ba41ae8

File tree

11 files changed

+283
-53
lines changed

11 files changed

+283
-53
lines changed

‎cli/provisionerjobs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (r *RootCmd) provisionerJobsCancel() *serpent.Command {
166166
err = client.CancelTemplateVersion(ctx, ptr.NilToEmpty(job.Input.TemplateVersionID))
167167
case codersdk.ProvisionerJobTypeWorkspaceBuild:
168168
_, _ = fmt.Fprintf(inv.Stdout, "Canceling workspace build job %s...\n", job.ID)
169-
err = client.CancelWorkspaceBuild(ctx, ptr.NilToEmpty(job.Input.WorkspaceBuildID))
169+
err = client.CancelWorkspaceBuild(ctx, ptr.NilToEmpty(job.Input.WorkspaceBuildID), codersdk.CancelWorkspaceBuildRequest{})
170170
}
171171
if err != nil {
172172
return xerrors.Errorf("cancel provisioner job: %w", err)

‎coderd/apidoc/docs.go

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

‎coderd/apidoc/swagger.json

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

‎coderd/workspacebuilds.go

Lines changed: 88 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -581,10 +581,12 @@ func (api *API) notifyWorkspaceUpdated(
581581
// @Produce json
582582
// @Tags Builds
583583
// @Param workspacebuild path string true "Workspace build ID"
584+
// @Param expect_state query string false "Expected state of the job"
584585
// @Success 200 {object} codersdk.Response
585586
// @Router /workspacebuilds/{workspacebuild}/cancel [patch]
586587
func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Request) {
587588
ctx := r.Context()
589+
expectState := r.URL.Query().Get("expect_state")
588590
workspaceBuild := httpmw.WorkspaceBuildParam(r)
589591
workspace, err := api.Database.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
590592
if err != nil {
@@ -609,46 +611,94 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques
609611
return
610612
}
611613

612-
job, err := api.Database.GetProvisionerJobByID(ctx, workspaceBuild.JobID)
613-
if err != nil {
614-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
615-
Message: "Internal error fetching provisioner job.",
616-
Detail: err.Error(),
617-
})
618-
return
619-
}
620-
if job.CompletedAt.Valid {
621-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
622-
Message: "Job has already completed!",
623-
})
624-
return
625-
}
626-
if job.CanceledAt.Valid {
627-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
628-
Message: "Job has already been marked as canceled!",
629-
})
630-
return
631-
}
632-
err = api.Database.UpdateProvisionerJobWithCancelByID(ctx, database.UpdateProvisionerJobWithCancelByIDParams{
633-
ID: job.ID,
634-
CanceledAt: sql.NullTime{
635-
Time: dbtime.Now(),
636-
Valid: true,
637-
},
638-
CompletedAt: sql.NullTime{
639-
Time: dbtime.Now(),
640-
// If the job is running, don't mark it completed!
641-
Valid: !job.WorkerID.Valid,
642-
},
643-
})
644-
if err != nil {
645-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
646-
Message: "Internal error updating provisioner job.",
647-
Detail: err.Error(),
614+
if expectState != "" {
615+
jobStatus := database.ProvisionerJobStatus(expectState)
616+
if !jobStatus.Valid() {
617+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
618+
Message: "Invalid expect_state. Only 'pending', 'running', 'succeeded', 'canceling', 'canceled', 'failed', or 'unknown' are allowed.",
619+
})
620+
return
621+
}
622+
623+
// use local error type to detect if the error is due to job state mismatch
624+
var errJobStateMismatch = xerrors.New("job is not in the expected state")
625+
626+
err := api.Database.InTx(func(store database.Store) error {
627+
job, err := store.GetProvisionerJobByIDForUpdate(ctx, workspaceBuild.JobID)
628+
if err != nil {
629+
return err
630+
}
631+
632+
if job.JobStatus != jobStatus {
633+
return errJobStateMismatch
634+
}
635+
636+
return store.UpdateProvisionerJobWithCancelByID(ctx, database.UpdateProvisionerJobWithCancelByIDParams{
637+
ID: job.ID,
638+
CanceledAt: sql.NullTime{
639+
Time: dbtime.Now(),
640+
Valid: true,
641+
},
642+
CompletedAt: sql.NullTime{
643+
Time: dbtime.Now(),
644+
Valid: !job.WorkerID.Valid,
645+
},
646+
})
647+
}, nil)
648+
if err != nil {
649+
if errors.Is(err, errJobStateMismatch) {
650+
httpapi.Write(ctx, rw, http.StatusPreconditionFailed, codersdk.Response{
651+
Message: "Job is not in the expected state.",
652+
})
653+
return
654+
}
655+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
656+
Message: "Internal error fetching provisioner job.",
657+
Detail: err.Error(),
658+
})
659+
return
660+
}
661+
} else {
662+
job, err := api.Database.GetProvisionerJobByID(ctx, workspaceBuild.JobID)
663+
if err != nil {
664+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
665+
Message: "Internal error fetching provisioner job.",
666+
Detail: err.Error(),
667+
})
668+
return
669+
}
670+
if job.CompletedAt.Valid {
671+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
672+
Message: "Job has already completed!",
673+
})
674+
return
675+
}
676+
if job.CanceledAt.Valid {
677+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
678+
Message: "Job has already been marked as canceled!",
679+
})
680+
return
681+
}
682+
err = api.Database.UpdateProvisionerJobWithCancelByID(ctx, database.UpdateProvisionerJobWithCancelByIDParams{
683+
ID: job.ID,
684+
CanceledAt: sql.NullTime{
685+
Time: dbtime.Now(),
686+
Valid: true,
687+
},
688+
CompletedAt: sql.NullTime{
689+
Time: dbtime.Now(),
690+
// If the job is running, don't mark it completed!
691+
Valid: !job.WorkerID.Valid,
692+
},
648693
})
649-
return
694+
if err != nil {
695+
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
696+
Message: "Internal error updating provisioner job.",
697+
Detail: err.Error(),
698+
})
699+
return
700+
}
650701
}
651-
652702
api.publishWorkspaceUpdate(ctx, workspace.OwnerID, wspubsub.WorkspaceEvent{
653703
Kind: wspubsub.WorkspaceEventKindStateChange,
654704
WorkspaceID: workspace.ID,

‎coderd/workspacebuilds_test.go

Lines changed: 153 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) {
573573
build, err = client.WorkspaceBuild(ctx, workspace.LatestBuild.ID)
574574
return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning
575575
}, testutil.WaitShort, testutil.IntervalFast)
576-
err := client.CancelWorkspaceBuild(ctx, build.ID)
576+
err := client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildRequest{})
577577
require.NoError(t, err)
578578
require.Eventually(t, func() bool {
579579
var err error
@@ -618,11 +618,161 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) {
618618
build, err = userClient.WorkspaceBuild(ctx, workspace.LatestBuild.ID)
619619
return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning
620620
}, testutil.WaitShort, testutil.IntervalFast)
621-
err := userClient.CancelWorkspaceBuild(ctx, build.ID)
621+
err := userClient.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildRequest{})
622622
var apiErr *codersdk.Error
623623
require.ErrorAs(t, err, &apiErr)
624624
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
625625
})
626+
627+
t.Run("Cancel with expect_state=pending", func(t *testing.T) {
628+
t.Parallel()
629+
if !dbtestutil.WillUsePostgres() {
630+
t.Skip("this test requires postgres")
631+
}
632+
// Given: a coderd instance with a provisioner daemon
633+
store, ps, db := dbtestutil.NewDBWithSQLDB(t)
634+
client, closeDaemon := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{
635+
Database: store,
636+
Pubsub: ps,
637+
IncludeProvisionerDaemon: true,
638+
})
639+
defer closeDaemon.Close()
640+
// Given: a user, template, and workspace
641+
user := coderdtest.CreateFirstUser(t, client)
642+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
643+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
644+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
645+
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
646+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
647+
648+
// Stop the provisioner daemon.
649+
require.NoError(t, closeDaemon.Close())
650+
ctx := testutil.Context(t, testutil.WaitLong)
651+
// Given: no provisioner daemons exist.
652+
_, err := db.ExecContext(ctx, `DELETE FROM provisioner_daemons;`)
653+
require.NoError(t, err)
654+
655+
// When: a new workspace build is created
656+
build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
657+
TemplateVersionID: template.ActiveVersionID,
658+
Transition: codersdk.WorkspaceTransitionStart,
659+
})
660+
// Then: the request should succeed.
661+
require.NoError(t, err)
662+
// Then: the provisioner job should remain pending.
663+
require.Equal(t, codersdk.ProvisionerJobPending, build.Job.Status)
664+
665+
// Then: the response should indicate no provisioners are available.
666+
if assert.NotNil(t, build.MatchedProvisioners) {
667+
assert.Zero(t, build.MatchedProvisioners.Count)
668+
assert.Zero(t, build.MatchedProvisioners.Available)
669+
assert.Zero(t, build.MatchedProvisioners.MostRecentlySeen.Time)
670+
assert.False(t, build.MatchedProvisioners.MostRecentlySeen.Valid)
671+
}
672+
673+
// When: the workspace build is canceled
674+
err = client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildRequest{
675+
ExpectState: codersdk.ProvisionerJobPending,
676+
})
677+
require.NoError(t, err)
678+
679+
// Then: the workspace build should be canceled.
680+
build, err = client.WorkspaceBuild(ctx, build.ID)
681+
require.NoError(t, err)
682+
require.Equal(t, codersdk.ProvisionerJobCanceled, build.Job.Status)
683+
})
684+
685+
t.Run("Cancel with expect_state=pending - should fail with 412", func(t *testing.T) {
686+
t.Parallel()
687+
if !dbtestutil.WillUsePostgres() {
688+
t.Skip("this test requires postgres")
689+
}
690+
691+
// Given: a coderd instance with a provisioner daemon
692+
store, ps, db := dbtestutil.NewDBWithSQLDB(t)
693+
client, closeDaemon, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
694+
Database: store,
695+
Pubsub: ps,
696+
IncludeProvisionerDaemon: true,
697+
})
698+
defer closeDaemon.Close()
699+
700+
// Given: a user, template, and workspace
701+
user := coderdtest.CreateFirstUser(t, client)
702+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
703+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
704+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
705+
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
706+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
707+
708+
// Stop the provisioner daemon.
709+
require.NoError(t, closeDaemon.Close())
710+
ctx := testutil.Context(t, testutil.WaitLong)
711+
712+
// Given: no provisioner daemons exist.
713+
_, err := db.ExecContext(ctx, `DELETE FROM provisioner_daemons;`)
714+
require.NoError(t, err)
715+
716+
// When: a new workspace build is created
717+
build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
718+
TemplateVersionID: template.ActiveVersionID,
719+
Transition: codersdk.WorkspaceTransitionStart,
720+
})
721+
// Then: the request should succeed.
722+
require.NoError(t, err)
723+
// Then: the provisioner job should remain pending.
724+
require.Equal(t, codersdk.ProvisionerJobPending, build.Job.Status)
725+
726+
// When: a provisioner daemon is started
727+
daemon := coderdtest.NewProvisionerDaemon(t, api)
728+
defer daemon.Close()
729+
// Then: the job should be acquired (status changes from pending)
730+
require.Eventually(t, func() bool {
731+
build, err = client.WorkspaceBuild(ctx, build.ID)
732+
return err == nil && build.Job.Status != codersdk.ProvisionerJobPending
733+
}, testutil.WaitShort, testutil.IntervalFast)
734+
735+
// When: a cancel request is made with expect_state=pending
736+
err = client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildRequest{
737+
ExpectState: codersdk.ProvisionerJobPending,
738+
})
739+
// Then: the request should fail with 412.
740+
require.Error(t, err)
741+
require.Equal(t, http.StatusPreconditionFailed, err.(*codersdk.Error).StatusCode())
742+
})
743+
744+
t.Run("Cancel with expect_state - invalid status", func(t *testing.T) {
745+
t.Parallel()
746+
747+
// Given: a coderd instance with a provisioner daemon
748+
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
749+
user := coderdtest.CreateFirstUser(t, client)
750+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
751+
Parse: echo.ParseComplete,
752+
ProvisionApply: []*proto.Response{{
753+
Type: &proto.Response_Log{
754+
Log: &proto.Log{},
755+
},
756+
}},
757+
ProvisionPlan: echo.PlanComplete,
758+
})
759+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
760+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
761+
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
762+
763+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
764+
defer cancel()
765+
766+
// When: a cancel request is made with invalid expect_state
767+
err := client.CancelWorkspaceBuild(ctx, workspace.LatestBuild.ID, codersdk.CancelWorkspaceBuildRequest{
768+
ExpectState: "invalid_status",
769+
})
770+
// Then: the request should fail with 400.
771+
var apiErr *codersdk.Error
772+
require.ErrorAs(t, err, &apiErr)
773+
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
774+
require.Contains(t, apiErr.Message, "Invalid expect_state")
775+
})
626776
}
627777

628778
func TestWorkspaceBuildResources(t *testing.T) {
@@ -968,7 +1118,7 @@ func TestWorkspaceBuildStatus(t *testing.T) {
9681118
_ = closeDaemon.Close()
9691119
// after successful cancel is "canceled"
9701120
build = coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart)
971-
err = client.CancelWorkspaceBuild(ctx, build.ID)
1121+
err = client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildRequest{})
9721122
require.NoError(t, err)
9731123

9741124
workspace, err = client.Workspace(ctx, workspace.ID)

‎coderd/workspaces_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3245,7 +3245,7 @@ func TestWorkspaceWatcher(t *testing.T) {
32453245
closeFunc.Close()
32463246
build := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart)
32473247
wait("first is for the workspace build itself", nil)
3248-
err = client.CancelWorkspaceBuild(ctx, build.ID)
3248+
err = client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildRequest{})
32493249
require.NoError(t, err)
32503250
wait("second is for the build cancel", nil)
32513251
}

0 commit comments

Comments
 (0)