Skip to content

Commit 725f97b

Browse files
SasSwartSasSwart
committed
Merge remote-tracking branch 'origin/main' into jjs/prebuilds-agent-reinit
2 parents 7e8dcee + d9ef6ed commit 725f97b

File tree

252 files changed

+3479
-3501
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

252 files changed

+3479
-3501
lines changed

‎CODEOWNERS

+2
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,5 @@ agent/proto/ @spikecurtis @johnstcn
44
tailnet/proto/ @spikecurtis @johnstcn
55
vpn/vpn.proto @spikecurtis @johnstcn
66
vpn/version.go @spikecurtis @johnstcn
7+
provisionerd/proto/ @spikecurtis @johnstcn
8+
provisionersdk/proto/ @spikecurtis @johnstcn

‎agent/agent.go

+10-6
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ type Options struct {
9191
ServiceBannerRefreshInterval time.Duration
9292
BlockFileTransfer bool
9393
Execer agentexec.Execer
94-
ContainerLister agentcontainers.Lister
9594

9695
ExperimentalDevcontainersEnabled bool
96+
ContainerAPIOptions []agentcontainers.Option // Enable ExperimentalDevcontainersEnabled for these to be effective.
9797
}
9898

9999
type Client interface {
@@ -156,9 +156,6 @@ func New(options Options) Agent {
156156
if options.Execer == nil {
157157
options.Execer = agentexec.DefaultExecer
158158
}
159-
if options.ContainerLister == nil {
160-
options.ContainerLister = agentcontainers.NoopLister{}
161-
}
162159

163160
hardCtx, hardCancel := context.WithCancel(context.Background())
164161
gracefulCtx, gracefulCancel := context.WithCancel(hardCtx)
@@ -194,9 +191,9 @@ func New(options Options) Agent {
194191
prometheusRegistry: prometheusRegistry,
195192
metrics: newAgentMetrics(prometheusRegistry),
196193
execer: options.Execer,
197-
lister: options.ContainerLister,
198194

199195
experimentalDevcontainersEnabled: options.ExperimentalDevcontainersEnabled,
196+
containerAPIOptions: options.ContainerAPIOptions,
200197
}
201198
// Initially, we have a closed channel, reflecting the fact that we are not initially connected.
202199
// Each time we connect we replace the channel (while holding the closeMutex) with a new one
@@ -276,9 +273,10 @@ type agent struct {
276273
// labeled in Coder with the agent + workspace.
277274
metrics *agentMetrics
278275
execer agentexec.Execer
279-
lister agentcontainers.Lister
280276

281277
experimentalDevcontainersEnabled bool
278+
containerAPIOptions []agentcontainers.Option
279+
containerAPI atomic.Pointer[agentcontainers.API] // Set by apiHandler.
282280
}
283281

284282
func (a *agent) TailnetConn() *tailnet.Conn {
@@ -1178,6 +1176,12 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
11781176
}
11791177
a.metrics.startupScriptSeconds.WithLabelValues(label).Set(dur)
11801178
a.scriptRunner.StartCron()
1179+
if containerAPI := a.containerAPI.Load(); containerAPI != nil {
1180+
// Inform the container API that the agent is ready.
1181+
// This allows us to start watching for changes to
1182+
// the devcontainer configuration files.
1183+
containerAPI.SignalReady()
1184+
}
11811185
})
11821186
if err != nil {
11831187
return xerrors.Errorf("track conn goroutine: %w", err)

‎agent/agentcontainers/api.go

+47-20
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
@@ -56,14 +57,6 @@ type API struct {
5657
// Option is a functional option for API.
5758
type Option func(*API)
5859

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-
6760
// WithClock sets the quartz.Clock implementation to use.
6861
// This is primarily used for testing to control time.
6962
func WithClock(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+
func WithExecer(execer agentexec.Execer) Option {
70+
return func(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+
func WithLister(cl Lister) Option {
78+
return func(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
func WithDevcontainerCLI(dccli DevcontainerCLI) Option {
@@ -113,6 +121,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
113121
done: make(chan struct{}),
114122
logger: logger,
115123
clock: quartz.NewReal(),
124+
execer: agentexec.DefaultExecer,
116125
cacheDuration: defaultGetContainersCacheDuration,
117126
lockCh: make(chan struct{}, 1),
118127
devcontainerNames: make(map[string]struct{}),
@@ -123,30 +132,46 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
123132
opt(api)
124133
}
125134
if api.cl == nil {
126-
api.cl = &DockerCLILister{}
135+
api.cl = NewDocker(api.execer)
127136
}
128137
if api.dccli == nil {
129-
api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), agentexec.DefaultExecer)
138+
api.dccli = NewDevcontainerCLI(logger.Named("devcontainer-cli"), api.execer)
130139
}
131140
if api.watcher == nil {
132-
api.watcher = watcher.NewNoop()
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()
146+
}
133147
}
134148

149+
go api.loop()
150+
151+
return api
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 := range api.knownDevcontainers {
137-
if devcontainer.ConfigPath != "" {
138-
if err := 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+
if devcontainer.ConfigPath == "" {
165+
continue
141166
}
142-
}
143167

144-
go api.start()
145-
146-
return api
168+
if err := 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
defer close(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
return r
194221
}
195222

‎agent/agentcontainers/api_test.go

+5
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
defer api.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
if len(tt.knownDevcontainers) > 0 {
@@ -631,6 +634,8 @@ func TestAPI(t *testing.T) {
631634
)
632635
defer api.Close()
633636

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

‎agent/agentscripts/agentscripts.go

+19-4
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"os/user"
1111
"path/filepath"
1212
"sync"
13-
"sync/atomic"
1413
"time"
1514

1615
"github.com/google/uuid"
@@ -104,7 +103,6 @@ type Runner struct {
104103
closed chan struct{}
105104
closeMutex sync.Mutex
106105
cron *cron.Cron
107-
initialized atomic.Bool
108106
scripts []runnerScript
109107
dataDir string
110108
scriptCompleted ScriptCompletedFunc
@@ -113,6 +111,9 @@ type Runner struct {
113111
// execute startup scripts, and scripts on a cron schedule. Both will increment
114112
// this counter.
115113
scriptsExecuted *prometheus.CounterVec
114+
115+
initMutex sync.Mutex
116+
initialized bool
116117
}
117118

118119
// DataDir returns the directory where scripts data is stored.
@@ -154,10 +155,12 @@ func WithPostStartScripts(scripts ...codersdk.WorkspaceAgentScript) InitOption {
154155
// It also schedules any scripts that have a schedule.
155156
// This function must be called before Execute.
156157
func (r *Runner) Init(scripts []codersdk.WorkspaceAgentScript, scriptCompleted ScriptCompletedFunc, opts ...InitOption) error {
157-
if r.initialized.Load() {
158+
r.initMutex.Lock()
159+
defer r.initMutex.Unlock()
160+
if r.initialized {
158161
return xerrors.New("init: already initialized")
159162
}
160-
r.initialized.Store(true)
163+
r.initialized = true
161164
r.scripts = toRunnerScript(scripts...)
162165
r.scriptCompleted = scriptCompleted
163166
for _, opt := range opts {
@@ -227,6 +230,18 @@ const (
227230

228231
// Execute runs a set of scripts according to a filter.
229232
func (r *Runner) Execute(ctx context.Context, option ExecuteOption) error {
233+
initErr := func() error {
234+
r.initMutex.Lock()
235+
defer r.initMutex.Unlock()
236+
if !r.initialized {
237+
return xerrors.New("execute: not initialized")
238+
}
239+
return nil
240+
}()
241+
if initErr != nil {
242+
return initErr
243+
}
244+
230245
var eg errgroup.Group
231246
for _, script := range r.scripts {
232247
runScript := (option == ExecuteStartScripts && script.RunOnStart) ||

‎agent/api.go

+21-6
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
if a.experimentalDevcontainersEnabled {
41+
containerAPIOpts := []agentcontainers.Option{
42+
agentcontainers.WithExecer(a.execer),
43+
}
4444
manifest := a.manifest.Load()
4545
if manifest != 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
return r, func() error {
67-
return containerAPI.Close()
79+
if containerAPI := a.containerAPI.Load(); containerAPI != nil {
80+
return containerAPI.Close()
81+
}
82+
return nil
6883
}
6984
}
7085

‎cli/agent.go

+3-8
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"github.com/coder/serpent"
2929

3030
"github.com/coder/coder/v2/agent"
31-
"github.com/coder/coder/v2/agent/agentcontainers"
3231
"github.com/coder/coder/v2/agent/agentexec"
3332
"github.com/coder/coder/v2/agent/agentssh"
3433
"github.com/coder/coder/v2/agent/reaper"
@@ -321,13 +320,10 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
321320
return xerrors.Errorf("create agent execer: %w", err)
322321
}
323322

324-
var containerLister agentcontainers.Lister
325-
if !experimentalDevcontainersEnabled {
326-
logger.Info(ctx, "agent devcontainer detection not enabled")
327-
containerLister = &agentcontainers.NoopLister{}
328-
} else {
323+
if experimentalDevcontainersEnabled {
329324
logger.Info(ctx, "agent devcontainer detection enabled")
330-
containerLister = agentcontainers.NewDocker(execer)
325+
} else {
326+
logger.Info(ctx, "agent devcontainer detection not enabled")
331327
}
332328

333329
reinitEvents := agentsdk.WaitForReinitLoop(ctx, logger, client)
@@ -365,7 +361,6 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
365361
PrometheusRegistry: prometheusRegistry,
366362
BlockFileTransfer: blockFileTransfer,
367363
Execer: execer,
368-
ContainerLister: containerLister,
369364
ExperimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
370365
})
371366

0 commit comments

Comments
 (0)