refactor(agent): update agentcontainers api initialization

coder · mafredri · Apr 29, 2025 · Apr 29, 2025 · Apr 29, 2025 · Apr 29, 2025
commit 2fd85f759a4b925db021a86dffb74d4a8ed48ac2
diff --git a/agent/agent.go b/agent/agent.go
@@ -89,8 +89,8 @@
 	ServiceBannerRefreshInterval time.Duration
 	BlockFileTransfer            bool
 	Execer                       agentexec.Execer
-	ContainerLister              agentcontainers.Lister
 
+	ContainerAPIOptions              []agentcontainers.Option
 	ExperimentalDevcontainersEnabled bool
 }
 
@@ -154,9 +154,6 @@
 	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,8 +189,8 @@
 		prometheusRegistry: prometheusRegistry,
 		metrics:            newAgentMetrics(prometheusRegistry),
 		execer:             options.Execer,
-		lister:             options.ContainerLister,
 
+		containerAPIOptions:              options.ContainerAPIOptions,
 		experimentalDevcontainersEnabled: options.ExperimentalDevcontainersEnabled,
 	}
 	// Initially, we have a closed channel, reflecting the fact that we are not initially connected.
@@ -274,8 +271,9 @@
 	// labeled in Coder with the agent + workspace.
 	metrics *agentMetrics
 	execer  agentexec.Execer
 	lister  agentcontainers.Lister
 
+	containerAPIOptions              []agentcontainers.Option
 	experimentalDevcontainersEnabled bool
 }
 

diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go
@@ -39,6 +39,7 @@
 	watcher watcher.Watcher
 
 	cacheDuration time.Duration
+	execer        agentexec.Execer
 	cl            Lister
 	dccli         DevcontainerCLI
 	clock         quartz.Clock
@@ -51,19 +52,15 @@
 	devcontainerNames       map[string]struct{}                   // Track devcontainer names to avoid duplicates.
 	knownDevcontainers      []codersdk.WorkspaceAgentDevcontainer // Track predefined and runtime-detected devcontainers.
 	configFileModifiedTimes map[string]time.Time                  // Track when config files were last modified.
+
+	// experimentalDevcontainersEnabled indicates if the agent is
+	// running in experimental mode with devcontainers enabled.
+	experimentalDevcontainersEnabled bool
 }
 
 // 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 +69,21 @@
 	}
 }
 
+// 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 {
@@ -105,30 +117,46 @@
 }
 
 // NewAPI returns a new API with the given options applied.
-func NewAPI(logger slog.Logger, options ...Option) *API {
+//
+//nolint:revive // experimentalDevcontainersEnabled is a control flag.
+func NewAPI(logger slog.Logger, experimentalDevcontainersEnabled bool, options ...Option) *API {
 	ctx, cancel := context.WithCancel(context.Background())
 	api := &API{
 		ctx:                     ctx,
 		cancel:                  cancel,
 		done:                    make(chan struct{}),
 		logger:                  logger,
 		clock:                   quartz.NewReal(),
+		execer:                  agentexec.DefaultExecer,
 		cacheDuration:           defaultGetContainersCacheDuration,
 		lockCh:                  make(chan struct{}, 1),
 		devcontainerNames:       make(map[string]struct{}),
 		knownDevcontainers:      []codersdk.WorkspaceAgentDevcontainer{},
 		configFileModifiedTimes: make(map[string]time.Time),
+
+		experimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
 	}
 	for _, opt := range options {
 		opt(api)
 	}
-	if api.cl == nil {
-		api.cl = &DockerCLILister{}
-	}
-	if api.dccli == nil {
-		api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), agentexec.DefaultExecer)
-	}
-	if api.watcher == nil {
+	if api.experimentalDevcontainersEnabled {
+		if api.cl == nil {
+			api.cl = NewDocker(api.execer)
+		}
+		if api.dccli == nil {
+			api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), api.execer)
+		}
+		if api.watcher == nil {
+			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()
+			}
+		}
+	} else {
+		api.cl = &NoopLister{}
+		api.dccli = &noopDevcontainerCLI{}
 		api.watcher = watcher.NewNoop()
 	}
 
@@ -187,12 +215,35 @@
 // Routes returns the HTTP handler for container-related routes.
 func (api *API) Routes() http.Handler {
 	r := chi.NewRouter()
+
+	if !api.experimentalDevcontainersEnabled {
+		r.Get("/", api.handleDisabledEmptyList)
+		r.Get("/devcontainers", api.handleDisabled)
+		r.Post("/{id}/recreate", api.handleDisabled)
+
+		return r
+	}
+
 	r.Get("/", api.handleList)
 	r.Get("/devcontainers", api.handleListDevcontainers)
 	r.Post("/{id}/recreate", api.handleRecreate)
+
 	return r
 }
 
+func (api *API) handleDisabled(w http.ResponseWriter, r *http.Request) {
+	httpapi.Write(r.Context(), w, http.StatusNotImplemented, codersdk.Response{
+		Message: "Devcontainers are not enabled in this agent.",
+		Detail:  "Devcontainers are not enabled in this agent.",
+	})
+}
+
+func (api *API) handleDisabledEmptyList(w http.ResponseWriter, r *http.Request) {
+	httpapi.Write(r.Context(), w, http.StatusOK, codersdk.WorkspaceAgentListContainersResponse{
+		Containers: []codersdk.WorkspaceAgentContainer{},
+	})
+}
+
 // handleList handles the HTTP request to list containers.
 func (api *API) handleList(rw http.ResponseWriter, r *http.Request) {
 	select {

diff --git a/agent/agentcontainers/api_internal_test.go b/agent/agentcontainers/api_internal_test.go
@@ -101,7 +101,7 @@ func TestAPI(t *testing.T) {
 					mockLister = acmock.NewMockLister(ctrl)
 					now        = time.Now().UTC()
 					logger     = slogtest.Make(t, nil).Leveled(slog.LevelDebug)
-					api        = NewAPI(logger, WithLister(mockLister))
+					api        = NewAPI(logger, true, WithLister(mockLister))
 				)
 				defer api.Close()
 

diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go
@@ -251,6 +251,7 @@ func TestAPI(t *testing.T) {
 				r := chi.NewRouter()
 				api := agentcontainers.NewAPI(
 					logger,
+					true,
 					agentcontainers.WithLister(tt.lister),
 					agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI),
 				)
@@ -564,7 +565,7 @@ func TestAPI(t *testing.T) {
 					apiOptions = append(apiOptions, agentcontainers.WithDevcontainers(tt.knownDevcontainers))
 				}
 
-				api := agentcontainers.NewAPI(logger, apiOptions...)
+				api := agentcontainers.NewAPI(logger, true, apiOptions...)
 				defer api.Close()
 				r.Mount("/", api.Routes())
 
@@ -625,6 +626,7 @@ func TestAPI(t *testing.T) {
 		logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
 		api := agentcontainers.NewAPI(
 			logger,
+			true,
 			agentcontainers.WithLister(fLister),
 			agentcontainers.WithWatcher(fWatcher),
 			agentcontainers.WithClock(mClock),

diff --git a/agent/agentcontainers/devcontainercli.go b/agent/agentcontainers/devcontainercli.go
@@ -191,3 +191,13 @@
 	}
 	return len(p), nil
 }
+
+// noopDevcontainerCLI is a no-op implementation of the DevcontainerCLI
+// interface. It is used when devcontainers are not enabled.
+type noopDevcontainerCLI struct{}
+
+func (n *noopDevcontainerCLI) Up(context.Context, string, string, ...DevcontainerCLIUpOptions) (string, error) {
+	return "", nil
+}
+
+var _ DevcontainerCLI = &noopDevcontainerCLI{}
diff --git a/agent/api.go b/agent/api.go
@@ -37,9 +37,7 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
 		cacheDuration: cacheDuration,
 	}
 
-	containerAPIOpts := []agentcontainers.Option{
-		agentcontainers.WithLister(a.lister),
-	}
+	var containerAPIOpts []agentcontainers.Option
 	if a.experimentalDevcontainersEnabled {
 		manifest := a.manifest.Load()
 		if manifest != nil && len(manifest.Devcontainers) > 0 {
@@ -49,7 +47,7 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
 			)
 		}
 	}
-	containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
+	containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), a.experimentalDevcontainersEnabled, containerAPIOpts...)
 
 	promHandler := PrometheusMetricsHandler(a.prometheusRegistry, a.logger)
 

diff --git 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
@@ -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
@@ -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_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()
 

diff --git 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")