Skip to content

Commit 9d3ea6f

Browse files
johnstcnjohnstcn
committed
fix: wsbuilder: complete job and mark workspace deleted if no provisioners available
1 parent bacdc28 commit 9d3ea6f

File tree

6 files changed

+346
-43
lines changed

6 files changed

+346
-43
lines changed

‎cli/delete_test.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"io"
7+
"net/http"
78
"testing"
89

910
"github.com/stretchr/testify/assert"
@@ -51,28 +52,35 @@ func TestDelete(t *testing.T) {
5152
t.Parallel()
5253
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
5354
owner := coderdtest.CreateFirstUser(t, client)
54-
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
55-
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
56-
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
57-
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
58-
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
55+
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
56+
version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, nil)
57+
coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID)
58+
template := coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID)
59+
workspace := coderdtest.CreateWorkspace(t, templateAdmin, template.ID)
60+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, workspace.LatestBuild.ID)
61+
62+
ctx := testutil.Context(t, testutil.WaitShort)
5963
inv, root := clitest.New(t, "delete", workspace.Name, "-y", "--orphan")
64+
clitest.SetupConfig(t, templateAdmin, root)
6065

61-
//nolint:gocritic // Deleting orphaned workspaces requires an admin.
62-
clitest.SetupConfig(t, client, root)
6366
doneChan := make(chan struct{})
6467
pty := ptytest.New(t).Attach(inv)
6568
inv.Stderr = pty.Output()
6669
go func() {
6770
defer close(doneChan)
68-
err := inv.Run()
71+
err := inv.WithContext(ctx).Run()
6972
// When running with the race detector on, we sometimes get an EOF.
7073
if err != nil {
7174
assert.ErrorIs(t, err, io.EOF)
7275
}
7376
}()
7477
pty.ExpectMatch("has been deleted")
75-
<-doneChan
78+
testutil.TryReceive(ctx, t, doneChan)
79+
80+
_, err := client.Workspace(ctx, workspace.ID)
81+
require.Error(t, err)
82+
cerr := coderdtest.SDKError(t, err)
83+
require.Equal(t, http.StatusGone, cerr.StatusCode())
7684
})
7785

7886
// Super orphaned, as the workspace doesn't even have a user.

‎coderd/database/dbmem/dbmem.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4464,7 +4464,7 @@ func (q *FakeQuerier) GetProvisionerDaemons(_ context.Context) ([]database.Provi
44644464
defer q.mutex.RUnlock()
44654465

44664466
if len(q.provisionerDaemons) == 0 {
4467-
return nil, sql.ErrNoRows
4467+
return []database.ProvisionerDaemon{}, nil
44684468
}
44694469
// copy the data so that the caller can't manipulate any data inside dbmem
44704470
// after returning

‎coderd/workspacebuilds.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,10 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
423423
return
424424
}
425425

426+
var qpr database.GetProvisionerJobsByIDsWithQueuePositionRow
426427
if provisionerJob != nil {
428+
qpr.ProvisionerJob = *provisionerJob
429+
qpr.QueuePosition = 0
427430
if err := provisionerjobs.PostJob(api.Pubsub, *provisionerJob); err != nil {
428431
// Client probably doesn't care about this error, so just log it.
429432
api.Logger.Error(ctx, "failed to post provisioner job to pubsub", slog.Error(err))
@@ -433,10 +436,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
433436
apiBuild, err := api.convertWorkspaceBuild(
434437
*workspaceBuild,
435438
workspace,
436-
database.GetProvisionerJobsByIDsWithQueuePositionRow{
437-
ProvisionerJob: *provisionerJob,
438-
QueuePosition: 0,
439-
},
439+
qpr,
440440
[]database.WorkspaceResource{},
441441
[]database.WorkspaceResourceMetadatum{},
442442
[]database.WorkspaceAgent{},

‎coderd/workspacebuilds_test.go

Lines changed: 128 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
2626
"github.com/coder/coder/v2/coderd/database"
2727
"github.com/coder/coder/v2/coderd/database/dbauthz"
28+
"github.com/coder/coder/v2/coderd/database/dbfake"
2829
"github.com/coder/coder/v2/coderd/database/dbgen"
2930
"github.com/coder/coder/v2/coderd/database/dbtestutil"
3031
"github.com/coder/coder/v2/coderd/database/dbtime"
@@ -371,42 +372,140 @@ func TestWorkspaceBuildsProvisionerState(t *testing.T) {
371372

372373
t.Run("Orphan", func(t *testing.T) {
373374
t.Parallel()
374-
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
375-
first := coderdtest.CreateFirstUser(t, client)
376-
377-
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
378-
defer cancel()
379375

380-
version := coderdtest.CreateTemplateVersion(t, client, first.OrganizationID, nil)
381-
template := coderdtest.CreateTemplate(t, client, first.OrganizationID, version.ID)
382-
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
376+
t.Run("WithoutDelete", func(t *testing.T) {
377+
t.Parallel()
378+
client, store := coderdtest.NewWithDatabase(t, nil)
379+
first := coderdtest.CreateFirstUser(t, client)
380+
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin())
381+
382+
r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
383+
OwnerID: templateAdminUser.ID,
384+
OrganizationID: first.OrganizationID,
385+
}).Do()
386+
387+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
388+
defer cancel()
389+
390+
// Trying to orphan without delete transition fails.
391+
_, err := templateAdmin.CreateWorkspaceBuild(ctx, r.Workspace.ID, codersdk.CreateWorkspaceBuildRequest{
392+
TemplateVersionID: r.TemplateVersion.ID,
393+
Transition: codersdk.WorkspaceTransitionStart,
394+
Orphan: true,
395+
})
396+
require.Error(t, err, "Orphan is only permitted when deleting a workspace.")
397+
cerr := coderdtest.SDKError(t, err)
398+
require.Equal(t, http.StatusBadRequest, cerr.StatusCode())
399+
})
383400

384-
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
385-
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
401+
t.Run("WithState", func(t *testing.T) {
402+
t.Parallel()
403+
client, store := coderdtest.NewWithDatabase(t, nil)
404+
first := coderdtest.CreateFirstUser(t, client)
405+
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin())
406+
407+
r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
408+
OwnerID: templateAdminUser.ID,
409+
OrganizationID: first.OrganizationID,
410+
}).Do()
411+
412+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
413+
defer cancel()
414+
415+
// Providing both state and orphan fails.
416+
_, err := templateAdmin.CreateWorkspaceBuild(ctx, r.Workspace.ID, codersdk.CreateWorkspaceBuildRequest{
417+
TemplateVersionID: r.TemplateVersion.ID,
418+
Transition: codersdk.WorkspaceTransitionDelete,
419+
ProvisionerState: []byte(" "),
420+
Orphan: true,
421+
})
422+
require.Error(t, err)
423+
cerr := coderdtest.SDKError(t, err)
424+
require.Equal(t, http.StatusBadRequest, cerr.StatusCode())
425+
})
386426

387-
// Providing both state and orphan fails.
388-
_, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
389-
TemplateVersionID: workspace.LatestBuild.TemplateVersionID,
390-
Transition: codersdk.WorkspaceTransitionDelete,
391-
ProvisionerState: []byte(" "),
392-
Orphan: true,
427+
t.Run("NoPermission", func(t *testing.T) {
428+
t.Parallel()
429+
client, store := coderdtest.NewWithDatabase(t, nil)
430+
first := coderdtest.CreateFirstUser(t, client)
431+
member, memberUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID)
432+
433+
r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
434+
OwnerID: memberUser.ID,
435+
OrganizationID: first.OrganizationID,
436+
}).Do()
437+
438+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
439+
defer cancel()
440+
441+
// Trying to orphan without being a template admin fails.
442+
_, err := member.CreateWorkspaceBuild(ctx, r.Workspace.ID, codersdk.CreateWorkspaceBuildRequest{
443+
TemplateVersionID: r.TemplateVersion.ID,
444+
Transition: codersdk.WorkspaceTransitionDelete,
445+
Orphan: true,
446+
})
447+
require.Error(t, err)
448+
cerr := coderdtest.SDKError(t, err)
449+
require.Equal(t, http.StatusForbidden, cerr.StatusCode())
393450
})
394-
require.Error(t, err)
395-
cerr := coderdtest.SDKError(t, err)
396-
require.Equal(t, http.StatusBadRequest, cerr.StatusCode())
397451

398-
// Regular orphan operation succeeds.
399-
build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
400-
TemplateVersionID: workspace.LatestBuild.TemplateVersionID,
401-
Transition: codersdk.WorkspaceTransitionDelete,
402-
Orphan: true,
452+
t.Run("OK", func(t *testing.T) {
453+
// Include a provisioner so that we can test that provisionerdserver
454+
// performs deletion.
455+
client, store := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
456+
first := coderdtest.CreateFirstUser(t, client)
457+
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin())
458+
459+
r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
460+
OwnerID: templateAdminUser.ID,
461+
OrganizationID: first.OrganizationID,
462+
}).Do()
463+
464+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
465+
defer cancel()
466+
467+
// Regular orphan operation succeeds.
468+
build, err := templateAdmin.CreateWorkspaceBuild(ctx, r.Workspace.ID, codersdk.CreateWorkspaceBuildRequest{
469+
TemplateVersionID: r.TemplateVersion.ID,
470+
Transition: codersdk.WorkspaceTransitionDelete,
471+
Orphan: true,
472+
})
473+
require.NoError(t, err)
474+
require.Equal(t, codersdk.WorkspaceTransitionDelete, build.Transition)
475+
require.Equal(t, codersdk.ProvisionerJobPending, build.Job.Status)
403476
})
404-
require.NoError(t, err)
405-
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, build.ID)
406477

407-
_, err = client.Workspace(ctx, workspace.ID)
408-
require.Error(t, err)
409-
require.Equal(t, http.StatusGone, coderdtest.SDKError(t, err).StatusCode())
478+
t.Run("NoProvisioners", func(t *testing.T) {
479+
t.Parallel()
480+
client, store := coderdtest.NewWithDatabase(t, nil)
481+
first := coderdtest.CreateFirstUser(t, client)
482+
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleTemplateAdmin())
483+
484+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
485+
defer cancel()
486+
r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{
487+
OwnerID: templateAdminUser.ID,
488+
OrganizationID: first.OrganizationID,
489+
}).Do()
490+
491+
// nolint:gocritic // For testing
492+
daemons, err := store.GetProvisionerDaemons(dbauthz.AsSystemReadProvisionerDaemons(ctx))
493+
require.NoError(t, err)
494+
require.Empty(t, daemons, "Provisioner daemons should be empty for this test")
495+
496+
// Orphan deletion still succeeds despite no provisioners being available.
497+
build, err := templateAdmin.CreateWorkspaceBuild(ctx, r.Workspace.ID, codersdk.CreateWorkspaceBuildRequest{
498+
TemplateVersionID: r.TemplateVersion.ID,
499+
Transition: codersdk.WorkspaceTransitionDelete,
500+
Orphan: true,
501+
})
502+
require.NoError(t, err)
503+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, templateAdmin, build.ID)
504+
505+
ws, err := client.Workspace(ctx, r.Workspace.ID)
506+
require.Empty(t, ws)
507+
require.Equal(t, http.StatusGone, coderdtest.SDKError(t, err).StatusCode())
508+
})
410509
})
411510
}
412511

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,35 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
464464
return BuildError{http.StatusInternalServerError, "get workspace build", err}
465465
}
466466

467+
// If the requestor is trying to orphan-delete a workspace and there are no
468+
// provisioners available, we should complete the build and mark the
469+
// workspace as deleted ourselves.
470+
// Orphan-deleting a workspace sends an empty state to Terraform, which means
471+
// it won't actually delete anything.
472+
// There are cases where tagged provisioner daemons have been decommissioned
473+
// without deleting the relevant workspaces, and without any provisioners
474+
// available these workspaces cannot be deleted.
475+
if b.state.orphan && len(provisionerDaemons) == 0 {
476+
// nolint: gocritic // At this moment, we are pretending to be provisionerd.
477+
if err := store.UpdateProvisionerJobWithCompleteWithStartedAtByID(dbauthz.AsProvisionerd(b.ctx), database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{
478+
CompletedAt: sql.NullTime{Valid: true, Time: now},
479+
Error: sql.NullString{Valid: true, String: "No provisioners were available to handle the orphan-delete request. The workspace was marked as deleted, but no resources were destroyed."},
480+
ErrorCode: sql.NullString{Valid: false},
481+
ID: provisionerJob.ID,
482+
StartedAt: sql.NullTime{Valid: true, Time: now},
483+
UpdatedAt: now,
484+
}); err != nil {
485+
return BuildError{http.StatusInternalServerError, "mark orphan-delete provisioner job as completed", err}
486+
}
487+
488+
if err := store.UpdateWorkspaceDeletedByID(b.ctx, database.UpdateWorkspaceDeletedByIDParams{
489+
ID: b.workspace.ID,
490+
Deleted: true,
491+
}); err != nil {
492+
return BuildError{http.StatusInternalServerError, "mark workspace as deleted", err}
493+
}
494+
}
495+
467496
return nil
468497
}, nil)
469498
if err != nil {

0 commit comments

Comments
 (0)