Skip to content

Commit 5a896ed

Browse files
johnstcnjohnstcn
committed
fixup! nits
1 parent 92a4b99 commit 5a896ed

File tree

4 files changed

+12
-9
lines changed

4 files changed

+12
-9
lines changed

‎coderd/workspacebuilds_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,8 +528,8 @@ func TestWorkspaceBuildsProvisionerState(t *testing.T) {
528528
})
529529
require.NoError(t, err)
530530
require.Equal(t, codersdk.WorkspaceTransitionDelete, build.Transition)
531-
require.Equal(t, codersdk.ProvisionerJobFailed, build.Job.Status)
532-
require.Contains(t, build.Job.Error, "No provisioners were available to handle the request")
531+
require.Equal(t, codersdk.ProvisionerJobSucceeded, build.Job.Status)
532+
require.Empty(t, build.Job.Error)
533533

534534
ws, err := client.Workspace(ctx, r.Workspace.ID)
535535
require.Empty(t, ws)

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -467,11 +467,13 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
467467
// If the requestor is trying to orphan-delete a workspace and there are no
468468
// provisioners available, we should complete the build and mark the
469469
// workspace as deleted ourselves.
470-
// Orphan-deleting a workspace sends an empty state to Terraform, which means
471-
// it won't actually delete anything.
472470
// There are cases where tagged provisioner daemons have been decommissioned
473471
// without deleting the relevant workspaces, and without any provisioners
474472
// available these workspaces cannot be deleted.
473+
// Orphan-deleting a workspace sends an empty state to Terraform, which means
474+
// it won't actually delete anything. So we actually don't need to execute a
475+
// provisioner job at all for an orphan delete, but deleting without a workspace
476+
// build or provisioner job would result in no audit log entry, which is a deal-breaker.
475477
hasActiveEligibleProvisioner := false
476478
for _, pd := range provisionerDaemons {
477479
age := now.Sub(pd.ProvisionerDaemon.LastSeenAt.Time)
@@ -484,7 +486,7 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
484486
// nolint: gocritic // At this moment, we are pretending to be provisionerd.
485487
if err := store.UpdateProvisionerJobWithCompleteWithStartedAtByID(dbauthz.AsProvisionerd(b.ctx), database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{
486488
CompletedAt: sql.NullTime{Valid: true, Time: now},
487-
Error: sql.NullString{Valid: true, String: "No provisioners were available to handle the request. The workspace has been deleted. No resources were destroyed."},
489+
Error: sql.NullString{Valid: false},
488490
ErrorCode: sql.NullString{Valid: false},
489491
ID: provisionerJob.ID,
490492
StartedAt: sql.NullTime{Valid: true, Time: now},

‎coderd/wsbuilder/wsbuilder_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ func TestWorkspaceBuildWithPreset(t *testing.T) {
842842
func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
843843
t.Parallel()
844844

845-
t.Run("Provisioners", func(t *testing.T) {
845+
t.Run("WithActiveProvisioners", func(t *testing.T) {
846846
t.Parallel()
847847
req := require.New(t)
848848
asrt := assert.New(t)
@@ -904,7 +904,7 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
904904
req.NoError(err)
905905
})
906906

907-
t.Run("NoProvisioners", func(t *testing.T) {
907+
t.Run("NoActiveProvisioners", func(t *testing.T) {
908908
t.Parallel()
909909
req := require.New(t)
910910
asrt := assert.New(t)
@@ -959,7 +959,7 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
959959
// Because no provisioners were available and the request was to delete --orphan
960960
expectUpdateProvisionerJobWithCompleteWithStartedAtByID(func(params database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams) {
961961
asrt.Equal(jobID, params.ID)
962-
asrt.Contains(params.Error.String, "No provisioners were available")
962+
asrt.False(params.Error.Valid)
963963
asrt.True(params.CompletedAt.Valid)
964964
asrt.True(params.StartedAt.Valid)
965965
}),

‎site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceDeleteDialog.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ export const WorkspaceDeleteDialog: FC<WorkspaceDeleteDialogProps> = ({
4949
// usually means that builds are failing as well.
5050
// b) No provisioner is available to delete the workspace, which will
5151
// cause the job to remain in the "pending" state indefinitely.
52-
// The assumption here is that an admin will cancel the job.
52+
// The assumption here is that an admin will cancel the job, in which
53+
// case we want to allow them to perform an orphan-delete.
5354
const canOrphan =
5455
canDeleteFailedWorkspace &&
5556
(workspace.latest_build.status === "failed" ||

0 commit comments

Comments
 (0)