From a8dec129f5ce1ff67610fde37fb8f31239326e00 Mon Sep 17 00:00:00 2001 From: Brian DeHamer Date: Tue, 21 Jul 2015 22:41:58 +0000 Subject: [PATCH] Refactor Client interface --- actions/check.go | 2 +- actions/check_test.go | 8 +- actions/update.go | 12 +- container/client.go | 78 ++++++------- container/client_test.go | 208 +++++++++++++++++------------------ container/mockclient/mock.go | 18 +-- 6 files changed, 164 insertions(+), 162 deletions(-) diff --git a/actions/check.go b/actions/check.go index 020bc8a..9ac2b8c 100644 --- a/actions/check.go +++ b/actions/check.go @@ -19,7 +19,7 @@ func CheckPrereqs(client container.Client) error { // Iterate over all containers execept the last one for _, c := range containers[0 : len(containers)-1] { - client.Stop(c, 60) + client.StopContainer(c, 60) } } diff --git a/actions/check_test.go b/actions/check_test.go index 1c0fa18..73a5f42 100644 --- a/actions/check_test.go +++ b/actions/check_test.go @@ -35,8 +35,8 @@ func TestCheckPrereqs_Success(t *testing.T) { cs := []container.Container{c1, c2} client := &mockclient.MockClient{} - client.On("ListContainers", mock.AnythingOfType("container.ContainerFilter")).Return(cs, nil) - client.On("Stop", c2, time.Duration(60)).Return(nil) + client.On("ListContainers", mock.AnythingOfType("container.Filter")).Return(cs, nil) + client.On("StopContainer", c2, time.Duration(60)).Return(nil) err := CheckPrereqs(client) @@ -59,7 +59,7 @@ func TestCheckPrereqs_OnlyOneContainer(t *testing.T) { cs := []container.Container{c1} client := &mockclient.MockClient{} - client.On("ListContainers", mock.AnythingOfType("container.ContainerFilter")).Return(cs, nil) + client.On("ListContainers", mock.AnythingOfType("container.Filter")).Return(cs, nil) err := CheckPrereqs(client) @@ -71,7 +71,7 @@ func TestCheckPrereqs_ListError(t *testing.T) { cs := []container.Container{} client := &mockclient.MockClient{} - client.On("ListContainers", mock.AnythingOfType("container.ContainerFilter")).Return(cs, errors.New("oops")) + client.On("ListContainers", mock.AnythingOfType("container.Filter")).Return(cs, errors.New("oops")) err := CheckPrereqs(client) diff --git a/actions/update.go b/actions/update.go index 0beeb5b..7ae9b61 100644 --- a/actions/update.go +++ b/actions/update.go @@ -18,10 +18,12 @@ func Update(client container.Client) error { return err } - for i := range containers { - if err := client.RefreshImage(&containers[i]); err != nil { + for i, container := range containers { + stale, err := client.IsContainerStale(container) + if err != nil { return err } + containers[i].Stale = stale } containers, err = container.SortByDependencies(containers) @@ -40,7 +42,7 @@ func Update(client container.Client) error { } if container.Stale { - if err := client.Stop(container, 10); err != nil { + if err := client.StopContainer(container, 10); err != nil { return err } } @@ -54,12 +56,12 @@ func Update(client container.Client) error { // from re-using the same container name so we first rename the current // instance so that the new one can adopt the old name. if container.IsWatchtower() { - if err := client.Rename(container, randName()); err != nil { + if err := client.RenameContainer(container, randName()); err != nil { return err } } - if err := client.Start(container); err != nil { + if err := client.StartContainer(container); err != nil { return err } } diff --git a/container/client.go b/container/client.go index ef36dd7..d590ed3 100644 --- a/container/client.go +++ b/container/client.go @@ -22,14 +22,14 @@ func init() { pullImages = true } -type ContainerFilter func(Container) bool +type Filter func(Container) bool type Client interface { - ListContainers(ContainerFilter) ([]Container, error) - RefreshImage(*Container) error - Stop(Container, time.Duration) error - Start(Container) error - Rename(Container, string) error + ListContainers(Filter) ([]Container, error) + StopContainer(Container, time.Duration) error + StartContainer(Container) error + RenameContainer(Container, string) error + IsContainerStale(Container) (bool, error) } func NewClient(dockerHost string) Client { @@ -46,7 +46,7 @@ type DockerClient struct { api dockerclient.Client } -func (client DockerClient) ListContainers(fn ContainerFilter) ([]Container, error) { +func (client DockerClient) ListContainers(fn Filter) ([]Container, error) { cs := []Container{} runningContainers, err := client.api.ListContainers(false, false, "") @@ -74,36 +74,7 @@ func (client DockerClient) ListContainers(fn ContainerFilter) ([]Container, erro return cs, nil } -func (client DockerClient) RefreshImage(c *Container) error { - containerInfo := c.containerInfo - oldImageInfo := c.imageInfo - imageName := containerInfo.Config.Image - - if pullImages { - if !strings.Contains(imageName, ":") { - imageName = fmt.Sprintf("%s:latest", imageName) - } - - log.Printf("Pulling %s for %s\n", imageName, c.Name()) - if err := client.api.PullImage(imageName, nil); err != nil { - return err - } - } - - newImageInfo, err := client.api.InspectImage(imageName) - if err != nil { - return err - } - - if newImageInfo.Id != oldImageInfo.Id { - log.Printf("Found new %s image (%s)\n", imageName, newImageInfo.Id) - c.Stale = true - } - - return nil -} - -func (client DockerClient) Stop(c Container, timeout time.Duration) error { +func (client DockerClient) StopContainer(c Container, timeout time.Duration) error { signal := defaultStopSignal if sig, ok := c.containerInfo.Config.Labels[signalLabel]; ok { @@ -122,7 +93,7 @@ func (client DockerClient) Stop(c Container, timeout time.Duration) error { return client.api.RemoveContainer(c.containerInfo.Id, true, false) } -func (client DockerClient) Start(c Container) error { +func (client DockerClient) StartContainer(c Container) error { config := c.runtimeConfig() hostConfig := c.hostConfig() name := c.Name() @@ -141,10 +112,39 @@ func (client DockerClient) Start(c Container) error { return client.api.StartContainer(newContainerID, hostConfig) } -func (client DockerClient) Rename(c Container, newName string) error { +func (client DockerClient) RenameContainer(c Container, newName string) error { return client.api.RenameContainer(c.containerInfo.Id, newName) } +func (client DockerClient) IsContainerStale(c Container) (bool, error) { + containerInfo := c.containerInfo + oldImageInfo := c.imageInfo + imageName := containerInfo.Config.Image + + if pullImages { + if !strings.Contains(imageName, ":") { + imageName = fmt.Sprintf("%s:latest", imageName) + } + + log.Printf("Pulling %s for %s\n", imageName, c.Name()) + if err := client.api.PullImage(imageName, nil); err != nil { + return false, err + } + } + + newImageInfo, err := client.api.InspectImage(imageName) + if err != nil { + return false, err + } + + if newImageInfo.Id != oldImageInfo.Id { + log.Printf("Found new %s image (%s)\n", imageName, newImageInfo.Id) + return true, nil + } + + return false, nil +} + func (client DockerClient) waitForStop(c Container, waitTime time.Duration) error { timeout := time.After(waitTime) diff --git a/container/client_test.go b/container/client_test.go index 18a6e50..f2e74a0 100644 --- a/container/client_test.go +++ b/container/client_test.go @@ -89,93 +89,7 @@ func TestListContainers_InspectImageError(t *testing.T) { api.AssertExpectations(t) } -func TestRefreshImage_NotStaleSuccess(t *testing.T) { - c := &Container{ - containerInfo: &dockerclient.ContainerInfo{ - Name: "foo", - Config: &dockerclient.ContainerConfig{Image: "bar"}, - }, - imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, - } - newImageInfo := &dockerclient.ImageInfo{Id: "abc123"} - - api := mockclient.NewMockClient() - api.On("PullImage", "bar:latest", mock.Anything).Return(nil) - api.On("InspectImage", "bar:latest").Return(newImageInfo, nil) - - client := DockerClient{api: api} - err := client.RefreshImage(c) - - assert.NoError(t, err) - assert.False(t, c.Stale) - api.AssertExpectations(t) -} - -func TestRefreshImage_StaleSuccess(t *testing.T) { - c := &Container{ - containerInfo: &dockerclient.ContainerInfo{ - Name: "foo", - Config: &dockerclient.ContainerConfig{Image: "bar:1.0"}, - }, - imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, - } - newImageInfo := &dockerclient.ImageInfo{Id: "xyz789"} - - api := mockclient.NewMockClient() - api.On("PullImage", "bar:1.0", mock.Anything).Return(nil) - api.On("InspectImage", "bar:1.0").Return(newImageInfo, nil) - - client := DockerClient{api: api} - err := client.RefreshImage(c) - - assert.NoError(t, err) - assert.True(t, c.Stale) - api.AssertExpectations(t) -} - -func TestRefreshImage_PullImageError(t *testing.T) { - c := &Container{ - containerInfo: &dockerclient.ContainerInfo{ - Name: "foo", - Config: &dockerclient.ContainerConfig{Image: "bar:latest"}, - }, - imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, - } - - api := mockclient.NewMockClient() - api.On("PullImage", "bar:latest", mock.Anything).Return(errors.New("oops")) - - client := DockerClient{api: api} - err := client.RefreshImage(c) - - assert.Error(t, err) - assert.EqualError(t, err, "oops") - api.AssertExpectations(t) -} - -func TestRefreshImage_InspectImageError(t *testing.T) { - c := &Container{ - containerInfo: &dockerclient.ContainerInfo{ - Name: "foo", - Config: &dockerclient.ContainerConfig{Image: "bar:latest"}, - }, - imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, - } - newImageInfo := &dockerclient.ImageInfo{} - - api := mockclient.NewMockClient() - api.On("PullImage", "bar:latest", mock.Anything).Return(nil) - api.On("InspectImage", "bar:latest").Return(newImageInfo, errors.New("uh-oh")) - - client := DockerClient{api: api} - err := client.RefreshImage(c) - - assert.Error(t, err) - assert.EqualError(t, err, "uh-oh") - api.AssertExpectations(t) -} - -func TestStop_DefaultSuccess(t *testing.T) { +func TestStopContainer_DefaultSuccess(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Name: "foo", @@ -196,13 +110,13 @@ func TestStop_DefaultSuccess(t *testing.T) { api.On("RemoveContainer", "abc123", true, false).Return(nil) client := DockerClient{api: api} - err := client.Stop(c, time.Second) + err := client.StopContainer(c, time.Second) assert.NoError(t, err) api.AssertExpectations(t) } -func TestStop_CustomSignalSuccess(t *testing.T) { +func TestStopContainer_CustomSignalSuccess(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Name: "foo", @@ -224,13 +138,13 @@ func TestStop_CustomSignalSuccess(t *testing.T) { api.On("RemoveContainer", "abc123", true, false).Return(nil) client := DockerClient{api: api} - err := client.Stop(c, time.Second) + err := client.StopContainer(c, time.Second) assert.NoError(t, err) api.AssertExpectations(t) } -func TestStop_KillContainerError(t *testing.T) { +func TestStopContainer_KillContainerError(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Name: "foo", @@ -243,14 +157,14 @@ func TestStop_KillContainerError(t *testing.T) { api.On("KillContainer", "abc123", "SIGTERM").Return(errors.New("oops")) client := DockerClient{api: api} - err := client.Stop(c, time.Second) + err := client.StopContainer(c, time.Second) assert.Error(t, err) assert.EqualError(t, err, "oops") api.AssertExpectations(t) } -func TestStop_RemoveContainerError(t *testing.T) { +func TestStopContainer_RemoveContainerError(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Name: "foo", @@ -265,14 +179,14 @@ func TestStop_RemoveContainerError(t *testing.T) { api.On("RemoveContainer", "abc123", true, false).Return(errors.New("whoops")) client := DockerClient{api: api} - err := client.Stop(c, time.Second) + err := client.StopContainer(c, time.Second) assert.Error(t, err) assert.EqualError(t, err, "whoops") api.AssertExpectations(t) } -func TestStart_Success(t *testing.T) { +func TestStartContainer_Success(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Name: "foo", @@ -289,13 +203,13 @@ func TestStart_Success(t *testing.T) { api.On("StartContainer", "def789", mock.AnythingOfType("*dockerclient.HostConfig")).Return(nil) client := DockerClient{api: api} - err := client.Start(c) + err := client.StartContainer(c) assert.NoError(t, err) api.AssertExpectations(t) } -func TestStart_CreateContainerError(t *testing.T) { +func TestStartContainer_CreateContainerError(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Name: "foo", @@ -311,14 +225,14 @@ func TestStart_CreateContainerError(t *testing.T) { api.On("CreateContainer", mock.Anything, "foo").Return("", errors.New("oops")) client := DockerClient{api: api} - err := client.Start(c) + err := client.StartContainer(c) assert.Error(t, err) assert.EqualError(t, err, "oops") api.AssertExpectations(t) } -func TestStart_StartContainerError(t *testing.T) { +func TestStartContainer_StartContainerError(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Name: "foo", @@ -335,14 +249,14 @@ func TestStart_StartContainerError(t *testing.T) { api.On("StartContainer", "def789", mock.Anything).Return(errors.New("whoops")) client := DockerClient{api: api} - err := client.Start(c) + err := client.StartContainer(c) assert.Error(t, err) assert.EqualError(t, err, "whoops") api.AssertExpectations(t) } -func TestRename_Success(t *testing.T) { +func TestRenameContainer_Success(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Id: "abc123", @@ -353,13 +267,13 @@ func TestRename_Success(t *testing.T) { api.On("RenameContainer", "abc123", "foo").Return(nil) client := DockerClient{api: api} - err := client.Rename(c, "foo") + err := client.RenameContainer(c, "foo") assert.NoError(t, err) api.AssertExpectations(t) } -func TestRename_Error(t *testing.T) { +func TestRenameContainer_Error(t *testing.T) { c := Container{ containerInfo: &dockerclient.ContainerInfo{ Id: "abc123", @@ -370,9 +284,95 @@ func TestRename_Error(t *testing.T) { api.On("RenameContainer", "abc123", "foo").Return(errors.New("oops")) client := DockerClient{api: api} - err := client.Rename(c, "foo") + err := client.RenameContainer(c, "foo") + + assert.Error(t, err) + assert.EqualError(t, err, "oops") + api.AssertExpectations(t) +} + +func TestIsContainerStale_NotStaleSuccess(t *testing.T) { + c := Container{ + containerInfo: &dockerclient.ContainerInfo{ + Name: "foo", + Config: &dockerclient.ContainerConfig{Image: "bar"}, + }, + imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, + } + newImageInfo := &dockerclient.ImageInfo{Id: "abc123"} + + api := mockclient.NewMockClient() + api.On("PullImage", "bar:latest", mock.Anything).Return(nil) + api.On("InspectImage", "bar:latest").Return(newImageInfo, nil) + + client := DockerClient{api: api} + stale, err := client.IsContainerStale(c) + + assert.NoError(t, err) + assert.False(t, stale) + api.AssertExpectations(t) +} + +func TestIsContainerStale_StaleSuccess(t *testing.T) { + c := Container{ + containerInfo: &dockerclient.ContainerInfo{ + Name: "foo", + Config: &dockerclient.ContainerConfig{Image: "bar:1.0"}, + }, + imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, + } + newImageInfo := &dockerclient.ImageInfo{Id: "xyz789"} + + api := mockclient.NewMockClient() + api.On("PullImage", "bar:1.0", mock.Anything).Return(nil) + api.On("InspectImage", "bar:1.0").Return(newImageInfo, nil) + + client := DockerClient{api: api} + stale, err := client.IsContainerStale(c) + + assert.NoError(t, err) + assert.True(t, stale) + api.AssertExpectations(t) +} + +func TestIsContainerStale_PullImageError(t *testing.T) { + c := Container{ + containerInfo: &dockerclient.ContainerInfo{ + Name: "foo", + Config: &dockerclient.ContainerConfig{Image: "bar:latest"}, + }, + imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, + } + + api := mockclient.NewMockClient() + api.On("PullImage", "bar:latest", mock.Anything).Return(errors.New("oops")) + + client := DockerClient{api: api} + _, err := client.IsContainerStale(c) assert.Error(t, err) assert.EqualError(t, err, "oops") api.AssertExpectations(t) } + +func TestIsContainerStale_InspectImageError(t *testing.T) { + c := Container{ + containerInfo: &dockerclient.ContainerInfo{ + Name: "foo", + Config: &dockerclient.ContainerConfig{Image: "bar:latest"}, + }, + imageInfo: &dockerclient.ImageInfo{Id: "abc123"}, + } + newImageInfo := &dockerclient.ImageInfo{} + + api := mockclient.NewMockClient() + api.On("PullImage", "bar:latest", mock.Anything).Return(nil) + api.On("InspectImage", "bar:latest").Return(newImageInfo, errors.New("uh-oh")) + + client := DockerClient{api: api} + _, err := client.IsContainerStale(c) + + assert.Error(t, err) + assert.EqualError(t, err, "uh-oh") + api.AssertExpectations(t) +} diff --git a/container/mockclient/mock.go b/container/mockclient/mock.go index d315962..bebd686 100644 --- a/container/mockclient/mock.go +++ b/container/mockclient/mock.go @@ -11,27 +11,27 @@ type MockClient struct { mock.Mock } -func (m *MockClient) ListContainers(cf container.ContainerFilter) ([]container.Container, error) { +func (m *MockClient) ListContainers(cf container.Filter) ([]container.Container, error) { args := m.Called(cf) return args.Get(0).([]container.Container), args.Error(1) } -func (m *MockClient) RefreshImage(c *container.Container) error { - args := m.Called(c) - return args.Error(0) -} - -func (m *MockClient) Stop(c container.Container, timeout time.Duration) error { +func (m *MockClient) StopContainer(c container.Container, timeout time.Duration) error { args := m.Called(c, timeout) return args.Error(0) } -func (m *MockClient) Start(c container.Container) error { +func (m *MockClient) StartContainer(c container.Container) error { args := m.Called(c) return args.Error(0) } -func (m *MockClient) Rename(c container.Container, name string) error { +func (m *MockClient) RenameContainer(c container.Container, name string) error { args := m.Called(c, name) return args.Error(0) } + +func (m *MockClient) IsContainerStale(c container.Container) (bool, error) { + args := m.Called(c) + return args.Bool(0), args.Error(1) +}