From b231e8736ca460f1618b066f0d92ff1534ef01a9 Mon Sep 17 00:00:00 2001 From: Rob Watson Date: Sun, 16 Mar 2025 14:51:50 +0100 Subject: [PATCH] feat: cleanup zombie networks --- .mockery.yaml | 8 +-- internal/app/app.go | 4 ++ internal/container/container.go | 43 ++++++++++++- internal/container/container_test.go | 50 ++++++++++++--- internal/container/events_test.go | 4 +- .../mocks}/dockerclient_mock.go | 61 ++++++++++++++++++- internal/container/stats_test.go | 6 +- internal/mediaserver/api_test.go | 2 +- .../mocks}/httpclient_mock.go | 2 +- 9 files changed, 160 insertions(+), 20 deletions(-) rename internal/{generated/mocks/container => container/mocks}/dockerclient_mock.go (93%) rename internal/{generated/mocks/mediaserver => mediaserver/mocks}/httpclient_mock.go (99%) diff --git a/.mockery.yaml b/.mockery.yaml index 7c06473..4b7ed37 100644 --- a/.mockery.yaml +++ b/.mockery.yaml @@ -1,15 +1,15 @@ with-expecter: true -dir: "generated/mocks/{{ .InterfaceDirRelative }}" +dir: "{{ .InterfaceDir }}/mocks" filename: "{{ .InterfaceName | lower }}_mock.go" mockname: "{{ .InterfaceName }}" -outpkg: "{{ .PackageName }}" +outpkg: mocks issue-845-fix: true resolve-type-alias: false packages: - git.netflux.io/rob/octoplex/container: + git.netflux.io/rob/octoplex/internal/container: interfaces: DockerClient: - git.netflux.io/rob/octoplex/mediaserver: + git.netflux.io/rob/octoplex/internal/mediaserver: interfaces: httpClient: config: diff --git a/internal/app/app.go b/internal/app/app.go index 9bb6507..d07304f 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -51,6 +51,7 @@ func Run(ctx context.Context, params RunParams) error { updateUI := func() { ui.SetState(*state) } updateUI() + // TODO: check for unused networks. if exists, err := containerClient.ContainerRunning(ctx, container.AllContainers()); err != nil { return fmt.Errorf("check existing containers: %w", err) } else if exists { @@ -58,6 +59,9 @@ func Run(ctx context.Context, params RunParams) error { if err = containerClient.RemoveContainers(ctx, container.AllContainers()); err != nil { return fmt.Errorf("remove existing containers: %w", err) } + if err = containerClient.RemoveUnusedNetworks(ctx); err != nil { + return fmt.Errorf("remove unused networks: %w", err) + } } else { return nil } diff --git a/internal/container/container.go b/internal/container/container.go index dff90f6..d263b6c 100644 --- a/internal/container/container.go +++ b/internal/container/container.go @@ -7,6 +7,7 @@ import ( "io" "log/slog" "maps" + "slices" "strings" "sync" "time" @@ -45,6 +46,7 @@ type DockerClient interface { ImagePull(context.Context, string, image.PullOptions) (io.ReadCloser, error) NetworkConnect(context.Context, string, string, *network.EndpointSettings) error NetworkCreate(context.Context, string, network.CreateOptions) (network.CreateResponse, error) + NetworkList(context.Context, network.ListOptions) ([]network.Summary, error) NetworkRemove(context.Context, string) error } @@ -73,7 +75,14 @@ type Client struct { // NewClient creates a new Client. func NewClient(ctx context.Context, apiClient DockerClient, logger *slog.Logger) (*Client, error) { id := shortid.New() - network, err := apiClient.NetworkCreate(ctx, domain.AppName+"-"+id.String(), network.CreateOptions{Driver: "bridge"}) + network, err := apiClient.NetworkCreate( + ctx, + domain.AppName+"-"+id.String(), + network.CreateOptions{ + Driver: "bridge", + Labels: map[string]string{LabelApp: domain.AppName, LabelAppID: id.String()}, + }, + ) if err != nil { return nil, fmt.Errorf("network create: %w", err) } @@ -441,6 +450,38 @@ func (a *Client) RemoveContainers(ctx context.Context, labelOptions LabelOptions return nil } +// RemoveUnusedNetworks removes all networks that are not used by any +// container. +func (a *Client) RemoveUnusedNetworks(ctx context.Context) error { + networks, err := a.otherNetworks(ctx) + if err != nil { + return fmt.Errorf("other networks: %w", err) + } + + for _, network := range networks { + a.logger.Info("Removing network", "id", shortID(network.ID)) + if err = a.apiClient.NetworkRemove(ctx, network.ID); err != nil { + a.logger.Error("Error removing network", "err", err, "id", shortID(network.ID)) + } + } + + return nil +} + +func (a *Client) otherNetworks(ctx context.Context) ([]network.Summary, error) { + filterArgs := filters.NewArgs() + filterArgs.Add("label", LabelApp+"="+domain.AppName) + + networks, err := a.apiClient.NetworkList(ctx, network.ListOptions{Filters: filterArgs}) + if err != nil { + return nil, fmt.Errorf("network list: %w", err) + } + + return slices.DeleteFunc(networks, func(n network.Summary) bool { + return n.ID == a.networkID + }), nil +} + // LabelOptions is a function that returns a map of labels. type LabelOptions func() map[string]string diff --git a/internal/container/container_test.go b/internal/container/container_test.go index 5dedeeb..a318a2e 100644 --- a/internal/container/container_test.go +++ b/internal/container/container_test.go @@ -8,7 +8,7 @@ import ( "time" "git.netflux.io/rob/octoplex/internal/container" - containermocks "git.netflux.io/rob/octoplex/internal/generated/mocks/container" + "git.netflux.io/rob/octoplex/internal/container/mocks" "git.netflux.io/rob/octoplex/internal/testhelpers" dockercontainer "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/events" @@ -32,12 +32,14 @@ func TestClientRunContainer(t *testing.T) { eventsC := make(chan events.Message) eventsErrC := make(chan error) - var dockerClient containermocks.DockerClient + var dockerClient mocks.DockerClient defer dockerClient.AssertExpectations(t) dockerClient. EXPECT(). - NetworkCreate(mock.Anything, mock.Anything, network.CreateOptions{Driver: "bridge"}). + NetworkCreate(mock.Anything, mock.Anything, mock.MatchedBy(func(opts network.CreateOptions) bool { + return opts.Driver == "bridge" && len(opts.Labels) > 0 + })). Return(network.CreateResponse{ID: "test-network"}, nil) dockerClient. EXPECT(). @@ -112,12 +114,14 @@ func TestClientRunContainer(t *testing.T) { func TestClientRunContainerErrorStartingContainer(t *testing.T) { logger := testhelpers.NewTestLogger() - var dockerClient containermocks.DockerClient + var dockerClient mocks.DockerClient defer dockerClient.AssertExpectations(t) dockerClient. EXPECT(). - NetworkCreate(mock.Anything, mock.Anything, network.CreateOptions{Driver: "bridge"}). + NetworkCreate(mock.Anything, mock.Anything, mock.MatchedBy(func(opts network.CreateOptions) bool { + return opts.Driver == "bridge" && len(opts.Labels) > 0 + })). Return(network.CreateResponse{ID: "test-network"}, nil) dockerClient. EXPECT(). @@ -156,12 +160,14 @@ func TestClientRunContainerErrorStartingContainer(t *testing.T) { func TestClientClose(t *testing.T) { logger := testhelpers.NewTestLogger() - var dockerClient containermocks.DockerClient + var dockerClient mocks.DockerClient defer dockerClient.AssertExpectations(t) dockerClient. EXPECT(). - NetworkCreate(mock.Anything, mock.Anything, network.CreateOptions{Driver: "bridge"}). + NetworkCreate(mock.Anything, mock.Anything, mock.MatchedBy(func(opts network.CreateOptions) bool { + return opts.Driver == "bridge" && len(opts.Labels) > 0 + })). Return(network.CreateResponse{ID: "test-network"}, nil) dockerClient. EXPECT(). @@ -189,3 +195,33 @@ func TestClientClose(t *testing.T) { require.NoError(t, containerClient.Close()) } + +func TestRemoveUnusedNetworks(t *testing.T) { + logger := testhelpers.NewTestLogger() + + var dockerClient mocks.DockerClient + defer dockerClient.AssertExpectations(t) + + dockerClient. + EXPECT(). + NetworkCreate(mock.Anything, mock.Anything, mock.MatchedBy(func(opts network.CreateOptions) bool { + return opts.Driver == "bridge" && len(opts.Labels) > 0 + })). + Return(network.CreateResponse{ID: "test-network"}, nil) + dockerClient. + EXPECT(). + NetworkList(mock.Anything, mock.Anything). + Return([]network.Summary{ + {ID: "test-network"}, + {ID: "another-network"}, + }, nil) + dockerClient. + EXPECT(). + NetworkRemove(mock.Anything, "another-network"). + Return(nil) + + containerClient, err := container.NewClient(t.Context(), &dockerClient, logger) + require.NoError(t, err) + + require.NoError(t, containerClient.RemoveUnusedNetworks(t.Context())) +} diff --git a/internal/container/events_test.go b/internal/container/events_test.go index 97be628..18ef9d5 100644 --- a/internal/container/events_test.go +++ b/internal/container/events_test.go @@ -5,7 +5,7 @@ import ( "io" "testing" - containermocks "git.netflux.io/rob/octoplex/internal/generated/mocks/container" + "git.netflux.io/rob/octoplex/internal/container/mocks" "git.netflux.io/rob/octoplex/internal/testhelpers" "github.com/docker/docker/api/types/events" "github.com/stretchr/testify/assert" @@ -19,7 +19,7 @@ func TestHandleEvents(t *testing.T) { containerID := "b905f51b47242090ae504c184c7bc84d6274511ef763c1847039dcaa00a3ad27" - var dockerClient containermocks.DockerClient + var dockerClient mocks.DockerClient defer dockerClient.AssertExpectations(t) dockerClient. diff --git a/internal/generated/mocks/container/dockerclient_mock.go b/internal/container/mocks/dockerclient_mock.go similarity index 93% rename from internal/generated/mocks/container/dockerclient_mock.go rename to internal/container/mocks/dockerclient_mock.go index 3c8fec4..edf889a 100644 --- a/internal/generated/mocks/container/dockerclient_mock.go +++ b/internal/container/mocks/dockerclient_mock.go @@ -1,6 +1,6 @@ // Code generated by mockery v2.52.2. DO NOT EDIT. -package container +package mocks import ( context "context" @@ -746,6 +746,65 @@ func (_c *DockerClient_NetworkCreate_Call) RunAndReturn(run func(context.Context return _c } +// NetworkList provides a mock function with given fields: _a0, _a1 +func (_m *DockerClient) NetworkList(_a0 context.Context, _a1 network.ListOptions) ([]network.Summary, error) { + ret := _m.Called(_a0, _a1) + + if len(ret) == 0 { + panic("no return value specified for NetworkList") + } + + var r0 []network.Summary + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, network.ListOptions) ([]network.Summary, error)); ok { + return rf(_a0, _a1) + } + if rf, ok := ret.Get(0).(func(context.Context, network.ListOptions) []network.Summary); ok { + r0 = rf(_a0, _a1) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]network.Summary) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, network.ListOptions) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// DockerClient_NetworkList_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'NetworkList' +type DockerClient_NetworkList_Call struct { + *mock.Call +} + +// NetworkList is a helper method to define mock.On call +// - _a0 context.Context +// - _a1 network.ListOptions +func (_e *DockerClient_Expecter) NetworkList(_a0 interface{}, _a1 interface{}) *DockerClient_NetworkList_Call { + return &DockerClient_NetworkList_Call{Call: _e.mock.On("NetworkList", _a0, _a1)} +} + +func (_c *DockerClient_NetworkList_Call) Run(run func(_a0 context.Context, _a1 network.ListOptions)) *DockerClient_NetworkList_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(network.ListOptions)) + }) + return _c +} + +func (_c *DockerClient_NetworkList_Call) Return(_a0 []network.Summary, _a1 error) *DockerClient_NetworkList_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *DockerClient_NetworkList_Call) RunAndReturn(run func(context.Context, network.ListOptions) ([]network.Summary, error)) *DockerClient_NetworkList_Call { + _c.Call.Return(run) + return _c +} + // NetworkRemove provides a mock function with given fields: _a0, _a1 func (_m *DockerClient) NetworkRemove(_a0 context.Context, _a1 string) error { ret := _m.Called(_a0, _a1) diff --git a/internal/container/stats_test.go b/internal/container/stats_test.go index 55510db..03cb3f2 100644 --- a/internal/container/stats_test.go +++ b/internal/container/stats_test.go @@ -7,7 +7,7 @@ import ( "io" "testing" - containermocks "git.netflux.io/rob/octoplex/internal/generated/mocks/container" + "git.netflux.io/rob/octoplex/internal/container/mocks" "git.netflux.io/rob/octoplex/internal/testhelpers" dockercontainer "github.com/docker/docker/api/types/container" "github.com/stretchr/testify/assert" @@ -24,7 +24,7 @@ func TestHandleStats(t *testing.T) { pr, pw := io.Pipe() containerID := "b905f51b47242090ae504c184c7bc84d6274511ef763c1847039dcaa00a3ad27" - var dockerClient containermocks.DockerClient + var dockerClient mocks.DockerClient defer dockerClient.AssertExpectations(t) dockerClient. @@ -70,7 +70,7 @@ func TestHandleStatsWithContainerRestart(t *testing.T) { pr, pw := io.Pipe() containerID := "d0adc747fb12b9ce2376408aed8538a0769de55aa9c239313f231d9d80402e39" - var dockerClient containermocks.DockerClient + var dockerClient mocks.DockerClient defer dockerClient.AssertExpectations(t) dockerClient. diff --git a/internal/mediaserver/api_test.go b/internal/mediaserver/api_test.go index b9ed358..f7e7e14 100644 --- a/internal/mediaserver/api_test.go +++ b/internal/mediaserver/api_test.go @@ -7,7 +7,7 @@ import ( "net/http" "testing" - mocks "git.netflux.io/rob/octoplex/internal/generated/mocks/mediaserver" + "git.netflux.io/rob/octoplex/internal/mediaserver/mocks" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) diff --git a/internal/generated/mocks/mediaserver/httpclient_mock.go b/internal/mediaserver/mocks/httpclient_mock.go similarity index 99% rename from internal/generated/mocks/mediaserver/httpclient_mock.go rename to internal/mediaserver/mocks/httpclient_mock.go index 43a4371..62d20f1 100644 --- a/internal/generated/mocks/mediaserver/httpclient_mock.go +++ b/internal/mediaserver/mocks/httpclient_mock.go @@ -1,6 +1,6 @@ // Code generated by mockery v2.52.2. DO NOT EDIT. -package mediaserver +package mocks import ( http "net/http"