add tests for check action, resolve wt cleanup bug (#284)

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))
+	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)
+}
 
-		// Iterate over all containers execept the last one
-		for _, c := range containers[0 : len(containers)-1] {
-			client.StopContainer(c, 60)
+func cleanupExcessWatchtowers(containers []container.Container, client container.Client, cleanup bool) error {
+	var cleanupErrors int
+	var stopErrors int
 
-			if cleanup {
-				client.RemoveImage(c)
+	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 nil
+	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
@@ -189,4 +187,4 @@ func (c Container) hostConfig() *dockercontainer.HostConfig {
 func ContainsWatchtowerLabel(labels map[string]string) bool {
 	val, ok := labels[watchtowerLabel]
 	return ok && val == "true"
-}
+}

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