Skip to content

Commit 49fcffc

Browse files
johnstcnjohnstcn
authored
fix!: stop workspace before update (#18425)
Fixes #17840 NOTE: calling this out as a breaking change so that it is highly visible in the changelog. * CLI: Modifies `coder update` to stop the workspace if already running. * UI: Modifies "update" button to always stop the workspace if already running.
1 parent 725bc37 commit 49fcffc

24 files changed

Lines changed: 426 additions & 232 deletions

File tree

‎cli/start_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ func TestStartAutoUpdate(t *testing.T) {
358358
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
359359

360360
if c.Cmd == "start" {
361-
coderdtest.MustTransitionWorkspace(t, member, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
361+
coderdtest.MustTransitionWorkspace(t, member, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
362362
}
363363
version2 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, prepareEchoResponses(stringRichParameters), func(ctvr *codersdk.CreateTemplateVersionRequest) {
364364
ctvr.TemplateID = template.ID

‎cli/stop.go‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -37,32 +37,11 @@ func (r *RootCmd) stop() *serpent.Command {
3737
if err != nil {
3838
return err
3939
}
40-
if workspace.LatestBuild.Job.Status == codersdk.ProvisionerJobPending {
41-
// cliutil.WarnMatchedProvisioners also checks if the job is pending
42-
// but we still want to avoid users spamming multiple builds that will
43-
// not be picked up.
44-
cliui.Warn(inv.Stderr, "The workspace is already stopping!")
45-
cliutil.WarnMatchedProvisioners(inv.Stderr, workspace.LatestBuild.MatchedProvisioners, workspace.LatestBuild.Job)
46-
if _, err := cliui.Prompt(inv, cliui.PromptOptions{
47-
Text: "Enqueue another stop?",
48-
IsConfirm: true,
49-
Default: cliui.ConfirmNo,
50-
}); err != nil {
51-
return err
52-
}
53-
}
5440

55-
wbr := codersdk.CreateWorkspaceBuildRequest{
56-
Transition: codersdk.WorkspaceTransitionStop,
57-
}
58-
if bflags.provisionerLogDebug {
59-
wbr.LogLevel = codersdk.ProvisionerLogLevelDebug
60-
}
61-
build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, wbr)
41+
build, err := stopWorkspace(inv, client, workspace, bflags)
6242
if err != nil {
6343
return err
6444
}
65-
cliutil.WarnMatchedProvisioners(inv.Stderr, build.MatchedProvisioners, build.Job)
6645

6746
err = cliui.WorkspaceBuild(inv.Context(), inv.Stdout, client, build.ID)
6847
if err != nil {
@@ -71,8 +50,8 @@ func (r *RootCmd) stop() *serpent.Command {
7150

7251
_, _ = fmt.Fprintf(
7352
inv.Stdout,
74-
"\nThe %s workspace has been stopped at %s!\n", cliui.Keyword(workspace.Name),
75-
53+
"\nThe %s workspace has been stopped at %s!\n",
54+
cliui.Keyword(workspace.Name),
7655
cliui.Timestamp(time.Now()),
7756
)
7857
return nil
@@ -82,3 +61,27 @@ func (r *RootCmd) stop() *serpent.Command {
8261

8362
return cmd
8463
}
64+
65+
func stopWorkspace(inv *serpent.Invocation, client *codersdk.Client, workspace codersdk.Workspace, bflags buildFlags) (codersdk.WorkspaceBuild, error) {
66+
if workspace.LatestBuild.Job.Status == codersdk.ProvisionerJobPending {
67+
// cliutil.WarnMatchedProvisioners also checks if the job is pending
68+
// but we still want to avoid users spamming multiple builds that will
69+
// not be picked up.
70+
cliui.Warn(inv.Stderr, "The workspace is already stopping!")
71+
cliutil.WarnMatchedProvisioners(inv.Stderr, workspace.LatestBuild.MatchedProvisioners, workspace.LatestBuild.Job)
72+
if _, err := cliui.Prompt(inv, cliui.PromptOptions{
73+
Text: "Enqueue another stop?",
74+
IsConfirm: true,
75+
Default: cliui.ConfirmNo,
76+
}); err != nil {
77+
return codersdk.WorkspaceBuild{}, err
78+
}
79+
}
80+
wbr := codersdk.CreateWorkspaceBuildRequest{
81+
Transition: codersdk.WorkspaceTransitionStop,
82+
}
83+
if bflags.provisionerLogDebug {
84+
wbr.LogLevel = codersdk.ProvisionerLogLevelDebug
85+
}
86+
return client.CreateWorkspaceBuild(inv.Context(), workspace.ID, wbr)
87+
}

‎cli/testdata/coder_--help.golden‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ SUBCOMMANDS:
5757
tokens Manage personal access tokens
5858
unfavorite Remove a workspace from your favorites
5959
update Will update and start a given workspace if it is out of
60-
date
60+
date. If the workspace is already running, it will be
61+
stopped first.
6162
users Manage users
6263
version Show coder version
6364
whoami Fetch authenticated user info for Coder deployment

‎cli/testdata/coder_update_--help.golden‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ coder v0.0.0-devel
33
USAGE:
44
coder update [flags] <workspace>
55

6-
Will update and start a given workspace if it is out of date
6+
Will update and start a given workspace if it is out of date. If the workspace
7+
is already running, it will be stopped first.
78

89
Use --always-prompt to change the parameter values of the workspace.
910

‎cli/update.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"golang.org/x/xerrors"
77

8+
"github.com/coder/coder/v2/cli/cliui"
89
"github.com/coder/coder/v2/codersdk"
910
"github.com/coder/serpent"
1011
)
@@ -18,7 +19,7 @@ func (r *RootCmd) update() *serpent.Command {
1819
cmd := &serpent.Command{
1920
Annotations: workspaceCommand,
2021
Use: "update <workspace>",
21-
Short: "Will update and start a given workspace if it is out of date",
22+
Short: "Will update and start a given workspace if it is out of date. If the workspace is already running, it will be stopped first.",
2223
Long: "Use --always-prompt to change the parameter values of the workspace.",
2324
Middleware: serpent.Chain(
2425
serpent.RequireNArgs(1),
@@ -34,6 +35,20 @@ func (r *RootCmd) update() *serpent.Command {
3435
return nil
3536
}
3637

38+
// #17840: If the workspace is already running, we will stop it before
39+
// updating. Simply performing a new start transition may not work if the
40+
// template specifies ignore_changes.
41+
if workspace.LatestBuild.Transition == codersdk.WorkspaceTransitionStart {
42+
build, err := stopWorkspace(inv, client, workspace, bflags)
43+
if err != nil {
44+
return xerrors.Errorf("stop workspace: %w", err)
45+
}
46+
// Wait for the stop to complete.
47+
if err := cliui.WorkspaceBuild(inv.Context(), inv.Stdout, client, build.ID); err != nil {
48+
return xerrors.Errorf("wait for stop: %w", err)
49+
}
50+
}
51+
3752
build, err := startWorkspace(inv, client, workspace, parameterFlags, bflags, WorkspaceUpdate)
3853
if err != nil {
3954
return xerrors.Errorf("start workspace: %w", err)

‎cli/update_test.go‎

Lines changed: 95 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,49 +34,125 @@ func TestUpdate(t *testing.T) {
3434
t.Run("OK", func(t *testing.T) {
3535
t.Parallel()
3636

37+
// Given: a workspace exists on the latest template version.
3738
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
3839
owner := coderdtest.CreateFirstUser(t, client)
39-
member, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
40+
member, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
4041
version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
4142

4243
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID)
4344
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID)
4445

45-
inv, root := clitest.New(t, "create",
46-
"my-workspace",
47-
"--template", template.Name,
48-
"-y",
49-
)
46+
ws := coderdtest.CreateWorkspace(t, member, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
47+
cwr.Name = "my-workspace"
48+
})
49+
require.False(t, ws.Outdated, "newly created workspace with active template version must not be outdated")
50+
51+
// Given: the template version is updated
52+
version2 := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{
53+
Parse: echo.ParseComplete,
54+
ProvisionApply: echo.ApplyComplete,
55+
ProvisionPlan: echo.PlanComplete,
56+
}, template.ID)
57+
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version2.ID)
58+
59+
ctx := testutil.Context(t, testutil.WaitShort)
60+
err := client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
61+
ID: version2.ID,
62+
})
63+
require.NoError(t, err, "failed to update active template version")
64+
65+
// Then: the workspace is marked as 'outdated'
66+
ws, err = member.WorkspaceByOwnerAndName(ctx, codersdk.Me, "my-workspace", codersdk.WorkspaceOptions{})
67+
require.NoError(t, err, "member failed to get workspace they themselves own")
68+
require.True(t, ws.Outdated, "workspace must be outdated after template version update")
69+
70+
// When: the workspace is updated
71+
inv, root := clitest.New(t, "update", ws.Name)
5072
clitest.SetupConfig(t, member, root)
5173

52-
err := inv.Run()
53-
require.NoError(t, err)
74+
err = inv.Run()
75+
require.NoError(t, err, "update command failed")
76+
77+
// Then: the workspace is no longer 'outdated'
78+
ws, err = member.WorkspaceByOwnerAndName(ctx, codersdk.Me, "my-workspace", codersdk.WorkspaceOptions{})
79+
require.NoError(t, err, "member failed to get workspace they themselves own after update")
80+
require.Equal(t, version2.ID.String(), ws.LatestBuild.TemplateVersionID.String(), "workspace must have latest template version after update")
81+
require.False(t, ws.Outdated, "workspace must not be outdated after update")
82+
83+
// Then: the workspace must have been started with the new template version
84+
require.Equal(t, int32(3), ws.LatestBuild.BuildNumber, "workspace must have 3 builds after update")
85+
require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "latest build must be a start transition")
86+
87+
// Then: the previous workspace build must be a stop transition with the old
88+
// template version.
89+
// This is important to ensure that the workspace resources are recreated
90+
// correctly. Simply running a start transition with the new template
91+
// version may not recreate resources that were changed in the new
92+
// template version. This can happen, for example, if a user specifies
93+
// ignore_changes in the template.
94+
prevBuild, err := member.WorkspaceBuildByUsernameAndWorkspaceNameAndBuildNumber(ctx, codersdk.Me, ws.Name, "2")
95+
require.NoError(t, err, "failed to get previous workspace build")
96+
require.Equal(t, codersdk.WorkspaceTransitionStop, prevBuild.Transition, "previous build must be a stop transition")
97+
require.Equal(t, version1.ID.String(), prevBuild.TemplateVersionID.String(), "previous build must have the old template version")
98+
})
5499

55-
ws, err := client.WorkspaceByOwnerAndName(context.Background(), memberUser.Username, "my-workspace", codersdk.WorkspaceOptions{})
56-
require.NoError(t, err)
57-
require.Equal(t, version1.ID.String(), ws.LatestBuild.TemplateVersionID.String())
100+
t.Run("Stopped", func(t *testing.T) {
101+
t.Parallel()
102+
103+
// Given: a workspace exists on the latest template version.
104+
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
105+
owner := coderdtest.CreateFirstUser(t, client)
106+
member, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
107+
version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
108+
109+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID)
110+
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID)
58111

112+
ws := coderdtest.CreateWorkspace(t, member, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
113+
cwr.Name = "my-workspace"
114+
})
115+
require.False(t, ws.Outdated, "newly created workspace with active template version must not be outdated")
116+
117+
// Given: the template version is updated
59118
version2 := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{
60119
Parse: echo.ParseComplete,
61120
ProvisionApply: echo.ApplyComplete,
62121
ProvisionPlan: echo.PlanComplete,
63122
}, template.ID)
64123
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version2.ID)
65124

66-
err = client.UpdateActiveTemplateVersion(context.Background(), template.ID, codersdk.UpdateActiveTemplateVersion{
125+
ctx := testutil.Context(t, testutil.WaitShort)
126+
err := client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
67127
ID: version2.ID,
68128
})
69-
require.NoError(t, err)
129+
require.NoError(t, err, "failed to update active template version")
130+
131+
// Given: the workspace is in a stopped state.
132+
coderdtest.MustTransitionWorkspace(t, member, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
70133

71-
inv, root = clitest.New(t, "update", ws.Name)
134+
// Then: the workspace is marked as 'outdated'
135+
ws, err = member.WorkspaceByOwnerAndName(ctx, codersdk.Me, "my-workspace", codersdk.WorkspaceOptions{})
136+
require.NoError(t, err, "member failed to get workspace they themselves own")
137+
require.True(t, ws.Outdated, "workspace must be outdated after template version update")
138+
139+
// When: the workspace is updated
140+
inv, root := clitest.New(t, "update", ws.Name)
72141
clitest.SetupConfig(t, member, root)
73142

74143
err = inv.Run()
75-
require.NoError(t, err)
76-
77-
ws, err = member.WorkspaceByOwnerAndName(context.Background(), memberUser.Username, "my-workspace", codersdk.WorkspaceOptions{})
78-
require.NoError(t, err)
79-
require.Equal(t, version2.ID.String(), ws.LatestBuild.TemplateVersionID.String())
144+
require.NoError(t, err, "update command failed")
145+
146+
// Then: the workspace is no longer 'outdated'
147+
ws, err = member.WorkspaceByOwnerAndName(ctx, codersdk.Me, "my-workspace", codersdk.WorkspaceOptions{})
148+
require.NoError(t, err, "member failed to get workspace they themselves own after update")
149+
require.Equal(t, version2.ID.String(), ws.LatestBuild.TemplateVersionID.String(), "workspace must have latest template version after update")
150+
require.False(t, ws.Outdated, "workspace must not be outdated after update")
151+
152+
// Then: the workspace must have been started with the new template version
153+
require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "latest build must be a start transition")
154+
// Then: we expect 3 builds, as we manually stopped the workspace.
155+
require.Equal(t, int32(3), ws.LatestBuild.BuildNumber, "workspace must have 3 builds after update")
80156
})
81157
}
82158

‎coderd/autobuild/lifecycle_executor_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestExecutorAutostartOK(t *testing.T) {
4747
})
4848
)
4949
// Given: workspace is stopped
50-
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
50+
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
5151

5252
// When: the autobuild executor ticks after the scheduled time
5353
go func() {
@@ -105,7 +105,7 @@ func TestMultipleLifecycleExecutors(t *testing.T) {
105105
)
106106

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

110110
// Get both clients to perform a lifecycle execution tick
111111
next := sched.Next(workspace.LatestBuild.CreatedAt)
@@ -203,7 +203,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
203203
)
204204
// Given: workspace is stopped
205205
workspace = coderdtest.MustTransitionWorkspace(
206-
t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
206+
t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
207207

208208
orgs, err := client.OrganizationsByUser(ctx, workspace.OwnerID.String())
209209
require.NoError(t, err)
@@ -344,7 +344,7 @@ func TestExecutorAutostartNotEnabled(t *testing.T) {
344344
require.Empty(t, workspace.AutostartSchedule)
345345

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

349349
// When: the autobuild executor ticks way into the future
350350
go func() {
@@ -384,7 +384,7 @@ func TestExecutorAutostartUserSuspended(t *testing.T) {
384384
workspace = coderdtest.MustWorkspace(t, userClient, workspace.ID)
385385

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

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

@@ -507,7 +507,7 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) {
507507
)
508508

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

512512
// When: the autobuild executor ticks past the TTL
513513
go func() {
@@ -578,7 +578,7 @@ func TestExecutorWorkspaceDeleted(t *testing.T) {
578578
)
579579

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

583583
// When: the autobuild executor ticks
584584
go func() {
@@ -767,7 +767,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) {
767767
})
768768
)
769769
// Given: workspace is stopped
770-
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
770+
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
771771

772772
// When: the autobuild executor ticks past the scheduled time
773773
go func() {
@@ -832,7 +832,7 @@ func TestExecutorAutostartWithParameters(t *testing.T) {
832832
})
833833
)
834834
// Given: workspace is stopped
835-
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
835+
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
836836

837837
// When: the autobuild executor ticks after the scheduled time
838838
go func() {
@@ -882,7 +882,7 @@ func TestExecutorAutostartTemplateDisabled(t *testing.T) {
882882
})
883883
)
884884
// Given: workspace is stopped
885-
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
885+
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
886886

887887
// When: the autobuild executor ticks before the next scheduled time
888888
go func() {
@@ -1001,7 +1001,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) {
10011001
cwr.AutostartSchedule = ptr.Ref(sched.String())
10021002
})
10031003
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, ownerClient, ws.LatestBuild.ID)
1004-
ws = coderdtest.MustTransitionWorkspace(t, memberClient, ws.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop, func(req *codersdk.CreateWorkspaceBuildRequest) {
1004+
ws = coderdtest.MustTransitionWorkspace(t, memberClient, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop, func(req *codersdk.CreateWorkspaceBuildRequest) {
10051005
req.TemplateVersionID = inactiveVersion.ID
10061006
})
10071007
require.Equal(t, inactiveVersion.ID, ws.LatestBuild.TemplateVersionID)
@@ -1159,7 +1159,7 @@ func TestNotifications(t *testing.T) {
11591159
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)
11601160

11611161
// Stop workspace
1162-
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
1162+
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
11631163
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)
11641164

11651165
// Wait for workspace to become dormant

0 commit comments

Comments
 (0)