feat(agent/agentcontainers): refactor Lister to ContainerCLI and implement new methods (#18243)

This commit is contained in:
Mathias Fredriksson
2025-06-06 10:33:09 +00:00
committed by GitHub
parent 533c6dcbbe
commit a12429e9f8
11 changed files with 367 additions and 95 deletions
+52 -36
View File
@@ -28,15 +28,31 @@ import (
"github.com/coder/quartz"
)
// fakeLister implements the agentcontainers.Lister interface for
// fakeContainerCLI implements the agentcontainers.ContainerCLI interface for
// testing.
type fakeLister struct {
type fakeContainerCLI struct {
containers codersdk.WorkspaceAgentListContainersResponse
err error
listErr error
arch string
archErr error
copyErr error
execErr error
}
func (f *fakeLister) List(_ context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) {
return f.containers, f.err
func (f *fakeContainerCLI) List(_ context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) {
return f.containers, f.listErr
}
func (f *fakeContainerCLI) DetectArchitecture(_ context.Context, _ string) (string, error) {
return f.arch, f.archErr
}
func (f *fakeContainerCLI) Copy(ctx context.Context, name, src, dst string) error {
return f.copyErr
}
func (f *fakeContainerCLI) ExecAs(ctx context.Context, name, user string, args ...string) ([]byte, error) {
return nil, f.execErr
}
// fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI
@@ -180,7 +196,7 @@ func TestAPI(t *testing.T) {
// initialData to be stored in the handler
initialData initialDataPayload
// function to set up expectations for the mock
setupMock func(mcl *acmock.MockLister, preReq *gomock.Call)
setupMock func(mcl *acmock.MockContainerCLI, preReq *gomock.Call)
// expected result
expected codersdk.WorkspaceAgentListContainersResponse
// expected error
@@ -189,7 +205,7 @@ func TestAPI(t *testing.T) {
{
name: "no initial data",
initialData: initialDataPayload{makeResponse(), nil},
setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) {
setupMock: func(mcl *acmock.MockContainerCLI, preReq *gomock.Call) {
mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).After(preReq).AnyTimes()
},
expected: makeResponse(fakeCt),
@@ -207,7 +223,7 @@ func TestAPI(t *testing.T) {
{
name: "lister error only during initial data",
initialData: initialDataPayload{makeResponse(), assert.AnError},
setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) {
setupMock: func(mcl *acmock.MockContainerCLI, preReq *gomock.Call) {
mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt), nil).After(preReq).AnyTimes()
},
expected: makeResponse(fakeCt),
@@ -215,7 +231,7 @@ func TestAPI(t *testing.T) {
{
name: "lister error after initial data",
initialData: initialDataPayload{makeResponse(fakeCt), nil},
setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) {
setupMock: func(mcl *acmock.MockContainerCLI, preReq *gomock.Call) {
mcl.EXPECT().List(gomock.Any()).Return(makeResponse(), assert.AnError).After(preReq).AnyTimes()
},
expectedErr: assert.AnError.Error(),
@@ -223,7 +239,7 @@ func TestAPI(t *testing.T) {
{
name: "updated data",
initialData: initialDataPayload{makeResponse(fakeCt), nil},
setupMock: func(mcl *acmock.MockLister, preReq *gomock.Call) {
setupMock: func(mcl *acmock.MockContainerCLI, preReq *gomock.Call) {
mcl.EXPECT().List(gomock.Any()).Return(makeResponse(fakeCt2), nil).After(preReq).AnyTimes()
},
expected: makeResponse(fakeCt2),
@@ -236,7 +252,7 @@ func TestAPI(t *testing.T) {
mClock = quartz.NewMock(t)
tickerTrap = mClock.Trap().TickerFunc("updaterLoop")
mCtrl = gomock.NewController(t)
mLister = acmock.NewMockLister(mCtrl)
mLister = acmock.NewMockContainerCLI(mCtrl)
logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)
r = chi.NewRouter()
)
@@ -250,7 +266,7 @@ func TestAPI(t *testing.T) {
api := agentcontainers.NewAPI(logger,
agentcontainers.WithClock(mClock),
agentcontainers.WithLister(mLister),
agentcontainers.WithContainerCLI(mLister),
)
defer api.Close()
r.Mount("/", api.Routes())
@@ -326,7 +342,7 @@ func TestAPI(t *testing.T) {
tests := []struct {
name string
containerID string
lister *fakeLister
lister *fakeContainerCLI
devcontainerCLI *fakeDevcontainerCLI
wantStatus []int
wantBody []string
@@ -334,7 +350,7 @@ func TestAPI(t *testing.T) {
{
name: "Missing container ID",
containerID: "",
lister: &fakeLister{},
lister: &fakeContainerCLI{},
devcontainerCLI: &fakeDevcontainerCLI{},
wantStatus: []int{http.StatusBadRequest},
wantBody: []string{"Missing container ID or name"},
@@ -342,8 +358,8 @@ func TestAPI(t *testing.T) {
{
name: "List error",
containerID: "container-id",
lister: &fakeLister{
err: xerrors.New("list error"),
lister: &fakeContainerCLI{
listErr: xerrors.New("list error"),
},
devcontainerCLI: &fakeDevcontainerCLI{},
wantStatus: []int{http.StatusInternalServerError},
@@ -352,7 +368,7 @@ func TestAPI(t *testing.T) {
{
name: "Container not found",
containerID: "nonexistent-container",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
},
@@ -364,7 +380,7 @@ func TestAPI(t *testing.T) {
{
name: "Missing workspace folder label",
containerID: "missing-folder-container",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{missingFolderContainer},
},
@@ -376,7 +392,7 @@ func TestAPI(t *testing.T) {
{
name: "Devcontainer CLI error",
containerID: "container-id",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
},
@@ -390,7 +406,7 @@ func TestAPI(t *testing.T) {
{
name: "OK",
containerID: "container-id",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{validContainer},
},
@@ -423,7 +439,7 @@ func TestAPI(t *testing.T) {
api := agentcontainers.NewAPI(
logger,
agentcontainers.WithClock(mClock),
agentcontainers.WithLister(tt.lister),
agentcontainers.WithContainerCLI(tt.lister),
agentcontainers.WithDevcontainerCLI(tt.devcontainerCLI),
agentcontainers.WithWatcher(watcher.NewNoop()),
)
@@ -559,7 +575,7 @@ func TestAPI(t *testing.T) {
tests := []struct {
name string
lister *fakeLister
lister *fakeContainerCLI
knownDevcontainers []codersdk.WorkspaceAgentDevcontainer
wantStatus int
wantCount int
@@ -567,20 +583,20 @@ func TestAPI(t *testing.T) {
}{
{
name: "List error",
lister: &fakeLister{
err: xerrors.New("list error"),
lister: &fakeContainerCLI{
listErr: xerrors.New("list error"),
},
wantStatus: http.StatusInternalServerError,
},
{
name: "Empty containers",
lister: &fakeLister{},
lister: &fakeContainerCLI{},
wantStatus: http.StatusOK,
wantCount: 0,
},
{
name: "Only known devcontainers, no containers",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{},
},
@@ -597,7 +613,7 @@ func TestAPI(t *testing.T) {
},
{
name: "Runtime-detected devcontainer",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{
{
@@ -631,7 +647,7 @@ func TestAPI(t *testing.T) {
},
{
name: "Mixed known and runtime-detected devcontainers",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{
{
@@ -679,7 +695,7 @@ func TestAPI(t *testing.T) {
},
{
name: "Both running and non-running containers have container references",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{
{
@@ -723,7 +739,7 @@ func TestAPI(t *testing.T) {
},
{
name: "Config path update",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{
{
@@ -759,7 +775,7 @@ func TestAPI(t *testing.T) {
},
{
name: "Name generation and uniqueness",
lister: &fakeLister{
lister: &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{
{
@@ -831,7 +847,7 @@ func TestAPI(t *testing.T) {
r := chi.NewRouter()
apiOptions := []agentcontainers.Option{
agentcontainers.WithClock(mClock),
agentcontainers.WithLister(tt.lister),
agentcontainers.WithContainerCLI(tt.lister),
agentcontainers.WithWatcher(watcher.NewNoop()),
}
@@ -914,7 +930,7 @@ func TestAPI(t *testing.T) {
ctx := testutil.Context(t, testutil.WaitShort)
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
fLister := &fakeLister{
fLister := &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{container},
},
@@ -926,7 +942,7 @@ func TestAPI(t *testing.T) {
api := agentcontainers.NewAPI(logger,
agentcontainers.WithClock(mClock),
agentcontainers.WithLister(fLister),
agentcontainers.WithContainerCLI(fLister),
agentcontainers.WithWatcher(fWatcher),
agentcontainers.WithDevcontainers(
[]codersdk.WorkspaceAgentDevcontainer{dc},
@@ -1013,7 +1029,7 @@ func TestAPI(t *testing.T) {
mClock.Set(startTime)
tickerTrap := mClock.Trap().TickerFunc("updaterLoop")
fWatcher := newFakeWatcher(t)
fLister := &fakeLister{
fLister := &fakeContainerCLI{
containers: codersdk.WorkspaceAgentListContainersResponse{
Containers: []codersdk.WorkspaceAgentContainer{container},
},
@@ -1022,7 +1038,7 @@ func TestAPI(t *testing.T) {
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
api := agentcontainers.NewAPI(
logger,
agentcontainers.WithLister(fLister),
agentcontainers.WithContainerCLI(fLister),
agentcontainers.WithWatcher(fWatcher),
agentcontainers.WithClock(mClock),
)