From 3fd97f80e13e0aa8e4b082fa2395fc12106773cb Mon Sep 17 00:00:00 2001 From: Simon Aronsson Date: Sat, 20 Apr 2019 11:15:33 +0200 Subject: [PATCH] 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 | 173 ++++++++++++++++++++++++++++++++++ actions/check.go | 76 ++++++++++++--- container/container.go | 8 +- main.go | 2 +- 4 files changed, 239 insertions(+), 20 deletions(-) create mode 100644 actions/actions_suite_test.go diff --git a/actions/actions_suite_test.go b/actions/actions_suite_test.go new file mode 100644 index 0000000..850fd4e --- /dev/null +++ b/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") +} diff --git a/actions/check.go b/actions/check.go index a95133e..16fd42e 100644 --- a/actions/check.go +++ b/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 + } - // Iterate over all containers execept the last one - for _, c := range containers[0 : len(containers)-1] { - client.StopContainer(c, 60) + log.Info("Found multiple running watchtower instances. Cleaning up.") + return cleanupExcessWatchtowers(containers, client, cleanup) +} - if cleanup { - client.RemoveImage(c) +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 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) } diff --git a/container/container.go b/container/container.go index 7c4d948..b32d4aa 100644 --- a/container/container.go +++ b/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" -} \ No newline at end of file +} diff --git a/main.go b/main.go index d9042de..a9b35c7 100644 --- a/main.go +++ b/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) }