add tests for check action, resolve wt cleanup bug

add unit tests for the check action to allow for some
refactoring and bug fixing without having to worry about
breaking stuff.

resolve watchtower cleanup bug by adding an initial 1 second sleep
in the check action. without the sleep, the docker client returns an
empty array, which is why we were left with two watchtowers.

actions/actions_suite_test.go

@@ -0,0 +1,173 @@
+package actions_test
+
+import (
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/containrrr/watchtower/actions"
+	"github.com/containrrr/watchtower/container"
+	"github.com/containrrr/watchtower/container/mocks"
+	"github.com/docker/docker/api/types"
+
+	cli "github.com/docker/docker/client"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestActions(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Actions Suite")
+}
+
+var _ = Describe("the actions package", func() {
+	var dockerClient cli.CommonAPIClient
+	var client mockClient
+	BeforeSuite(func() {
+		server := mocks.NewMockAPIServer()
+		dockerClient, _ = cli.NewClientWithOpts(
+			cli.WithHost(server.URL),
+			cli.WithHTTPClient(server.Client()))
+	})
+	BeforeEach(func() {
+		client = mockClient{
+			api:        dockerClient,
+			pullImages: false,
+			TestData:   &TestData{},
+		}
+	})
+
+	Describe("the check prerequisites method", func() {
+		When("given an empty array", func() {
+			It("should not do anything", func() {
+				client.TestData.Containers = []container.Container{}
+				err := actions.CheckForMultipleWatchtowerInstances(client, false)
+				Expect(err).NotTo(HaveOccurred())
+			})
+		})
+		When("given an array of one", func() {
+			It("should not do anything", func() {
+				client.TestData.Containers = []container.Container{
+					createMockContainer(
+						"test-container",
+						"test-container",
+						"watchtower",
+						time.Now()),
+				}
+				err := actions.CheckForMultipleWatchtowerInstances(client, false)
+				Expect(err).NotTo(HaveOccurred())
+			})
+		})
+		When("given multiple containers", func() {
+			BeforeEach(func() {
+				client = mockClient{
+					api:        dockerClient,
+					pullImages: false,
+					TestData: &TestData{
+						NameOfContainerToKeep: "test-container-02",
+						Containers: []container.Container{
+							createMockContainer(
+								"test-container-01",
+								"test-container-01",
+								"watchtower",
+								time.Now().AddDate(0, 0, -1)),
+							createMockContainer(
+								"test-container-02",
+								"test-container-02",
+								"watchtower",
+								time.Now()),
+						},
+					},
+				}
+			})
+			It("should stop all but the latest one", func() {
+				err := actions.CheckForMultipleWatchtowerInstances(client, false)
+				Expect(err).NotTo(HaveOccurred())
+			})
+		})
+		When("deciding whether to cleanup images", func() {
+			BeforeEach(func() {
+				client = mockClient{
+					api:        dockerClient,
+					pullImages: false,
+					TestData: &TestData{
+						Containers: []container.Container{
+							createMockContainer(
+								"test-container-01",
+								"test-container-01",
+								"watchtower",
+								time.Now().AddDate(0, 0, -1)),
+							createMockContainer(
+								"test-container-02",
+								"test-container-02",
+								"watchtower",
+								time.Now()),
+						},
+					},
+				}
+			})
+			It("should try to delete the image if the cleanup flag is true", func() {
+				err := actions.CheckForMultipleWatchtowerInstances(client, true)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(client.TestData.TriedToRemoveImage).To(BeTrue())
+			})
+			It("should not try to delete the image if the cleanup flag is false", func() {
+				err := actions.CheckForMultipleWatchtowerInstances(client, false)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(client.TestData.TriedToRemoveImage).To(BeFalse())
+			})
+		})
+	})
+})
+
+func createMockContainer(id string, name string, image string, created time.Time) container.Container {
+	content := types.ContainerJSON{
+		ContainerJSONBase: &types.ContainerJSONBase{
+			ID:      id,
+			Image:   image,
+			Name:    name,
+			Created: created.String(),
+		},
+	}
+	return *container.NewContainer(&content, nil)
+}
+
+type mockClient struct {
+	TestData   *TestData
+	api        cli.CommonAPIClient
+	pullImages bool
+}
+
+type TestData struct {
+	TriedToRemoveImage    bool
+	NameOfContainerToKeep string
+	Containers            []container.Container
+}
+
+func (client mockClient) ListContainers(f container.Filter) ([]container.Container, error) {
+	return client.TestData.Containers, nil
+}
+
+func (client mockClient) StopContainer(c container.Container, d time.Duration) error {
+	if c.Name() == client.TestData.NameOfContainerToKeep {
+		return errors.New("tried to stop the instance we want to keep")
+	}
+	return nil
+}
+func (client mockClient) StartContainer(c container.Container) error {
+	panic("Not implemented")
+}
+
+func (client mockClient) RenameContainer(c container.Container, s string) error {
+	panic("Not implemented")
+}
+
+func (client mockClient) RemoveImage(c container.Container) error {
+	client.TestData.TriedToRemoveImage = true
+	return nil
+}
+
+func (client mockClient) IsContainerStale(c container.Container) (bool, error) {
+	panic("Not implemented")
+}

actions/check.go

@@ -1,36 +1,84 @@
 package actions
 
 import (
+	"errors"
+	"fmt"
 	"sort"
+	"strings"
+	"time"
+
+	"github.com/opencontainers/runc/Godeps/_workspace/src/github.com/Sirupsen/logrus"
 
 	log "github.com/sirupsen/logrus"
 
 	"github.com/containrrr/watchtower/container"
 )
 
-// CheckPrereqs will ensure that there are not multiple instances of the
-// watchtower running simultaneously. If multiple watchtower containers are
-// detected, this function will stop and remove all but the most recently
-// started container.
-func CheckPrereqs(client container.Client, cleanup bool) error {
+// CheckForMultipleWatchtowerInstances will ensure that there are not multiple instances of the
+// watchtower running simultaneously. If multiple watchtower containers are detected, this function
+// will stop and remove all but the most recently started container.
+func CheckForMultipleWatchtowerInstances(client container.Client, cleanup bool) error {
+	awaitDockerClient()
 	containers, err := client.ListContainers(container.WatchtowerContainersFilter)
+
 	if err != nil {
+		log.Fatal(err)
 		return err
 	}
 
-	if len(containers) > 1 {
-		log.Info("Found multiple running watchtower instances. Cleaning up")
-		sort.Sort(container.ByCreated(containers))
-
-		// Iterate over all containers execept the last one
-		for _, c := range containers[0 : len(containers)-1] {
-			client.StopContainer(c, 60)
-
-			if cleanup {
-				client.RemoveImage(c)
-			}
-		}
-	}
-
+	if len(containers) <= 1 {
+		log.Debug("There are no additional watchtower containers")
 		return nil
+	}
+
+	log.Info("Found multiple running watchtower instances. Cleaning up.")
+	return cleanupExcessWatchtowers(containers, client, cleanup)
+}
+
+func cleanupExcessWatchtowers(containers []container.Container, client container.Client, cleanup bool) error {
+	var cleanupErrors int
+	var stopErrors int
+
+	sort.Sort(container.ByCreated(containers))
+	allContainersExceptLast := containers[0 : len(containers)-1]
+
+	for _, c := range allContainersExceptLast {
+		if err := client.StopContainer(c, 60); err != nil {
+			// logging the original here as we're just returning a count
+			logrus.Error(err)
+			stopErrors++
+			continue
+		}
+
+		if cleanup == true {
+			if err := client.RemoveImage(c); err != nil {
+				// logging the original here as we're just returning a count
+				logrus.Error(err)
+				cleanupErrors++
+			}
+		}
+	}
+
+	return createErrorIfAnyHaveOccurred(stopErrors, cleanupErrors)
+}
+
+func createErrorIfAnyHaveOccurred(c int, i int) error {
+	if c == 0 && i == 0 {
+		return nil
+	}
+
+	var output strings.Builder
+
+	if c > 0 {
+		output.WriteString(fmt.Sprintf("%d errors while stopping containers", c))
+	}
+	if i > 0 {
+		output.WriteString(fmt.Sprintf("%d errors while cleaning up images", c))
+	}
+	return errors.New(output.String())
+}
+
+func awaitDockerClient() {
+	log.Debug("Sleeping for a seconds to ensure the docker api client has been properly initialized.")
+	time.Sleep(1 * time.Second)
 }

container/container.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
-	log "github.com/sirupsen/logrus"
+
 	"github.com/docker/docker/api/types"
 	dockercontainer "github.com/docker/docker/api/types/container"
 )
@@ -102,9 +102,7 @@ func (c Container) Links() []string {
 // identified by the presence of the "com.centurylinklabs.watchtower" label in
 // the container metadata.
 func (c Container) IsWatchtower() bool {
-	log.Debugf("Checking if %s is a watchtower instance.", c.Name())
-	wasWatchtower := ContainsWatchtowerLabel(c.containerInfo.Config.Labels)
-	return wasWatchtower
+	return ContainsWatchtowerLabel(c.containerInfo.Config.Labels)
 }
 
 // StopSignal returns the custom stop signal (if any) that is encoded in the

main.go

@@ -106,7 +106,7 @@ func start(c *cli.Context) error {
 		return nil
 	}
 
-	if err := actions.CheckPrereqs(client, cleanup); err != nil {
+	if err := actions.CheckForMultipleWatchtowerInstances(client, cleanup); err != nil {
 		log.Fatal(err)
 	}