diff --git a/cli/ssh.go b/cli/ssh.go index 7df590946fd6b..7a1d5940bfd01 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -657,12 +657,19 @@ func getWorkspaceAndAgent(ctx context.Context, inv *serpent.Invocation, client * // workspaces with the active version. _, _ = fmt.Fprintf(inv.Stderr, "Workspace was stopped, starting workspace to allow connecting to %q...\n", workspace.Name) _, err = startWorkspace(inv, client, workspace, workspaceParameterFlags{}, buildFlags{}, WorkspaceStart) - if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusForbidden { - _, err = startWorkspace(inv, client, workspace, workspaceParameterFlags{}, buildFlags{}, WorkspaceUpdate) - if err != nil { - return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, xerrors.Errorf("start workspace with active template version: %w", err) + if cerr, ok := codersdk.AsError(err); ok { + switch cerr.StatusCode() { + case http.StatusConflict: + _, _ = fmt.Fprintln(inv.Stderr, "Unable to start the workspace due to conflict, the workspace may be starting, retrying without autostart...") + return getWorkspaceAndAgent(ctx, inv, client, false, input) + + case http.StatusForbidden: + _, err = startWorkspace(inv, client, workspace, workspaceParameterFlags{}, buildFlags{}, WorkspaceUpdate) + if err != nil { + return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, xerrors.Errorf("start workspace with active template version: %w", err) + } + _, _ = fmt.Fprintln(inv.Stdout, "Unable to start the workspace with template version from last build. Your workspace has been updated to the current active template version.") } - _, _ = fmt.Fprintln(inv.Stdout, "Unable to start the workspace with template version from last build. Your workspace has been updated to the current active template version.") } else if err != nil { return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, xerrors.Errorf("start workspace with current template version: %w", err) } diff --git a/cli/ssh_test.go b/cli/ssh_test.go index 62feaf2b61e95..bd107852251f7 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -17,6 +17,7 @@ import ( "os/exec" "path" "path/filepath" + "regexp" "runtime" "strings" "testing" @@ -145,6 +146,87 @@ func TestSSH(t *testing.T) { pty.WriteLine("exit") <-cmdDone }) + t.Run("StartStoppedWorkspaceConflict", func(t *testing.T) { + t.Parallel() + + // Intercept builds to synchronize execution of the SSH command. + // The purpose here is to make sure all commands try to trigger + // a start build of the workspace. + isFirstBuild := true + buildURL := regexp.MustCompile("/api/v2/workspaces/.*/builds") + buildReq := make(chan struct{}) + buildResume := make(chan struct{}) + buildSyncMW := func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && buildURL.MatchString(r.URL.Path) { + if !isFirstBuild { + t.Log("buildSyncMW: blocking build") + buildReq <- struct{}{} + <-buildResume + t.Log("buildSyncMW: resuming build") + } else { + isFirstBuild = false + } + } + next.ServeHTTP(w, r) + }) + } + + authToken := uuid.NewString() + ownerClient := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + APIMiddleware: buildSyncMW, + }) + owner := coderdtest.CreateFirstUser(t, ownerClient) + client, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.PlanComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, 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) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) + defer cancel() + + var ptys []*ptytest.PTY + for i := 0; i < 3; i++ { + // SSH to the workspace which should autostart it + inv, root := clitest.New(t, "ssh", workspace.Name) + + pty := ptytest.New(t).Attach(inv) + ptys = append(ptys, pty) + clitest.SetupConfig(t, client, root) + testutil.Go(t, func() { + _ = inv.WithContext(ctx).Run() + }) + } + + for _, pty := range ptys { + pty.ExpectMatchContext(ctx, "Workspace was stopped, starting workspace to allow connecting to") + } + for range ptys { + testutil.RequireRecvCtx(ctx, t, buildReq) + } + close(buildResume) + + var foundConflict int + for _, pty := range ptys { + // Either allow the command to start the workspace or fail + // due to conflict (race), in which case it retries. + match := pty.ExpectRegexMatchContext(ctx, "Waiting for the workspace agent to connect") + if strings.Contains(match, "Unable to start the workspace due to conflict, the workspace may be starting, retrying without autostart...") { + foundConflict++ + } + } + require.Equal(t, 2, foundConflict, "expected 2 conflicts") + }) t.Run("RequireActiveVersion", func(t *testing.T) { t.Parallel() diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index bd1ed740a7ce7..aa096707b8fb7 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -33,6 +33,7 @@ import ( "cloud.google.com/go/compute/metadata" "github.com/fullsailor/pkcs7" + "github.com/go-chi/chi/v5" "github.com/golang-jwt/jwt/v4" "github.com/google/uuid" "github.com/moby/moby/pkg/namesgenerator" @@ -146,6 +147,11 @@ type Options struct { Database database.Store Pubsub pubsub.Pubsub + // APIMiddleware inserts middleware before api.RootHandler, this can be + // useful in certain tests where you want to intercept requests before + // passing them on to the API, e.g. for synchronization of execution. + APIMiddleware func(http.Handler) http.Handler + ConfigSSH codersdk.SSHConfigResponse SwaggerEndpoint bool @@ -555,7 +561,14 @@ func NewWithAPI(t testing.TB, options *Options) (*codersdk.Client, io.Closer, *c setHandler, cancelFunc, serverURL, newOptions := NewOptions(t, options) // We set the handler after server creation for the access URL. coderAPI := coderd.New(newOptions) - setHandler(coderAPI.RootHandler) + rootHandler := coderAPI.RootHandler + if options.APIMiddleware != nil { + r := chi.NewRouter() + r.Use(options.APIMiddleware) + r.Mount("/", rootHandler) + rootHandler = r + } + setHandler(rootHandler) var provisionerCloser io.Closer = nopcloser{} if options.IncludeProvisionerDaemon { provisionerCloser = NewTaggedProvisionerDaemon(t, coderAPI, "test", options.ProvisionerDaemonTags) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 2123322356a3c..3d757f4c5590b 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -381,6 +381,10 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object code := http.StatusInternalServerError if rbac.IsUnauthorizedError(err) { code = http.StatusForbidden + } else if database.IsUniqueViolation(err) { + // Concurrent builds may result in duplicate + // workspace_builds_workspace_id_build_number_key. + code = http.StatusConflict } return BuildError{code, "insert workspace build", err} }