diff --git a/agent/agent.go b/agent/agent.go index b195368338242..7525ecf051f69 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -89,9 +89,9 @@ type Options struct { ServiceBannerRefreshInterval time.Duration BlockFileTransfer bool Execer agentexec.Execer - ContainerLister agentcontainers.Lister ExperimentalDevcontainersEnabled bool + ContainerAPIOptions []agentcontainers.Option // Enable ExperimentalDevcontainersEnabled for these to be effective. } type Client interface { @@ -154,9 +154,6 @@ func New(options Options) Agent { if options.Execer == nil { options.Execer = agentexec.DefaultExecer } - if options.ContainerLister == nil { - options.ContainerLister = agentcontainers.NoopLister{} - } hardCtx, hardCancel := context.WithCancel(context.Background()) gracefulCtx, gracefulCancel := context.WithCancel(hardCtx) @@ -192,9 +189,9 @@ func New(options Options) Agent { prometheusRegistry: prometheusRegistry, metrics: newAgentMetrics(prometheusRegistry), execer: options.Execer, - lister: options.ContainerLister, experimentalDevcontainersEnabled: options.ExperimentalDevcontainersEnabled, + containerAPIOptions: options.ContainerAPIOptions, } // Initially, we have a closed channel, reflecting the fact that we are not initially connected. // Each time we connect we replace the channel (while holding the closeMutex) with a new one @@ -274,9 +271,10 @@ type agent struct { // labeled in Coder with the agent + workspace. metrics *agentMetrics execer agentexec.Execer - lister agentcontainers.Lister experimentalDevcontainersEnabled bool + containerAPIOptions []agentcontainers.Option + containerAPI atomic.Pointer[agentcontainers.API] // Set by apiHandler. } func (a *agent) TailnetConn() *tailnet.Conn { @@ -1170,6 +1168,12 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context, } a.metrics.startupScriptSeconds.WithLabelValues(label).Set(dur) a.scriptRunner.StartCron() + if containerAPI := a.containerAPI.Load(); containerAPI != nil { + // Inform the container API that the agent is ready. + // This allows us to start watching for changes to + // the devcontainer configuration files. + containerAPI.SignalReady() + } }) if err != nil { return xerrors.Errorf("track conn goroutine: %w", err) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 489bc1e55194c..c3779af67633a 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -39,6 +39,7 @@ type API struct { watcher watcher.Watcher cacheDuration time.Duration + execer agentexec.Execer cl Lister dccli DevcontainerCLI clock quartz.Clock @@ -56,14 +57,6 @@ type API struct { // Option is a functional option for API. type Option func(*API) -// WithLister sets the agentcontainers.Lister implementation to use. -// The default implementation uses the Docker CLI to list containers. -func WithLister(cl Lister) Option { - return func(api *API) { - api.cl = cl - } -} - // WithClock sets the quartz.Clock implementation to use. // This is primarily used for testing to control time. func WithClock(clock quartz.Clock) Option { @@ -72,6 +65,21 @@ func WithClock(clock quartz.Clock) Option { } } +// WithExecer sets the agentexec.Execer implementation to use. +func WithExecer(execer agentexec.Execer) Option { + return func(api *API) { + api.execer = execer + } +} + +// WithLister sets the agentcontainers.Lister implementation to use. +// The default implementation uses the Docker CLI to list containers. +func WithLister(cl Lister) Option { + return func(api *API) { + api.cl = cl + } +} + // WithDevcontainerCLI sets the DevcontainerCLI implementation to use. // This can be used in tests to modify @devcontainer/cli behavior. func WithDevcontainerCLI(dccli DevcontainerCLI) Option { @@ -113,6 +121,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API { done: make(chan struct{}), logger: logger, clock: quartz.NewReal(), + execer: agentexec.DefaultExecer, cacheDuration: defaultGetContainersCacheDuration, lockCh: make(chan struct{}, 1), devcontainerNames: make(map[string]struct{}), @@ -123,30 +132,46 @@ func NewAPI(logger slog.Logger, options ...Option) *API { opt(api) } if api.cl == nil { - api.cl = &DockerCLILister{} + api.cl = NewDocker(api.execer) } if api.dccli == nil { - api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), agentexec.DefaultExecer) + api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), api.execer) } if api.watcher == nil { - api.watcher = watcher.NewNoop() + var err error + api.watcher, err = watcher.NewFSNotify() + if err != nil { + logger.Error(ctx, "create file watcher service failed", slog.Error(err)) + api.watcher = watcher.NewNoop() + } } + go api.loop() + + return api +} + +// SignalReady signals the API that we are ready to begin watching for +// file changes. This is used to prime the cache with the current list +// of containers and to start watching the devcontainer config files for +// changes. It should be called after the agent ready. +func (api *API) SignalReady() { + // Prime the cache with the current list of containers. + _, _ = api.cl.List(api.ctx) + // Make sure we watch the devcontainer config files for changes. for _, devcontainer := range api.knownDevcontainers { - if devcontainer.ConfigPath != "" { - if err := api.watcher.Add(devcontainer.ConfigPath); err != nil { - api.logger.Error(ctx, "watch devcontainer config file failed", slog.Error(err), slog.F("file", devcontainer.ConfigPath)) - } + if devcontainer.ConfigPath == "" { + continue } - } - go api.start() - - return api + if err := api.watcher.Add(devcontainer.ConfigPath); err != nil { + api.logger.Error(api.ctx, "watch devcontainer config file failed", slog.Error(err), slog.F("file", devcontainer.ConfigPath)) + } + } } -func (api *API) start() { +func (api *API) loop() { defer close(api.done) for { @@ -187,9 +212,11 @@ func (api *API) start() { // Routes returns the HTTP handler for container-related routes. 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 } diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index a246d929d9089..45044b4e43e2e 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -18,6 +18,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/agent/agentcontainers" + "github.com/coder/coder/v2/agent/agentcontainers/watcher" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" "github.com/coder/quartz" @@ -253,6 +254,7 @@ func TestAPI(t *testing.T) { logger, agentcontainers.WithLister(tt.lister), agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI), + agentcontainers.WithWatcher(watcher.NewNoop()), ) defer api.Close() r.Mount("/", api.Routes()) @@ -558,6 +560,7 @@ func TestAPI(t *testing.T) { r := chi.NewRouter() apiOptions := []agentcontainers.Option{ agentcontainers.WithLister(tt.lister), + agentcontainers.WithWatcher(watcher.NewNoop()), } if len(tt.knownDevcontainers) > 0 { @@ -631,6 +634,8 @@ func TestAPI(t *testing.T) { ) defer api.Close() + api.SignalReady() + r := chi.NewRouter() r.Mount("/", api.Routes()) diff --git a/agent/api.go b/agent/api.go index 97a04333f147e..f09d39b172bd5 100644 --- a/agent/api.go +++ b/agent/api.go @@ -37,10 +37,10 @@ func (a *agent) apiHandler() (http.Handler, func() error) { cacheDuration: cacheDuration, } - containerAPIOpts := []agentcontainers.Option{ - agentcontainers.WithLister(a.lister), - } if a.experimentalDevcontainersEnabled { + containerAPIOpts := []agentcontainers.Option{ + agentcontainers.WithExecer(a.execer), + } manifest := a.manifest.Load() if manifest != nil && len(manifest.Devcontainers) > 0 { containerAPIOpts = append( @@ -48,12 +48,24 @@ func (a *agent) apiHandler() (http.Handler, func() error) { agentcontainers.WithDevcontainers(manifest.Devcontainers), ) } + + // Append after to allow the agent options to override the default options. + containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...) + + containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...) + r.Mount("/api/v0/containers", containerAPI.Routes()) + a.containerAPI.Store(containerAPI) + } else { + r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusForbidden, codersdk.Response{ + Message: "The agent dev containers feature is experimental and not enabled by default.", + Detail: "To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.", + }) + }) } - containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...) promHandler := PrometheusMetricsHandler(a.prometheusRegistry, a.logger) - r.Mount("/api/v0/containers", containerAPI.Routes()) r.Get("/api/v0/listening-ports", lp.handler) r.Get("/api/v0/netcheck", a.HandleNetcheck) r.Post("/api/v0/list-directory", a.HandleLS) @@ -64,7 +76,10 @@ func (a *agent) apiHandler() (http.Handler, func() error) { r.Get("/debug/prometheus", promHandler.ServeHTTP) return r, func() error { - return containerAPI.Close() + if containerAPI := a.containerAPI.Load(); containerAPI != nil { + return containerAPI.Close() + } + return nil } } diff --git a/cli/agent.go b/cli/agent.go index 18c4542a6c3a0..5d6cdbd66b4e0 100644 --- a/cli/agent.go +++ b/cli/agent.go @@ -26,7 +26,6 @@ import ( "cdr.dev/slog/sloggers/slogjson" "cdr.dev/slog/sloggers/slogstackdriver" "github.com/coder/coder/v2/agent" - "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentexec" "github.com/coder/coder/v2/agent/agentssh" "github.com/coder/coder/v2/agent/reaper" @@ -319,13 +318,10 @@ func (r *RootCmd) workspaceAgent() *serpent.Command { return xerrors.Errorf("create agent execer: %w", err) } - var containerLister agentcontainers.Lister - if !experimentalDevcontainersEnabled { - logger.Info(ctx, "agent devcontainer detection not enabled") - containerLister = &agentcontainers.NoopLister{} - } else { + if experimentalDevcontainersEnabled { logger.Info(ctx, "agent devcontainer detection enabled") - containerLister = agentcontainers.NewDocker(execer) + } else { + logger.Info(ctx, "agent devcontainer detection not enabled") } agnt := agent.New(agent.Options{ @@ -354,7 +350,6 @@ func (r *RootCmd) workspaceAgent() *serpent.Command { PrometheusRegistry: prometheusRegistry, BlockFileTransfer: blockFileTransfer, Execer: execer, - ContainerLister: containerLister, ExperimentalDevcontainersEnabled: experimentalDevcontainersEnabled, }) diff --git a/cli/exp_rpty_test.go b/cli/exp_rpty_test.go index b7f26beb87f2f..355cc1741b5a9 100644 --- a/cli/exp_rpty_test.go +++ b/cli/exp_rpty_test.go @@ -9,7 +9,6 @@ import ( "github.com/ory/dockertest/v3/docker" "github.com/coder/coder/v2/agent" - "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agenttest" "github.com/coder/coder/v2/cli/clitest" "github.com/coder/coder/v2/coderd/coderdtest" @@ -112,7 +111,6 @@ func TestExpRpty(t *testing.T) { _ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) { o.ExperimentalDevcontainersEnabled = true - o.ContainerLister = agentcontainers.NewDocker(o.Execer) }) _ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait() diff --git a/cli/open_test.go b/cli/open_test.go index f0183022782d9..9ba16a32674e2 100644 --- a/cli/open_test.go +++ b/cli/open_test.go @@ -14,6 +14,7 @@ import ( "go.uber.org/mock/gomock" "github.com/coder/coder/v2/agent" + "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentcontainers/acmock" "github.com/coder/coder/v2/agent/agenttest" "github.com/coder/coder/v2/cli/clitest" @@ -335,7 +336,8 @@ func TestOpenVSCodeDevContainer(t *testing.T) { }) _ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) { - o.ContainerLister = mcl + o.ExperimentalDevcontainersEnabled = true + o.ContainerAPIOptions = append(o.ContainerAPIOptions, agentcontainers.WithLister(mcl)) }) _ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait() @@ -508,7 +510,8 @@ func TestOpenVSCodeDevContainer_NoAgentDirectory(t *testing.T) { }) _ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) { - o.ContainerLister = mcl + o.ExperimentalDevcontainersEnabled = true + o.ContainerAPIOptions = append(o.ContainerAPIOptions, agentcontainers.WithLister(mcl)) }) _ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait() diff --git a/cli/ssh.go b/cli/ssh.go index e02443e7032c6..2025c1691b7d7 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -299,8 +299,6 @@ func (r *RootCmd) ssh() *serpent.Command { } if len(cts.Containers) == 0 { cliui.Info(inv.Stderr, "No containers found!") - cliui.Info(inv.Stderr, "Tip: Agent container integration is experimental and not enabled by default.") - cliui.Info(inv.Stderr, " To enable it, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.") return nil } var found bool diff --git a/cli/ssh_test.go b/cli/ssh_test.go index c8ad072270169..2603c81e88cec 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -2029,7 +2029,6 @@ func TestSSH_Container(t *testing.T) { _ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) { o.ExperimentalDevcontainersEnabled = true - o.ContainerLister = agentcontainers.NewDocker(o.Execer) }) _ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait() @@ -2058,7 +2057,7 @@ func TestSSH_Container(t *testing.T) { mLister := acmock.NewMockLister(ctrl) _ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) { o.ExperimentalDevcontainersEnabled = true - o.ContainerLister = mLister + o.ContainerAPIOptions = append(o.ContainerAPIOptions, agentcontainers.WithLister(mLister)) }) _ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait() @@ -2097,16 +2096,9 @@ func TestSSH_Container(t *testing.T) { inv, root := clitest.New(t, "ssh", workspace.Name, "-c", uuid.NewString()) clitest.SetupConfig(t, client, root) - ptty := ptytest.New(t).Attach(inv) - - cmdDone := tGo(t, func() { - err := inv.WithContext(ctx).Run() - assert.NoError(t, err) - }) - ptty.ExpectMatch("No containers found!") - ptty.ExpectMatch("Tip: Agent container integration is experimental and not enabled by default.") - <-cmdDone + err := inv.WithContext(ctx).Run() + require.ErrorContains(t, err, "The agent dev containers feature is experimental and not enabled by default.") }) } diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 1388b61030d38..98e803581b946 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -848,6 +848,11 @@ func (api *API) workspaceAgentListContainers(rw http.ResponseWriter, r *http.Req }) return } + // If the agent returns a codersdk.Error, we can return that directly. + if cerr, ok := codersdk.AsError(err); ok { + httpapi.Write(ctx, rw, cerr.StatusCode(), cerr.Response) + return + } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching containers.", Detail: err.Error(), diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index a6e10ea5fdabf..7e3d141ebb09d 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -35,7 +35,6 @@ import ( "github.com/coder/coder/v2/agent" "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentcontainers/acmock" - "github.com/coder/coder/v2/agent/agentexec" "github.com/coder/coder/v2/agent/agenttest" agentproto "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/coderd/coderdtest" @@ -1171,8 +1170,8 @@ func TestWorkspaceAgentContainers(t *testing.T) { }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { return agents }).Do() - _ = agenttest.New(t, client.URL, r.AgentToken, func(opts *agent.Options) { - opts.ContainerLister = agentcontainers.NewDocker(agentexec.DefaultExecer) + _ = agenttest.New(t, client.URL, r.AgentToken, func(o *agent.Options) { + o.ExperimentalDevcontainersEnabled = true }) resources := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID).Wait() require.Len(t, resources, 1, "expected one resource") @@ -1273,8 +1272,9 @@ func TestWorkspaceAgentContainers(t *testing.T) { }).WithAgent(func(agents []*proto.Agent) []*proto.Agent { return agents }).Do() - _ = agenttest.New(t, client.URL, r.AgentToken, func(opts *agent.Options) { - opts.ContainerLister = mcl + _ = agenttest.New(t, client.URL, r.AgentToken, func(o *agent.Options) { + o.ExperimentalDevcontainersEnabled = true + o.ContainerAPIOptions = append(o.ContainerAPIOptions, agentcontainers.WithLister(mcl)) }) resources := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID).Wait() require.Len(t, resources, 1, "expected one resource") diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 0e29fa969c903..260f5d4880ef2 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -2447,11 +2447,20 @@ class ApiMethods { labels?.map((label) => ["label", label]), ); - const res = - await this.axios.get( - `/api/v2/workspaceagents/${agentId}/containers?${params.toString()}`, - ); - return res.data; + try { + const res = + await this.axios.get( + `/api/v2/workspaceagents/${agentId}/containers?${params.toString()}`, + ); + return res.data; + } catch (err) { + // If the error is a 403, it means that experimental + // containers are not enabled on the agent. + if (isAxiosError(err) && err.response?.status === 403) { + return { containers: [] }; + } + throw err; + } }; getInboxNotifications = async (startingBeforeId?: string) => {