Skip to content

fix(agent/agentcontainers): always derive devcontainer name from workspace folder #18666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions agent/agentcontainers/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,25 @@ func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer, scri
if dc.Status == "" {
dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting
}
logger := api.logger.With(
slog.F("devcontainer_id", dc.ID),
slog.F("devcontainer_name", dc.Name),
slog.F("workspace_folder", dc.WorkspaceFolder),
slog.F("config_path", dc.ConfigPath),
)

// Devcontainers have a name originating from Terraform, but
// we need to ensure that the name is unique. We will use
// the workspace folder name to generate a unique agent name,
// and if that fails, we will fall back to the devcontainers
// original name.
name, usingWorkspaceFolder := api.makeAgentName(dc.WorkspaceFolder, dc.Name)
if name != dc.Name {
logger = logger.With(slog.F("devcontainer_name", name))
logger.Debug(api.ctx, "updating devcontainer name", slog.F("devcontainer_old_name", dc.Name))
dc.Name = name
api.usingWorkspaceFolderName[dc.WorkspaceFolder] = usingWorkspaceFolder
}

api.knownDevcontainers[dc.WorkspaceFolder] = dc
api.devcontainerNames[dc.Name] = true
Expand All @@ -223,12 +242,7 @@ func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer, scri
}
}
if api.devcontainerLogSourceIDs[dc.WorkspaceFolder] == uuid.Nil {
api.logger.Error(api.ctx, "devcontainer log source ID not found for devcontainer",
slog.F("devcontainer_id", dc.ID),
slog.F("devcontainer_name", dc.Name),
slog.F("workspace_folder", dc.WorkspaceFolder),
slog.F("config_path", dc.ConfigPath),
)
logger.Error(api.ctx, "devcontainer log source ID not found for devcontainer")
}
}
}
Expand Down Expand Up @@ -872,7 +886,7 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse,
devcontainers = append(devcontainers, dc)
}
slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int {
return strings.Compare(a.Name, b.Name)
return strings.Compare(a.WorkspaceFolder, b.WorkspaceFolder)
})
}

Expand Down
79 changes: 79 additions & 0 deletions agent/agentcontainers/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2596,3 +2596,82 @@ func fakeContainer(t *testing.T, mut ...func(*codersdk.WorkspaceAgentContainer))
}
return ct
}

func TestWithDevcontainersNameGeneration(t *testing.T) {
t.Parallel()

if runtime.GOOS == "windows" {
t.Skip("Dev Container tests are not supported on Windows")
}

devcontainers := []codersdk.WorkspaceAgentDevcontainer{
{
ID: uuid.New(),
Name: "original-name",
WorkspaceFolder: "/home/coder/foo/project",
ConfigPath: "/home/coder/foo/project/.devcontainer/devcontainer.json",
},
{
ID: uuid.New(),
Name: "another-name",
WorkspaceFolder: "/home/coder/bar/project",
ConfigPath: "/home/coder/bar/project/.devcontainer/devcontainer.json",
},
}

scripts := []codersdk.WorkspaceAgentScript{
{ID: devcontainers[0].ID, LogSourceID: uuid.New()},
{ID: devcontainers[1].ID, LogSourceID: uuid.New()},
}

logger := testutil.Logger(t)

// This should trigger the WithDevcontainers code path where names are generated
api := agentcontainers.NewAPI(logger,
agentcontainers.WithDevcontainers(devcontainers, scripts),
agentcontainers.WithContainerCLI(&fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{
fakeContainer(t, func(c *codersdk.WorkspaceAgentContainer) {
c.ID = "some-container-id-1"
c.FriendlyName = "container-name-1"
c.Labels[agentcontainers.DevcontainerLocalFolderLabel] = "/home/coder/baz/project"
c.Labels[agentcontainers.DevcontainerConfigFileLabel] = "/home/coder/baz/project/.devcontainer/devcontainer.json"
}),
},
},
}),
agentcontainers.WithDevcontainerCLI(&fakeDevcontainerCLI{}),
agentcontainers.WithSubAgentClient(&fakeSubAgentClient{}),
agentcontainers.WithWatcher(watcher.NewNoop()),
)
defer api.Close()
api.Start()

r := chi.NewRouter()
r.Mount("/", api.Routes())

ctx := context.Background()

err := api.RefreshContainers(ctx)
require.NoError(t, err, "RefreshContainers should not error")

// Initial request returns the initial data.
req := httptest.NewRequest(http.MethodGet, "/", nil).
WithContext(ctx)
rec := httptest.NewRecorder()
r.ServeHTTP(rec, req)

require.Equal(t, http.StatusOK, rec.Code)
var response codersdk.WorkspaceAgentListContainersResponse
err = json.NewDecoder(rec.Body).Decode(&response)
require.NoError(t, err)

// Verify the devcontainers have the expected names.
require.Len(t, response.Devcontainers, 3, "should have two devcontainers")
assert.NotEqual(t, "original-name", response.Devcontainers[2].Name, "first devcontainer should not keep original name")
assert.Equal(t, "project", response.Devcontainers[2].Name, "first devcontainer should use the project folder name")
assert.NotEqual(t, "another-name", response.Devcontainers[0].Name, "second devcontainer should not keep original name")
assert.Equal(t, "bar-project", response.Devcontainers[0].Name, "second devcontainer should has a collision and uses the folder name with a prefix")
assert.Equal(t, "baz-project", response.Devcontainers[1].Name, "third devcontainer should use the folder name with a prefix since it collides with the first two")
}
Loading