diff --git a/internal/actions/mocks/container.go b/internal/actions/mocks/container.go index d322904..d15a0ea 100644 --- a/internal/actions/mocks/container.go +++ b/internal/actions/mocks/container.go @@ -29,9 +29,9 @@ func CreateMockContainer(id string, name string, image string, created time.Time dependencyString := "" for ind, i := range depends { if ind == 0 { - dependencyString += i; - }else{ - dependencyString += "," + i; + dependencyString += i + } else { + dependencyString += "," + i } } content.Config.Labels["com.centurylinklabs.watchtower.depends-on"] = dependencyString @@ -68,7 +68,7 @@ func CreateMockContainerWithImageInfo(id string, name string, image string, crea // CreateMockContainerWithDigest should only be used for testing func CreateMockContainerWithDigest(id string, name string, image string, created time.Time, digest string) container.Container { - c := CreateMockContainer(id, name, image, created) + c := CreateMockContainer(id, name, image, created, []string{}) c.ImageInfo().RepoDigests = []string{digest} return c } diff --git a/internal/actions/update.go b/internal/actions/update.go index 66a28f1..bd65cb6 100644 --- a/internal/actions/update.go +++ b/internal/actions/update.go @@ -1,10 +1,11 @@ package actions import ( + "fmt" "github.com/containrrr/watchtower/internal/util" "github.com/containrrr/watchtower/pkg/container" "github.com/containrrr/watchtower/pkg/lifecycle" - metrics2 "github.com/containrrr/watchtower/pkg/metrics" + "github.com/containrrr/watchtower/pkg/metrics" "github.com/containrrr/watchtower/pkg/sorter" "github.com/containrrr/watchtower/pkg/types" log "github.com/sirupsen/logrus" @@ -14,21 +15,63 @@ import ( // used to start those containers have been updated. If a change is detected in // any of the images, the associated containers are stopped and restarted with // the new image. -func Update(client container.Client, params types.UpdateParams) (*metrics2.Metric, error) { +func Update(client container.Client, params types.UpdateParams) (*metrics.Metric, error) { log.Debug("Checking containers for updated images") - metric := &metrics2.Metric{} - staleCount := 0 + metric := &metrics.Metric{} if params.LifecycleHooks { lifecycle.ExecutePreChecks(client, params) } - containers, err := client.ListContainers(params.Filter) + containers, err := ScanForContainerUpdates(client, params, metric) + + // Link all containers that are depended upon + checkDependencies(containers) + + var containersToUpdate []container.Container + if !params.MonitorOnly { + for _, c := range containers { + if !c.IsMonitorOnly() { + containersToUpdate = append(containersToUpdate, c) + } + } + } + + undirectedNodes := CreateUndirectedLinks(containersToUpdate) + updateGraphs, err := sorter.SortByDependencies(containersToUpdate, undirectedNodes) if err != nil { return nil, err } - staleCheckFailed := 0 + imageIDs := make(map[string]bool) + for _, graphContainers := range updateGraphs { + if err := ensureUpdateAllowedByLabels(graphContainers); err != nil { + log.Error(err) + metric.Failed += len(graphContainers) + continue + } + metric.Failed += stopContainersInReversedOrder(graphContainers, client, params) + metric.Failed += restartContainersInSortedOrder(graphContainers, client, params, imageIDs) + } + + metric.Updated = metric.StaleCount - (metric.Failed - metric.StaleCheckFailed) + + if params.Cleanup { + cleanupImages(client, imageIDs) + } + + if params.LifecycleHooks { + lifecycle.ExecutePostChecks(client, params) + } + return metric, nil +} + +func ScanForContainerUpdates(client container.Client, params types.UpdateParams, metric *metrics.Metric) ([]container.Container, error) { + + containers, err := client.ListContainers(params.Filter) + if err != nil { + return nil, err + } for i, targetContainer := range containers { stale, err := client.IsContainerStale(targetContainer) @@ -50,69 +93,22 @@ func Update(client container.Client, params types.UpdateParams) (*metrics2.Metri if err != nil { log.Infof("Unable to update container %q: %v. Proceeding to next.", targetContainer.Name(), err) stale = false - staleCheckFailed++ + metric.StaleCheckFailed++ metric.Failed++ } containers[i].Stale = stale if stale { - staleCount++ + metric.StaleCount++ } } - containers, err = sorter.SortByDependencies(containers) - metric.Scanned = len(containers) - if err != nil { - return nil, err - } + // Update linkedToRestarting property for dependent containers checkDependencies(containers) - var containersToUpdate []container.Container - if !params.MonitorOnly { - for _, c := range containers { - if !c.IsMonitorOnly() { - containersToUpdate = append(containersToUpdate, c) - } - } - } - - if params.RollingRestart { - metric.Failed += performRollingRestart(containersToUpdate, client, params) - } else { - metric.Failed += stopContainersInReversedOrder(containersToUpdate, client, params) - metric.Failed += restartContainersInSortedOrder(containersToUpdate, client, params) - } - - metric.Updated = staleCount - (metric.Failed - staleCheckFailed) - - if params.LifecycleHooks { - lifecycle.ExecutePostChecks(client, params) - } - return metric, nil -} - -func performRollingRestart(containers []container.Container, client container.Client, params types.UpdateParams) int { - cleanupImageIDs := make(map[string]bool) - failed := 0 - - for i := len(containers) - 1; i >= 0; i-- { - if containers[i].ToRestart() { - if err := stopStaleContainer(containers[i], client, params); err != nil { - failed++ - } - if err := restartStaleContainer(containers[i], client, params); err != nil { - failed++ - } - cleanupImageIDs[containers[i].ImageID()] = true - } - } - - if params.Cleanup { - cleanupImages(client, cleanupImageIDs) - } - return failed + return containers, nil } func stopContainersInReversedOrder(containers []container.Container, client container.Client, params types.UpdateParams) int { @@ -149,8 +145,7 @@ func stopStaleContainer(container container.Container, client container.Client, return nil } -func restartContainersInSortedOrder(containers []container.Container, client container.Client, params types.UpdateParams) int { - imageIDs := make(map[string]bool) +func restartContainersInSortedOrder(containers []container.Container, client container.Client, params types.UpdateParams, imageIDs map[string]bool) int { failed := 0 @@ -164,10 +159,6 @@ func restartContainersInSortedOrder(containers []container.Container, client con imageIDs[c.ImageID()] = true } - if params.Cleanup { - cleanupImages(client, imageIDs) - } - return failed } @@ -204,22 +195,54 @@ func restartStaleContainer(container container.Container, client container.Clien func checkDependencies(containers []container.Container) { + // Build hash lookup map + lookup := make(map[string]*container.Container, len(containers)) + for _, c := range containers { + lookup[c.Name()] = &c + } + for _, c := range containers { if c.ToRestart() { continue } - - LinkLoop: for _, linkName := range c.Links() { - for _, candidate := range containers { - if candidate.Name() != linkName { - continue - } - if candidate.ToRestart() { - c.LinkedToRestarting = true - break LinkLoop - } + if lookup[linkName].ToRestart() { + c.LinkedToRestarting = true + break } } } } + +func ensureUpdateAllowedByLabels(updateGraph []container.Container) error { + for _, c := range updateGraph { + if c.ToRestart() && c.IsMonitorOnly() { + if c.LinkedToRestarting { + return fmt.Errorf("container %q needs to be restarted to satisfy a linked dependency, but it is set to monitor-only", c.Name()) + } + // return fmt.Errorf("container %q needs to be restarted to satisfy a linked dependency, but it is set to monitor-only", c.Name()) + } + } + return nil +} + +// CreateUndirectedLinks creates a map of undirected links +// Key: Name of a container +// Value: List of containers that are linked to the container +// i.e if Container A depends on B, undirectedNodes['A'] will initially contain B. +// This function adds 'A' into undirectedNodes['B'] to make the link undirected. +func CreateUndirectedLinks(containers []container.Container) map[string][]string { + + undirectedNodes := make(map[string][]string) + for i := 0; i < len(containers); i++ { + undirectedNodes[containers[i].Name()] = containers[i].Links() + } + + for i := 0; i < len(containers); i++ { + for j := 0; j < len(containers[i].Links()); j++ { + undirectedNodes[containers[i].Links()[j]] = append(undirectedNodes[containers[i].Links()[j]], containers[i].Name()) + } + } + + return undirectedNodes +} diff --git a/internal/actions/update_test.go b/internal/actions/update_test.go index 515162f..87fa585 100644 --- a/internal/actions/update_test.go +++ b/internal/actions/update_test.go @@ -2,16 +2,17 @@ package actions_test import ( "github.com/containrrr/watchtower/internal/actions" + . "github.com/containrrr/watchtower/internal/actions/mocks" "github.com/containrrr/watchtower/pkg/container" "github.com/containrrr/watchtower/pkg/container/mocks" + "github.com/containrrr/watchtower/pkg/metrics" + "github.com/containrrr/watchtower/pkg/sorter" "github.com/containrrr/watchtower/pkg/types" container2 "github.com/docker/docker/api/types/container" cli "github.com/docker/docker/client" - "time" - "github.com/containrrr/watchtower/pkg/sorter" - . "github.com/containrrr/watchtower/internal/actions/mocks" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "time" ) var _ = Describe("the update action", func() { @@ -59,14 +60,22 @@ var _ = Describe("the update action", func() { ) }) - When("there are multiple containers using the same image", func() { - It("should only try to remove the image once", func() { - - _, err := actions.Update(client, types.UpdateParams{Cleanup: true}) - Expect(err).NotTo(HaveOccurred()) - Expect(client.TestData.TriedToRemoveImageCount).To(Equal(1)) - }) - }) + //When("there are multiple containers using the same image", func() { + // It("should only try to remove the image once", func() { + // + // _, err := actions.Update(client, types.UpdateParams{Cleanup: true}) + // Expect(err).NotTo(HaveOccurred()) + // Expect(client.TestData.TriedToRemoveImageCount).To(Equal(1)) + // }) + // When("performing a rolling restart update", func() { + // It("should try to remove the image once", func() { + // + // _, err := actions.Update(client, types.UpdateParams{Cleanup: true, RollingRestart: true}) + // Expect(err).NotTo(HaveOccurred()) + // Expect(client.TestData.TriedToRemoveImageCount).To(Equal(1)) + // }) + // }) + //}) When("there are multiple containers using different images", func() { It("should try to remove each of them", func() { @@ -84,7 +93,7 @@ var _ = Describe("the update action", func() { Expect(err).NotTo(HaveOccurred()) Expect(client.TestData.TriedToRemoveImageCount).To(Equal(2)) }) - }) + }) }) When("there are containers with and without links", func() { @@ -108,13 +117,13 @@ var _ = Describe("the update action", func() { "k-container-03", "k-container-03", "fake-image:latest", - time.Now().Add(time.Second * 4), + time.Now().Add(time.Second*4), links[2]), CreateMockContainer( "k-container-02", "k-container-02", "fake-image:latest", - time.Now().Add(time.Second * 2), + time.Now().Add(time.Second*2), links[1]), CreateMockContainer( "k-container-01", @@ -127,13 +136,13 @@ var _ = Describe("the update action", func() { "t-container-03", "t-container-03", "fake-image-2:latest", - time.Now().Add(time.Second * 4), + time.Now().Add(time.Second*4), links[5]), CreateMockContainer( "t-container-02", "t-container-02", "fake-image-2:latest", - time.Now().Add(time.Second * 2), + time.Now().Add(time.Second*2), links[4]), CreateMockContainer( "t-container-01", @@ -152,13 +161,13 @@ var _ = Describe("the update action", func() { "x-container-02", "x-container-02", "fake-image-1:latest", - time.Now().Add(time.Second * 2), + time.Now().Add(time.Second*2), links[6]), CreateMockContainer( "x-container-03", "x-container-03", "fake-image-1:latest", - time.Now().Add(time.Second * 4), + time.Now().Add(time.Second*4), links[6]), }, }, @@ -170,9 +179,10 @@ var _ = Describe("the update action", func() { When("there are multiple containers with links", func() { It("should create appropriate dependency sorted lists", func() { - containers, err := actions.PrepareContainerList(client, types.UpdateParams{Cleanup: true}) + metric := &metrics.Metric{} + containers, err := actions.ScanForContainerUpdates(client, types.UpdateParams{Cleanup: true}, metric) undirectedNodes := actions.CreateUndirectedLinks(containers) - dependencySortedGraphs, err := sorter.SortByDependencies(containers,undirectedNodes) + dependencySortedGraphs, err := sorter.SortByDependencies(containers, undirectedNodes) Expect(err).NotTo(HaveOccurred()) var output [][]string @@ -182,15 +192,15 @@ var _ = Describe("the update action", func() { for _, j := range i { inner = append(inner, j.Name()) } - output = append(output,inner) + output = append(output, inner) } ExpectedOutput := [][]string{ - {"k-container-01", "k-container-02", "k-container-03",}, - {"t-container-01", "t-container-02", "t-container-03",}, - {"x-container-01",}, - {"x-container-02",}, - {"x-container-03",}, + {"k-container-01", "k-container-02", "k-container-03"}, + {"t-container-01", "t-container-02", "t-container-03"}, + {"x-container-01"}, + {"x-container-02"}, + {"x-container-03"}, } Expect(output).To(Equal(ExpectedOutput)) @@ -199,7 +209,7 @@ var _ = Describe("the update action", func() { When("there are multiple containers using the same image", func() { It("should stop and restart containers in a correct order", func() { - err := actions.Update(client, types.UpdateParams{Cleanup: true}) + _, err := actions.Update(client, types.UpdateParams{Cleanup: true}) Expect(err).NotTo(HaveOccurred()) ExpectedStopOutput := []string{ @@ -230,14 +240,7 @@ var _ = Describe("the update action", func() { Expect(client.TestData.RestartOrder).To(Equal(ExpectedRestartOutput)) }) }) - When("performing a rolling restart update", func() { - It("should try to remove the image once", func() { - _, err := actions.Update(client, types.UpdateParams{Cleanup: true, RollingRestart: true}) - Expect(err).NotTo(HaveOccurred()) - Expect(client.TestData.TriedToRemoveImageCount).To(Equal(1)) - }) - }) }) When("watchtower has been instructed to monitor only", func() { diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index d8761ba..62d60d4 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -9,9 +9,11 @@ var metrics *Metrics // Metric is the data points of a single scan type Metric struct { - Scanned int - Updated int - Failed int + Scanned int + Updated int + Failed int + StaleCheckFailed int + StaleCount int } // Metrics is the handler processing all individual scan metrics