8000 fix!: stop workspace before update by johnstcn · Pull Request #18425 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

fix!: stop workspace before update #18425

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Jun 23, 2025
Merged

fix!: stop workspace before update #18425

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
9443453
chore: TestUpdate: remove extraneous cli invocation
johnstcn Jun 17, 2025
594d1d7
assert previous build state when updating workspace
johnstcn Jun 17, 2025
873794f
chore: coderdtest: change argument types to remove unnecessary conver…
johnstcn Jun 17, 2025
f73b3f6
cli: stop before starting on update
johnstcn Jun 17, 2025
8a48d85
chore: refactor and extract stopWorkspace function
johnstcn Jun 18, 2025
123b51f
update cli usage doc
johnstcn Jun 18, 2025
3d6106f
confirm before stop
johnstcn Jun 18, 2025
f25c333
Revert "confirm before stop"
johnstcn Jun 18, 2025
9bf5141
ui: make update button stop before start
johnstcn Jun 18, 2025
8dd13d3
make fmt
johnstcn Jun 18, 2025
be0c176
site: fix some tests
johnstcn Jun 18, 2025
1c703ca
fixup! site: fix some tests
johnstcn Jun 18, 2025
da8bd12
reduce button explosion
johnstcn Jun 19, 2025 10000
ed6399a
adjust icons
johnstcn Jun 19, 2025
0cf0110
add testid to bulk update button and fix bulk update tests
johnstcn Jun 19, 2025
d4529c8
fix e2e race
johnstcn Jun 20, 2025
cc236ab
Merge branch 'main' into cj/prebuild-template-upgrade
johnstcn Jun 20, 2025
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
Prev Previous commit
Next Next commit
chore: coderdtest: change argument types to remove unnecessary conver… 
…sion
  • Loading branch information
@johnstcn
johnstcn committed Jun 19, 2025
commit 873794fa1cbbc637457da3e56d2db91cb011d941
2 changes: 1 addition & 1 deletion cli/start_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ func TestStartAutoUpdate(t *testing.T) {
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)

if c.Cmd == "start" {
coderdtest.MustTransitionWorkspace(t, member, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, member, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
}
version2 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, prepareEchoResponses(stringRichParameters), func(ctvr *codersdk.CreateTemplateVersionRequest) {
ctvr.TemplateID = template.ID
Expand Down
24 changes: 12 additions & 12 deletions coderd/autobuild/lifecycle_executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestExecutorAutostartOK(t *testing.T) {
})
)
// Given: workspace is stopped
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// When: the autobuild executor ticks after the scheduled time
go func() {
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestMultipleLifecycleExecutors(t *testing.T) {
)

// Have the workspace stopped so we can perform an autostart
workspace = coderdtest.MustTransitionWorkspace(t, clientA, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, clientA, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// Get both clients to perform a lifecycle execution tick
next := sched.Next(workspace.LatestBuild.CreatedAt)
Expand Down Expand Up @@ -204,7 +204,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
)
// Given: workspace is stopped
workspace = coderdtest.MustTransitionWorkspace(
t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

orgs, err := client.OrganizationsByUser(ctx, workspace.OwnerID.String())
require.NoError(t, err)
Expand Down Expand Up @@ -345,7 +345,7 @@ func TestExecutorAutostartNotEnabled(t *testing.T) {
require.Empty(t, workspace.AutostartSchedule)

// Given: workspace is stopped
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// When: the autobuild executor ticks way into the future
go func() {
Expand Down Expand Up @@ -385,7 +385,7 @@ func TestExecutorAutostartUserSuspended(t *testing.T) {
workspace = coderdtest.MustWorkspace(t, userClient, workspace.ID)

// Given: workspace is stopped, and the user is suspended.
workspace = coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

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

Expand Down Expand Up @@ -508,7 +508,7 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) {
)

// Given: workspace is stopped
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// When: the autobuild executor ticks past the TTL
go func() {
Expand Down Expand Up @@ -579,7 +579,7 @@ func TestExecutorWorkspaceDeleted(t *testing.T) {
)

// Given: workspace is deleted
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionDelete)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionDelete)

// When: the autobuild executor ticks
go func() {
Expand Down Expand Up @@ -768,7 +768,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) {
})
)
// Given: workspace is stopped
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// When: the autobuild executor ticks past the scheduled time
go func() {
Expand Down Expand Up @@ -833,7 +833,7 @@ func TestExecutorAutostartWithParameters(t *testing.T) {
})
)
// Given: workspace is stopped
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// When: the autobuild executor ticks after the scheduled time
go func() {
Expand Down Expand Up @@ -883,7 +883,7 @@ func TestExecutorAutostartTemplateDisabled(t *testing.T) {
})
)
// Given: workspace is stopped
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// When: the autobuild executor ticks before the next scheduled time
go func() {
Expand Down Expand Up @@ -1002,7 +1002,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) {
cwr.AutostartSchedule = ptr.Ref(sched.String())
})
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, ownerClient, ws.LatestBuild.ID)
ws = coderdtest.MustTransitionWorkspace(t, memberClient, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop, func(req *codersdk.CreateWorkspaceBuildRequest) {
ws = coderdtest.MustTransitionWorkspace(t, memberClient, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop, func(req *codersdk.CreateWorkspaceBuildRequest) {
req.TemplateVersionID = inactiveVersion.ID
})
require.Equal(t, inactiveVersion.ID, ws.LatestBuild.TemplateVersionID)
Expand Down Expand Up @@ -1160,7 +1160,7 @@ func TestNotifications(t *testing.T) {
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)

// Stop workspace
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)

// Wait for workspace to become dormant
Expand Down
8 changes: 4 additions & 4 deletions coderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,16 +1245,16 @@ func CreateWorkspace(t testing.TB, client *codersdk.Client, templateID uuid.UUID
}

// TransitionWorkspace is a convenience method for transitioning a workspace from one state to another.
func MustTransitionWorkspace(t testing.TB, client *codersdk.Client, workspaceID uuid.UUID, from, to database.WorkspaceTransition, muts ...func(req *codersdk.CreateWorkspaceBuildRequest)) codersdk.Workspace {
func MustTransitionWorkspace(t testing.TB, client *codersdk.Client, workspaceID uuid.UUID, from, to codersdk.WorkspaceTransition, muts ...func(req *codersdk.CreateWorkspaceBuildRequest)) codersdk.Workspace {
Copy link
Member Author
@johnstcn johnstcn Jun 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: This is annoying to have as a database type, so migrated to use the codersdk type instead. This inflated the diff a bit but removes an unnecessary conversion.

t.Helper()
ctx := context.Background()
workspace, err := client.Workspace(ctx, workspaceID)
require.NoError(t, err, "unexpected error fetching workspace")
require.Equal(t, workspace.LatestBuild.Transition, codersdk.WorkspaceTransition(from), "expected workspace state: %s got: %s", from, workspace.LatestBuild.Transition)
require.Equal(t, workspace.LatestBuild.Transition, from, "expected workspace state: %s got: %s", from, workspace.LatestBuild.Transition)

req := codersdk.CreateWorkspaceBuildRequest{
TemplateVersionID: workspace.LatestBuild.TemplateVersionID,
Transition: codersdk.WorkspaceTransition(to),
Transition: to,
}

for _, mut := range muts {
Expand All @@ -1267,7 +1267,7 @@ func MustTransitionWorkspace(t testing.TB, client *codersdk.Client, workspaceID
_ = AwaitWorkspaceBuildJobCompleted(t, client, build.ID)

updated := MustWorkspace(t, client, workspace.ID)
require.Equal(t, codersdk.WorkspaceTransition(to), updated.LatestBuild.Transition, "expected workspace to be in state %s but got %s", to, updated.LatestBuild.Transition)
require.Equal(t, to, updated.LatestBuild.Transition, "expected workspace to be in state %s but got %s", to, updated.LatestBuild.Transition)
return updated
}

Expand Down
3 changes: 1 addition & 2 deletions coderd/workspaceapps/apptest/apptest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"golang.org/x/xerrors"

"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/jwtutils"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/workspaceapps"
Expand Down Expand Up @@ -1824,7 +1823,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

_ = coderdtest.MustTransitionWorkspace(t, appDetails.SDKClient, appDetails.Workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
_ = coderdtest.MustTransitionWorkspace(t, appDetails.SDKClient, appDetails.Workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

u := appDetails.PathAppURL(appDetails.Apps.Owner)
resp, err := appDetails.AppClient(t).Request(ctx, http.MethodGet, u.String(), nil)
Expand Down
10 changes: 5 additions & 5 deletions coderd/workspacebuilds_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
// Create a workspace using this template
workspace := coderdtest.CreateWorkspace(t, userClient, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// Create a new version of the template
newVersion := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) {
Expand All @@ -598,7 +598,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
cwbr.TemplateVersionID = newVersion.ID
})
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, build.ID)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// Create the workspace build _again_. We are doing this to
// ensure we do not create _another_ notification. This is
Expand All @@ -610,7 +610,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
cwbr.TemplateVersionID = newVersion.ID
})
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, build.ID)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// We're going to have two notifications (one for the first user and one for the template admin)
// By ensuring we only have these two, we are sure the second build didn't trigger more
Expand Down Expand Up @@ -659,7 +659,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
// Create a workspace using this template
workspace := coderdtest.CreateWorkspace(t, userClient, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// Create a new version of the template
newVersion := coderdtest.CreateTemplateVersion(t, templateAdminClient, first.OrganizationID, nil, func(ctvr *codersdk.CreateTemplateVersionRequest) {
Expand All @@ -675,7 +675,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
})
require.NoError(t, err)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, build.ID)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, userClient, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// Ensure we receive only 1 workspace manually updated notification and to the right user
sent := notify.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceManuallyUpdated))
Expand Down
4 changes: 2 additions & 2 deletions coderd/workspaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3998,7 +3998,7 @@ func TestWorkspaceDormant(t *testing.T) {
require.NoError(t, err)

// Should be able to stop a workspace while it is dormant.
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)

// Should not be able to start a workspace while it is dormant.
_, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
Expand All @@ -4011,7 +4011,7 @@ func TestWorkspaceDormant(t *testing.T) {
Dormant: false,
})
require.NoError(t, err)
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart)
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStop, codersdk.WorkspaceTransitionStart)
})
}

Expand Down
5 changes: 2 additions & 3 deletions enterprise/cli/start_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/coder/coder/v2/cli/clitest"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
Expand Down Expand Up @@ -146,7 +145,7 @@ func TestStart(t *testing.T) {

if cmd == "start" {
// Stop the workspace so that we can start it.
coderdtest.MustTransitionWorkspace(t, c.Client, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, c.Client, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
}
// Start the workspace. Every test permutation should
// pass.
Expand Down Expand Up @@ -198,7 +197,7 @@ func TestStart(t *testing.T) {
memberClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
workspace := coderdtest.CreateWorkspace(t, memberClient, template.ID)
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, memberClient, workspace.LatestBuild.ID)
_ = coderdtest.MustTransitionWorkspace(t, memberClient, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
_ = coderdtest.MustTransitionWorkspace(t, memberClient, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
err := memberClient.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{
Dormant: true,
})
Expand Down
Loading
0