Skip to content

Commit df8ced5

Browse files
mafredriEdwardAngert
mafredri
authored and
EdwardAngert
committed
feat(agent/agentcontainers): add Exec method to devcontainers CLI (#18244)
This change adds Exec to the devcontainer CLI. Updates coder/internal#621
1 parent 0ba748e commit df8ced5

File tree

5 files changed

+326
-59
lines changed

5 files changed

+326
-59
lines changed

‎agent/agentcontainers/acmock/acmock.go

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎agent/agentcontainers/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con
684684

685685
logger.Debug(ctx, "starting devcontainer recreation")
686686

687-
_, err = api.dccli.Up(ctx, dc.WorkspaceFolder, configPath, WithOutput(infoW, errW), WithRemoveExistingContainer())
687+
_, err = api.dccli.Up(ctx, dc.WorkspaceFolder, configPath, WithUpOutput(infoW, errW), WithRemoveExistingContainer())
688688
if err != nil {
689689
// No need to log if the API is closing (context canceled), as this
690690
// is expected behavior when the API is shutting down.

‎agent/agentcontainers/api_test.go

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,39 @@ func (f *fakeContainerCLI) ExecAs(ctx context.Context, name, user string, args .
5858
// fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI
5959
// interface for testing.
6060
type fakeDevcontainerCLI struct {
61-
id string
62-
err error
63-
continueUp chan struct{}
61+
upID string
62+
upErr error
63+
upErrC chan error // If set, send to return err, close to return upErr.
64+
execErr error
65+
execErrC chan error // If set, send to return err, close to return execErr.
6466
}
6567

6668
func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
67-
if f.continueUp != nil {
69+
if f.upErrC != nil {
6870
select {
6971
case <-ctx.Done():
70-
return "", xerrors.New("test timeout")
71-
case <-f.continueUp:
72+
return "", ctx.Err()
73+
case err, ok := <-f.upErrC:
74+
if ok {
75+
return f.upID, err
76+
}
7277
}
7378
}
74-
return f.id, f.err
79+
return f.upID, f.upErr
80+
}
81+
82+
func (f *fakeDevcontainerCLI) Exec(ctx context.Context, _, _ string, _ string, _ []string, _ ...agentcontainers.DevcontainerCLIExecOptions) error {
83+
if f.execErrC != nil {
84+
select {
85+
case <-ctx.Done():
86+
return ctx.Err()
87+
case err, ok := <-f.execErrC:
88+
if ok {
89+
return err
90+
}
91+
}
92+
}
93+
return f.execErr
7594
}
7695

7796
// fakeWatcher implements the watcher.Watcher interface for testing.
@@ -398,7 +417,7 @@ func TestAPI(t *testing.T) {
398417
},
399418
},
400419
devcontainerCLI: &fakeDevcontainerCLI{
401-
err: xerrors.New("devcontainer CLI error"),
420+
upErr: xerrors.New("devcontainer CLI error"),
402421
},
403422
wantStatus: []int{http.StatusAccepted, http.StatusConflict},
404423
wantBody: []string{"Devcontainer recreation initiated", "Devcontainer recreation already in progress"},
@@ -432,7 +451,7 @@ func TestAPI(t *testing.T) {
432451
nowRecreateErrorTrap := mClock.Trap().Now("recreate", "errorTimes")
433452
nowRecreateSuccessTrap := mClock.Trap().Now("recreate", "successTimes")
434453

435-
tt.devcontainerCLI.continueUp = make(chan struct{})
454+
tt.devcontainerCLI.upErrC = make(chan error)
436455

437456
// Setup router with the handler under test.
438457
r := chi.NewRouter()
@@ -470,7 +489,7 @@ func TestAPI(t *testing.T) {
470489
// because we must check what state the devcontainer ends up in
471490
// after the recreation process is initiated and finished.
472491
if tt.wantStatus[0] != http.StatusAccepted {
473-
close(tt.devcontainerCLI.continueUp)
492+
close(tt.devcontainerCLI.upErrC)
474493
nowRecreateSuccessTrap.Close()
475494
nowRecreateErrorTrap.Close()
476495
return
@@ -497,10 +516,10 @@ func TestAPI(t *testing.T) {
497516
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not starting")
498517

499518
// Allow the devcontainer CLI to continue the up process.
500-
close(tt.devcontainerCLI.continueUp)
519+
close(tt.devcontainerCLI.upErrC)
501520

502521
// Ensure the devcontainer ends up in error state if the up call fails.
503-
if tt.devcontainerCLI.err != nil {
522+
if tt.devcontainerCLI.upErr != nil {
504523
nowRecreateSuccessTrap.Close()
505524
// The timestamp for the error will be stored, which gives
506525
// us a good anchor point to know when to do our request.

‎agent/agentcontainers/devcontainercli.go

Lines changed: 91 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,38 +17,83 @@ import (
1717
// DevcontainerCLI is an interface for the devcontainer CLI.
1818
type DevcontainerCLI interface {
1919
Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error)
20+
Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIExecOptions) error
2021
}
2122

22-
// DevcontainerCLIUpOptions are options for the devcontainer CLI up
23+
// DevcontainerCLIUpOptions are options for the devcontainer CLI Up
2324
// command.
2425
type DevcontainerCLIUpOptions func(*devcontainerCLIUpConfig)
2526

27+
type devcontainerCLIUpConfig struct {
28+
args []string // Additional arguments for the Up command.
29+
stdout io.Writer
30+
stderr io.Writer
31+
}
32+
2633
// WithRemoveExistingContainer is an option to remove the existing
2734
// container.
2835
func WithRemoveExistingContainer() DevcontainerCLIUpOptions {
2936
return func(o *devcontainerCLIUpConfig) {
30-
o.removeExistingContainer = true
37+
o.args = append(o.args, "--remove-existing-container")
3138
}
3239
}
3340

34-
// WithOutput sets stdout and stderr writers for Up command logs.
35-
func WithOutput(stdout, stderr io.Writer) DevcontainerCLIUpOptions {
41+
// WithUpOutput sets additional stdout and stderr writers for logs
42+
// during Up operations.
43+
func WithUpOutput(stdout, stderr io.Writer) DevcontainerCLIUpOptions {
3644
return func(o *devcontainerCLIUpConfig) {
3745
o.stdout = stdout
3846
o.stderr = stderr
3947
}
4048
}
4149

42-
type devcontainerCLIUpConfig struct {
43-
removeExistingContainer bool
44-
stdout io.Writer
45-
stderr io.Writer
50+
// DevcontainerCLIExecOptions are options for the devcontainer CLI Exec
51+
// command.
52+
type DevcontainerCLIExecOptions func(*devcontainerCLIExecConfig)
53+
54+
type devcontainerCLIExecConfig struct {
55+
args []string // Additional arguments for the Exec command.
56+
stdout io.Writer
57+
stderr io.Writer
58+
}
59+
60+
// WithExecOutput sets additional stdout and stderr writers for logs
61+
// during Exec operations.
62+
func WithExecOutput(stdout, stderr io.Writer) DevcontainerCLIExecOptions {
63+
return func(o *devcontainerCLIExecConfig) {
64+
o.stdout = stdout
65+
o.stderr = stderr
66+
}
67+
}
68+
69+
// WithContainerID sets the container ID to target a specific container.
70+
func WithContainerID(id string) DevcontainerCLIExecOptions {
71+
return func(o *devcontainerCLIExecConfig) {
72+
o.args = append(o.args, "--container-id", id)
73+
}
74+
}
75+
76+
// WithRemoteEnv sets environment variables for the Exec command.
77+
func WithRemoteEnv(env ...string) DevcontainerCLIExecOptions {
78+
return func(o *devcontainerCLIExecConfig) {
79+
for _, e := range env {
80+
o.args = append(o.args, "--remote-env", e)
81+
}
82+
}
4683
}
4784

4885
func applyDevcontainerCLIUpOptions(opts []DevcontainerCLIUpOptions) devcontainerCLIUpConfig {
49-
conf := devcontainerCLIUpConfig{
50-
removeExistingContainer: false,
86+
conf := devcontainerCLIUpConfig{}
87+
for _, opt := range opts {
88+
if opt != nil {
89+
opt(&conf)
90+
}
5191
}
92+
return conf
93+
}
94+
95+
func applyDevcontainerCLIExecOptions(opts []DevcontainerCLIExecOptions) devcontainerCLIExecConfig {
96+
conf := devcontainerCLIExecConfig{}
5297
for _, opt := range opts {
5398
if opt != nil {
5499
opt(&conf)
@@ -73,7 +118,7 @@ func NewDevcontainerCLI(logger slog.Logger, execer agentexec.Execer) Devcontaine
73118

74119
func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (string, error) {
75120
conf := applyDevcontainerCLIUpOptions(opts)
76-
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath), slog.F("recreate", conf.removeExistingContainer))
121+
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath))
77122

78123
args := []string{
79124
"up",
@@ -83,9 +128,7 @@ func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath st
83128
if configPath != "" {
84129
args = append(args, "--config", configPath)
85130
}
86-
if conf.removeExistingContainer {
87-
args = append(args, "--remove-existing-container")
88-
}
131+
args = append(args, conf.args...)
89132
cmd := d.execer.CommandContext(ctx, "devcontainer", args...)
90133

91134
// Capture stdout for parsing and stream logs for both default and provided writers.
@@ -117,6 +160,40 @@ func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath st
117160
return result.ContainerID, nil
118161
}
119162

163+
func (d *devcontainerCLI) Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIExecOptions) error {
164+
conf := applyDevcontainerCLIExecOptions(opts)
165+
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath))
166+
167+
args := []string{"exec"}
168+
if workspaceFolder != "" {
169+
args = append(args, "--workspace-folder", workspaceFolder)
170+
}
171+
if configPath != "" {
172+
args = append(args, "--config", configPath)
173+
}
174+
args = append(args, conf.args...)
175+
args = append(args, cmd)
176+
args = append(args, cmdArgs...)
177+
c := d.execer.CommandContext(ctx, "devcontainer", args...)
178+
179+
stdoutWriters := []io.Writer{&devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stdout", true))}}
180+
if conf.stdout != nil {
181+
stdoutWriters = append(stdoutWriters, conf.stdout)
182+
}
183+
c.Stdout = io.MultiWriter(stdoutWriters...)
184+
stderrWriters := []io.Writer{&devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stderr", true))}}
185+
if conf.stderr != nil {
186+
stderrWriters = append(stderrWriters, conf.stderr)
187+
}
188+
c.Stderr = io.MultiWriter(stderrWriters...)
189+
190+
if err := c.Run(); err != nil {
191+
return xerrors.Errorf("devcontainer exec failed: %w", err)
192+
}
193+
194+
return nil
195+
}
196+
120197
// parseDevcontainerCLILastLine parses the last line of the devcontainer CLI output
121198
// which is a JSON object.
122199
func parseDevcontainerCLILastLine(ctx context.Context, logger slog.Logger, p []byte) (result devcontainerCLIResult, err error) {

0 commit comments

Comments
 (0)