Skip to content

Commit 48cde34

Browse files
mafredrimafredri
committed
refactor, remove control flag
1 parent 44cc9a6 commit 48cde34

File tree

4 files changed

+34
-46
lines changed

4 files changed

+34
-46
lines changed

‎agent/agentcontainers/api.go

+13-38
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,6 @@ type API struct {
5252
devcontainerNames map[string]struct{} // Track devcontainer names to avoid duplicates.
5353
knownDevcontainers []codersdk.WorkspaceAgentDevcontainer // Track predefined and runtime-detected devcontainers.
5454
configFileModifiedTimes map[string]time.Time // Track when config files were last modified.
55-
56-
// experimentalDevcontainersEnabled indicates if the agent is
57-
// running in experimental mode with devcontainers enabled.
58-
experimentalDevcontainersEnabled bool
5955
}
6056

6157
// Option is a functional option for API.
@@ -117,9 +113,7 @@ func WithWatcher(w watcher.Watcher) Option {
117113
}
118114

119115
// NewAPI returns a new API with the given options applied.
120-
//
121-
//nolint:revive // experimentalDevcontainersEnabled is a control flag.
122-
func NewAPI(logger slog.Logger, experimentalDevcontainersEnabled bool, options ...Option) *API {
116+
func NewAPI(logger slog.Logger, options ...Option) *API {
123117
ctx, cancel := context.WithCancel(context.Background())
124118
api := &API{
125119
ctx: ctx,
@@ -133,34 +127,23 @@ func NewAPI(logger slog.Logger, experimentalDevcontainersEnabled bool, options .
133127
devcontainerNames: make(map[string]struct{}),
134128
knownDevcontainers: []codersdk.WorkspaceAgentDevcontainer{},
135129
configFileModifiedTimes: make(map[string]time.Time),
136-
137-
experimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
138130
}
139131
for _, opt := range options {
140132
opt(api)
141133
}
142-
if api.experimentalDevcontainersEnabled {
143-
if api.cl == nil {
144-
api.cl = NewDocker(api.execer)
145-
}
146-
if api.dccli == nil {
147-
api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), api.execer)
148-
}
149-
if api.watcher == nil {
150-
var err error
151-
api.watcher, err = watcher.NewFSNotify()
152-
if err != nil {
153-
logger.Error(ctx, "create file watcher service failed", slog.Error(err))
154-
api.watcher = watcher.NewNoop()
155-
}
156-
}
157-
} else {
158-
if api.cl != nil || api.dccli != nil || api.watcher != nil {
159-
logger.Warn(ctx, "devcontainers are disabled but API is configured with devcontainer services")
134+
if api.cl == nil {
135+
api.cl = NewDocker(api.execer)
136+
}
137+
if api.dccli == nil {
138+
api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), api.execer)
139+
}
140+
if api.watcher == nil {
141+
var err error
142+
api.watcher, err = watcher.NewFSNotify()
143+
if err != nil {
144+
logger.Error(ctx, "create file watcher service failed", slog.Error(err))
145+
api.watcher = watcher.NewNoop()
160146
}
161-
api.cl = &NoopLister{}
162-
api.dccli = &noopDevcontainerCLI{}
163-
api.watcher = watcher.NewNoop()
164147
}
165148

166149
// Make sure we watch the devcontainer config files for changes.
@@ -219,14 +202,6 @@ func (api *API) start() {
219202
func (api *API) Routes() http.Handler {
220203
r := chi.NewRouter()
221204

222-
if !api.experimentalDevcontainersEnabled {
223-
r.Get("/", api.handleDisabledEmptyList)
224-
r.Get("/devcontainers", api.handleDisabled)
225-
r.Post("/{id}/recreate", api.handleDisabled)
226-
227-
return r
228-
}
229-
230205
r.Get("/", api.handleList)
231206
r.Get("/devcontainers", api.handleListDevcontainers)
232207
r.Post("/{id}/recreate", api.handleRecreate)

‎agent/agentcontainers/api_internal_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func TestAPI(t *testing.T) {
101101
mockLister = acmock.NewMockLister(ctrl)
102102
now = time.Now().UTC()
103103
logger = slogtest.Make(t, nil).Leveled(slog.LevelDebug)
104-
api = NewAPI(logger, true, WithLister(mockLister))
104+
api = NewAPI(logger, WithLister(mockLister))
105105
)
106106
defer api.Close()
107107

‎agent/agentcontainers/api_test.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,6 @@ func TestAPI(t *testing.T) {
251251
r := chi.NewRouter()
252252
api := agentcontainers.NewAPI(
253253
logger,
254-
true,
255254
agentcontainers.WithLister(tt.lister),
256255
agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI),
257256
)
@@ -565,7 +564,7 @@ func TestAPI(t *testing.T) {
565564
apiOptions = append(apiOptions, agentcontainers.WithDevcontainers(tt.knownDevcontainers))
566565
}
567566

568-
api := agentcontainers.NewAPI(logger, true, apiOptions...)
567+
api := agentcontainers.NewAPI(logger, apiOptions...)
569568
defer api.Close()
570569
r.Mount("/", api.Routes())
571570

@@ -626,7 +625,6 @@ func TestAPI(t *testing.T) {
626625
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
627626
api := agentcontainers.NewAPI(
628627
logger,
629-
true,
630628
agentcontainers.WithLister(fLister),
631629
agentcontainers.WithWatcher(fWatcher),
632630
agentcontainers.WithClock(mClock),

‎agent/api.go

+19-4
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,33 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
3737
cacheDuration: cacheDuration,
3838
}
3939

40-
var containerAPIOpts []agentcontainers.Option
40+
var containerAPI *agentcontainers.API
4141
if a.experimentalDevcontainersEnabled {
42+
containerAPIOpts := []agentcontainers.Option{
43+
agentcontainers.WithExecer(a.execer),
44+
}
4245
manifest := a.manifest.Load()
4346
if manifest != nil && len(manifest.Devcontainers) > 0 {
4447
containerAPIOpts = append(
4548
containerAPIOpts,
4649
agentcontainers.WithDevcontainers(manifest.Devcontainers),
4750
)
4851
}
52+
53+
containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
54+
55+
r.Mount("/api/v0/containers", containerAPI.Routes())
56+
} else {
57+
r.HandleFunc("/api/v0/containers", func(w http.ResponseWriter, r *http.Request) {
58+
httpapi.Write(r.Context(), w, http.StatusNotFound, codersdk.Response{
59+
Message: "Container API is not enabled.",
60+
Detail: "Set the --experimental-devcontainers flag to enable the container API.",
61+
})
62+
})
4963
}
50-
containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), a.experimentalDevcontainersEnabled, containerAPIOpts...)
5164

5265
promHandler := PrometheusMetricsHandler(a.prometheusRegistry, a.logger)
5366

54-
r.Mount("/api/v0/containers", containerAPI.Routes())
5567
r.Get("/api/v0/listening-ports", lp.handler)
5668
r.Get("/api/v0/netcheck", a.HandleNetcheck)
5769
r.Post("/api/v0/list-directory", a.HandleLS)
@@ -62,7 +74,10 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
6274
r.Get("/debug/prometheus", promHandler.ServeHTTP)
6375

6476
return r, func() error {
65-
return containerAPI.Close()
77+
if containerAPI != nil {
78+
return containerAPI.Close()
79+
}
80+
return nil
6681
}
6782
}
6883

0 commit comments

Comments
 (0)