opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: wait for initial update before marking API as ready (#21363)

Browse Source 
_Disclaimer: investigation done by Claude Opus 4.5_

Closes https://github.com/coder/internal/issues/1173
Closes https://github.com/coder/internal/issues/1174

The agent containers API is only marked "ready" under this condition in
`agent/agentcontainers/api.go`:

```go
	// For now, all endpoints require the initial update to be done.
	// If we want to allow some endpoints to be available before
	// the initial update, we can enable this per-route.
```

However, what was actually being checked for was that the _init_ was
done, not the _initial update_.

In agent/agentcontainers/api.go, the `Start()` method:
  1. Called `Init()` which closed `initDone` <--- API marked ready here
  2. Then launched `go api.updaterLoop()` asynchronously
3. `updaterLoop()` performs the initial container update <--- should
have marked it ready after this

This PR fixes these semantics to avoid the race which was causing the
above two flakes.

Signed-off-by: Danny Kopping <danny@coder.com>
This commit is contained in:
Danny Kopping
2025-12-22 14:18:01 +02:00
committed by GitHub
parent 72e478e461
commit 8991a5966e
1 changed files with 12 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agent/agentcontainers/api.go
+12 -15
View File
@@ -87,7 +87,8 @@ type API struct {
agentDirectory string
mu sync.RWMutex // Protects the following fields.
initDone chan struct{} // Closed by Init.
initDone bool // Whether Init has been called.
initialUpdateDone chan struct{} // Closed after first updateContainers call in updaterLoop.
updateChans []chan struct{}
closed bool
containers codersdk.WorkspaceAgentListContainersResponse // Output from the last list operation.
@@ -325,7 +326,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
api := &API{
ctx: ctx,
cancel: cancel,
initDone: make(chan struct{}),
initialUpdateDone: make(chan struct{}),
updateTrigger: make(chan chan error),
updateInterval: defaultUpdateInterval,
logger: logger,
@@ -379,20 +380,15 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
return api
}
// Init applies a final set of options to the API and then
// closes initDone. This method can only be called once.
// Init applies a final set of options to the API and marks
// initialization as done. This method can only be called once.
func (api *API) Init(opts ...Option) {
api.mu.Lock()
defer api.mu.Unlock()
if api.closed {
if api.closed || api.initDone {
return
}
select {
case <-api.initDone:
return
default:
}
defer close(api.initDone)
api.initDone = true
for _, opt := range opts {
opt(api)
@@ -651,6 +647,7 @@ func (api *API) updaterLoop() {
} else {
api.logger.Debug(api.ctx, "initial containers update complete")
}
close(api.initialUpdateDone)
// We utilize a TickerFunc here instead of a regular Ticker so that
// we can guarantee execution of the updateContainers method after
@@ -715,7 +712,7 @@ func (api *API) UpdateSubAgentClient(client SubAgentClient) {
func (api *API) Routes() http.Handler {
r := chi.NewRouter()
ensureInitDoneMW := func(next http.Handler) http.Handler {
ensureInitialUpdateDoneMW := func(next http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
select {
case <-api.ctx.Done():
@@ -726,8 +723,8 @@ func (api *API) Routes() http.Handler {
return
case <-r.Context().Done():
return
case <-api.initDone:
// API init is done, we can start processing requests.
case <-api.initialUpdateDone:
// Initial update is done, we can start processing requests.
}
next.ServeHTTP(rw, r)
})
@@ -736,7 +733,7 @@ func (api *API) Routes() http.Handler {
// For now, all endpoints require the initial update to be done.
// If we want to allow some endpoints to be available before
// the initial update, we can enable this per-route.
r.Use(ensureInitDoneMW)
r.Use(ensureInitialUpdateDoneMW)
r.Get("/", api.handleList)
r.Get("/watch", api.watchContainers)