Skip to content

Commit ba6e84d

Browse files
mafredrimafredri
authored
fix(cli/ssh): retry on autostart conflict (coder#16058)
1 parent 53d9c7e commit ba6e84d

File tree

4 files changed

+112
-6
lines changed

4 files changed

+112
-6
lines changed

‎cli/ssh.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -657,12 +657,19 @@ func getWorkspaceAndAgent(ctx context.Context, inv *serpent.Invocation, client *
657657
// workspaces with the active version.
658658
_, _ = fmt.Fprintf(inv.Stderr, "Workspace was stopped, starting workspace to allow connecting to %q...\n", workspace.Name)
659659
_, err = startWorkspace(inv, client, workspace, workspaceParameterFlags{}, buildFlags{}, WorkspaceStart)
660-
if cerr, ok := codersdk.AsError(err); ok && cerr.StatusCode() == http.StatusForbidden {
661-
_, err = startWorkspace(inv, client, workspace, workspaceParameterFlags{}, buildFlags{}, WorkspaceUpdate)
662-
if err != nil {
663-
return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, xerrors.Errorf("start workspace with active template version: %w", err)
660+
if cerr, ok := codersdk.AsError(err); ok {
661+
switch cerr.StatusCode() {
662+
case http.StatusConflict:
663+
_, _ = fmt.Fprintln(inv.Stderr, "Unable to start the workspace due to conflict, the workspace may be starting, retrying without autostart...")
664+
return getWorkspaceAndAgent(ctx, inv, client, false, input)
665+
666+
case http.StatusForbidden:
667+
_, err = startWorkspace(inv, client, workspace, workspaceParameterFlags{}, buildFlags{}, WorkspaceUpdate)
668+
if err != nil {
669+
return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, xerrors.Errorf("start workspace with active template version: %w", err)
670+
}
671+
_, _ = 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.")
664672
}
665-
_, _ = 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.")
666673
} else if err != nil {
667674
return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, xerrors.Errorf("start workspace with current template version: %w", err)
668675
}

‎cli/ssh_test.go

+82
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"os/exec"
1818
"path"
1919
"path/filepath"
20+
"regexp"
2021
"runtime"
2122
"strings"
2223
"testing"
@@ -145,6 +146,87 @@ func TestSSH(t *testing.T) {
145146
pty.WriteLine("exit")
146147
<-cmdDone
147148
})
149+
t.Run("StartStoppedWorkspaceConflict", func(t *testing.T) {
150+
t.Parallel()
151+
152+
// Intercept builds to synchronize execution of the SSH command.
153+
// The purpose here is to make sure all commands try to trigger
154+
// a start build of the workspace.
155+
isFirstBuild := true
156+
buildURL := regexp.MustCompile("/api/v2/workspaces/.*/builds")
157+
buildReq := make(chan struct{})
158+
buildResume := make(chan struct{})
159+
buildSyncMW := func(next http.Handler) http.Handler {
160+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
161+
if r.Method == http.MethodPost && buildURL.MatchString(r.URL.Path) {
162+
if !isFirstBuild {
163+
t.Log("buildSyncMW: blocking build")
164+
buildReq <- struct{}{}
165+
<-buildResume
166+
t.Log("buildSyncMW: resuming build")
167+
} else {
168+
isFirstBuild = false
169+
}
170+
}
171+
next.ServeHTTP(w, r)
172+
})
173+
}
174+
175+
authToken := uuid.NewString()
176+
ownerClient := coderdtest.New(t, &coderdtest.Options{
177+
IncludeProvisionerDaemon: true,
178+
APIMiddleware: buildSyncMW,
179+
})
180+
owner := coderdtest.CreateFirstUser(t, ownerClient)
181+
client, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
182+
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{
183+
Parse: echo.ParseComplete,
184+
ProvisionPlan: echo.PlanComplete,
185+
ProvisionApply: echo.ProvisionApplyWithAgent(authToken),
186+
})
187+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
188+
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
189+
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
190+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
191+
// Stop the workspace
192+
workspaceBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop)
193+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspaceBuild.ID)
194+
195+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
196+
defer cancel()
197+
198+
var ptys []*ptytest.PTY
199+
for i := 0; i < 3; i++ {
200+
// SSH to the workspace which should autostart it
201+
inv, root := clitest.New(t, "ssh", workspace.Name)
202+
203+
pty := ptytest.New(t).Attach(inv)
204+
ptys = append(ptys, pty)
205+
clitest.SetupConfig(t, client, root)
206+
testutil.Go(t, func() {
207+
_ = inv.WithContext(ctx).Run()
208+
})
209+
}
210+
211+
for _, pty := range ptys {
212+
pty.ExpectMatchContext(ctx, "Workspace was stopped, starting workspace to allow connecting to")
213+
}
214+
for range ptys {
215+
testutil.RequireRecvCtx(ctx, t, buildReq)
216+
}
217+
close(buildResume)
218+
219+
var foundConflict int
220+
for _, pty := range ptys {
221+
// Either allow the command to start the workspace or fail
222+
// due to conflict (race), in which case it retries.
223+
match := pty.ExpectRegexMatchContext(ctx, "Waiting for the workspace agent to connect")
224+
if strings.Contains(match, "Unable to start the workspace due to conflict, the workspace may be starting, retrying without autostart...") {
225+
foundConflict++
226+
}
227+
}
228+
require.Equal(t, 2, foundConflict, "expected 2 conflicts")
229+
})
148230
t.Run("RequireActiveVersion", func(t *testing.T) {
149231
t.Parallel()
150232

‎coderd/coderdtest/coderdtest.go

+14-1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333

3434
"cloud.google.com/go/compute/metadata"
3535
"github.com/fullsailor/pkcs7"
36+
"github.com/go-chi/chi/v5"
3637
"github.com/golang-jwt/jwt/v4"
3738
"github.com/google/uuid"
3839
"github.com/moby/moby/pkg/namesgenerator"
@@ -146,6 +147,11 @@ type Options struct {
146147
Database database.Store
147148
Pubsub pubsub.Pubsub
148149

150+
// APIMiddleware inserts middleware before api.RootHandler, this can be
151+
// useful in certain tests where you want to intercept requests before
152+
// passing them on to the API, e.g. for synchronization of execution.
153+
APIMiddleware func(http.Handler) http.Handler
154+
149155
ConfigSSH codersdk.SSHConfigResponse
150156

151157
SwaggerEndpoint bool
@@ -555,7 +561,14 @@ func NewWithAPI(t testing.TB, options *Options) (*codersdk.Client, io.Closer, *c
555561
setHandler, cancelFunc, serverURL, newOptions := NewOptions(t, options)
556562
// We set the handler after server creation for the access URL.
557563
coderAPI := coderd.New(newOptions)
558-
setHandler(coderAPI.RootHandler)
564+
rootHandler := coderAPI.RootHandler
565+
if options.APIMiddleware != nil {
566+
r := chi.NewRouter()
567+
r.Use(options.APIMiddleware)
568+
r.Mount("/", rootHandler)
569+
rootHandler = r
570+
}
571+
setHandler(rootHandler)
559572
var provisionerCloser io.Closer = nopcloser{}
560573
if options.IncludeProvisionerDaemon {
561574
provisionerCloser = NewTaggedProvisionerDaemon(t, coderAPI, "test", options.ProvisionerDaemonTags)

‎coderd/wsbuilder/wsbuilder.go

+4
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,10 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
381381
code := http.StatusInternalServerError
382382
if rbac.IsUnauthorizedError(err) {
383383
code = http.StatusForbidden
384+
} else if database.IsUniqueViolation(err) {
385+
// Concurrent builds may result in duplicate
386+
// workspace_builds_workspace_id_build_number_key.
387+
code = http.StatusConflict
384388
}
385389
return BuildError{code, "insert workspace build", err}
386390
}

0 commit comments

Comments
 (0)