Skip to content

Commit d61353f

Browse files
DanielleMaywoodDanielleMaywood
authored
fix(agent/agentcontainers): read WorkspaceFolder from config (#18467)
Instead of exec'ing `pwd` inside of the container, we instead read `WorkspaceFolder` from the outcome of `read-configuration`.
1 parent 9c1feff commit d61353f

File tree

3 files changed

+20
-51
lines changed

3 files changed

+20
-51
lines changed

‎agent/agentcontainers/api.go

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
package agentcontainers
22

33
import (
4-
"bytes"
54
"context"
65
"errors"
76
"fmt"
8-
"io"
97
"net/http"
108
"os"
119
"path"
@@ -1114,27 +1112,6 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11141112
if proc.agent.ID == uuid.Nil || maybeRecreateSubAgent {
11151113
subAgentConfig.Architecture = arch
11161114

1117-
// Detect workspace folder by executing `pwd` in the container.
1118-
// NOTE(mafredri): This is a quick and dirty way to detect the
1119-
// workspace folder inside the container. In the future we will
1120-
// rely more on `devcontainer read-configuration`.
1121-
var pwdBuf bytes.Buffer
1122-
err = api.dccli.Exec(ctx, dc.WorkspaceFolder, dc.ConfigPath, "pwd", []string{},
1123-
WithExecOutput(&pwdBuf, io.Discard),
1124-
WithExecContainerID(container.ID),
1125-
)
1126-
if err != nil {
1127-
return xerrors.Errorf("check workspace folder in container: %w", err)
1128-
}
1129-
directory := strings.TrimSpace(pwdBuf.String())
1130-
if directory == "" {
1131-
logger.Warn(ctx, "detected workspace folder is empty, using default workspace folder",
1132-
slog.F("default_workspace_folder", DevcontainerDefaultContainerWorkspaceFolder),
1133-
)
1134-
directory = DevcontainerDefaultContainerWorkspaceFolder
1135-
}
1136-
subAgentConfig.Directory = directory
1137-
11381115
displayAppsMap := map[codersdk.DisplayApp]bool{
11391116
// NOTE(DanielleMaywood):
11401117
// We use the same defaults here as set in terraform-provider-coder.
@@ -1146,7 +1123,10 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11461123
codersdk.DisplayAppPortForward: true,
11471124
}
11481125

1149-
var appsWithPossibleDuplicates []SubAgentApp
1126+
var (
1127+
appsWithPossibleDuplicates []SubAgentApp
1128+
workspaceFolder = DevcontainerDefaultContainerWorkspaceFolder
1129+
)
11501130

11511131
if err := func() error {
11521132
var (
@@ -1167,6 +1147,8 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
11671147
return err
11681148
}
11691149

1150+
workspaceFolder = config.Workspace.WorkspaceFolder
1151+
11701152
// NOTE(DanielleMaywood):
11711153
// We only want to take an agent name specified in the root customization layer.
11721154
// This restricts the ability for a feature to specify the agent name. We may revisit
@@ -1241,6 +1223,7 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c
12411223

12421224
subAgentConfig.DisplayApps = displayApps
12431225
subAgentConfig.Apps = apps
1226+
subAgentConfig.Directory = workspaceFolder
12441227
}
12451228

12461229
deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig)

‎agent/agentcontainers/api_test.go

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,6 +1262,11 @@ func TestAPI(t *testing.T) {
12621262
deleteErrC: make(chan error, 1),
12631263
}
12641264
fakeDCCLI = &fakeDevcontainerCLI{
1265+
readConfig: agentcontainers.DevcontainerConfig{
1266+
Workspace: agentcontainers.DevcontainerWorkspace{
1267+
WorkspaceFolder: "/workspaces/coder",
1268+
},
1269+
},
12651270
execErrC: make(chan func(cmd string, args ...string) error, 1),
12661271
readConfigErrC: make(chan func(envs []string) error, 1),
12671272
}
@@ -1273,8 +1278,8 @@ func TestAPI(t *testing.T) {
12731278
Running: true,
12741279
CreatedAt: time.Now(),
12751280
Labels: map[string]string{
1276-
agentcontainers.DevcontainerLocalFolderLabel: "/workspaces",
1277-
agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json",
1281+
agentcontainers.DevcontainerLocalFolderLabel: "/home/coder/coder",
1282+
agentcontainers.DevcontainerConfigFileLabel: "/home/coder/coder/.devcontainer/devcontainer.json",
12781283
},
12791284
}
12801285
)
@@ -1320,11 +1325,6 @@ func TestAPI(t *testing.T) {
13201325

13211326
// Allow initial agent creation and injection to succeed.
13221327
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil)
1323-
testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error {
1324-
assert.Equal(t, "pwd", cmd)
1325-
assert.Empty(t, args)
1326-
return nil
1327-
}) // Exec pwd.
13281328
testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error {
13291329
assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container")
13301330
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace")
@@ -1350,7 +1350,7 @@ func TestAPI(t *testing.T) {
13501350
// Verify agent was created.
13511351
require.Len(t, fakeSAC.created, 1)
13521352
assert.Equal(t, "test-container", fakeSAC.created[0].Name)
1353-
assert.Equal(t, "/workspaces", fakeSAC.created[0].Directory)
1353+
assert.Equal(t, "/workspaces/coder", fakeSAC.created[0].Directory)
13541354
assert.Len(t, fakeSAC.deleted, 0)
13551355

13561356
t.Log("Agent injected successfully, now testing reinjection into the same container...")
@@ -1467,11 +1467,6 @@ func TestAPI(t *testing.T) {
14671467
testutil.RequireSend(ctx, t, fakeSAC.deleteErrC, nil)
14681468
// Expect the agent to be recreated.
14691469
testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil)
1470-
testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error {
1471-
assert.Equal(t, "pwd", cmd)
1472-
assert.Empty(t, args)
1473-
return nil
1474-
}) // Exec pwd.
14751470
testutil.RequireSend(ctx, t, fakeDCCLI.readConfigErrC, func(envs []string) error {
14761471
assert.Contains(t, envs, "CODER_WORKSPACE_AGENT_NAME=test-container")
14771472
assert.Contains(t, envs, "CODER_WORKSPACE_NAME=test-workspace")
@@ -1814,7 +1809,6 @@ func TestAPI(t *testing.T) {
18141809
},
18151810
},
18161811
},
1817-
execErrC: make(chan func(cmd string, args ...string) error, 1),
18181812
}
18191813

18201814
testContainer = codersdk.WorkspaceAgentContainer{
@@ -1861,15 +1855,9 @@ func TestAPI(t *testing.T) {
18611855

18621856
// Close before api.Close() defer to avoid deadlock after test.
18631857
defer close(fSAC.createErrC)
1864-
defer close(fDCCLI.execErrC)
18651858

18661859
// Given: We allow agent creation and injection to succeed.
18671860
testutil.RequireSend(ctx, t, fSAC.createErrC, nil)
1868-
testutil.RequireSend(ctx, t, fDCCLI.execErrC, func(cmd string, args ...string) error {
1869-
assert.Equal(t, "pwd", cmd)
1870-
assert.Empty(t, args)
1871-
return nil
1872-
})
18731861

18741862
// Wait until the ticker has been registered.
18751863
tickerTrap.MustWait(ctx).MustRelease(ctx)
@@ -1913,7 +1901,6 @@ func TestAPI(t *testing.T) {
19131901
},
19141902
},
19151903
readConfigErrC: make(chan func(envs []string) error, 2),
1916-
execErrC: make(chan func(cmd string, args ...string) error, 1),
19171904
}
19181905

19191906
testContainer = codersdk.WorkspaceAgentContainer{
@@ -1960,16 +1947,10 @@ func TestAPI(t *testing.T) {
19601947

19611948
// Close before api.Close() defer to avoid deadlock after test.
19621949
defer close(fSAC.createErrC)
1963-
defer close(fDCCLI.execErrC)
19641950
defer close(fDCCLI.readConfigErrC)
19651951

19661952
// Given: We allow agent creation and injection to succeed.
19671953
testutil.RequireSend(ctx, t, fSAC.createErrC, nil)
1968-
testutil.RequireSend(ctx, t, fDCCLI.execErrC, func(cmd string, args ...string) error {
1969-
assert.Equal(t, "pwd", cmd)
1970-
assert.Empty(t, args)
1971-
return nil
1972-
})
19731954
testutil.RequireSend(ctx, t, fDCCLI.readConfigErrC, func(env []string) error {
19741955
// We expect the wrong workspace agent name passed in first.
19751956
assert.Contains(t, env, "CODER_WORKSPACE_AGENT_NAME=test-container")

‎agent/agentcontainers/devcontainercli.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
type DevcontainerConfig struct {
2323
MergedConfiguration DevcontainerMergedConfiguration `json:"mergedConfiguration"`
2424
Configuration DevcontainerConfiguration `json:"configuration"`
25+
Workspace DevcontainerWorkspace `json:"workspace"`
2526
}
2627

2728
type DevcontainerMergedConfiguration struct {
@@ -46,6 +47,10 @@ type CoderCustomization struct {
4647
Name string `json:"name,omitempty"`
4748
}
4849

50+
type DevcontainerWorkspace struct {
51+
WorkspaceFolder string `json:"workspaceFolder"`
52+
}
53+
4954
// DevcontainerCLI is an interface for the devcontainer CLI.
5055
type DevcontainerCLI interface {
5156
Up(ctx context.Context, workspaceFolder, configPath string, opts ...DevcontainerCLIUpOptions) (id string, err error)

0 commit comments

Comments
 (0)