Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitfcf9371

Browse files
feat(agent/agentcontainers): retry with longer name on failure (#18513)
Closescoder/internal#732We now try (up to 5 times) when attempting to create an agent using theworkspace folder as the name.It is important to note this flow is only ever ran when attempting tocreate an agent using the workspace folder as the name. If a deploymentuses terraform or the devcontainer customization, we do not fall back tothis approach.
1 parent4fd0312 commitfcf9371

File tree

3 files changed

+403
-22
lines changed

3 files changed

+403
-22
lines changed

‎agent/agentcontainers/api.go

Lines changed: 111 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ const (
4242
// read-write, which seems sensible for devcontainers.
4343
coderPathInsideContainer="/.coder-agent/coder"
4444

45-
maxAgentNameLength=64
45+
maxAgentNameLength=64
46+
maxAttemptsToNameAgent=5
4647
)
4748

4849
// API is responsible for container-related operations in the agent.
@@ -71,17 +72,18 @@ type API struct {
7172
ownerNamestring
7273
workspaceNamestring
7374

74-
mu sync.RWMutex
75-
closedbool
76-
containers codersdk.WorkspaceAgentListContainersResponse// Output from the last list operation.
77-
containersErrerror// Error from the last list operation.
78-
devcontainerNamesmap[string]bool// By devcontainer name.
79-
knownDevcontainersmap[string]codersdk.WorkspaceAgentDevcontainer// By workspace folder.
80-
configFileModifiedTimesmap[string]time.Time// By config file path.
81-
recreateSuccessTimesmap[string]time.Time// By workspace folder.
82-
recreateErrorTimesmap[string]time.Time// By workspace folder.
83-
injectedSubAgentProcsmap[string]subAgentProcess// By workspace folder.
84-
asyncWg sync.WaitGroup
75+
mu sync.RWMutex
76+
closedbool
77+
containers codersdk.WorkspaceAgentListContainersResponse// Output from the last list operation.
78+
containersErrerror// Error from the last list operation.
79+
devcontainerNamesmap[string]bool// By devcontainer name.
80+
knownDevcontainersmap[string]codersdk.WorkspaceAgentDevcontainer// By workspace folder.
81+
configFileModifiedTimesmap[string]time.Time// By config file path.
82+
recreateSuccessTimesmap[string]time.Time// By workspace folder.
83+
recreateErrorTimesmap[string]time.Time// By workspace folder.
84+
injectedSubAgentProcsmap[string]subAgentProcess// By workspace folder.
85+
usingWorkspaceFolderNamemap[string]bool// By workspace folder.
86+
asyncWg sync.WaitGroup
8587

8688
devcontainerLogSourceIDsmap[string]uuid.UUID// By workspace folder.
8789
}
@@ -278,6 +280,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
278280
recreateErrorTimes:make(map[string]time.Time),
279281
scriptLogger:func(uuid.UUID)ScriptLogger {returnnoopScriptLogger{} },
280282
injectedSubAgentProcs:make(map[string]subAgentProcess),
283+
usingWorkspaceFolderName:make(map[string]bool),
281284
}
282285
// The ctx and logger must be set before applying options to avoid
283286
// nil pointer dereference.
@@ -630,7 +633,7 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
630633
// folder's name. If it is not possible to generate a valid
631634
// agent name based off of the folder name (i.e. no valid characters),
632635
// we will instead fall back to using the container's friendly name.
633-
dc.Name=safeAgentName(path.Base(filepath.ToSlash(dc.WorkspaceFolder)),dc.Container.FriendlyName)
636+
dc.Name,api.usingWorkspaceFolderName[dc.WorkspaceFolder]=api.makeAgentName(dc.WorkspaceFolder,dc.Container.FriendlyName)
634637
}
635638
}
636639

@@ -678,8 +681,10 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
678681
varconsecutiveHyphenRegex=regexp.MustCompile("-+")
679682

680683
// `safeAgentName` returns a safe agent name derived from a folder name,
681-
// falling back to the container’s friendly name if needed.
682-
funcsafeAgentName(namestring,friendlyNamestring)string {
684+
// falling back to the container’s friendly name if needed. The second
685+
// return value will be `true` if it succeeded and `false` if it had
686+
// to fallback to the friendly name.
687+
funcsafeAgentName(namestring,friendlyNamestring) (string,bool) {
683688
// Keep only ASCII letters and digits, replacing everything
684689
// else with a hyphen.
685690
varsb strings.Builder
@@ -701,10 +706,10 @@ func safeAgentName(name string, friendlyName string) string {
701706
name=name[:min(len(name),maxAgentNameLength)]
702707

703708
ifprovisioner.AgentNameRegex.Match([]byte(name)) {
704-
returnname
709+
returnname,true
705710
}
706711

707-
returnsafeFriendlyName(friendlyName)
712+
returnsafeFriendlyName(friendlyName),false
708713
}
709714

710715
// safeFriendlyName returns a API safe version of the container's
@@ -719,6 +724,47 @@ func safeFriendlyName(name string) string {
719724
returnname
720725
}
721726

727+
// expandedAgentName creates an agent name by including parent directories
728+
// from the workspace folder path to avoid name collisions. Like `safeAgentName`,
729+
// the second returned value will be true if using the workspace folder name,
730+
// and false if it fell back to the friendly name.
731+
funcexpandedAgentName(workspaceFolderstring,friendlyNamestring,depthint) (string,bool) {
732+
varparts []string
733+
forpart:=rangestrings.SplitSeq(filepath.ToSlash(workspaceFolder),"/") {
734+
ifpart=strings.TrimSpace(part);part!="" {
735+
parts=append(parts,part)
736+
}
737+
}
738+
iflen(parts)==0 {
739+
returnsafeFriendlyName(friendlyName),false
740+
}
741+
742+
components:=parts[max(0,len(parts)-depth-1):]
743+
expanded:=strings.Join(components,"-")
744+
745+
returnsafeAgentName(expanded,friendlyName)
746+
}
747+
748+
// makeAgentName attempts to create an agent name. It will first attempt to create an
749+
// agent name based off of the workspace folder, and will eventually fallback to a
750+
// friendly name. Like `safeAgentName`, the second returned value will be true if the
751+
// agent name utilizes the workspace folder, and false if it falls back to the
752+
// friendly name.
753+
func (api*API)makeAgentName(workspaceFolderstring,friendlyNamestring) (string,bool) {
754+
forattempt:=0;attempt<=maxAttemptsToNameAgent;attempt++ {
755+
agentName,usingWorkspaceFolder:=expandedAgentName(workspaceFolder,friendlyName,attempt)
756+
if!usingWorkspaceFolder {
757+
returnagentName,false
758+
}
759+
760+
if!api.devcontainerNames[agentName] {
761+
returnagentName,true
762+
}
763+
}
764+
765+
returnsafeFriendlyName(friendlyName),false
766+
}
767+
722768
// RefreshContainers triggers an immediate update of the container list
723769
// and waits for it to complete.
724770
func (api*API)RefreshContainers(ctx context.Context) (errerror) {
@@ -1234,6 +1280,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12341280
ifprovisioner.AgentNameRegex.Match([]byte(name)) {
12351281
subAgentConfig.Name=name
12361282
configOutdated=true
1283+
delete(api.usingWorkspaceFolderName,dc.WorkspaceFolder)
12371284
}else {
12381285
logger.Warn(ctx,"invalid name in devcontainer customization, ignoring",
12391286
slog.F("name",name),
@@ -1320,12 +1367,55 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
13201367
)
13211368

13221369
// Create new subagent record in the database to receive the auth token.
1370+
// If we get a unique constraint violation, try with expanded names that
1371+
// include parent directories to avoid collisions.
13231372
client:=*api.subAgentClient.Load()
1324-
newSubAgent,err:=client.Create(ctx,subAgentConfig)
1325-
iferr!=nil {
1326-
returnxerrors.Errorf("create subagent failed: %w",err)
1373+
1374+
originalName:=subAgentConfig.Name
1375+
1376+
forattempt:=1;attempt<=maxAttemptsToNameAgent;attempt++ {
1377+
ifproc.agent,err=client.Create(ctx,subAgentConfig);err==nil {
1378+
ifapi.usingWorkspaceFolderName[dc.WorkspaceFolder] {
1379+
api.devcontainerNames[dc.Name]=true
1380+
delete(api.usingWorkspaceFolderName,dc.WorkspaceFolder)
1381+
}
1382+
1383+
break
1384+
}
1385+
1386+
// NOTE(DanielleMaywood):
1387+
// Ordinarily we'd use `errors.As` here, but it didn't appear to work. Not
1388+
// sure if this is because of the communication protocol? Instead I've opted
1389+
// for a slightly more janky string contains approach.
1390+
//
1391+
// We only care if sub agent creation has failed due to a unique constraint
1392+
// violation on the agent name, as we can _possibly_ rectify this.
1393+
if!strings.Contains(err.Error(),"workspace agent name") {
1394+
returnxerrors.Errorf("create subagent failed: %w",err)
1395+
}
1396+
1397+
// If there has been a unique constraint violation but the user is *not*
1398+
// using an auto-generated name, then we should error. This is because
1399+
// we do not want to surprise the user with a name they did not ask for.
1400+
ifusingFolderName:=api.usingWorkspaceFolderName[dc.WorkspaceFolder];!usingFolderName {
1401+
returnxerrors.Errorf("create subagent failed: %w",err)
1402+
}
1403+
1404+
ifattempt==maxAttemptsToNameAgent {
1405+
returnxerrors.Errorf("create subagent failed after %d attempts: %w",attempt,err)
1406+
}
1407+
1408+
// We increase how much of the workspace folder is used for generating
1409+
// the agent name. With each iteration there is greater chance of this
1410+
// being successful.
1411+
subAgentConfig.Name,api.usingWorkspaceFolderName[dc.WorkspaceFolder]=expandedAgentName(dc.WorkspaceFolder,dc.Container.FriendlyName,attempt)
1412+
1413+
logger.Debug(ctx,"retrying subagent creation with expanded name",
1414+
slog.F("original_name",originalName),
1415+
slog.F("expanded_name",subAgentConfig.Name),
1416+
slog.F("attempt",attempt+1),
1417+
)
13271418
}
1328-
proc.agent=newSubAgent
13291419

13301420
logger.Info(ctx,"created new subagent",slog.F("agent_id",proc.agent.ID))
13311421
}else {

‎agent/agentcontainers/api_internal_test.go

Lines changed: 158 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ func TestSafeAgentName(t *testing.T) {
1515
namestring
1616
folderNamestring
1717
expectedstring
18+
fallbackbool
1819
}{
1920
// Basic valid names
2021
{
@@ -110,18 +111,22 @@ func TestSafeAgentName(t *testing.T) {
110111
{
111112
folderName:"",
112113
expected:"friendly-fallback",
114+
fallback:true,
113115
},
114116
{
115117
folderName:"---",
116118
expected:"friendly-fallback",
119+
fallback:true,
117120
},
118121
{
119122
folderName:"___",
120123
expected:"friendly-fallback",
124+
fallback:true,
121125
},
122126
{
123127
folderName:"@#$",
124128
expected:"friendly-fallback",
129+
fallback:true,
125130
},
126131

127132
// Additional edge cases
@@ -192,10 +197,162 @@ func TestSafeAgentName(t *testing.T) {
192197
for_,tt:=rangetests {
193198
t.Run(tt.folderName,func(t*testing.T) {
194199
t.Parallel()
195-
name:=safeAgentName(tt.folderName,"friendly-fallback")
200+
name,usingWorkspaceFolder:=safeAgentName(tt.folderName,"friendly-fallback")
196201

197202
assert.Equal(t,tt.expected,name)
198203
assert.True(t,provisioner.AgentNameRegex.Match([]byte(name)))
204+
assert.Equal(t,tt.fallback,!usingWorkspaceFolder)
205+
})
206+
}
207+
}
208+
209+
funcTestExpandedAgentName(t*testing.T) {
210+
t.Parallel()
211+
212+
tests:= []struct {
213+
namestring
214+
workspaceFolderstring
215+
friendlyNamestring
216+
depthint
217+
expectedstring
218+
fallbackbool
219+
}{
220+
{
221+
name:"simple path depth 1",
222+
workspaceFolder:"/home/coder/project",
223+
friendlyName:"friendly-fallback",
224+
depth:0,
225+
expected:"project",
226+
},
227+
{
228+
name:"simple path depth 2",
229+
workspaceFolder:"/home/coder/project",
230+
friendlyName:"friendly-fallback",
231+
depth:1,
232+
expected:"coder-project",
233+
},
234+
{
235+
name:"simple path depth 3",
236+
workspaceFolder:"/home/coder/project",
237+
friendlyName:"friendly-fallback",
238+
depth:2,
239+
expected:"home-coder-project",
240+
},
241+
{
242+
name:"simple path depth exceeds available",
243+
workspaceFolder:"/home/coder/project",
244+
friendlyName:"friendly-fallback",
245+
depth:9,
246+
expected:"home-coder-project",
247+
},
248+
// Cases with special characters that need sanitization
249+
{
250+
name:"path with spaces and special chars",
251+
workspaceFolder:"/home/coder/My Project_v2",
252+
friendlyName:"friendly-fallback",
253+
depth:1,
254+
expected:"coder-my-project-v2",
255+
},
256+
{
257+
name:"path with dots and underscores",
258+
workspaceFolder:"/home/user.name/project_folder.git",
259+
friendlyName:"friendly-fallback",
260+
depth:1,
261+
expected:"user-name-project-folder-git",
262+
},
263+
// Edge cases
264+
{
265+
name:"empty path",
266+
workspaceFolder:"",
267+
friendlyName:"friendly-fallback",
268+
depth:0,
269+
expected:"friendly-fallback",
270+
fallback:true,
271+
},
272+
{
273+
name:"root path",
274+
workspaceFolder:"/",
275+
friendlyName:"friendly-fallback",
276+
depth:0,
277+
expected:"friendly-fallback",
278+
fallback:true,
279+
},
280+
{
281+
name:"single component",
282+
workspaceFolder:"project",
283+
friendlyName:"friendly-fallback",
284+
depth:0,
285+
expected:"project",
286+
},
287+
{
288+
name:"single component with depth 2",
289+
workspaceFolder:"project",
290+
friendlyName:"friendly-fallback",
291+
depth:1,
292+
expected:"project",
293+
},
294+
// Collision simulation cases
295+
{
296+
name:"foo/project depth 1",
297+
workspaceFolder:"/home/coder/foo/project",
298+
friendlyName:"friendly-fallback",
299+
depth:0,
300+
expected:"project",
301+
},
302+
{
303+
name:"foo/project depth 2",
304+
workspaceFolder:"/home/coder/foo/project",
305+
friendlyName:"friendly-fallback",
306+
depth:1,
307+
expected:"foo-project",
308+
},
309+
{
310+
name:"bar/project depth 1",
311+
workspaceFolder:"/home/coder/bar/project",
312+
friendlyName:"friendly-fallback",
313+
depth:0,
314+
expected:"project",
315+
},
316+
{
317+
name:"bar/project depth 2",
318+
workspaceFolder:"/home/coder/bar/project",
319+
friendlyName:"friendly-fallback",
320+
depth:1,
321+
expected:"bar-project",
322+
},
323+
// Path with trailing slashes
324+
{
325+
name:"path with trailing slash",
326+
workspaceFolder:"/home/coder/project/",
327+
friendlyName:"friendly-fallback",
328+
depth:1,
329+
expected:"coder-project",
330+
},
331+
{
332+
name:"path with multiple trailing slashes",
333+
workspaceFolder:"/home/coder/project///",
334+
friendlyName:"friendly-fallback",
335+
depth:1,
336+
expected:"coder-project",
337+
},
338+
// Path with leading slashes
339+
{
340+
name:"path with multiple leading slashes",
341+
workspaceFolder:"///home/coder/project",
342+
friendlyName:"friendly-fallback",
343+
depth:1,
344+
expected:"coder-project",
345+
},
346+
}
347+
348+
for_,tt:=rangetests {
349+
t.Run(tt.name,func(t*testing.T) {
350+
t.Parallel()
351+
name,usingWorkspaceFolder:=expandedAgentName(tt.workspaceFolder,tt.friendlyName,tt.depth)
352+
353+
assert.Equal(t,tt.expected,name)
354+
assert.True(t,provisioner.AgentNameRegex.Match([]byte(name)))
355+
assert.Equal(t,tt.fallback,!usingWorkspaceFolder)
199356
})
200357
}
201358
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp