Skip to content

Commit 377187f

Browse files
spikecurtisspikecurtis
committed
fix: change coder start to be a no-op if workspace is started
1 parent 9031b49 commit 377187f

File tree

3 files changed

+173
-23
lines changed

3 files changed

+173
-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

+66
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55
"testing"
66

7+
"github.com/coder/coder/v2/coderd/database/dbfake"
8+
79
"github.com/stretchr/testify/assert"
810
"github.com/stretchr/testify/require"
911
"golang.org/x/net/context"
@@ -109,6 +111,9 @@ func TestStart(t *testing.T) {
109111
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
110112
workspace := coderdtest.CreateWorkspace(t, member, owner.OrganizationID, template.ID)
111113
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
114+
// Stop the workspace
115+
workspaceBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
116+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspaceBuild.ID)
112117

113118
inv, root := clitest.New(t, "start", workspace.Name, "--build-options")
114119
clitest.SetupConfig(t, member, root)
@@ -160,6 +165,9 @@ func TestStart(t *testing.T) {
160165
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
161166
workspace := coderdtest.CreateWorkspace(t, member, owner.OrganizationID, template.ID)
162167
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
168+
// Stop the workspace
169+
workspaceBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
170+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspaceBuild.ID)
163171

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

‎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)