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

spikecurtis · web-flow · commit 5d76210b0d58 · 2024-01-03T13:24:37.000+04:00
Fixes coder#11380
diff --git a/cli/start.go b/cli/start.go
@@ -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)
diff --git a/cli/start_test.go b/cli/start_test.go
@@ -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"
@@ -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)
@@ -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))
@@ -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)
+}
diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go
@@ -5,6 +5,7 @@ import (
 	"database/sql"
 	"encoding/json"
 	"testing"
+	"time"
 
 	"github.com/google/uuid"
 	"github.com/sqlc-dev/pqtype"
@@ -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.
@@ -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.
@@ -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
@@ -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)
