Skip to content

feat!: add ability to cancel pending workspace build #18713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/provisionerjobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (r *RootCmd) provisionerJobsCancel() *serpent.Command {
err = client.CancelTemplateVersion(ctx, ptr.NilToEmpty(job.Input.TemplateVersionID))
case codersdk.ProvisionerJobTypeWorkspaceBuild:
_, _ = fmt.Fprintf(inv.Stdout, "Canceling workspace build job %s...\n", job.ID)
err = client.CancelWorkspaceBuild(ctx, ptr.NilToEmpty(job.Input.WorkspaceBuildID))
err = client.CancelWorkspaceBuild(ctx, ptr.NilToEmpty(job.Input.WorkspaceBuildID), codersdk.CancelWorkspaceBuildParams{})
}
if err != nil {
return xerrors.Errorf("cancel provisioner job: %w", err)
Expand Down
10 changes: 10 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

138 changes: 86 additions & 52 deletions coderd/workspacebuilds.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,10 +581,12 @@ func (api *API) notifyWorkspaceUpdated(
// @Produce json
// @Tags Builds
// @Param workspacebuild path string true "Workspace build ID"
// @Param expect_status query string false "Expected status of the job" Enums(running, pending)
// @Success 200 {object} codersdk.Response
// @Router /workspacebuilds/{workspacebuild}/cancel [patch]
func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
expectStatus := r.URL.Query().Get("expect_status")
workspaceBuild := httpmw.WorkspaceBuildParam(r)
workspace, err := api.Database.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
if err != nil {
Expand All @@ -594,58 +596,85 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques
return
}

valid, err := api.verifyUserCanCancelWorkspaceBuilds(ctx, httpmw.APIKey(r).UserID, workspace.TemplateID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error verifying permission to cancel workspace build.",
Detail: err.Error(),
})
return
}
if !valid {
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
Message: "User is not allowed to cancel workspace builds. Owner role is required.",
})
return
}
code := http.StatusOK
resp := codersdk.Response{}
err = api.Database.InTx(func(db database.Store) error {
valid, err := verifyUserCanCancelWorkspaceBuilds(ctx, db, httpmw.APIKey(r).UserID, workspace.TemplateID, expectStatus)
if err != nil {
code = http.StatusInternalServerError
resp.Message = "Internal error verifying permission to cancel workspace build."
resp.Detail = err.Error()

job, err := api.Database.GetProvisionerJobByID(ctx, workspaceBuild.JobID)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching provisioner job.",
Detail: err.Error(),
})
return
}
if job.CompletedAt.Valid {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Job has already completed!",
})
return
}
if job.CanceledAt.Valid {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Job has already been marked as canceled!",
return xerrors.Errorf("verify user can cancel workspace builds: %w", err)
}
if !valid {
code = http.StatusForbidden
resp.Message = "User is not allowed to cancel workspace builds. Owner role is required."

return xerrors.New("user is not allowed to cancel workspace builds")
}

job, err := db.GetProvisionerJobByIDForUpdate(ctx, workspaceBuild.JobID)
if err != nil {
code = http.StatusInternalServerError
resp.Message = "Internal error fetching provisioner job."
resp.Detail = err.Error()

return xerrors.Errorf("get provisioner job: %w", err)
}
if job.CompletedAt.Valid {
code = http.StatusBadRequest
resp.Message = "Job has already completed!"

return xerrors.New("job has already completed")
}
if job.CanceledAt.Valid {
code = http.StatusBadRequest
resp.Message = "Job has already been marked as canceled!"

return xerrors.New("job has already been marked as canceled")
}

if expectStatus != "" {
if expectStatus != "running" && expectStatus != "pending" {
code = http.StatusBadRequest
resp.Message = fmt.Sprintf("Invalid expect_status %q. Only 'running' or 'pending' are allowed.", expectStatus)

return xerrors.Errorf("invalid expect_status %q", expectStatus)
}

if job.JobStatus != database.ProvisionerJobStatus(expectStatus) {
code = http.StatusPreconditionFailed
resp.Message = "Job is not in the expected state."

return xerrors.Errorf("job is not in the expected state: expected: %q, got %q", expectStatus, job.JobStatus)
}
}

err = db.UpdateProvisionerJobWithCancelByID(ctx, database.UpdateProvisionerJobWithCancelByIDParams{
ID: job.ID,
CanceledAt: sql.NullTime{
Time: dbtime.Now(),
Valid: true,
},
CompletedAt: sql.NullTime{
Time: dbtime.Now(),
// If the job is running, don't mark it completed!
Valid: !job.WorkerID.Valid,
},
})
return
}
err = api.Database.UpdateProvisionerJobWithCancelByID(ctx, database.UpdateProvisionerJobWithCancelByIDParams{
ID: job.ID,
CanceledAt: sql.NullTime{
Time: dbtime.Now(),
Valid: true,
},
CompletedAt: sql.NullTime{
Time: dbtime.Now(),
// If the job is running, don't mark it completed!
Valid: !job.WorkerID.Valid,
},
})
if err != nil {
code = http.StatusInternalServerError
resp.Message = "Internal error updating provisioner job."
resp.Detail = err.Error()

return xerrors.Errorf("update provisioner job: %w", err)
}

return nil
}, nil)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error updating provisioner job.",
Detail: err.Error(),
})
httpapi.Write(ctx, rw, code, resp)
return
}

Expand All @@ -659,8 +688,13 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques
})
}

func (api *API) verifyUserCanCancelWorkspaceBuilds(ctx context.Context, userID uuid.UUID, templateID uuid.UUID) (bool, error) {
template, err := api.Database.GetTemplateByID(ctx, templateID)
func verifyUserCanCancelWorkspaceBuilds(ctx context.Context, store database.Store, userID uuid.UUID, templateID uuid.UUID, expectStatus string) (bool, error) {
// If the expectStatus is pending, we can cancel it.
if expectStatus == "pending" {
return true, nil
}

template, err := store.GetTemplateByID(ctx, templateID)
if err != nil {
return false, xerrors.New("no template exists for this workspace")
}
Expand All @@ -669,7 +703,7 @@ func (api *API) verifyUserCanCancelWorkspaceBuilds(ctx context.Context, userID u
return true, nil // all users can cancel workspace builds
}

user, err := api.Database.GetUserByID(ctx, userID)
user, err := store.GetUserByID(ctx, userID)
if err != nil {
return false, xerrors.New("user does not exist")
}
Expand Down
136 changes: 133 additions & 3 deletions coderd/workspacebuilds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) {
build, err = client.WorkspaceBuild(ctx, workspace.LatestBuild.ID)
return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning
}, testutil.WaitShort, testutil.IntervalFast)
err := client.CancelWorkspaceBuild(ctx, build.ID)
err := client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildParams{})
require.NoError(t, err)
require.Eventually(t, func() bool {
var err error
Expand Down Expand Up @@ -618,11 +618,141 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) {
build, err = userClient.WorkspaceBuild(ctx, workspace.LatestBuild.ID)
return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning
}, testutil.WaitShort, testutil.IntervalFast)
err := userClient.CancelWorkspaceBuild(ctx, build.ID)
err := userClient.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildParams{})
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
})

t.Run("Cancel with expect_state=pending", func(t *testing.T) {
t.Parallel()
if !dbtestutil.WillUsePostgres() {
t.Skip("this test requires postgres")
}
// Given: a coderd instance with a provisioner daemon
store, ps, db := dbtestutil.NewDBWithSQLDB(t)
client, closeDaemon := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{
Database: store,
Pubsub: ps,
IncludeProvisionerDaemon: true,
})
defer closeDaemon.Close()
// Given: a user, template, and workspace
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)

// Stop the provisioner daemon.
require.NoError(t, closeDaemon.Close())
ctx := testutil.Context(t, testutil.WaitLong)
// Given: no provisioner daemons exist.
_, err := db.ExecContext(ctx, `DELETE FROM provisioner_daemons;`)
require.NoError(t, err)

// When: a new workspace build is created
build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
TemplateVersionID: template.ActiveVersionID,
Transition: codersdk.WorkspaceTransitionStart,
})
// Then: the request should succeed.
require.NoError(t, err)
// Then: the provisioner job should remain pending.
require.Equal(t, codersdk.ProvisionerJobPending, build.Job.Status)

// Then: the response should indicate no provisioners are available.
if assert.NotNil(t, build.MatchedProvisioners) {
assert.Zero(t, build.MatchedProvisioners.Count)
assert.Zero(t, build.MatchedProvisioners.Available)
assert.Zero(t, build.MatchedProvisioners.MostRecentlySeen.Time)
assert.False(t, build.MatchedProvisioners.MostRecentlySeen.Valid)
}

// When: the workspace build is canceled
err = client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildParams{
ExpectStatus: codersdk.CancelWorkspaceBuildStatusPending,
})
require.NoError(t, err)

// Then: the workspace build should be canceled.
build, err = client.WorkspaceBuild(ctx, build.ID)
require.NoError(t, err)
require.Equal(t, codersdk.ProvisionerJobCanceled, build.Job.Status)
})

t.Run("Cancel with expect_state=pending - should fail with 412", 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, &echo.Responses{
Parse: echo.ParseComplete,
ProvisionApply: []*proto.Response{{
Type: &proto.Response_Log{
Log: &proto.Log{},
},
}},
ProvisionPlan: echo.PlanComplete,
})
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
var build codersdk.WorkspaceBuild

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

require.Eventually(t, func() bool {
var err error
build, err = client.WorkspaceBuild(ctx, workspace.LatestBuild.ID)
return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning
}, testutil.WaitShort, testutil.IntervalFast)

// When: a cancel request is made with expect_state=pending
err := client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildParams{
ExpectStatus: codersdk.CancelWorkspaceBuildStatusPending,
})
// Then: the request should fail with 412.
require.Error(t, err)

var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusPreconditionFailed, apiErr.StatusCode())
})

t.Run("Cancel with expect_state - invalid status", func(t *testing.T) {
t.Parallel()

// Given: a coderd instance with a provisioner daemon
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
ProvisionApply: []*proto.Response{{
Type: &proto.Response_Log{
Log: &proto.Log{},
},
}},
ProvisionPlan: echo.PlanComplete,
})
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, template.ID)

ctx := testutil.Context(t, testutil.WaitLong)

// When: a cancel request is made with invalid expect_state
err := client.CancelWorkspaceBuild(ctx, workspace.LatestBuild.ID, codersdk.CancelWorkspaceBuildParams{
ExpectStatus: "invalid_status",
})
// Then: the request should fail with 400.
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
require.Contains(t, apiErr.Message, "Invalid expect_status")
})
}

func TestWorkspaceBuildResources(t *testing.T) {
Expand Down Expand Up @@ -968,7 +1098,7 @@ func TestWorkspaceBuildStatus(t *testing.T) {
_ = closeDaemon.Close()
// after successful cancel is "canceled"
build = coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart)
err = client.CancelWorkspaceBuild(ctx, build.ID)
err = client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildParams{})
require.NoError(t, err)

workspace, err = client.Workspace(ctx, workspace.ID)
Expand Down
2 changes: 1 addition & 1 deletion coderd/workspaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3245,7 +3245,7 @@ func TestWorkspaceWatcher(t *testing.T) {
closeFunc.Close()
build := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart)
wait("first is for the workspace build itself", nil)
err = client.CancelWorkspaceBuild(ctx, build.ID)
err = client.CancelWorkspaceBuild(ctx, build.ID, codersdk.CancelWorkspaceBuildParams{})
require.NoError(t, err)
wait("second is for the build cancel", nil)
}
Expand Down
Loading
Loading