feat(agent/agentcontainers): add Exec method to devcontainers CLI (#18244)

This change adds Exec to the devcontainer CLI.

Updates coder/internal#621
This commit is contained in:
Mathias Fredriksson
2025-06-06 14:38:58 +03:00
committed by GitHub
parent a12429e9f8
commit 709f374fe0
5 changed files with 324 additions and 57 deletions

View File

@ -130,6 +130,25 @@ func (m *MockDevcontainerCLI) EXPECT() *MockDevcontainerCLIMockRecorder {
return m.recorder return m.recorder
} }
// Exec mocks base method.
func (m *MockDevcontainerCLI) Exec(ctx context.Context, workspaceFolder, configPath, cmd string, cmdArgs []string, opts ...agentcontainers.DevcontainerCLIExecOptions) error {
m.ctrl.T.Helper()
varargs := []any{ctx, workspaceFolder, configPath, cmd, cmdArgs}
for _, a := range opts {
varargs = append(varargs, a)
}
ret := m.ctrl.Call(m, "Exec", varargs...)
ret0, _ := ret[0].(error)
return ret0
}
// Exec indicates an expected call of Exec.
func (mr *MockDevcontainerCLIMockRecorder) Exec(ctx, workspaceFolder, configPath, cmd, cmdArgs any, opts ...any) *gomock.Call {
mr.mock.ctrl.T.Helper()
varargs := append([]any{ctx, workspaceFolder, configPath, cmd, cmdArgs}, opts...)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Exec", reflect.TypeOf((*MockDevcontainerCLI)(nil).Exec), varargs...)
}
// Up mocks base method. // Up mocks base method.
func (m *MockDevcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...agentcontainers.DevcontainerCLIUpOptions) (string, error) { func (m *MockDevcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
m.ctrl.T.Helper() m.ctrl.T.Helper()

View File

@ -684,7 +684,7 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con
logger.Debug(ctx, "starting devcontainer recreation") logger.Debug(ctx, "starting devcontainer recreation")
_, err = api.dccli.Up(ctx, dc.WorkspaceFolder, configPath, WithOutput(infoW, errW), WithRemoveExistingContainer()) _, err = api.dccli.Up(ctx, dc.WorkspaceFolder, configPath, WithUpOutput(infoW, errW), WithRemoveExistingContainer())
if err != nil { if err != nil {
// No need to log if the API is closing (context canceled), as this // No need to log if the API is closing (context canceled), as this
// is expected behavior when the API is shutting down. // is expected behavior when the API is shutting down.

View File

@ -58,20 +58,39 @@ func (f *fakeContainerCLI) ExecAs(ctx context.Context, name, user string, args .
// fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI // fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI
// interface for testing. // interface for testing.
type fakeDevcontainerCLI struct { type fakeDevcontainerCLI struct {
id string upID string
err error upErr error
continueUp chan struct{} upErrC chan error // If set, send to return err, close to return upErr.
execErr error
execErrC chan error // If set, send to return err, close to return execErr.
} }
func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) { func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
if f.continueUp != nil { if f.upErrC != nil {
select { select {
case <-ctx.Done(): case <-ctx.Done():
return "", xerrors.New("test timeout") return "", ctx.Err()
case <-f.continueUp: case err, ok := <-f.upErrC:
if ok {
return f.upID, err
}
} }
} }
return f.id, f.err return f.upID, f.upErr
}
func (f *fakeDevcontainerCLI) Exec(ctx context.Context, _, _ string, _ string, _ []string, _ ...agentcontainers.DevcontainerCLIExecOptions) error {
if f.execErrC != nil {
select {
case <-ctx.Done():
return ctx.Err()
case err, ok := <-f.execErrC:
if ok {
return err
}
}
}
return f.execErr
} }
// fakeWatcher implements the watcher.Watcher interface for testing. // fakeWatcher implements the watcher.Watcher interface for testing.
@ -398,7 +417,7 @@ func TestAPI(t *testing.T) {
}, },
}, },
devcontainerCLI: &fakeDevcontainerCLI{ devcontainerCLI: &fakeDevcontainerCLI{
err: xerrors.New("devcontainer CLI error"), upErr: xerrors.New("devcontainer CLI error"),
}, },
wantStatus: []int{http.StatusAccepted, http.StatusConflict}, wantStatus: []int{http.StatusAccepted, http.StatusConflict},
wantBody: []string{"Devcontainer recreation initiated", "Devcontainer recreation already in progress"}, wantBody: []string{"Devcontainer recreation initiated", "Devcontainer recreation already in progress"},
@ -432,7 +451,7 @@ func TestAPI(t *testing.T) {
nowRecreateErrorTrap := mClock.Trap().Now("recreate", "errorTimes") nowRecreateErrorTrap := mClock.Trap().Now("recreate", "errorTimes")
nowRecreateSuccessTrap := mClock.Trap().Now("recreate", "successTimes") nowRecreateSuccessTrap := mClock.Trap().Now("recreate", "successTimes")
tt.devcontainerCLI.continueUp = make(chan struct{}) tt.devcontainerCLI.upErrC = make(chan error)
// Setup router with the handler under test. // Setup router with the handler under test.
r := chi.NewRouter() r := chi.NewRouter()
@ -470,7 +489,7 @@ func TestAPI(t *testing.T) {
// because we must check what state the devcontainer ends up in // because we must check what state the devcontainer ends up in
// after the recreation process is initiated and finished. // after the recreation process is initiated and finished.
if tt.wantStatus[0] != http.StatusAccepted { if tt.wantStatus[0] != http.StatusAccepted {
close(tt.devcontainerCLI.continueUp) close(tt.devcontainerCLI.upErrC)
nowRecreateSuccessTrap.Close() nowRecreateSuccessTrap.Close()
nowRecreateErrorTrap.Close() nowRecreateErrorTrap.Close()
return return
@ -497,10 +516,10 @@ func TestAPI(t *testing.T) {
assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not starting") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not starting")
// Allow the devcontainer CLI to continue the up process. // Allow the devcontainer CLI to continue the up process.
close(tt.devcontainerCLI.continueUp) close(tt.devcontainerCLI.upErrC)
// Ensure the devcontainer ends up in error state if the up call fails. // Ensure the devcontainer ends up in error state if the up call fails.
if tt.devcontainerCLI.err != nil { if tt.devcontainerCLI.upErr != nil {
nowRecreateSuccessTrap.Close() nowRecreateSuccessTrap.Close()
// The timestamp for the error will be stored, which gives // The timestamp for the error will be stored, which gives
// us a good anchor point to know when to do our request. // us a good anchor point to know when to do our request.

View File

@ -17,38 +17,83 @@ import (
// DevcontainerCLI is an interface for the devcontainer CLI. // DevcontainerCLI is an interface for the devcontainer CLI.
type DevcontainerCLI interface { type DevcontainerCLI interface {
Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error) Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error)
Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIExecOptions) error
} }
// DevcontainerCLIUpOptions are options for the devcontainer CLI up // DevcontainerCLIUpOptions are options for the devcontainer CLI Up
// command. // command.
type DevcontainerCLIUpOptions func(*devcontainerCLIUpConfig) type DevcontainerCLIUpOptions func(*devcontainerCLIUpConfig)
type devcontainerCLIUpConfig struct {
args []string // Additional arguments for the Up command.
stdout io.Writer
stderr io.Writer
}
// WithRemoveExistingContainer is an option to remove the existing // WithRemoveExistingContainer is an option to remove the existing
// container. // container.
func WithRemoveExistingContainer() DevcontainerCLIUpOptions { func WithRemoveExistingContainer() DevcontainerCLIUpOptions {
return func(o *devcontainerCLIUpConfig) { return func(o *devcontainerCLIUpConfig) {
o.removeExistingContainer = true o.args = append(o.args, "--remove-existing-container")
} }
} }
// WithOutput sets stdout and stderr writers for Up command logs. // WithUpOutput sets additional stdout and stderr writers for logs
func WithOutput(stdout, stderr io.Writer) DevcontainerCLIUpOptions { // during Up operations.
func WithUpOutput(stdout, stderr io.Writer) DevcontainerCLIUpOptions {
return func(o *devcontainerCLIUpConfig) { return func(o *devcontainerCLIUpConfig) {
o.stdout = stdout o.stdout = stdout
o.stderr = stderr o.stderr = stderr
} }
} }
type devcontainerCLIUpConfig struct { // DevcontainerCLIExecOptions are options for the devcontainer CLI Exec
removeExistingContainer bool // command.
stdout io.Writer type DevcontainerCLIExecOptions func(*devcontainerCLIExecConfig)
stderr io.Writer
type devcontainerCLIExecConfig struct {
args []string // Additional arguments for the Exec command.
stdout io.Writer
stderr io.Writer
}
// WithExecOutput sets additional stdout and stderr writers for logs
// during Exec operations.
func WithExecOutput(stdout, stderr io.Writer) DevcontainerCLIExecOptions {
return func(o *devcontainerCLIExecConfig) {
o.stdout = stdout
o.stderr = stderr
}
}
// WithContainerID sets the container ID to target a specific container.
func WithContainerID(id string) DevcontainerCLIExecOptions {
return func(o *devcontainerCLIExecConfig) {
o.args = append(o.args, "--container-id", id)
}
}
// WithRemoteEnv sets environment variables for the Exec command.
func WithRemoteEnv(env ...string) DevcontainerCLIExecOptions {
return func(o *devcontainerCLIExecConfig) {
for _, e := range env {
o.args = append(o.args, "--remote-env", e)
}
}
} }
func applyDevcontainerCLIUpOptions(opts []DevcontainerCLIUpOptions) devcontainerCLIUpConfig { func applyDevcontainerCLIUpOptions(opts []DevcontainerCLIUpOptions) devcontainerCLIUpConfig {
conf := devcontainerCLIUpConfig{ conf := devcontainerCLIUpConfig{}
removeExistingContainer: false, for _, opt := range opts {
if opt != nil {
opt(&conf)
}
} }
return conf
}
func applyDevcontainerCLIExecOptions(opts []DevcontainerCLIExecOptions) devcontainerCLIExecConfig {
conf := devcontainerCLIExecConfig{}
for _, opt := range opts { for _, opt := range opts {
if opt != nil { if opt != nil {
opt(&conf) opt(&conf)
@ -73,7 +118,7 @@ func NewDevcontainerCLI(logger slog.Logger, execer agentexec.Execer) Devcontaine
func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (string, error) { func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (string, error) {
conf := applyDevcontainerCLIUpOptions(opts) conf := applyDevcontainerCLIUpOptions(opts)
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath), slog.F("recreate", conf.removeExistingContainer)) logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath))
args := []string{ args := []string{
"up", "up",
@ -83,9 +128,7 @@ func (d *devcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath st
if configPath != "" { if configPath != "" {
args = append(args, "--config", configPath) args = append(args, "--config", configPath)
} }
if conf.removeExistingContainer { args = append(args, conf.args...)
args = append(args, "--remove-existing-container")
}
cmd := d.execer.CommandContext(ctx, "devcontainer", args...) cmd := d.execer.CommandContext(ctx, "devcontainer", args...)
// Capture stdout for parsing and stream logs for both default and provided writers. // 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
return result.ContainerID, nil return result.ContainerID, nil
} }
func (d *devcontainerCLI) Exec(ctx context.Context, workspaceFolder, configPath string, cmd string, cmdArgs []string, opts ...DevcontainerCLIExecOptions) error {
conf := applyDevcontainerCLIExecOptions(opts)
logger := d.logger.With(slog.F("workspace_folder", workspaceFolder), slog.F("config_path", configPath))
args := []string{"exec"}
if workspaceFolder != "" {
args = append(args, "--workspace-folder", workspaceFolder)
}
if configPath != "" {
args = append(args, "--config", configPath)
}
args = append(args, conf.args...)
args = append(args, cmd)
args = append(args, cmdArgs...)
c := d.execer.CommandContext(ctx, "devcontainer", args...)
stdoutWriters := []io.Writer{&devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stdout", true))}}
if conf.stdout != nil {
stdoutWriters = append(stdoutWriters, conf.stdout)
}
c.Stdout = io.MultiWriter(stdoutWriters...)
stderrWriters := []io.Writer{&devcontainerCLILogWriter{ctx: ctx, logger: logger.With(slog.F("stderr", true))}}
if conf.stderr != nil {
stderrWriters = append(stderrWriters, conf.stderr)
}
c.Stderr = io.MultiWriter(stderrWriters...)
if err := c.Run(); err != nil {
return xerrors.Errorf("devcontainer exec failed: %w", err)
}
return nil
}
// parseDevcontainerCLILastLine parses the last line of the devcontainer CLI output // parseDevcontainerCLILastLine parses the last line of the devcontainer CLI output
// which is a JSON object. // which is a JSON object.
func parseDevcontainerCLILastLine(ctx context.Context, logger slog.Logger, p []byte) (result devcontainerCLIResult, err error) { func parseDevcontainerCLILastLine(ctx context.Context, logger slog.Logger, p []byte) (result devcontainerCLIResult, err error) {

View File

@ -126,9 +126,116 @@ func TestDevcontainerCLI_ArgsAndParsing(t *testing.T) {
}) })
} }
}) })
t.Run("Exec", func(t *testing.T) {
t.Parallel()
tests := []struct {
name string
workspaceFolder string
configPath string
cmd string
cmdArgs []string
opts []agentcontainers.DevcontainerCLIExecOptions
wantArgs string
wantError bool
}{
{
name: "simple command",
workspaceFolder: "/test/workspace",
configPath: "",
cmd: "echo",
cmdArgs: []string{"hello"},
wantArgs: "exec --workspace-folder /test/workspace echo hello",
wantError: false,
},
{
name: "command with multiple args",
workspaceFolder: "/test/workspace",
configPath: "/test/config.json",
cmd: "ls",
cmdArgs: []string{"-la", "/workspace"},
wantArgs: "exec --workspace-folder /test/workspace --config /test/config.json ls -la /workspace",
wantError: false,
},
{
name: "empty command args",
workspaceFolder: "/test/workspace",
configPath: "",
cmd: "bash",
cmdArgs: nil,
wantArgs: "exec --workspace-folder /test/workspace bash",
wantError: false,
},
{
name: "workspace not found",
workspaceFolder: "/nonexistent/workspace",
configPath: "",
cmd: "echo",
cmdArgs: []string{"test"},
wantArgs: "exec --workspace-folder /nonexistent/workspace echo test",
wantError: true,
},
{
name: "with container ID",
workspaceFolder: "/test/workspace",
configPath: "",
cmd: "echo",
cmdArgs: []string{"hello"},
opts: []agentcontainers.DevcontainerCLIExecOptions{agentcontainers.WithContainerID("test-container-123")},
wantArgs: "exec --workspace-folder /test/workspace --container-id test-container-123 echo hello",
wantError: false,
},
{
name: "with container ID and config",
workspaceFolder: "/test/workspace",
configPath: "/test/config.json",
cmd: "bash",
cmdArgs: []string{"-c", "ls -la"},
opts: []agentcontainers.DevcontainerCLIExecOptions{agentcontainers.WithContainerID("my-container")},
wantArgs: "exec --workspace-folder /test/workspace --config /test/config.json --container-id my-container bash -c ls -la",
wantError: false,
},
{
name: "with container ID and output capture",
workspaceFolder: "/test/workspace",
configPath: "",
cmd: "cat",
cmdArgs: []string{"/etc/hostname"},
opts: []agentcontainers.DevcontainerCLIExecOptions{
agentcontainers.WithContainerID("test-container-789"),
},
wantArgs: "exec --workspace-folder /test/workspace --container-id test-container-789 cat /etc/hostname",
wantError: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitMedium)
testExecer := &testDevcontainerExecer{
testExePath: testExePath,
wantArgs: tt.wantArgs,
wantError: tt.wantError,
logFile: "", // Exec doesn't need log file parsing
}
dccli := agentcontainers.NewDevcontainerCLI(logger, testExecer)
err := dccli.Exec(ctx, tt.workspaceFolder, tt.configPath, tt.cmd, tt.cmdArgs, tt.opts...)
if tt.wantError {
assert.Error(t, err, "want error")
} else {
assert.NoError(t, err, "want no error")
}
})
}
})
} }
// TestDevcontainerCLI_WithOutput tests that WithOutput captures CLI // TestDevcontainerCLI_WithOutput tests that WithUpOutput and WithExecOutput capture CLI
// logs to provided writers. // logs to provided writers.
func TestDevcontainerCLI_WithOutput(t *testing.T) { func TestDevcontainerCLI_WithOutput(t *testing.T) {
t.Parallel() t.Parallel()
@ -136,35 +243,77 @@ func TestDevcontainerCLI_WithOutput(t *testing.T) {
// Prepare test executable and logger. // Prepare test executable and logger.
testExePath, err := os.Executable() testExePath, err := os.Executable()
require.NoError(t, err, "get test executable path") require.NoError(t, err, "get test executable path")
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
ctx := testutil.Context(t, testutil.WaitMedium)
// Buffers to capture stdout and stderr. t.Run("Up", func(t *testing.T) {
outBuf := &bytes.Buffer{} t.Parallel()
errBuf := &bytes.Buffer{}
// Simulate CLI execution with a standard up.log file. // Buffers to capture stdout and stderr.
wantArgs := "up --log-format json --workspace-folder /test/workspace" outBuf := &bytes.Buffer{}
testExecer := &testDevcontainerExecer{ errBuf := &bytes.Buffer{}
testExePath: testExePath,
wantArgs: wantArgs,
wantError: false,
logFile: filepath.Join("testdata", "devcontainercli", "parse", "up.log"),
}
dccli := agentcontainers.NewDevcontainerCLI(logger, testExecer)
// Call Up with WithOutput to capture CLI logs. // Simulate CLI execution with a standard up.log file.
containerID, err := dccli.Up(ctx, "/test/workspace", "", agentcontainers.WithOutput(outBuf, errBuf)) wantArgs := "up --log-format json --workspace-folder /test/workspace"
require.NoError(t, err, "Up should succeed") testExecer := &testDevcontainerExecer{
require.NotEmpty(t, containerID, "expected non-empty container ID") testExePath: testExePath,
wantArgs: wantArgs,
wantError: false,
logFile: filepath.Join("testdata", "devcontainercli", "parse", "up.log"),
}
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
dccli := agentcontainers.NewDevcontainerCLI(logger, testExecer)
// Read expected log content. // Call Up with WithUpOutput to capture CLI logs.
expLog, err := os.ReadFile(filepath.Join("testdata", "devcontainercli", "parse", "up.log")) ctx := testutil.Context(t, testutil.WaitMedium)
require.NoError(t, err, "reading expected log file") containerID, err := dccli.Up(ctx, "/test/workspace", "", agentcontainers.WithUpOutput(outBuf, errBuf))
require.NoError(t, err, "Up should succeed")
require.NotEmpty(t, containerID, "expected non-empty container ID")
// Verify stdout buffer contains the CLI logs and stderr is empty. // Read expected log content.
assert.Equal(t, string(expLog), outBuf.String(), "stdout buffer should match CLI logs") expLog, err := os.ReadFile(filepath.Join("testdata", "devcontainercli", "parse", "up.log"))
assert.Empty(t, errBuf.String(), "stderr buffer should be empty on success") require.NoError(t, err, "reading expected log file")
// Verify stdout buffer contains the CLI logs and stderr is empty.
assert.Equal(t, string(expLog), outBuf.String(), "stdout buffer should match CLI logs")
assert.Empty(t, errBuf.String(), "stderr buffer should be empty on success")
})
t.Run("Exec", func(t *testing.T) {
t.Parallel()
logFile := filepath.Join(t.TempDir(), "exec.log")
f, err := os.Create(logFile)
require.NoError(t, err, "create exec log file")
_, err = f.WriteString("exec command log\n")
require.NoError(t, err, "write to exec log file")
err = f.Close()
require.NoError(t, err, "close exec log file")
// Buffers to capture stdout and stderr.
outBuf := &bytes.Buffer{}
errBuf := &bytes.Buffer{}
// Simulate CLI execution for exec command with container ID.
wantArgs := "exec --workspace-folder /test/workspace --container-id test-container-456 echo hello"
testExecer := &testDevcontainerExecer{
testExePath: testExePath,
wantArgs: wantArgs,
wantError: false,
logFile: logFile,
}
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
dccli := agentcontainers.NewDevcontainerCLI(logger, testExecer)
// Call Exec with WithExecOutput and WithContainerID to capture any command output.
ctx := testutil.Context(t, testutil.WaitMedium)
err = dccli.Exec(ctx, "/test/workspace", "", "echo", []string{"hello"},
agentcontainers.WithContainerID("test-container-456"),
agentcontainers.WithExecOutput(outBuf, errBuf),
)
require.NoError(t, err, "Exec should succeed")
assert.NotEmpty(t, outBuf.String(), "stdout buffer should not be empty for exec with log file")
assert.Empty(t, errBuf.String(), "stderr buffer should be empty")
})
} }
// testDevcontainerExecer implements the agentexec.Execer interface for testing. // testDevcontainerExecer implements the agentexec.Execer interface for testing.
@ -243,13 +392,16 @@ func TestDevcontainerHelperProcess(t *testing.T) {
} }
logFilePath := os.Getenv("TEST_DEVCONTAINER_LOG_FILE") logFilePath := os.Getenv("TEST_DEVCONTAINER_LOG_FILE")
output, err := os.ReadFile(logFilePath) if logFilePath != "" {
if err != nil { // Read and output log file for commands that need it (like "up")
fmt.Fprintf(os.Stderr, "Reading log file %s failed: %v\n", logFilePath, err) output, err := os.ReadFile(logFilePath)
os.Exit(2) if err != nil {
fmt.Fprintf(os.Stderr, "Reading log file %s failed: %v\n", logFilePath, err)
os.Exit(2)
}
_, _ = io.Copy(os.Stdout, bytes.NewReader(output))
} }
_, _ = io.Copy(os.Stdout, bytes.NewReader(output))
if os.Getenv("TEST_DEVCONTAINER_WANT_ERROR") == "true" { if os.Getenv("TEST_DEVCONTAINER_WANT_ERROR") == "true" {
os.Exit(1) os.Exit(1)
} }