Skip to content

Commit

Permalink
perf: introduces a timeout when fetching host info (#3276)
Browse files Browse the repository at this point in the history
  • Loading branch information
amir20 authored Sep 20, 2024
1 parent 03fdd82 commit 92614ea
Show file tree
Hide file tree
Showing 27 changed files with 213 additions and 178 deletions.
19 changes: 10 additions & 9 deletions internal/agent/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import (
"crypto/tls"
"crypto/x509"
"fmt"
"github.com/goccy/go-json"
"io"
"time"

"github.com/goccy/go-json"

"github.com/amir20/dozzle/internal/agent/pb"
"github.com/amir20/dozzle/internal/docker"
"github.com/amir20/dozzle/internal/utils"
Expand Down Expand Up @@ -260,8 +261,8 @@ func (c *Client) StreamNewContainers(ctx context.Context, containers chan<- dock
}
}

func (c *Client) FindContainer(containerID string) (docker.Container, error) {
response, err := c.client.FindContainer(context.Background(), &pb.FindContainerRequest{ContainerId: containerID})
func (c *Client) FindContainer(ctx context.Context, containerID string) (docker.Container, error) {
response, err := c.client.FindContainer(ctx, &pb.FindContainerRequest{ContainerId: containerID})
if err != nil {
return docker.Container{}, err
}
Expand Down Expand Up @@ -294,8 +295,8 @@ func (c *Client) FindContainer(containerID string) (docker.Container, error) {
}, nil
}

func (c *Client) ListContainers() ([]docker.Container, error) {
response, err := c.client.ListContainers(context.Background(), &pb.ListContainersRequest{})
func (c *Client) ListContainers(ctx context.Context) ([]docker.Container, error) {
response, err := c.client.ListContainers(ctx, &pb.ListContainersRequest{})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -332,8 +333,8 @@ func (c *Client) ListContainers() ([]docker.Container, error) {
return containers, nil
}

func (c *Client) Host() (docker.Host, error) {
info, err := c.client.HostInfo(context.Background(), &pb.HostInfoRequest{})
func (c *Client) Host(ctx context.Context) (docker.Host, error) {
info, err := c.client.HostInfo(ctx, &pb.HostInfoRequest{})
if err != nil {
return docker.Host{
Endpoint: c.endpoint,
Expand All @@ -354,7 +355,7 @@ func (c *Client) Host() (docker.Host, error) {
}, nil
}

func (c *Client) ContainerAction(containerId string, action docker.ContainerAction) error {
func (c *Client) ContainerAction(ctx context.Context, containerId string, action docker.ContainerAction) error {
var containerAction pb.ContainerAction
switch action {
case docker.Start:
Expand All @@ -368,7 +369,7 @@ func (c *Client) ContainerAction(containerId string, action docker.ContainerActi

}

_, err := c.client.ContainerAction(context.Background(), &pb.ContainerActionRequest{ContainerId: containerId, Action: containerAction})
_, err := c.client.ContainerAction(ctx, &pb.ContainerActionRequest{ContainerId: containerId, Action: containerAction})

return err
}
Expand Down
20 changes: 10 additions & 10 deletions internal/agent/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ type MockedClient struct {
docker.Client
}

func (m *MockedClient) FindContainer(id string) (docker.Container, error) {
args := m.Called(id)
func (m *MockedClient) FindContainer(ctx context.Context, id string) (docker.Container, error) {
args := m.Called(ctx, id)
return args.Get(0).(docker.Container), args.Error(1)
}

func (m *MockedClient) ContainerActions(action docker.ContainerAction, containerID string) error {
args := m.Called(action, containerID)
func (m *MockedClient) ContainerActions(ctx context.Context, action docker.ContainerAction, containerID string) error {
args := m.Called(ctx, action, containerID)
return args.Error(0)
}

Expand All @@ -46,8 +46,8 @@ func (m *MockedClient) ContainerEvents(ctx context.Context, events chan<- docker
return args.Error(0)
}

func (m *MockedClient) ListContainers() ([]docker.Container, error) {
args := m.Called()
func (m *MockedClient) ListContainers(ctx context.Context) ([]docker.Container, error) {
args := m.Called(ctx)
return args.Get(0).([]docker.Container), args.Error(1)
}

Expand Down Expand Up @@ -92,7 +92,7 @@ func init() {
}

client = &MockedClient{}
client.On("ListContainers").Return([]docker.Container{
client.On("ListContainers", mock.Anything).Return([]docker.Container{
{
ID: "123456",
Name: "test",
Expand All @@ -111,7 +111,7 @@ func init() {
time.Sleep(5 * time.Second)
})

client.On("FindContainer", "123456").Return(docker.Container{
client.On("FindContainer", mock.Anything, "123456").Return(docker.Container{
ID: "123456",
Name: "test",
Host: "localhost",
Expand Down Expand Up @@ -142,7 +142,7 @@ func TestFindContainer(t *testing.T) {
t.Fatal(err)
}

container, _ := rpc.FindContainer("123456")
container, _ := rpc.FindContainer(context.Background(), "123456")

assert.Equal(t, container, docker.Container{
ID: "123456",
Expand All @@ -167,7 +167,7 @@ func TestListContainers(t *testing.T) {
t.Fatal(err)
}

containers, _ := rpc.ListContainers()
containers, _ := rpc.ListContainers(context.Background())

assert.Equal(t, containers, []docker.Container{
{
Expand Down
4 changes: 2 additions & 2 deletions internal/agent/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (s *server) LogsBetweenDates(in *pb.LogsBetweenDatesRequest, out pb.AgentSe
return err
}

container, err := s.client.FindContainer(in.ContainerId)
container, err := s.client.FindContainer(out.Context(), in.ContainerId)
if err != nil {
return err
}
Expand Down Expand Up @@ -295,7 +295,7 @@ func (s *server) ContainerAction(ctx context.Context, in *pb.ContainerActionRequ
return nil, status.Error(codes.InvalidArgument, "invalid action")
}

err := s.client.ContainerActions(action, in.ContainerId)
err := s.client.ContainerActions(ctx, action, in.ContainerId)

if err != nil {
return nil, status.Error(codes.Internal, err.Error())
Expand Down
22 changes: 11 additions & 11 deletions internal/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ type DockerCLI interface {
}

type Client interface {
ListContainers() ([]Container, error)
FindContainer(string) (Container, error)
ListContainers(context.Context) ([]Container, error)
FindContainer(context.Context, string) (Container, error)
ContainerLogs(context.Context, string, time.Time, StdType) (io.ReadCloser, error)
ContainerEvents(context.Context, chan<- ContainerEvent) error
ContainerLogsBetweenDates(context.Context, string, time.Time, time.Time, StdType) (io.ReadCloser, error)
ContainerStats(context.Context, string, chan<- ContainerStat) error
Ping(context.Context) (types.Ping, error)
Host() Host
ContainerActions(action ContainerAction, containerID string) error
ContainerActions(ctx context.Context, action ContainerAction, containerID string) error
IsSwarmMode() bool
SystemInfo() system.Info
}
Expand Down Expand Up @@ -179,36 +179,36 @@ func NewRemoteClient(f map[string][]string, host Host) (Client, error) {
}

// Finds a container by id, skipping the filters
func (d *httpClient) FindContainer(id string) (Container, error) {
func (d *httpClient) FindContainer(ctx context.Context, id string) (Container, error) {
log.Debug().Str("id", id).Msg("Finding container")
if json, err := d.cli.ContainerInspect(context.Background(), id); err == nil {
if json, err := d.cli.ContainerInspect(ctx, id); err == nil {
return newContainerFromJSON(json, d.host.ID), nil
} else {
return Container{}, err
}

}

func (d *httpClient) ContainerActions(action ContainerAction, containerID string) error {
func (d *httpClient) ContainerActions(ctx context.Context, action ContainerAction, containerID string) error {
switch action {
case Start:
return d.cli.ContainerStart(context.Background(), containerID, container.StartOptions{})
return d.cli.ContainerStart(ctx, containerID, container.StartOptions{})
case Stop:
return d.cli.ContainerStop(context.Background(), containerID, container.StopOptions{})
return d.cli.ContainerStop(ctx, containerID, container.StopOptions{})
case Restart:
return d.cli.ContainerRestart(context.Background(), containerID, container.StopOptions{})
return d.cli.ContainerRestart(ctx, containerID, container.StopOptions{})
default:
return fmt.Errorf("unknown action: %s", action)
}
}

func (d *httpClient) ListContainers() ([]Container, error) {
func (d *httpClient) ListContainers(ctx context.Context) ([]Container, error) {
log.Debug().Interface("filter", d.filters).Str("host", d.host.Name).Msg("Listing containers")
containerListOptions := container.ListOptions{
Filters: d.filters,
All: true,
}
list, err := d.cli.ContainerList(context.Background(), containerListOptions)
list, err := d.cli.ContainerList(ctx, containerListOptions)
if err != nil {
return nil, err
}
Expand Down
18 changes: 9 additions & 9 deletions internal/docker/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func Test_dockerClient_ListContainers_null(t *testing.T) {
proxy.On("ContainerList", mock.Anything, mock.Anything).Return(nil, nil)
client := &httpClient{proxy, filters.NewArgs(), Host{ID: "localhost"}, system.Info{}}

list, err := client.ListContainers()
list, err := client.ListContainers(context.Background())
assert.Empty(t, list, "list should be empty")
require.NoError(t, err, "error should not return an error.")

Expand All @@ -104,7 +104,7 @@ func Test_dockerClient_ListContainers_error(t *testing.T) {
proxy.On("ContainerList", mock.Anything, mock.Anything).Return(nil, errors.New("test"))
client := &httpClient{proxy, filters.NewArgs(), Host{ID: "localhost"}, system.Info{}}

list, err := client.ListContainers()
list, err := client.ListContainers(context.Background())
assert.Nil(t, list, "list should be nil")
require.Error(t, err, "test.")

Expand All @@ -127,7 +127,7 @@ func Test_dockerClient_ListContainers_happy(t *testing.T) {
proxy.On("ContainerList", mock.Anything, mock.Anything).Return(containers, nil)
client := &httpClient{proxy, filters.NewArgs(), Host{ID: "localhost"}, system.Info{}}

list, err := client.ListContainers()
list, err := client.ListContainers(context.Background())
require.NoError(t, err, "error should not return an error.")

Ids := []string{"1234567890_a", "abcdefghijkl"}
Expand Down Expand Up @@ -191,7 +191,7 @@ func Test_dockerClient_FindContainer_happy(t *testing.T) {

client := &httpClient{proxy, filters.NewArgs(), Host{ID: "localhost"}, system.Info{}}

container, err := client.FindContainer("abcdefghijkl")
container, err := client.FindContainer(context.Background(), "abcdefghijkl")
require.NoError(t, err, "error should not be thrown")

assert.Equal(t, container.ID, "abcdefghijkl")
Expand All @@ -204,7 +204,7 @@ func Test_dockerClient_FindContainer_error(t *testing.T) {
proxy.On("ContainerInspect", mock.Anything, "not_valid").Return(types.ContainerJSON{}, errors.New("not found"))
client := &httpClient{proxy, filters.NewArgs(), Host{ID: "localhost"}, system.Info{}}

_, err := client.FindContainer("not_valid")
_, err := client.FindContainer(context.Background(), "not_valid")
require.Error(t, err, "error should be thrown")

proxy.AssertExpectations(t)
Expand All @@ -222,14 +222,14 @@ func Test_dockerClient_ContainerActions_happy(t *testing.T) {
proxy.On("ContainerStop", mock.Anything, "abcdefghijkl", mock.Anything).Return(nil)
proxy.On("ContainerRestart", mock.Anything, "abcdefghijkl", mock.Anything).Return(nil)

container, err := client.FindContainer("abcdefghijkl")
container, err := client.FindContainer(context.Background(), "abcdefghijkl")
require.NoError(t, err, "error should not be thrown")

assert.Equal(t, container.ID, "abcdefghijkl")

actions := []string{"start", "stop", "restart"}
for _, action := range actions {
err := client.ContainerActions(ContainerAction(action), container.ID)
err := client.ContainerActions(context.Background(), ContainerAction(action), container.ID)
require.NoError(t, err, "error should not be thrown")
assert.Equal(t, err, nil)
}
Expand All @@ -246,12 +246,12 @@ func Test_dockerClient_ContainerActions_error(t *testing.T) {
proxy.On("ContainerStop", mock.Anything, mock.Anything, mock.Anything).Return(errors.New("test"))
proxy.On("ContainerRestart", mock.Anything, mock.Anything, mock.Anything).Return(errors.New("test"))

container, err := client.FindContainer("random-id")
container, err := client.FindContainer(context.Background(), "random-id")
require.Error(t, err, "error should be thrown")

actions := []string{"start", "stop", "restart"}
for _, action := range actions {
err := client.ContainerActions(ContainerAction(action), container.ID)
err := client.ContainerActions(context.Background(), ContainerAction(action), container.ID)
require.Error(t, err, "error should be thrown")
assert.Error(t, err, "error should have been returned")
}
Expand Down
16 changes: 12 additions & 4 deletions internal/docker/container_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"sync"
"sync/atomic"
"time"

"github.com/puzpuzpuz/xsync/v3"
"github.com/rs/zerolog/log"
Expand Down Expand Up @@ -59,7 +60,9 @@ func (s *ContainerStore) checkConnectivity() error {
s.connected.Store(false)
}()

if containers, err := s.client.ListContainers(); err != nil {
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) // 3s is enough to fetch all containers
defer cancel()
if containers, err := s.client.ListContainers(ctx); err != nil {
return err
} else {
s.containers.Clear()
Expand All @@ -81,7 +84,9 @@ func (s *ContainerStore) checkConnectivity() error {
}
go func(c Container, i int) {
defer sem.Release(1)
if container, err := s.client.FindContainer(c.ID); err == nil {
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) // 2s is hardcoded timeout for fetching container
defer cancel()
if container, err := s.client.FindContainer(ctx, c.ID); err == nil {
s.containers.Store(c.ID, &container)
}
}(c, i)
Expand Down Expand Up @@ -173,8 +178,10 @@ func (s *ContainerStore) init() {
log.Trace().Str("event", event.Name).Str("id", event.ActorID).Msg("received container event")
switch event.Name {
case "start":
if container, err := s.client.FindContainer(event.ActorID); err == nil {
list, _ := s.client.ListContainers()
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)

if container, err := s.client.FindContainer(ctx, event.ActorID); err == nil {
list, _ := s.client.ListContainers(ctx)

// make sure the container is in the list of containers when using filter
valid := lo.ContainsBy(list, func(item Container) bool {
Expand All @@ -193,6 +200,7 @@ func (s *ContainerStore) init() {
})
}
}
cancel()
case "destroy":
log.Debug().Str("id", event.ActorID).Msg("container destroyed")
s.containers.Delete(event.ActorID)
Expand Down
16 changes: 8 additions & 8 deletions internal/docker/container_store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ type mockedClient struct {
Client
}

func (m *mockedClient) ListContainers() ([]Container, error) {
args := m.Called()
func (m *mockedClient) ListContainers(ctx context.Context) ([]Container, error) {
args := m.Called(ctx)
return args.Get(0).([]Container), args.Error(1)
}

func (m *mockedClient) FindContainer(id string) (Container, error) {
args := m.Called(id)
func (m *mockedClient) FindContainer(ctx context.Context, id string) (Container, error) {
args := m.Called(ctx, id)
return args.Get(0).(Container), args.Error(1)
}

Expand All @@ -42,7 +42,7 @@ func (m *mockedClient) Host() Host {
func TestContainerStore_List(t *testing.T) {

client := new(mockedClient)
client.On("ListContainers").Return([]Container{
client.On("ListContainers", mock.Anything).Return([]Container{
{
ID: "1234",
Name: "test",
Expand All @@ -56,7 +56,7 @@ func TestContainerStore_List(t *testing.T) {
ID: "localhost",
})

client.On("FindContainer", "1234").Return(Container{
client.On("FindContainer", mock.Anything, "1234").Return(Container{
ID: "1234",
Name: "test",
Image: "test",
Expand All @@ -74,7 +74,7 @@ func TestContainerStore_List(t *testing.T) {

func TestContainerStore_die(t *testing.T) {
client := new(mockedClient)
client.On("ListContainers").Return([]Container{
client.On("ListContainers", mock.Anything).Return([]Container{
{
ID: "1234",
Name: "test",
Expand All @@ -100,7 +100,7 @@ func TestContainerStore_die(t *testing.T) {

client.On("ContainerStats", mock.Anything, "1234", mock.AnythingOfType("chan<- docker.ContainerStat")).Return(nil)

client.On("FindContainer", "1234").Return(Container{
client.On("FindContainer", mock.Anything, "1234").Return(Container{
ID: "1234",
Name: "test",
Image: "test",
Expand Down
Loading

0 comments on commit 92614ea

Please sign in to comment.