Skip to content

Commit 63f93bc

Browse files
mafredrimafredri
committed
feat(agent/agentcontainers): add Exec method to devcontainers CLI
This change adds Exec to the devcontainer CLI. Updates coder/internal#621
1 parent 5cf4ae7 commit 63f93bc

File tree

4 files changed

+314
-62
lines changed

4 files changed

+314
-62
lines changed

‎agent/agentcontainers/acmock/acmock.go

Lines changed: 20 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎agent/agentcontainers/api_test.go

Lines changed: 32 additions & 13 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

66-
func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
67-
if f.continueUp != nil {
68+
func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIOptions) (string, error) {
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.DevcontainerCLIOptions) 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: 77 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,37 +16,67 @@ import (
1616

1717
// DevcontainerCLI is an interface for the devcontainer CLI.
1818
type DevcontainerCLI interface {
19-
Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error)
19+
Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIOptions) (id string, err error)
20+
Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIOptions) error
2021
}
2122

22-
// DevcontainerCLIUpOptions are options for the devcontainer CLI up
23-
// command.
24-
type DevcontainerCLIUpOptions func(*devcontainerCLIUpConfig)
23+
// DevcontainerCLIOptions are options for the devcontainer CLI commands.
24+
type DevcontainerCLIOptions func(*devcontainerCLIUpConfig)
2525

2626
// WithRemoveExistingContainer is an option to remove the existing
27-
// container.
28-
func WithRemoveExistingContainer() DevcontainerCLIUpOptions {
27+
// container. Can only be used with the Up command.
28+
func WithRemoveExistingContainer() DevcontainerCLIOptions {
2929
return func(o *devcontainerCLIUpConfig) {
30-
o.removeExistingContainer = true
30+
if o.command != "up" {
31+
panic("developer error: WithRemoveExistingContainer can only be used with the Up command")
32+
}
33+
o.args = append(o.args, "--remove-existing-container")
3134
}
3235
}
3336

34-
// WithOutput sets stdout and stderr writers for Up command logs.
35-
func WithOutput(stdout, stderr io.Writer) DevcontainerCLIUpOptions {
37+
// WithOutput sets additional stdout and stderr writers for logs.
38+
func WithOutput(stdout, stderr io.Writer) DevcontainerCLIOptions {
3639
return func(o *devcontainerCLIUpConfig) {
3740
o.stdout = stdout
3841
o.stderr = stderr
3942
}
4043
}
4144

45+
// WithContainerID sets the container ID to target a specific container.
46+
// Can only be used with the Exec command.
47+
func WithContainerID(id string) DevcontainerCLIOptions {
48+
return func(o *devcontainerCLIUpConfig) {
49+
if o.command != "exec" {
50+
panic("developer error: WithContainerID can only be used with the Exec command")
51+
}
52+
o.args = append(o.args, "--container-id", id)
53+
}
54+
}
55+
56+
// WithRemoteEnv sets environment variables for the Exec command.
57+
// Can only be used with the Exec command.
58+
func WithRemoteEnv(env ...string) DevcontainerCLIOptions {
59+
return func(o *devcontainerCLIUpConfig) {
60+
if o.command != "exec" {
61+
panic("developer error: WithRemoteEnv can only be used with the Exec command")
62+
}
63+
for _, e := range env {
64+
o.args = append(o.args, "--remote-env", e)
65+
}
66+
}
67+
}
68+
4269
type devcontainerCLIUpConfig struct {
70+
command string // The devcontainer CLI command to run, e.g. "up", "exec".
4371
removeExistingContainer bool
72+
args []string // Additional arguments for the command.
4473
stdout io.Writer
4574
stderr io.Writer
4675
}
4776

48-
func applyDevcontainerCLIUpOptions(opts []DevcontainerCLIUpOptions) devcontainerCLIUpConfig {
77+
func applyDevcontainerCLIOptions(command string, opts []DevcontainerCLIOptions) devcontainerCLIUpConfig {
4978
conf := devcontainerCLIUpConfig{
79+
command: command,
5080
removeExistingContainer: false,
5181
}
5282
for _, opt := range opts {
@@ -71,8 +101,8 @@ func NewDevcontainerCLI(logger slog.Logger, execer agentexec.Execer) Devcontaine
71101
}
72102
}
73103

74-
func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (string, error) {
75-
conf := applyDevcontainerCLIUpOptions(opts)
104+
func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIOptions) (string, error) {
105+
conf := applyDevcontainerCLIOptions("up", opts)
76106
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath), slog.F("recreate", conf.removeExistingContainer))
77107

78108
args := []string{
@@ -83,9 +113,7 @@ func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath st
83113
if configPath != "" {
84114
args = append(args, "--config", configPath)
85115
}
86-
if conf.removeExistingContainer {
87-
args = append(args, "--remove-existing-container")
88-
}
116+
args = append(args, conf.args...)
89117
cmd := d.execer.CommandContext(ctx, "devcontainer", args...)
90118

91119
// Capture stdout for parsing and stream logs for both default and provided writers.
@@ -117,6 +145,40 @@ func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath st
117145
return result.ContainerID, nil
118146
}
119147

148+
func (d *devcontainerCLI) Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIOptions) error {
149+
conf := applyDevcontainerCLIOptions("exec", opts)
150+
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath))
151+
152+
args := []string{"exec"}
153+
if workspaceFolder != "" {
154+
args = append(args, "--workspace-folder", workspaceFolder)
155+
}
156+
if configPath != "" {
157+
args = append(args, "--config", configPath)
158+
}
159+
args = append(args, conf.args...)
160+
args = append(args, cmd)
161+
args = append(args, cmdArgs...)
162+
c := d.execer.CommandContext(ctx, "devcontainer", args...)
163+
164+
stdoutWriters := []io.Writer{&devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stdout", true))}}
165+
if conf.stdout != nil {
166+
stdoutWriters = append(stdoutWriters, conf.stdout)
167+
}
168+
c.Stdout = io.MultiWriter(stdoutWriters...)
169+
stderrWriters := []io.Writer{&devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stderr", true))}}
170+
if conf.stderr != nil {
171+
stderrWriters = append(stderrWriters, conf.stderr)
172+
}
173+
c.Stderr = io.MultiWriter(stderrWriters...)
174+
175+
if err := c.Run(); err != nil {
176+
return xerrors.Errorf("devcontainer exec failed: %w", err)
177+
}
178+
179+
return nil
180+
}
181+
120182
// parseDevcontainerCLILastLine parses the last line of the devcontainer CLI output
121183
// which is a JSON object.
122184
func parseDevcontainerCLILastLine(ctx context.Context, logger slog.Logger, p []byte) (result devcontainerCLIResult, err error) {

0 commit comments

Comments
 (0)