Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1fc74f6

Browse files
authored
refactor(agent): update agentcontainers api initialization (#17600)
There were too many ways to configure the agentcontainers API resultingin inconsistent behavior or features not being enabled. This refactorintroduces a control flag for enabling or disabling the containers API.When disabled, all implementations are no-op and explicit endpointbehaviors are defined. When enabled, concrete implementations are usedby default but can be overridden by passing options.
1 parent22b932a commit1fc74f6

File tree

12 files changed

+118
-67
lines changed

12 files changed

+118
-67
lines changed

‎agent/agent.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ type Options struct {
8989
ServiceBannerRefreshInterval time.Duration
9090
BlockFileTransferbool
9191
Execer agentexec.Execer
92-
ContainerLister agentcontainers.Lister
9392

9493
ExperimentalDevcontainersEnabledbool
94+
ContainerAPIOptions []agentcontainers.Option// Enable ExperimentalDevcontainersEnabled for these to be effective.
9595
}
9696

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

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

197193
experimentalDevcontainersEnabled:options.ExperimentalDevcontainersEnabled,
194+
containerAPIOptions:options.ContainerAPIOptions,
198195
}
199196
// Initially, we have a closed channel, reflecting the fact that we are not initially connected.
200197
// Each time we connect we replace the channel (while holding the closeMutex) with a new one
@@ -274,9 +271,10 @@ type agent struct {
274271
// labeled in Coder with the agent + workspace.
275272
metrics*agentMetrics
276273
execer agentexec.Execer
277-
lister agentcontainers.Lister
278274

279275
experimentalDevcontainersEnabledbool
276+
containerAPIOptions []agentcontainers.Option
277+
containerAPI atomic.Pointer[agentcontainers.API]// Set by apiHandler.
280278
}
281279

282280
func (a*agent)TailnetConn()*tailnet.Conn {
@@ -1170,6 +1168,12 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
11701168
}
11711169
a.metrics.startupScriptSeconds.WithLabelValues(label).Set(dur)
11721170
a.scriptRunner.StartCron()
1171+
ifcontainerAPI:=a.containerAPI.Load();containerAPI!=nil {
1172+
// Inform the container API that the agent is ready.
1173+
// This allows us to start watching for changes to
1174+
// the devcontainer configuration files.
1175+
containerAPI.SignalReady()
1176+
}
11731177
})
11741178
iferr!=nil {
11751179
returnxerrors.Errorf("track conn goroutine: %w",err)

‎agent/agentcontainers/api.go‎

Lines changed: 47 additions & 20 deletions
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
clLister
4344
dccliDevcontainerCLI
4445
clock quartz.Clock
@@ -56,14 +57,6 @@ type API struct {
5657
// Option is a functional option for API.
5758
typeOptionfunc(*API)
5859

59-
// WithLister sets the agentcontainers.Lister implementation to use.
60-
// The default implementation uses the Docker CLI to list containers.
61-
funcWithLister(clLister)Option {
62-
returnfunc(api*API) {
63-
api.cl=cl
64-
}
65-
}
66-
6760
// WithClock sets the quartz.Clock implementation to use.
6861
// This is primarily used for testing to control time.
6962
funcWithClock(clock quartz.Clock)Option {
@@ -72,6 +65,21 @@ func WithClock(clock quartz.Clock) Option {
7265
}
7366
}
7467

68+
// WithExecer sets the agentexec.Execer implementation to use.
69+
funcWithExecer(execer agentexec.Execer)Option {
70+
returnfunc(api*API) {
71+
api.execer=execer
72+
}
73+
}
74+
75+
// WithLister sets the agentcontainers.Lister implementation to use.
76+
// The default implementation uses the Docker CLI to list containers.
77+
funcWithLister(clLister)Option {
78+
returnfunc(api*API) {
79+
api.cl=cl
80+
}
81+
}
82+
7583
// WithDevcontainerCLI sets the DevcontainerCLI implementation to use.
7684
// This can be used in tests to modify @devcontainer/cli behavior.
7785
funcWithDevcontainerCLI(dccliDevcontainerCLI)Option {
@@ -113,6 +121,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
113121
done:make(chanstruct{}),
114122
logger:logger,
115123
clock:quartz.NewReal(),
124+
execer:agentexec.DefaultExecer,
116125
cacheDuration:defaultGetContainersCacheDuration,
117126
lockCh:make(chanstruct{},1),
118127
devcontainerNames:make(map[string]struct{}),
@@ -123,30 +132,46 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
123132
opt(api)
124133
}
125134
ifapi.cl==nil {
126-
api.cl=&DockerCLILister{}
135+
api.cl=NewDocker(api.execer)
127136
}
128137
ifapi.dccli==nil {
129-
api.dccli=NewDevcontainerCLI(logger.Named("devcontainer-cli"),agentexec.DefaultExecer)
138+
api.dccli=NewDevcontainerCLI(logger.Named("devcontainer-cli"),api.execer)
130139
}
131140
ifapi.watcher==nil {
132-
api.watcher=watcher.NewNoop()
141+
varerrerror
142+
api.watcher,err=watcher.NewFSNotify()
143+
iferr!=nil {
144+
logger.Error(ctx,"create file watcher service failed",slog.Error(err))
145+
api.watcher=watcher.NewNoop()
146+
}
133147
}
134148

149+
goapi.loop()
150+
151+
returnapi
152+
}
153+
154+
// SignalReady signals the API that we are ready to begin watching for
155+
// file changes. This is used to prime the cache with the current list
156+
// of containers and to start watching the devcontainer config files for
157+
// changes. It should be called after the agent ready.
158+
func (api*API)SignalReady() {
159+
// Prime the cache with the current list of containers.
160+
_,_=api.cl.List(api.ctx)
161+
135162
// Make sure we watch the devcontainer config files for changes.
136163
for_,devcontainer:=rangeapi.knownDevcontainers {
137-
ifdevcontainer.ConfigPath!="" {
138-
iferr:=api.watcher.Add(devcontainer.ConfigPath);err!=nil {
139-
api.logger.Error(ctx,"watch devcontainer config file failed",slog.Error(err),slog.F("file",devcontainer.ConfigPath))
140-
}
164+
ifdevcontainer.ConfigPath=="" {
165+
continue
141166
}
142-
}
143167

144-
goapi.start()
145-
146-
returnapi
168+
iferr:=api.watcher.Add(devcontainer.ConfigPath);err!=nil {
169+
api.logger.Error(api.ctx,"watch devcontainer config file failed",slog.Error(err),slog.F("file",devcontainer.ConfigPath))
170+
}
171+
}
147172
}
148173

149-
func (api*API)start() {
174+
func (api*API)loop() {
150175
deferclose(api.done)
151176

152177
for {
@@ -187,9 +212,11 @@ func (api *API) start() {
187212
// Routes returns the HTTP handler for container-related routes.
188213
func (api*API)Routes() http.Handler {
189214
r:=chi.NewRouter()
215+
190216
r.Get("/",api.handleList)
191217
r.Get("/devcontainers",api.handleListDevcontainers)
192218
r.Post("/{id}/recreate",api.handleRecreate)
219+
193220
returnr
194221
}
195222

‎agent/agentcontainers/api_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"cdr.dev/slog"
1919
"cdr.dev/slog/sloggers/slogtest"
2020
"github.com/coder/coder/v2/agent/agentcontainers"
21+
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
2122
"github.com/coder/coder/v2/codersdk"
2223
"github.com/coder/coder/v2/testutil"
2324
"github.com/coder/quartz"
@@ -253,6 +254,7 @@ func TestAPI(t *testing.T) {
253254
logger,
254255
agentcontainers.WithLister(tt.lister),
255256
agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI),
257+
agentcontainers.WithWatcher(watcher.NewNoop()),
256258
)
257259
deferapi.Close()
258260
r.Mount("/",api.Routes())
@@ -558,6 +560,7 @@ func TestAPI(t *testing.T) {
558560
r:=chi.NewRouter()
559561
apiOptions:= []agentcontainers.Option{
560562
agentcontainers.WithLister(tt.lister),
563+
agentcontainers.WithWatcher(watcher.NewNoop()),
561564
}
562565

563566
iflen(tt.knownDevcontainers)>0 {
@@ -631,6 +634,8 @@ func TestAPI(t *testing.T) {
631634
)
632635
deferapi.Close()
633636

637+
api.SignalReady()
638+
634639
r:=chi.NewRouter()
635640
r.Mount("/",api.Routes())
636641

‎agent/api.go‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,23 +37,35 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
3737
cacheDuration:cacheDuration,
3838
}
3939

40-
containerAPIOpts:= []agentcontainers.Option{
41-
agentcontainers.WithLister(a.lister),
42-
}
4340
ifa.experimentalDevcontainersEnabled {
41+
containerAPIOpts:= []agentcontainers.Option{
42+
agentcontainers.WithExecer(a.execer),
43+
}
4444
manifest:=a.manifest.Load()
4545
ifmanifest!=nil&&len(manifest.Devcontainers)>0 {
4646
containerAPIOpts=append(
4747
containerAPIOpts,
4848
agentcontainers.WithDevcontainers(manifest.Devcontainers),
4949
)
5050
}
51+
52+
// Append after to allow the agent options to override the default options.
53+
containerAPIOpts=append(containerAPIOpts,a.containerAPIOptions...)
54+
55+
containerAPI:=agentcontainers.NewAPI(a.logger.Named("containers"),containerAPIOpts...)
56+
r.Mount("/api/v0/containers",containerAPI.Routes())
57+
a.containerAPI.Store(containerAPI)
58+
}else {
59+
r.HandleFunc("/api/v0/containers",func(w http.ResponseWriter,r*http.Request) {
60+
httpapi.Write(r.Context(),w,http.StatusForbidden, codersdk.Response{
61+
Message:"The agent dev containers feature is experimental and not enabled by default.",
62+
Detail:"To enable this feature, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.",
63+
})
64+
})
5165
}
52-
containerAPI:=agentcontainers.NewAPI(a.logger.Named("containers"),containerAPIOpts...)
5366

5467
promHandler:=PrometheusMetricsHandler(a.prometheusRegistry,a.logger)
5568

56-
r.Mount("/api/v0/containers",containerAPI.Routes())
5769
r.Get("/api/v0/listening-ports",lp.handler)
5870
r.Get("/api/v0/netcheck",a.HandleNetcheck)
5971
r.Post("/api/v0/list-directory",a.HandleLS)
@@ -64,7 +76,10 @@ func (a *agent) apiHandler() (http.Handler, func() error) {
6476
r.Get("/debug/prometheus",promHandler.ServeHTTP)
6577

6678
returnr,func()error {
67-
returncontainerAPI.Close()
79+
ifcontainerAPI:=a.containerAPI.Load();containerAPI!=nil {
80+
returncontainerAPI.Close()
81+
}
82+
returnnil
6883
}
6984
}
7085

‎cli/agent.go‎

Lines changed: 3 additions & 8 deletions
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
returnxerrors.Errorf("create agent execer: %w",err)
320319
}
321320

322-
varcontainerLister agentcontainers.Lister
323-
if!experimentalDevcontainersEnabled {
324-
logger.Info(ctx,"agent devcontainer detection not enabled")
325-
containerLister=&agentcontainers.NoopLister{}
326-
}else {
321+
ifexperimentalDevcontainersEnabled {
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‎

Lines changed: 0 additions & 2 deletions
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‎

Lines changed: 5 additions & 2 deletions
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.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,6 @@ func (r *RootCmd) ssh() *serpent.Command {
299299
}
300300
iflen(cts.Containers)==0 {
301301
cliui.Info(inv.Stderr,"No containers found!")
302-
cliui.Info(inv.Stderr,"Tip: Agent container integration is experimental and not enabled by default.")
303-
cliui.Info(inv.Stderr," To enable it, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.")
304302
returnnil
305303
}
306304
varfoundbool

‎cli/ssh_test.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,7 +2029,6 @@ func TestSSH_Container(t *testing.T) {
20292029

20302030
_=agenttest.New(t,client.URL,agentToken,func(o*agent.Options) {
20312031
o.ExperimentalDevcontainersEnabled=true
2032-
o.ContainerLister=agentcontainers.NewDocker(o.Execer)
20332032
})
20342033
_=coderdtest.NewWorkspaceAgentWaiter(t,client,workspace.ID).Wait()
20352034

@@ -2058,7 +2057,7 @@ func TestSSH_Container(t *testing.T) {
20582057
mLister:=acmock.NewMockLister(ctrl)
20592058
_=agenttest.New(t,client.URL,agentToken,func(o*agent.Options) {
20602059
o.ExperimentalDevcontainersEnabled=true
2061-
o.ContainerLister=mLister
2060+
o.ContainerAPIOptions=append(o.ContainerAPIOptions,agentcontainers.WithLister(mLister))
20622061
})
20632062
_=coderdtest.NewWorkspaceAgentWaiter(t,client,workspace.ID).Wait()
20642063

@@ -2097,16 +2096,9 @@ func TestSSH_Container(t *testing.T) {
20972096

20982097
inv,root:=clitest.New(t,"ssh",workspace.Name,"-c",uuid.NewString())
20992098
clitest.SetupConfig(t,client,root)
2100-
ptty:=ptytest.New(t).Attach(inv)
2101-
2102-
cmdDone:=tGo(t,func() {
2103-
err:=inv.WithContext(ctx).Run()
2104-
assert.NoError(t,err)
2105-
})
21062099

2107-
ptty.ExpectMatch("No containers found!")
2108-
ptty.ExpectMatch("Tip: Agent container integration is experimental and not enabled by default.")
2109-
<-cmdDone
2100+
err:=inv.WithContext(ctx).Run()
2101+
require.ErrorContains(t,err,"The agent dev containers feature is experimental and not enabled by default.")
21102102
})
21112103
}
21122104

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp