Refactor Client interface

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)
 		}
 	}
 

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("ListContainers", mock.AnythingOfType("container.Filter")).Return(cs, nil)
-	client.On("Stop", c2, time.Duration(60)).Return(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)
 

actions/update.go

@@ -18,10 +18,12 @@ func Update(client container.Client) error {
 		return err
 	}
 
-	for i := range containers {
+	for i, container := range containers {
-		if err := client.RefreshImage(&containers[i]); err != nil {
+		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
 			}
 		}

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)
+	ListContainers(Filter) ([]Container, error)
-	RefreshImage(*Container) error
+	StopContainer(Container, time.Duration) error
-	Stop(Container, time.Duration) error
+	StartContainer(Container) error
-	Start(Container) error
+	RenameContainer(Container, string) error
-	Rename(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 {
+func (client DockerClient) StopContainer(c Container, timeout time.Duration) 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 {
 	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)
 

container/client_test.go

@@ -89,93 +89,7 @@ func TestListContainers_InspectImageError(t *testing.T) {
 	api.AssertExpectations(t)
 }
 
-func TestRefreshImage_NotStaleSuccess(t *testing.T) {
+func TestStopContainer_DefaultSuccess(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) {
 	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)
+}

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 {
+func (m *MockClient) StopContainer(c container.Container, timeout time.Duration) error {
-	args := m.Called(c)
-	return args.Error(0)
-}
-
-func (m *MockClient) Stop(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)
+}