refactor, remove control flag

coder · mafredri · Apr 29, 2025 · Apr 29, 2025 · Apr 29, 2025 · Apr 29, 2025
commit ebed3de758dde43f813c08d5164f2f511898d2e8
diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go
@@ -52,10 +52,6 @@ type API struct {
 	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.
@@ -117,9 +113,7 @@ func WithWatcher(w watcher.Watcher) Option {
 }
 
 // NewAPI returns a new API with the given options applied.
-//
-//nolint:revive // experimentalDevcontainersEnabled is a control flag.
-func NewAPI(logger slog.Logger, experimentalDevcontainersEnabled bool, options ...Option) *API {
+func NewAPI(logger slog.Logger, options ...Option) *API {
 	ctx, cancel := context.WithCancel(context.Background())
 	api := &API{
 		ctx:                     ctx,
@@ -133,34 +127,23 @@ func NewAPI(logger slog.Logger, experimentalDevcontainersEnabled bool, options .
 		devcontainerNames:       make(map[string]struct{}),
 		knownDevcontainers:      []codersdk.WorkspaceAgentDevcontainer{},
 		configFileModifiedTimes: make(map[string]time.Time),
-
-		experimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
 	}
 	for _, opt := range options {
 		opt(api)
 	}
-	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 {
-		if api.cl != nil || api.dccli != nil || api.watcher != nil {
-			logger.Warn(ctx, "devcontainers are disabled but API is configured with devcontainer services")
+	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()
 		}
-		api.cl = &NoopLister{}
-		api.dccli = &noopDevcontainerCLI{}
-		api.watcher = watcher.NewNoop()
 	}
 
 	// Make sure we watch the devcontainer config files for changes.
@@ -219,34 +202,13 @@ func (api *API) start() {
 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) 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) 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, true, WithLister(mockLister))
+					api        = NewAPI(logger, WithLister(mockLister))
 				)
 				defer api.Close()
 

diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go
@@ -251,7 +251,6 @@ func TestAPI(t *testing.T) {
 				r := chi.NewRouter()
 				api := agentcontainers.NewAPI(
 					logger,
-					true,
 					agentcontainers.WithLister(tt.lister),
 					agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI),
 				)
@@ -565,7 +564,7 @@ func TestAPI(t *testing.T) {
 					apiOptions = append(apiOptions, agentcontainers.WithDevcontainers(tt.knownDevcontainers))
 				}
 
-				api := agentcontainers.NewAPI(logger, true, apiOptions...)
+				api := agentcontainers.NewAPI(logger, apiOptions...)
 				defer api.Close()
 				r.Mount("/", api.Routes())
 
@@ -626,7 +625,6 @@ 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/api.go b/agent/api.go
@@ -37,21 +37,33 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
 		cacheDuration: cacheDuration,
 	}
 
-	var containerAPIOpts []agentcontainers.Option
+	var containerAPI *agentcontainers.API
 	if a.experimentalDevcontainersEnabled {
+		containerAPIOpts := []agentcontainers.Option{
+			agentcontainers.WithExecer(a.execer),
+		}
 		manifest := a.manifest.Load()
 		if manifest != nil && len(manifest.Devcontainers) > 0 {
 			containerAPIOpts = append(
 				containerAPIOpts,
 				agentcontainers.WithDevcontainers(manifest.Devcontainers),
 			)
 		}
+
+		containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
+
+		r.Mount("/api/v0/containers", containerAPI.Routes())
+	} else {
+		r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) {
+			httpapi.Write(r.Context(), w, http.StatusNotFound, codersdk.Response{
+				Message: "Container API is not enabled.",
+				Detail:  "Set the --experimental-devcontainers flag to enable the container API.",
+			})
+		})
 	}
-	containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), a.experimentalDevcontainersEnabled, 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)
@@ -62,7 +74,10 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
 	r.Get("/debug/prometheus", promHandler.ServeHTTP)
 
 	return r, func() error {
-		return containerAPI.Close()
+		if containerAPI != nil {
+			return containerAPI.Close()
+		}
+		return nil
 	}
 }