Skip to content

feat(agent/agentcontainers): add devcontainers list endpoint #17389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 120 additions & 14 deletions agent/agentcontainers/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@ package agentcontainers
import (
"context"
"errors"
"fmt"
"net/http"
"path"
"slices"
"strings"
"time"

"github.com/go-chi/chi/v5"
"github.com/google/uuid"
"golang.org/x/xerrors"

"cdr.dev/slog"
Expand All @@ -31,11 +35,13 @@ type API struct {
dccli DevcontainerCLI
clock quartz.Clock

// lockCh protects the below fields. We use a channel instead of a mutex so we
// can handle cancellation properly.
lockCh chan struct{}
containers codersdk.WorkspaceAgentListContainersResponse
mtime time.Time
// lockCh protects the below fields. We use a channel instead of a
// mutex so we can handle cancellation properly.
lockCh chan struct{}
containers codersdk.WorkspaceAgentListContainersResponse
mtime time.Time
devcontainerNames map[string]struct{} // Track devcontainer names to avoid duplicates.
knownDevcontainers []codersdk.WorkspaceAgentDevcontainer // Track predefined and runtime-detected devcontainers.
}

// Option is a functional option for API.
Expand All @@ -55,12 +61,29 @@ func WithDevcontainerCLI(dccli DevcontainerCLI) Option {
}
}

// WithDevcontainers sets the known devcontainers for the API. This
// allows the API to be aware of devcontainers defined in the workspace
// agent manifest.
func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer) Option {
return func(api *API) {
if len(devcontainers) > 0 {
api.knownDevcontainers = slices.Clone(devcontainers)
api.devcontainerNames = make(map[string]struct{}, len(devcontainers))
for _, devcontainer := range devcontainers {
api.devcontainerNames[devcontainer.Name] = struct{}{}
}
}
}
}

// NewAPI returns a new API with the given options applied.
func NewAPI(logger slog.Logger, options ...Option) *API {
api := &API{
clock: quartz.NewReal(),
cacheDuration: defaultGetContainersCacheDuration,
lockCh: make(chan struct{}, 1),
clock: quartz.NewReal(),
cacheDuration: defaultGetContainersCacheDuration,
lockCh: make(chan struct{}, 1),
devcontainerNames: make(map[string]struct{}),
knownDevcontainers: []codersdk.WorkspaceAgentDevcontainer{},
}
for _, opt := range options {
opt(api)
Expand All @@ -79,6 +102,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
func (api *API) Routes() http.Handler {
r := chi.NewRouter()
r.Get("/", api.handleList)
r.Get("/devcontainers", api.handleListDevcontainers)
r.Post("/{id}/recreate", api.handleRecreate)
return r
}
Expand Down Expand Up @@ -121,12 +145,11 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC
select {
case <-ctx.Done():
return codersdk.WorkspaceAgentListContainersResponse{}, ctx.Err()
default:
api.lockCh <- struct{}{}
case api.lockCh <- struct{}{}:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Drive-by bug fix.

defer func() {
<-api.lockCh
}()
}
defer func() {
<-api.lockCh
}()

now := api.clock.Now()
if now.Sub(api.mtime) < api.cacheDuration {
Expand All @@ -142,6 +165,53 @@ func (api *API) getContainers(ctx context.Context) (codersdk.WorkspaceAgentListC
api.containers = updated
api.mtime = now

// Reset all known devcontainers to not running.
for i := range api.knownDevcontainers {
api.knownDevcontainers[i].Running = false
api.knownDevcontainers[i].Container = nil
}

// Check if the container is running and update the known devcontainers.
for _, container := range updated.Containers {
workspaceFolder := container.Labels[DevcontainerLocalFolderLabel]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought, non-blocking: Given that a WorkspaceAgentDevcontainer is a superset of a WorkspaceAgentContainer, I wonder if we could instead store the optional devcontainer-specific fields in WorkspaceAgentContainer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main issue with that approach is: how do we represent a devcontainer we know about but that has no container? (I.e. either it wasn't started, or it was deleted.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha 👍

if workspaceFolder != "" {
// Check if this is already in our known list.
if knownIndex := slices.IndexFunc(api.knownDevcontainers, func(dc codersdk.WorkspaceAgentDevcontainer) bool {
return dc.WorkspaceFolder == workspaceFolder
}); knownIndex != -1 {
// Update existing entry with runtime information.
if api.knownDevcontainers[knownIndex].ConfigPath == "" {
api.knownDevcontainers[knownIndex].ConfigPath = container.Labels[DevcontainerConfigFileLabel]
}
api.knownDevcontainers[knownIndex].Running = container.Running
api.knownDevcontainers[knownIndex].Container = &container
continue
}

// If not in our known list, add as a runtime detected entry.
name := path.Base(workspaceFolder)
if _, ok := api.devcontainerNames[name]; ok {
// Try to find a unique name by appending a number.
for i := 2; ; i++ {
newName := fmt.Sprintf("%s-%d", name, i)
if _, ok := api.devcontainerNames[newName]; !ok {
name = newName
break
}
}
}
api.devcontainerNames[name] = struct{}{}
api.knownDevcontainers = append(api.knownDevcontainers, codersdk.WorkspaceAgentDevcontainer{
ID: uuid.New(),
Name: name,
WorkspaceFolder: workspaceFolder,
ConfigPath: container.Labels[DevcontainerConfigFileLabel],
Running: container.Running,
Container: &container,
})
}
}

return copyListContainersResponse(api.containers), nil
}

Expand All @@ -158,7 +228,7 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) {
return
}

containers, err := api.cl.List(ctx)
containers, err := api.getContainers(ctx)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Drive-by bug fix.

if err != nil {
httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{
Message: "Could not list containers",
Expand Down Expand Up @@ -203,3 +273,39 @@ func (api *API) handleRecreate(w http.ResponseWriter, r *http.Request) {

w.WriteHeader(http.StatusNoContent)
}

// handleListDevcontainers handles the HTTP request to list known devcontainers.
func (api *API) handleListDevcontainers(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()

// Run getContainers to detect the latest devcontainers and their state.
_, err := api.getContainers(ctx)
if err != nil {
httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{
Message: "Could not list containers",
Detail: err.Error(),
})
return
}

select {
case <-ctx.Done():
return
case api.lockCh <- struct{}{}:
}
devcontainers := slices.Clone(api.knownDevcontainers)
<-api.lockCh

slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int {
if cmp := strings.Compare(a.WorkspaceFolder, b.WorkspaceFolder); cmp != 0 {
return cmp
}
return strings.Compare(a.ConfigPath, b.ConfigPath)
})

response := codersdk.WorkspaceAgentDevcontainersResponse{
Devcontainers: devcontainers,
}

httpapi.Write(ctx, w, http.StatusOK, response)
}
Loading
Loading