Skip to content

Commit 5d76210

Browse files
spikecurtisspikecurtis
authored
fix: change coder start to be a no-op if workspace is started
Fixes #11380
1 parent 1ef9602 commit 5d76210

File tree

3 files changed

+172
-23
lines changed

3 files changed

+172
-23
lines changed

‎cli/start.go

+26-11
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,33 @@ func (r *RootCmd) start() *clibase.Cmd {
3030
if err != nil {
3131
return err
3232
}
33-
34-
build, err := startWorkspace(inv, client, workspace, parameterFlags, WorkspaceStart)
35-
// It's possible for a workspace build to fail due to the template requiring starting
36-
// workspaces with the active version.
37-
if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusForbidden {
38-
_, _ = fmt.Fprintln(inv.Stdout, "Failed to restart with the template version from your last build. Policy may require you to restart with the current active template version.")
39-
build, err = startWorkspace(inv, client, workspace, parameterFlags, WorkspaceUpdate)
40-
if err != nil {
41-
return xerrors.Errorf("start workspace with active template version: %w", err)
33+
var build codersdk.WorkspaceBuild
34+
switch workspace.LatestBuild.Status {
35+
case codersdk.WorkspaceStatusRunning:
36+
_, _ = fmt.Fprintf(
37+
inv.Stdout, "\nThe %s workspace is already running!\n",
38+
cliui.Keyword(workspace.Name),
39+
)
40+
return nil
41+
case codersdk.WorkspaceStatusStarting:
42+
_, _ = fmt.Fprintf(
43+
inv.Stdout, "\nThe %s workspace is already starting.\n",
44+
cliui.Keyword(workspace.Name),
45+
)
46+
build = workspace.LatestBuild
47+
default:
48+
build, err = startWorkspace(inv, client, workspace, parameterFlags, WorkspaceStart)
49+
// It's possible for a workspace build to fail due to the template requiring starting
50+
// workspaces with the active version.
51+
if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusForbidden {
52+
_, _ = fmt.Fprintln(inv.Stdout, "Failed to restart with the template version from your last build. Policy may require you to restart with the current active template version.")
53+
build, err = startWorkspace(inv, client, workspace, parameterFlags, WorkspaceUpdate)
54+
if err != nil {
55+
return xerrors.Errorf("start workspace with active template version: %w", err)
56+
}
57+
} else if err != nil {
58+
return err
4259
}
43-
} else if err != nil {
44-
return err
4560
}
4661

4762
err = cliui.WorkspaceBuild(inv.Context(), inv.Stdout, client, build.ID)

‎cli/start_test.go

+65
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/coder/coder/v2/cli/clitest"
1212
"github.com/coder/coder/v2/coderd/coderdtest"
1313
"github.com/coder/coder/v2/coderd/database"
14+
"github.com/coder/coder/v2/coderd/database/dbfake"
1415
"github.com/coder/coder/v2/codersdk"
1516
"github.com/coder/coder/v2/provisioner/echo"
1617
"github.com/coder/coder/v2/provisionersdk/proto"
@@ -109,6 +110,9 @@ func TestStart(t *testing.T) {
109110
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
110111
workspace := coderdtest.CreateWorkspace(t, member, owner.OrganizationID, template.ID)
111112
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
113+
// Stop the workspace
114+
workspaceBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
115+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspaceBuild.ID)
112116

113117
inv, root := clitest.New(t, "start", workspace.Name, "--build-options")
114118
clitest.SetupConfig(t, member, root)
@@ -160,6 +164,9 @@ func TestStart(t *testing.T) {
160164
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
161165
workspace := coderdtest.CreateWorkspace(t, member, owner.OrganizationID, template.ID)
162166
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
167+
// Stop the workspace
168+
workspaceBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
169+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspaceBuild.ID)
163170

164171
inv, root := clitest.New(t, "start", workspace.Name,
165172
"--build-option", fmt.Sprintf("%s=%s", ephemeralParameterName, ephemeralParameterValue))
@@ -374,3 +381,61 @@ func TestStartAutoUpdate(t *testing.T) {
374381
})
375382
}
376383
}
384+
385+
func TestStart_AlreadyRunning(t *testing.T) {
386+
t.Parallel()
387+
ctx := testutil.Context(t, testutil.WaitShort)
388+
389+
client, db := coderdtest.NewWithDatabase(t, nil)
390+
owner := coderdtest.CreateFirstUser(t, client)
391+
memberClient, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
392+
r := dbfake.WorkspaceBuild(t, db, database.Workspace{
393+
OwnerID: member.ID,
394+
OrganizationID: owner.OrganizationID,
395+
}).Do()
396+
397+
inv, root := clitest.New(t, "start", r.Workspace.Name)
398+
clitest.SetupConfig(t, memberClient, root)
399+
doneChan := make(chan struct{})
400+
pty := ptytest.New(t).Attach(inv)
401+
go func() {
402+
defer close(doneChan)
403+
err := inv.Run()
404+
assert.NoError(t, err)
405+
}()
406+
407+
pty.ExpectMatch("workspace is already running")
408+
_ = testutil.RequireRecvCtx(ctx, t, doneChan)
409+
}
410+
411+
func TestStart_Starting(t *testing.T) {
412+
t.Parallel()
413+
ctx := testutil.Context(t, testutil.WaitShort)
414+
415+
client, db := coderdtest.NewWithDatabase(t, nil)
416+
owner := coderdtest.CreateFirstUser(t, client)
417+
memberClient, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
418+
r := dbfake.WorkspaceBuild(t, db, database.Workspace{
419+
OwnerID: member.ID,
420+
OrganizationID: owner.OrganizationID,
421+
}).
422+
Starting().
423+
Do()
424+
425+
inv, root := clitest.New(t, "start", r.Workspace.Name)
426+
clitest.SetupConfig(t, memberClient, root)
427+
doneChan := make(chan struct{})
428+
pty := ptytest.New(t).Attach(inv)
429+
go func() {
430+
defer close(doneChan)
431+
err := inv.Run()
432+
assert.NoError(t, err)
433+
}()
434+
435+
pty.ExpectMatch("workspace is already starting")
436+
437+
_ = dbfake.JobComplete(t, db, r.Build.JobID).Do()
438+
pty.ExpectMatch("workspace has been started")
439+
440+
_ = testutil.RequireRecvCtx(ctx, t, doneChan)
441+
}

‎coderd/database/dbfake/dbfake.go

+81-12
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"database/sql"
66
"encoding/json"
77
"testing"
8+
"time"
89

910
"github.com/google/uuid"
1011
"github.com/sqlc-dev/pqtype"
@@ -47,6 +48,11 @@ type WorkspaceBuildBuilder struct {
4748
resources []*sdkproto.Resource
4849
params []database.WorkspaceBuildParameter
4950
agentToken string
51+
dispo workspaceBuildDisposition
52+
}
53+
54+
type workspaceBuildDisposition struct {
55+
starting bool
5056
}
5157

5258
// WorkspaceBuild generates a workspace build for the provided workspace.
@@ -100,6 +106,12 @@ func (b WorkspaceBuildBuilder) WithAgent(mutations ...func([]*sdkproto.Agent) []
100106
return b
101107
}
102108

109+
func (b WorkspaceBuildBuilder) Starting() WorkspaceBuildBuilder {
110+
//nolint: revive // returns modified struct
111+
b.dispo.starting = true
112+
return b
113+
}
114+
103115
// Do generates all the resources associated with a workspace build.
104116
// Template and TemplateVersion will be optionally populated if no
105117
// TemplateID is set on the provided workspace.
@@ -166,20 +178,43 @@ func (b WorkspaceBuildBuilder) Do() WorkspaceResponse {
166178
})
167179
require.NoError(b.t, err, "insert job")
168180

169-
err = b.db.UpdateProvisionerJobWithCompleteByID(ownerCtx, database.UpdateProvisionerJobWithCompleteByIDParams{
170-
ID: job.ID,
171-
UpdatedAt: dbtime.Now(),
172-
Error: sql.NullString{},
173-
ErrorCode: sql.NullString{},
174-
CompletedAt: sql.NullTime{
175-
Time: dbtime.Now(),
176-
Valid: true,
177-
},
178-
})
179-
require.NoError(b.t, err, "complete job")
181+
if b.dispo.starting {
182+
// might need to do this multiple times if we got a template version
183+
// import job as well
184+
for {
185+
j, err := b.db.AcquireProvisionerJob(ownerCtx, database.AcquireProvisionerJobParams{
186+
StartedAt: sql.NullTime{
187+
Time: dbtime.Now(),
188+
Valid: true,
189+
},
190+
WorkerID: uuid.NullUUID{
191+
UUID: uuid.New(),
192+
Valid: true,
193+
},
194+
Types: []database.ProvisionerType{database.ProvisionerTypeEcho},
195+
Tags: nil,
196+
})
197+
require.NoError(b.t, err, "acquire starting job")
198+
if j.ID == job.ID {
199+
break
200+
}
201+
}
202+
} else {
203+
err = b.db.UpdateProvisionerJobWithCompleteByID(ownerCtx, database.UpdateProvisionerJobWithCompleteByIDParams{
204+
ID: job.ID,
205+
UpdatedAt: dbtime.Now(),
206+
Error: sql.NullString{},
207+
ErrorCode: sql.NullString{},
208+
CompletedAt: sql.NullTime{
209+
Time: dbtime.Now(),
210+
Valid: true,
211+
},
212+
})
213+
require.NoError(b.t, err, "complete job")
214+
ProvisionerJobResources(b.t, b.db, job.ID, b.seed.Transition, b.resources...).Do()
215+
}
180216

181217
resp.Build = dbgen.WorkspaceBuild(b.t, b.db, b.seed)
182-
ProvisionerJobResources(b.t, b.db, job.ID, b.seed.Transition, b.resources...).Do()
183218

184219
for i := range b.params {
185220
b.params[i].WorkspaceBuildID = resp.Build.ID
@@ -340,6 +375,40 @@ func (t TemplateVersionBuilder) Do() TemplateVersionResponse {
340375
return resp
341376
}
342377

378+
type JobCompleteBuilder struct {
379+
t testing.TB
380+
db database.Store
381+
jobID uuid.UUID
382+
}
383+
384+
type JobCompleteResponse struct {
385+
CompletedAt time.Time
386+
}
387+
388+
func JobComplete(t testing.TB, db database.Store, jobID uuid.UUID) JobCompleteBuilder {
389+
return JobCompleteBuilder{
390+
t: t,
391+
db: db,
392+
jobID: jobID,
393+
}
394+
}
395+
396+
func (b JobCompleteBuilder) Do() JobCompleteResponse {
397+
r := JobCompleteResponse{CompletedAt: dbtime.Now()}
398+
err := b.db.UpdateProvisionerJobWithCompleteByID(ownerCtx, database.UpdateProvisionerJobWithCompleteByIDParams{
399+
ID: b.jobID,
400+
UpdatedAt: r.CompletedAt,
401+
Error: sql.NullString{},
402+
ErrorCode: sql.NullString{},
403+
CompletedAt: sql.NullTime{
404+
Time: r.CompletedAt,
405+
Valid: true,
406+
},
407+
})
408+
require.NoError(b.t, err, "complete job")
409+
return r
410+
}
411+
343412
func must[V any](v V, err error) V {
344413
if err != nil {
345414
panic(err)

0 commit comments

Comments
 (0)