Skip to content

Commit ef75f7b

Browse files
mafredrimafredri
committed
refactor(agent): update agentcontainers api initialization
1 parent 268a50c commit ef75f7b

11 files changed

+99
-46
lines changed

‎agent/agent.go

+3-5
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ type Options struct {
8989
ServiceBannerRefreshInterval time.Duration
9090
BlockFileTransfer bool
9191
Execer agentexec.Execer
92-
ContainerLister agentcontainers.Lister
9392

93+
ContainerAPIOptions []agentcontainers.Option
9494
ExperimentalDevcontainersEnabled bool
9595
}
9696

@@ -154,9 +154,6 @@ func New(options Options) Agent {
154154
if options.Execer == nil {
155155
options.Execer = agentexec.DefaultExecer
156156
}
157-
if options.ContainerLister == nil {
158-
options.ContainerLister = agentcontainers.NoopLister{}
159-
}
160157

161158
hardCtx, hardCancel := context.WithCancel(context.Background())
162159
gracefulCtx, gracefulCancel := context.WithCancel(hardCtx)
@@ -192,8 +189,8 @@ func New(options Options) Agent {
192189
prometheusRegistry: prometheusRegistry,
193190
metrics: newAgentMetrics(prometheusRegistry),
194191
execer: options.Execer,
195-
lister: options.ContainerLister,
196192

193+
containerAPIOptions: options.ContainerAPIOptions,
197194
experimentalDevcontainersEnabled: options.ExperimentalDevcontainersEnabled,
198195
}
199196
// Initially, we have a closed channel, reflecting the fact that we are not initially connected.
@@ -276,6 +273,7 @@ type agent struct {
276273
execer agentexec.Execer
277274
lister agentcontainers.Lister
278275

276+
containerAPIOptions []agentcontainers.Option
279277
experimentalDevcontainersEnabled bool
280278
}
281279

‎agent/agentcontainers/api.go

+67-16
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ type API struct {
3939
watcher watcher.Watcher
4040

4141
cacheDuration time.Duration
42+
execer agentexec.Execer
4243
cl Lister
4344
dccli DevcontainerCLI
4445
clock quartz.Clock
@@ -51,19 +52,15 @@ type API struct {
5152
devcontainerNames map[string]struct{} // Track devcontainer names to avoid duplicates.
5253
knownDevcontainers []codersdk.WorkspaceAgentDevcontainer // Track predefined and runtime-detected devcontainers.
5354
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
5459
}
5560

5661
// Option is a functional option for API.
5762
type Option func(*API)
5863

59-
// WithLister sets the agentcontainers.Lister implementation to use.
60-
// The default implementation uses the Docker CLI to list containers.
61-
func WithLister(cl Lister) Option {
62-
return func(api *API) {
63-
api.cl = cl
64-
}
65-
}
66-
6764
// WithClock sets the quartz.Clock implementation to use.
6865
// This is primarily used for testing to control time.
6966
func WithClock(clock quartz.Clock) Option {
@@ -72,6 +69,21 @@ func WithClock(clock quartz.Clock) Option {
7269
}
7370
}
7471

72+
// WithExecer sets the agentexec.Execer implementation to use.
73+
func WithExecer(execer agentexec.Execer) Option {
74+
return func(api *API) {
75+
api.execer = execer
76+
}
77+
}
78+
79+
// WithLister sets the agentcontainers.Lister implementation to use.
80+
// The default implementation uses the Docker CLI to list containers.
81+
func WithLister(cl Lister) Option {
82+
return func(api *API) {
83+
api.cl = cl
84+
}
85+
}
86+
7587
// WithDevcontainerCLI sets the DevcontainerCLI implementation to use.
7688
// This can be used in tests to modify @devcontainer/cli behavior.
7789
func WithDevcontainerCLI(dccli DevcontainerCLI) Option {
@@ -105,30 +117,46 @@ func WithWatcher(w watcher.Watcher) Option {
105117
}
106118

107119
// NewAPI returns a new API with the given options applied.
108-
func NewAPI(logger slog.Logger, options ...Option) *API {
120+
//
121+
//nolint:revive // experimentalDevcontainersEnabled is a control flag.
122+
func NewAPI(logger slog.Logger, experimentalDevcontainersEnabled bool, options ...Option) *API {
109123
ctx, cancel := context.WithCancel(context.Background())
110124
api := &API{
111125
ctx: ctx,
112126
cancel: cancel,
113127
done: make(chan struct{}),
114128
logger: logger,
115129
clock: quartz.NewReal(),
130+
execer: agentexec.DefaultExecer,
116131
cacheDuration: defaultGetContainersCacheDuration,
117132
lockCh: make(chan struct{}, 1),
118133
devcontainerNames: make(map[string]struct{}),
119134
knownDevcontainers: []codersdk.WorkspaceAgentDevcontainer{},
120135
configFileModifiedTimes: make(map[string]time.Time),
136+
137+
experimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
121138
}
122139
for _, opt := range options {
123140
opt(api)
124141
}
125-
if api.cl == nil {
126-
api.cl = &DockerCLILister{}
127-
}
128-
if api.dccli == nil {
129-
api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), agentexec.DefaultExecer)
130-
}
131-
if api.watcher == nil {
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+
api.cl = &NoopLister{}
159+
api.dccli = &noopDevcontainerCLI{}
132160
api.watcher = watcher.NewNoop()
133161
}
134162

@@ -187,12 +215,35 @@ func (api *API) start() {
187215
// Routes returns the HTTP handler for container-related routes.
188216
func (api *API) Routes() http.Handler {
189217
r := chi.NewRouter()
218+
219+
if !api.experimentalDevcontainersEnabled {
220+
r.Get("/", api.handleDisabledEmptyList)
221+
r.Get("/devcontainers", api.handleDisabled)
222+
r.Post("/{id}/recreate", api.handleDisabled)
223+
224+
return r
225+
}
226+
190227
r.Get("/", api.handleList)
191228
r.Get("/devcontainers", api.handleListDevcontainers)
192229
r.Post("/{id}/recreate", api.handleRecreate)
230+
193231
return r
194232
}
195233

234+
func (api *API) handleDisabled(w http.ResponseWriter, r *http.Request) {
235+
httpapi.Write(r.Context(), w, http.StatusNotImplemented, codersdk.Response{
236+
Message: "Devcontainers are not enabled in this agent.",
237+
Detail: "Devcontainers are not enabled in this agent.",
238+
})
239+
}
240+
241+
func (api *API) handleDisabledEmptyList(w http.ResponseWriter, r *http.Request) {
242+
httpapi.Write(r.Context(), w, http.StatusOK, codersdk.WorkspaceAgentListContainersResponse{
243+
Containers: []codersdk.WorkspaceAgentContainer{},
244+
})
245+
}
246+
196247
// handleList handles the HTTP request to list containers.
197248
func (api *API) handleList(rw http.ResponseWriter, r *http.Request) {
198249
select {

‎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, WithLister(mockLister))
104+
api = NewAPI(logger, true, WithLister(mockLister))
105105
)
106106
defer api.Close()
107107

‎agent/agentcontainers/api_test.go

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

567-
api := agentcontainers.NewAPI(logger, apiOptions...)
568+
api := agentcontainers.NewAPI(logger, true, apiOptions...)
568569
defer api.Close()
569570
r.Mount("/", api.Routes())
570571

@@ -625,6 +626,7 @@ func TestAPI(t *testing.T) {
625626
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
626627
api := agentcontainers.NewAPI(
627628
logger,
629+
true,
628630
agentcontainers.WithLister(fLister),
629631
agentcontainers.WithWatcher(fWatcher),
630632
agentcontainers.WithClock(mClock),

‎agent/agentcontainers/devcontainercli.go

+10
Original file line numberDiff line numberDiff line change
@@ -191,3 +191,13 @@ func (l *devcontainerCLILogWriter) Write(p []byte) (n int, err error) {
191191
}
192192
return len(p), nil
193193
}
194+
195+
// noopDevcontainerCLI is a no-op implementation of the DevcontainerCLI
196+
// interface. It is used when devcontainers are not enabled.
197+
type noopDevcontainerCLI struct{}
198+
199+
func (n *noopDevcontainerCLI) Up(context.Context, string, string, ...DevcontainerCLIUpOptions) (string, error) {
200+
return "", nil
201+
}
202+
203+
var _ DevcontainerCLI = &noopDevcontainerCLI{}

‎agent/api.go

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

40-
containerAPIOpts := []agentcontainers.Option{
41-
agentcontainers.WithLister(a.lister),
42-
}
40+
var containerAPIOpts []agentcontainers.Option
4341
if a.experimentalDevcontainersEnabled {
4442
manifest := a.manifest.Load()
4543
if manifest != nil && len(manifest.Devcontainers) > 0 {
@@ -49,7 +47,7 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
4947
)
5048
}
5149
}
52-
containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
50+
containerAPI := agentcontainers.NewAPI(a.logger.Named("containers"), a.experimentalDevcontainersEnabled, containerAPIOpts...)
5351

5452
promHandler := PrometheusMetricsHandler(a.prometheusRegistry, a.logger)
5553

‎cli/agent.go

+3-8
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
"cdr.dev/slog/sloggers/slogjson"
2727
"cdr.dev/slog/sloggers/slogstackdriver"
2828
"github.com/coder/coder/v2/agent"
29-
"github.com/coder/coder/v2/agent/agentcontainers"
3029
"github.com/coder/coder/v2/agent/agentexec"
3130
"github.com/coder/coder/v2/agent/agentssh"
3231
"github.com/coder/coder/v2/agent/reaper"
@@ -319,13 +318,10 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
319318
return xerrors.Errorf("create agent execer: %w", err)
320319
}
321320

322-
var containerLister agentcontainers.Lister
323-
if !experimentalDevcontainersEnabled {
324-
logger.Info(ctx, "agent devcontainer detection not enabled")
325-
containerLister = &agentcontainers.NoopLister{}
326-
} else {
321+
if experimentalDevcontainersEnabled {
327322
logger.Info(ctx, "agent devcontainer detection enabled")
328-
containerLister = agentcontainers.NewDocker(execer)
323+
} else {
324+
logger.Info(ctx, "agent devcontainer detection not enabled")
329325
}
330326

331327
agnt := agent.New(agent.Options{
@@ -354,7 +350,6 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
354350
PrometheusRegistry: prometheusRegistry,
355351
BlockFileTransfer: blockFileTransfer,
356352
Execer: execer,
357-
ContainerLister: containerLister,
358353

359354
ExperimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
360355
})

‎cli/exp_rpty_test.go

-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/ory/dockertest/v3/docker"
1010

1111
"github.com/coder/coder/v2/agent"
12-
"github.com/coder/coder/v2/agent/agentcontainers"
1312
"github.com/coder/coder/v2/agent/agenttest"
1413
"github.com/coder/coder/v2/cli/clitest"
1514
"github.com/coder/coder/v2/coderd/coderdtest"
@@ -112,7 +111,6 @@ func TestExpRpty(t *testing.T) {
112111

113112
_ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) {
114113
o.ExperimentalDevcontainersEnabled = true
115-
o.ContainerLister = agentcontainers.NewDocker(o.Execer)
116114
})
117115
_ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait()
118116

‎cli/open_test.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"go.uber.org/mock/gomock"
1515

1616
"github.com/coder/coder/v2/agent"
17+
"github.com/coder/coder/v2/agent/agentcontainers"
1718
"github.com/coder/coder/v2/agent/agentcontainers/acmock"
1819
"github.com/coder/coder/v2/agent/agenttest"
1920
"github.com/coder/coder/v2/cli/clitest"
@@ -335,7 +336,8 @@ func TestOpenVSCodeDevContainer(t *testing.T) {
335336
})
336337

337338
_ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) {
338-
o.ContainerLister = mcl
339+
o.ExperimentalDevcontainersEnabled = true
340+
o.ContainerAPIOptions = append(o.ContainerAPIOptions, agentcontainers.WithLister(mcl))
339341
})
340342
_ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait()
341343

@@ -508,7 +510,8 @@ func TestOpenVSCodeDevContainer_NoAgentDirectory(t *testing.T) {
508510
})
509511

510512
_ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) {
511-
o.ContainerLister = mcl
513+
o.ExperimentalDevcontainersEnabled = true
514+
o.ContainerAPIOptions = append(o.ContainerAPIOptions, agentcontainers.WithLister(mcl))
512515
})
513516
_ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait()
514517

‎cli/ssh_test.go

-2
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636
"golang.org/x/xerrors"
3737

3838
"github.com/coder/coder/v2/agent"
39-
"github.com/coder/coder/v2/agent/agentcontainers"
4039
"github.com/coder/coder/v2/agent/agentcontainers/acmock"
4140
"github.com/coder/coder/v2/agent/agentssh"
4241
"github.com/coder/coder/v2/agent/agenttest"
@@ -2029,7 +2028,6 @@ func TestSSH_Container(t *testing.T) {
20292028

20302029
_ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) {
20312030
o.ExperimentalDevcontainersEnabled = true
2032-
o.ContainerLister = agentcontainers.NewDocker(o.Execer)
20332031
})
20342032
_ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait()
20352033

‎coderd/workspaceagents_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import (
3535
"github.com/coder/coder/v2/agent"
3636
"github.com/coder/coder/v2/agent/agentcontainers"
3737
"github.com/coder/coder/v2/agent/agentcontainers/acmock"
38-
"github.com/coder/coder/v2/agent/agentexec"
3938
"github.com/coder/coder/v2/agent/agenttest"
4039
agentproto "github.com/coder/coder/v2/agent/proto"
4140
"github.com/coder/coder/v2/coderd/coderdtest"
@@ -1171,8 +1170,8 @@ func TestWorkspaceAgentContainers(t *testing.T) {
11711170
}).WithAgent(func(agents []*proto.Agent) []*proto.Agent {
11721171
return agents
11731172
}).Do()
1174-
_ = agenttest.New(t, client.URL, r.AgentToken, func(opts *agent.Options) {
1175-
opts.ContainerLister = agentcontainers.NewDocker(agentexec.DefaultExecer)
1173+
_ = agenttest.New(t, client.URL, r.AgentToken, func(o *agent.Options) {
1174+
o.ExperimentalDevcontainersEnabled = true
11761175
})
11771176
resources := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID).Wait()
11781177
require.Len(t, resources, 1, "expected one resource")
@@ -1273,8 +1272,9 @@ func TestWorkspaceAgentContainers(t *testing.T) {
12731272
}).WithAgent(func(agents []*proto.Agent) []*proto.Agent {
12741273
return agents
12751274
}).Do()
1276-
_ = agenttest.New(t, client.URL, r.AgentToken, func(opts *agent.Options) {
1277-
opts.ContainerLister = mcl
1275+
_ = agenttest.New(t, client.URL, r.AgentToken, func(o *agent.Options) {
1276+
o.ExperimentalDevcontainersEnabled = true
1277+
o.ContainerAPIOptions = append(o.ContainerAPIOptions, agentcontainers.WithLister(mcl))
12781278
})
12791279
resources := coderdtest.NewWorkspaceAgentWaiter(t, client, r.Workspace.ID).Wait()
12801280
require.Len(t, resources, 1, "expected one resource")

0 commit comments

Comments
 (0)