Skip to content

fix: change coder start to be a no-op if workspace is started #11381

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.

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 1 commit into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 26 additions & 11 deletions cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,33 @@ func (r *RootCmd) start() *clibase.Cmd {
if err != nil {
return err
}

build, err := startWorkspace(inv, client, workspace, parameterFlags, WorkspaceStart)
// It's possible for a workspace build to fail due to the template requiring starting
// workspaces with the active version.
if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusForbidden {
_, _ = 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.")
build, err = startWorkspace(inv, client, workspace, parameterFlags, WorkspaceUpdate)
if err != nil {
return xerrors.Errorf("start workspace with active template version: %w", err)
var build codersdk.WorkspaceBuild
switch workspace.LatestBuild.Status {
case codersdk.WorkspaceStatusRunning:
_, _ = fmt.Fprintf(
inv.Stdout, "\nThe %s workspace is already running!\n",
cliui.Keyword(workspace.Name),
)
return nil
case codersdk.WorkspaceStatusStarting:
_, _ = fmt.Fprintf(
inv.Stdout, "\nThe %s workspace is already starting.\n",
cliui.Keyword(workspace.Name),
)
build = workspace.LatestBuild
default:
build, err = startWorkspace(inv, client, workspace, parameterFlags, WorkspaceStart)
// It's possible for a workspace build to fail due to the template requiring starting
// workspaces with the active version.
if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusForbidden {
_, _ = 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.")
build, err = startWorkspace(inv, client, workspace, parameterFlags, WorkspaceUpdate)
if err != nil {
return xerrors.Errorf("start workspace with active template version: %w", err)
}
} else if err != nil {
return err
}
} else if err != nil {
return err
}

err = cliui.WorkspaceBuild(inv.Context(), inv.Stdout, client, build.ID)
Expand Down
65 changes: 65 additions & 0 deletions cli/start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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/database/dbfake"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/provisioner/echo"
"github.com/coder/coder/v2/provisionersdk/proto"
Expand Down Expand Up @@ -109,6 +110,9 @@ func TestStart(t *testing.T) {
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, member, owner.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
// Stop the workspace
workspaceBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspaceBuild.ID)

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

inv, root := clitest.New(t, "start", workspace.Name,
"--build-option", fmt.Sprintf("%s=%s", ephemeralParameterName, ephemeralParameterValue))
Expand Down Expand Up @@ -374,3 +381,61 @@ func TestStartAutoUpdate(t *testing.T) {
})
}
}

func TestStart_AlreadyRunning(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)

client, db := coderdtest.NewWithDatabase(t, nil)
owner := coderdtest.CreateFirstUser(t, client)
memberClient, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
r := dbfake.WorkspaceBuild(t, db, database.Workspace{
OwnerID: member.ID,
OrganizationID: owner.OrganizationID,
}).Do()

inv, root := clitest.New(t, "start", r.Workspace.Name)
clitest.SetupConfig(t, memberClient, root)
doneChan := make(chan struct{})
pty := ptytest.New(t).Attach(inv)
go func() {
defer close(doneChan)
err := inv.Run()
assert.NoError(t, err)
}()

pty.ExpectMatch("workspace is already running")
_ = testutil.RequireRecvCtx(ctx, t, doneChan)
}

func TestStart_Starting(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)

client, db := coderdtest.NewWithDatabase(t, nil)
owner := coderdtest.CreateFirstUser(t, client)
memberClient, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
r := dbfake.WorkspaceBuild(t, db, database.Workspace{
OwnerID: member.ID,
OrganizationID: owner.OrganizationID,
}).
Starting().
Do()

inv, root := clitest.New(t, "start", r.Workspace.Name)
clitest.SetupConfig(t, memberClient, root)
doneChan := make(chan struct{})
pty := ptytest.New(t).Attach(inv)
go func() {
defer close(doneChan)
err := inv.Run()
assert.NoError(t, err)
}()

pty.ExpectMatch("workspace is already starting")

_ = dbfake.JobComplete(t, db, r.Build.JobID).Do()
pty.ExpectMatch("workspace has been started")

_ = testutil.RequireRecvCtx(ctx, t, doneChan)
}
93 changes: 81 additions & 12 deletions coderd/database/dbfake/dbfake.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"database/sql"
"encoding/json"
"testing"
"time"

"github.com/google/uuid"
"github.com/sqlc-dev/pqtype"
Expand Down Expand Up @@ -47,6 +48,11 @@ type WorkspaceBuildBuilder struct {
resources []*sdkproto.Resource
params []database.WorkspaceBuildParameter
agentToken string
dispo workspaceBuildDisposition
}

type workspaceBuildDisposition struct {
starting bool
}

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

func (b WorkspaceBuildBuilder) Starting() WorkspaceBuildBuilder {
//nolint: revive // returns modified struct
b.dispo.starting = true
return b
}

// Do generates all the resources associated with a workspace build.
// Template and TemplateVersion will be optionally populated if no
// TemplateID is set on the provided workspace.
Expand Down Expand Up @@ -166,20 +178,43 @@ func (b WorkspaceBuildBuilder) Do() WorkspaceResponse {
})
require.NoError(b.t, err, "insert job")

err = b.db.UpdateProvisionerJobWithCompleteByID(ownerCtx, database.UpdateProvisionerJobWithCompleteByIDParams{
ID: job.ID,
UpdatedAt: dbtime.Now(),
Error: sql.NullString{},
ErrorCode: sql.NullString{},
CompletedAt: sql.NullTime{
Time: dbtime.Now(),
Valid: true,
},
})
require.NoError(b.t, err, "complete job")
if b.dispo.starting {
// might need to do this multiple times if we got a template version
// import job as well
for {
j, err := b.db.AcquireProvisionerJob(ownerCtx, database.AcquireProvisionerJobParams{
StartedAt: sql.NullTime{
Time: dbtime.Now(),
Valid: true,
},
WorkerID: uuid.NullUUID{
UUID: uuid.New(),
Valid: true,
},
Types: []database.ProvisionerType{database.ProvisionerTypeEcho},
Tags: nil,
})
require.NoError(b.t, err, "acquire starting job")
if j.ID == job.ID {
break
}
}
} else {
err = b.db.UpdateProvisionerJobWithCompleteByID(ownerCtx, database.UpdateProvisionerJobWithCompleteByIDParams{
ID: job.ID,
UpdatedAt: dbtime.Now(),
Error: sql.NullString{},
ErrorCode: sql.NullString{},
CompletedAt: sql.NullTime{
Time: dbtime.Now(),
Valid: true,
},
})
require.NoError(b.t, err, "complete job")
ProvisionerJobResources(b.t, b.db, job.ID, b.seed.Transition, b.resources...).Do()
}

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

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

type JobCompleteBuilder struct {
t testing.TB
db database.Store
jobID uuid.UUID
}

type JobCompleteResponse struct {
CompletedAt time.Time
}

func JobComplete(t testing.TB, db database.Store, jobID uuid.UUID) JobCompleteBuilder {
return JobCompleteBuilder{
t: t,
db: db,
jobID: jobID,
}
}

func (b JobCompleteBuilder) Do() JobCompleteResponse {
r := JobCompleteResponse{CompletedAt: dbtime.Now()}
err := b.db.UpdateProvisionerJobWithCompleteByID(ownerCtx, database.UpdateProvisionerJobWithCompleteByIDParams{
ID: b.jobID,
UpdatedAt: r.CompletedAt,
Error: sql.NullString{},
ErrorCode: sql.NullString{},
CompletedAt: sql.NullTime{
Time: r.CompletedAt,
Valid: true,
},
})
require.NoError(b.t, err, "complete job")
return r
}

func must[V any](v V, err error) V {
if err != nil {
panic(err)
Expand Down