Skip to content

Commit adbfd45

Browse files
mafredrimafredri
committed
separate options for Up and Exec
1 parent cb275a2 commit adbfd45

File tree

5 files changed

+70
-55
lines changed

5 files changed

+70
-55
lines changed

‎agent/agentcontainers/acmock/acmock.go

Lines changed: 2 additions & 2 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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ type fakeDevcontainerCLI struct {
6565
execErrC chan error // If set, send to return err, close to return execErr.
6666
}
6767

68-
func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIOptions) (string, error) {
68+
func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
6969
if f.upErrC != nil {
7070
select {
7171
case <-ctx.Done():
@@ -79,7 +79,7 @@ func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcon
7979
return f.upID, f.upErr
8080
}
8181

82-
func (f *fakeDevcontainerCLI) Exec(ctx context.Context, _, _ string, _ string, _ []string, _ ...agentcontainers.DevcontainerCLIOptions) error {
82+
func (f *fakeDevcontainerCLI) Exec(ctx context.Context, _, _ string, _ string, _ []string, _ ...agentcontainers.DevcontainerCLIExecOptions) error {
8383
if f.execErrC != nil {
8484
select {
8585
case <-ctx.Done():

‎agent/agentcontainers/devcontainercli.go

Lines changed: 54 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,69 +16,84 @@ import (
1616

1717
// DevcontainerCLI is an interface for the devcontainer CLI.
1818
type DevcontainerCLI interface {
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
19+
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
2121
}
2222

23-
// DevcontainerCLIOptions are options for the devcontainer CLI commands.
24-
type DevcontainerCLIOptions func(*devcontainerCLIUpConfig)
23+
// DevcontainerCLIUpOptions are options for the devcontainer CLI Up
24+
// command.
25+
type DevcontainerCLIUpOptions func(*devcontainerCLIUpConfig)
26+
27+
type devcontainerCLIUpConfig struct {
28+
args []string // Additional arguments for the Up command.
29+
stdout io.Writer
30+
stderr io.Writer
31+
}
2532

2633
// WithRemoveExistingContainer is an option to remove the existing
27-
// container. Can only be used with the Up command.
28-
func WithRemoveExistingContainer() DevcontainerCLIOptions {
34+
// container.
35+
func WithRemoveExistingContainer() DevcontainerCLIUpOptions {
2936
return func(o *devcontainerCLIUpConfig) {
30-
if o.command != "up" {
31-
panic("developer error: WithRemoveExistingContainer can only be used with the Up command")
32-
}
3337
o.args = append(o.args, "--remove-existing-container")
3438
}
3539
}
3640

37-
// WithOutput sets additional stdout and stderr writers for logs.
38-
func WithOutput(stdout, stderr io.Writer) DevcontainerCLIOptions {
41+
// WithUpOutput sets additional stdout and stderr writers for logs
42+
// during Up operations.
43+
func WithUpOutput(stdout, stderr io.Writer) DevcontainerCLIUpOptions {
3944
return func(o *devcontainerCLIUpConfig) {
4045
o.stdout = stdout
4146
o.stderr = stderr
4247
}
4348
}
4449

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+
4569
// 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-
}
70+
func WithContainerID(id string) DevcontainerCLIExecOptions {
71+
return func(o *devcontainerCLIExecConfig) {
5272
o.args = append(o.args, "--container-id", id)
5373
}
5474
}
5575

5676
// 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-
}
77+
func WithRemoteEnv(env ...string) DevcontainerCLIExecOptions {
78+
return func(o *devcontainerCLIExecConfig) {
6379
for _, e := range env {
6480
o.args = append(o.args, "--remote-env", e)
6581
}
6682
}
6783
}
6884

69-
type devcontainerCLIUpConfig struct {
70-
command string // The devcontainer CLI command to run, e.g. "up", "exec".
71-
removeExistingContainer bool
72-
args []string // Additional arguments for the command.
73-
stdout io.Writer
74-
stderr io.Writer
85+
func applyDevcontainerCLIUpOptions(opts []DevcontainerCLIUpOptions) devcontainerCLIUpConfig {
86+
conf := devcontainerCLIUpConfig{}
87+
for _, opt := range opts {
88+
if opt != nil {
89+
opt(&conf)
90+
}
91+
}
92+
return conf
7593
}
7694

77-
func applyDevcontainerCLIOptions(command string, opts []DevcontainerCLIOptions) devcontainerCLIUpConfig {
78-
conf := devcontainerCLIUpConfig{
79-
command: command,
80-
removeExistingContainer: false,
81-
}
95+
func applyDevcontainerCLIExecOptions(opts []DevcontainerCLIExecOptions) devcontainerCLIExecConfig {
96+
conf := devcontainerCLIExecConfig{}
8297
for _, opt := range opts {
8398
if opt != nil {
8499
opt(&conf)
@@ -101,9 +116,9 @@ func NewDevcontainerCLI(logger slog.Logger, execer agentexec.Execer) Devcontaine
101116
}
102117
}
103118

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

108123
args := []string{
109124
"up",
@@ -145,8 +160,8 @@ func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath st
145160
return result.ContainerID, nil
146161
}
147162

148-
func (d *devcontainerCLI) Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIOptions) error {
149-
conf := applyDevcontainerCLIOptions("exec", opts)
163+
func (d *devcontainerCLI) Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIExecOptions) error {
164+
conf := applyDevcontainerCLIExecOptions(opts)
150165
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath))
151166

152167
args := []string{"exec"}

‎agent/agentcontainers/devcontainercli_test.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
4242
logFile string
4343
workspace string
4444
config string
45-
opts []agentcontainers.DevcontainerCLIOptions
45+
opts []agentcontainers.DevcontainerCLIUpOptions
4646
wantArgs string
4747
wantError bool
4848
}{
@@ -93,7 +93,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
9393
name: "with remove existing container",
9494
logFile: "up.log",
9595
workspace: "/test/workspace",
96-
opts: []agentcontainers.DevcontainerCLIOptions{
96+
opts: []agentcontainers.DevcontainerCLIUpOptions{
9797
agentcontainers.WithRemoveExistingContainer(),
9898
},
9999
wantArgs: "up --log-format json --workspace-folder /test/workspace --remove-existing-container",
@@ -136,7 +136,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
136136
configPath string
137137
cmd string
138138
cmdArgs []string
139-
opts []agentcontainers.DevcontainerCLIOptions
139+
opts []agentcontainers.DevcontainerCLIExecOptions
140140
wantArgs string
141141
wantError bool
142142
}{
@@ -182,7 +182,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
182182
configPath: "",
183183
cmd: "echo",
184184
cmdArgs: []string{"hello"},
185-
opts: []agentcontainers.DevcontainerCLIOptions{agentcontainers.WithContainerID("test-container-123")},
185+
opts: []agentcontainers.DevcontainerCLIExecOptions{agentcontainers.WithContainerID("test-container-123")},
186186
wantArgs: "exec --workspace-folder /test/workspace --container-id test-container-123 echo hello",
187187
wantError: false,
188188
},
@@ -192,7 +192,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
192192
configPath: "/test/config.json",
193193
cmd: "bash",
194194
cmdArgs: []string{"-c", "ls -la"},
195-
opts: []agentcontainers.DevcontainerCLIOptions{agentcontainers.WithContainerID("my-container")},
195+
opts: []agentcontainers.DevcontainerCLIExecOptions{agentcontainers.WithContainerID("my-container")},
196196
wantArgs: "exec --workspace-folder /test/workspace --config /test/config.json --container-id my-container bash -c ls -la",
197197
wantError: false,
198198
},
@@ -202,7 +202,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
202202
configPath: "",
203203
cmd: "cat",
204204
cmdArgs: []string{"/etc/hostname"},
205-
opts: []agentcontainers.DevcontainerCLIOptions{
205+
opts: []agentcontainers.DevcontainerCLIExecOptions{
206206
agentcontainers.WithContainerID("test-container-789"),
207207
},
208208
wantArgs: "exec --workspace-folder /test/workspace --container-id test-container-789 cat /etc/hostname",
@@ -235,7 +235,7 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
235235
})
236236
}
237237

238-
// TestDevcontainerCLI_WithOutput tests that WithOutput captures CLI
238+
// TestDevcontainerCLI_WithOutput tests that WithUpOutput and WithExecOutput capture CLI
239239
// logs to provided writers.
240240
func TestDevcontainerCLI_WithOutput(t *testing.T) {
241241
t.Parallel()
@@ -262,9 +262,9 @@ func TestDevcontainerCLI_WithOutput(t *testing.T) {
262262
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
263263
dccli := agentcontainers.NewDevcontainerCLI(logger, testExecer)
264264

265-
// Call Up with WithOutput to capture CLI logs.
265+
// Call Up with WithUpOutput to capture CLI logs.
266266
ctx := testutil.Context(t, testutil.WaitMedium)
267-
containerID, err := dccli.Up(ctx, "/test/workspace", "", agentcontainers.WithOutput(outBuf, errBuf))
267+
containerID, err := dccli.Up(ctx, "/test/workspace", "", agentcontainers.WithUpOutput(outBuf, errBuf))
268268
require.NoError(t, err, "Up should succeed")
269269
require.NotEmpty(t, containerID, "expected non-empty container ID")
270270

@@ -303,11 +303,11 @@ func TestDevcontainerCLI_WithOutput(t *testing.T) {
303303
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
304304
dccli := agentcontainers.NewDevcontainerCLI(logger, testExecer)
305305

306-
// Call Exec with WithOutput and WithContainerID to capture any command output.
306+
// Call Exec with WithExecOutput and WithContainerID to capture any command output.
307307
ctx := testutil.Context(t, testutil.WaitMedium)
308308
err = dccli.Exec(ctx, "/test/workspace", "", "echo", []string{"hello"},
309309
agentcontainers.WithContainerID("test-container-456"),
310-
agentcontainers.WithOutput(outBuf, errBuf),
310+
agentcontainers.WithExecOutput(outBuf, errBuf),
311311
)
312312
require.NoError(t, err, "Exec should succeed")
313313

0 commit comments

Comments
 (0)