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

Commiteb6412a

Browse files
authored
refactor(agent/agentcontainers): update routes and locking in container api (#17768)
This refactor updates the devcontainer routes and in-api locking forbetter clarity.Updates#16424
1 parentb6d72c8 commiteb6412a

File tree

2 files changed

+83
-57
lines changed

2 files changed

+83
-57
lines changed

‎agent/agentcontainers/api.go‎

Lines changed: 81 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,10 @@ func (api *API) Routes() http.Handler {
214214
r:=chi.NewRouter()
215215

216216
r.Get("/",api.handleList)
217-
r.Get("/devcontainers",api.handleListDevcontainers)
218-
r.Post("/{id}/recreate",api.handleRecreate)
217+
r.Route("/devcontainers",func(r chi.Router) {
218+
r.Get("/",api.handleDevcontainersList)
219+
r.Post("/container/{container}/recreate",api.handleDevcontainerRecreate)
220+
})
219221

220222
returnr
221223
}
@@ -376,12 +378,13 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC
376378
returncopyListContainersResponse(api.containers),nil
377379
}
378380

379-
// handleRecreate handles the HTTP request to recreate a container.
380-
func (api*API)handleRecreate(w http.ResponseWriter,r*http.Request) {
381+
// handleDevcontainerRecreate handles the HTTP request to recreate a
382+
// devcontainer by referencing the container.
383+
func (api*API)handleDevcontainerRecreate(w http.ResponseWriter,r*http.Request) {
381384
ctx:=r.Context()
382-
id:=chi.URLParam(r,"id")
385+
containerID:=chi.URLParam(r,"container")
383386

384-
ifid=="" {
387+
ifcontainerID=="" {
385388
httpapi.Write(ctx,w,http.StatusBadRequest, codersdk.Response{
386389
Message:"Missing container ID or name",
387390
Detail:"Container ID or name is required to recreate a devcontainer.",
@@ -399,7 +402,7 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) {
399402
}
400403

401404
containerIdx:=slices.IndexFunc(containers.Containers,func(c codersdk.WorkspaceAgentContainer)bool {
402-
returnc.Match(id)
405+
returnc.Match(containerID)
403406
})
404407
ifcontainerIdx==-1 {
405408
httpapi.Write(ctx,w,http.StatusNotFound, codersdk.Response{
@@ -418,7 +421,7 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) {
418421
ifworkspaceFolder=="" {
419422
httpapi.Write(ctx,w,http.StatusBadRequest, codersdk.Response{
420423
Message:"Missing workspace folder label",
421-
Detail:"The workspace folder labelis requiredtorecreate a devcontainer.",
424+
Detail:"Thecontainer is not a devcontainer, the container must have theworkspace folder label tosupport recreation.",
422425
})
423426
return
424427
}
@@ -434,32 +437,28 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) {
434437

435438
// TODO(mafredri): Temporarily handle clearing the dirty state after
436439
// recreation, later on this should be handled by a "container watcher".
437-
select {
438-
case<-api.ctx.Done():
439-
return
440-
case<-ctx.Done():
441-
return
442-
caseapi.lockCh<-struct{}{}:
443-
deferfunc() {<-api.lockCh }()
444-
}
445-
fori:=rangeapi.knownDevcontainers {
446-
ifapi.knownDevcontainers[i].WorkspaceFolder==workspaceFolder {
447-
ifapi.knownDevcontainers[i].Dirty {
448-
api.logger.Info(ctx,"clearing dirty flag after recreation",
449-
slog.F("workspace_folder",workspaceFolder),
450-
slog.F("name",api.knownDevcontainers[i].Name),
451-
)
452-
api.knownDevcontainers[i].Dirty=false
440+
if!api.doLockedHandler(w,r,func() {
441+
fori:=rangeapi.knownDevcontainers {
442+
ifapi.knownDevcontainers[i].WorkspaceFolder==workspaceFolder {
443+
ifapi.knownDevcontainers[i].Dirty {
444+
api.logger.Info(ctx,"clearing dirty flag after recreation",
445+
slog.F("workspace_folder",workspaceFolder),
446+
slog.F("name",api.knownDevcontainers[i].Name),
447+
)
448+
api.knownDevcontainers[i].Dirty=false
449+
}
450+
return
453451
}
454-
break
455452
}
453+
}) {
454+
return
456455
}
457456

458457
w.WriteHeader(http.StatusNoContent)
459458
}
460459

461-
//handleListDevcontainers handles the HTTP request to list known devcontainers.
462-
func (api*API)handleListDevcontainers(w http.ResponseWriter,r*http.Request) {
460+
//handleDevcontainersList handles the HTTP request to list known devcontainers.
461+
func (api*API)handleDevcontainersList(w http.ResponseWriter,r*http.Request) {
463462
ctx:=r.Context()
464463

465464
// Run getContainers to detect the latest devcontainers and their state.
@@ -472,15 +471,12 @@ func (api *API) handleListDevcontainers(w http.ResponseWriter, r *http.Request)
472471
return
473472
}
474473

475-
select {
476-
case<-api.ctx.Done():
474+
vardevcontainers []codersdk.WorkspaceAgentDevcontainer
475+
if!api.doLockedHandler(w,r,func() {
476+
devcontainers=slices.Clone(api.knownDevcontainers)
477+
}) {
477478
return
478-
case<-ctx.Done():
479-
return
480-
caseapi.lockCh<-struct{}{}:
481479
}
482-
devcontainers:=slices.Clone(api.knownDevcontainers)
483-
<-api.lockCh
484480

485481
slices.SortFunc(devcontainers,func(a,b codersdk.WorkspaceAgentDevcontainer)int {
486482
ifcmp:=strings.Compare(a.WorkspaceFolder,b.WorkspaceFolder);cmp!=0 {
@@ -499,34 +495,64 @@ func (api *API) handleListDevcontainers(w http.ResponseWriter, r *http.Request)
499495
// markDevcontainerDirty finds the devcontainer with the given config file path
500496
// and marks it as dirty. It acquires the lock before modifying the state.
501497
func (api*API)markDevcontainerDirty(configPathstring,modifiedAt time.Time) {
498+
ok:=api.doLocked(func() {
499+
// Record the timestamp of when this configuration file was modified.
500+
api.configFileModifiedTimes[configPath]=modifiedAt
501+
502+
fori:=rangeapi.knownDevcontainers {
503+
ifapi.knownDevcontainers[i].ConfigPath!=configPath {
504+
continue
505+
}
506+
507+
// TODO(mafredri): Simplistic mark for now, we should check if the
508+
// container is running and if the config file was modified after
509+
// the container was created.
510+
if!api.knownDevcontainers[i].Dirty {
511+
api.logger.Info(api.ctx,"marking devcontainer as dirty",
512+
slog.F("file",configPath),
513+
slog.F("name",api.knownDevcontainers[i].Name),
514+
slog.F("workspace_folder",api.knownDevcontainers[i].WorkspaceFolder),
515+
slog.F("modified_at",modifiedAt),
516+
)
517+
api.knownDevcontainers[i].Dirty=true
518+
}
519+
}
520+
})
521+
if!ok {
522+
api.logger.Debug(api.ctx,"mark devcontainer dirty failed",slog.F("file",configPath))
523+
}
524+
}
525+
526+
func (api*API)doLockedHandler(w http.ResponseWriter,r*http.Request,ffunc())bool {
502527
select {
528+
case<-r.Context().Done():
529+
httpapi.Write(r.Context(),w,http.StatusRequestTimeout, codersdk.Response{
530+
Message:"Request canceled",
531+
Detail:"Request was canceled before we could process it.",
532+
})
533+
returnfalse
503534
case<-api.ctx.Done():
504-
return
535+
httpapi.Write(r.Context(),w,http.StatusServiceUnavailable, codersdk.Response{
536+
Message:"API closed",
537+
Detail:"The API is closed and cannot process requests.",
538+
})
539+
returnfalse
505540
caseapi.lockCh<-struct{}{}:
506541
deferfunc() {<-api.lockCh }()
507542
}
543+
f()
544+
returntrue
545+
}
508546

509-
// Record the timestamp of when this configuration file was modified.
510-
api.configFileModifiedTimes[configPath]=modifiedAt
511-
512-
fori:=rangeapi.knownDevcontainers {
513-
ifapi.knownDevcontainers[i].ConfigPath!=configPath {
514-
continue
515-
}
516-
517-
// TODO(mafredri): Simplistic mark for now, we should check if the
518-
// container is running and if the config file was modified after
519-
// the container was created.
520-
if!api.knownDevcontainers[i].Dirty {
521-
api.logger.Info(api.ctx,"marking devcontainer as dirty",
522-
slog.F("file",configPath),
523-
slog.F("name",api.knownDevcontainers[i].Name),
524-
slog.F("workspace_folder",api.knownDevcontainers[i].WorkspaceFolder),
525-
slog.F("modified_at",modifiedAt),
526-
)
527-
api.knownDevcontainers[i].Dirty=true
528-
}
547+
func (api*API)doLocked(ffunc())bool {
548+
select {
549+
case<-api.ctx.Done():
550+
returnfalse
551+
caseapi.lockCh<-struct{}{}:
552+
deferfunc() {<-api.lockCh }()
529553
}
554+
f()
555+
returntrue
530556
}
531557

532558
func (api*API)Close()error {

‎agent/agentcontainers/api_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func TestAPI(t *testing.T) {
173173
wantBodystring
174174
}{
175175
{
176-
name:"Missing ID",
176+
name:"MissingcontainerID",
177177
containerID:"",
178178
lister:&fakeLister{},
179179
devcontainerCLI:&fakeDevcontainerCLI{},
@@ -260,7 +260,7 @@ func TestAPI(t *testing.T) {
260260
r.Mount("/",api.Routes())
261261

262262
// Simulate HTTP request to the recreate endpoint.
263-
req:=httptest.NewRequest(http.MethodPost,"/"+tt.containerID+"/recreate",nil)
263+
req:=httptest.NewRequest(http.MethodPost,"/devcontainers/container/"+tt.containerID+"/recreate",nil)
264264
rec:=httptest.NewRecorder()
265265
r.ServeHTTP(rec,req)
266266

0 commit comments

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