From 46ffa16ee27ebb3ffa4c497e5bd18e27812e1afe Mon Sep 17 00:00:00 2001 From: Simon Aronsson Date: Sat, 20 Apr 2019 16:44:41 +0200 Subject: [PATCH] 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 | 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) }