From e983beb52a48810ce650368468c81c44bda7c595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nils=20m=C3=A5s=C3=A9n?= Date: Fri, 27 May 2022 12:16:18 +0200 Subject: [PATCH] fix: gracefully skip pinned images (#1277) * move client args to opts struct * gracefully skip pinned images * replace newClientNoAPI with literals --- cmd/root.go | 25 ++++++------- pkg/container/client.go | 66 ++++++++++++++++++++------------- pkg/container/client_test.go | 57 +++++++++++++++------------- pkg/container/container_test.go | 38 +++++++------------ 4 files changed, 98 insertions(+), 88 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 1be52a8..9041157 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,7 +1,6 @@ package cmd import ( - "github.com/containrrr/watchtower/internal/meta" "math" "net/http" "os" @@ -11,12 +10,12 @@ import ( "syscall" "time" - apiMetrics "github.com/containrrr/watchtower/pkg/api/metrics" - "github.com/containrrr/watchtower/pkg/api/update" - "github.com/containrrr/watchtower/internal/actions" "github.com/containrrr/watchtower/internal/flags" + "github.com/containrrr/watchtower/internal/meta" "github.com/containrrr/watchtower/pkg/api" + apiMetrics "github.com/containrrr/watchtower/pkg/api/metrics" + "github.com/containrrr/watchtower/pkg/api/update" "github.com/containrrr/watchtower/pkg/container" "github.com/containrrr/watchtower/pkg/filters" "github.com/containrrr/watchtower/pkg/metrics" @@ -139,14 +138,14 @@ func PreRun(cmd *cobra.Command, _ []string) { log.Warn("Using `WATCHTOWER_NO_PULL` and `WATCHTOWER_MONITOR_ONLY` simultaneously might lead to no action being taken at all. If this is intentional, you may safely ignore this message.") } - client = container.NewClient( - !noPull, - includeStopped, - reviveStopped, - removeVolumes, - includeRestarting, - warnOnHeadPullFailed, - ) + client = container.NewClient(container.ClientOptions{ + PullImages: !noPull, + IncludeStopped: includeStopped, + ReviveStopped: reviveStopped, + RemoveVolumes: removeVolumes, + IncludeRestarting: includeRestarting, + WarnOnHeadFailed: container.WarningStrategy(warnOnHeadPullFailed), + }) notifier = notifications.NewNotifier(cmd) } @@ -293,7 +292,7 @@ func writeStartupMessage(c *cobra.Command, sched time.Time, filtering string) { startupLog.Info("Scheduling first run: " + sched.Format("2006-01-02 15:04:05 -0700 MST")) startupLog.Info("Note that the first check will be performed in " + until) } else if runOnce, _ := c.PersistentFlags().GetBool("run-once"); runOnce { - startupLog.Info("Running a one time update.") + startupLog.Info("Running a one time update.") } else { startupLog.Info("Periodic runs are not enabled.") } diff --git a/pkg/container/client.go b/pkg/container/client.go index 371206b..df4b8da 100644 --- a/pkg/container/client.go +++ b/pkg/container/client.go @@ -42,7 +42,7 @@ type Client interface { // * DOCKER_HOST the docker-engine host to send api requests to // * DOCKER_TLS_VERIFY whether to verify tls certificates // * DOCKER_API_VERSION the minimum docker api version to work with -func NewClient(pullImages, includeStopped, reviveStopped, removeVolumes, includeRestarting bool, warnOnHeadFailed string) Client { +func NewClient(opts ClientOptions) Client { cli, err := sdkClient.NewClientWithOpts(sdkClient.FromEnv) if err != nil { @@ -50,31 +50,43 @@ func NewClient(pullImages, includeStopped, reviveStopped, removeVolumes, include } return dockerClient{ - api: cli, - pullImages: pullImages, - removeVolumes: removeVolumes, - includeStopped: includeStopped, - reviveStopped: reviveStopped, - includeRestarting: includeRestarting, - warnOnHeadFailed: warnOnHeadFailed, + api: cli, + ClientOptions: opts, } } +// ClientOptions contains the options for how the docker client wrapper should behave +type ClientOptions struct { + PullImages bool + RemoveVolumes bool + IncludeStopped bool + ReviveStopped bool + IncludeRestarting bool + WarnOnHeadFailed WarningStrategy +} + +// WarningStrategy is a value determining when to show warnings +type WarningStrategy string + +const ( + // WarnAlways warns whenever the problem occurs + WarnAlways WarningStrategy = "always" + // WarnNever never warns when the problem occurs + WarnNever WarningStrategy = "never" + // WarnAuto skips warning when the problem was expected + WarnAuto WarningStrategy = "auto" +) + type dockerClient struct { - api sdkClient.CommonAPIClient - pullImages bool - removeVolumes bool - includeStopped bool - reviveStopped bool - includeRestarting bool - warnOnHeadFailed string + api sdkClient.CommonAPIClient + ClientOptions } func (client dockerClient) WarnOnHeadPullFailed(container Container) bool { - if client.warnOnHeadFailed == "always" { + if client.WarnOnHeadFailed == WarnAlways { return true } - if client.warnOnHeadFailed == "never" { + if client.WarnOnHeadFailed == WarnNever { return false } @@ -85,11 +97,11 @@ func (client dockerClient) ListContainers(fn t.Filter) ([]Container, error) { cs := []Container{} bg := context.Background() - if client.includeStopped && client.includeRestarting { + if client.IncludeStopped && client.IncludeRestarting { log.Debug("Retrieving running, stopped, restarting and exited containers") - } else if client.includeStopped { + } else if client.IncludeStopped { log.Debug("Retrieving running, stopped and exited containers") - } else if client.includeRestarting { + } else if client.IncludeRestarting { log.Debug("Retrieving running and restarting containers") } else { log.Debug("Retrieving running containers") @@ -125,12 +137,12 @@ func (client dockerClient) createListFilter() filters.Args { filterArgs := filters.NewArgs() filterArgs.Add("status", "running") - if client.includeStopped { + if client.IncludeStopped { filterArgs.Add("status", "created") filterArgs.Add("status", "exited") } - if client.includeRestarting { + if client.IncludeRestarting { filterArgs.Add("status", "restarting") } @@ -179,7 +191,7 @@ func (client dockerClient) StopContainer(c Container, timeout time.Duration) err } else { log.Debugf("Removing container %s", shortID) - if err := client.api.ContainerRemove(bg, idStr, types.ContainerRemoveOptions{Force: true, RemoveVolumes: client.removeVolumes}); err != nil { + if err := client.api.ContainerRemove(bg, idStr, types.ContainerRemoveOptions{Force: true, RemoveVolumes: client.RemoveVolumes}); err != nil { return err } } @@ -236,7 +248,7 @@ func (client dockerClient) StartContainer(c Container) (t.ContainerID, error) { } createdContainerID := t.ContainerID(createdContainer.ID) - if !c.IsRunning() && !client.reviveStopped { + if !c.IsRunning() && !client.ReviveStopped { return createdContainerID, nil } @@ -264,7 +276,7 @@ func (client dockerClient) RenameContainer(c Container, newName string) error { func (client dockerClient) IsContainerStale(container Container) (stale bool, latestImage t.ImageID, err error) { ctx := context.Background() - if !client.pullImages { + if !client.PullImages { log.Debugf("Skipping image pull.") } else if err := client.PullImage(ctx, container); err != nil { return false, container.SafeImageID(), err @@ -303,6 +315,10 @@ func (client dockerClient) PullImage(ctx context.Context, container Container) e "container": containerName, } + if strings.HasPrefix(imageName, "sha256:") { + return fmt.Errorf("container uses a pinned image, and cannot be updated by watchtower") + } + log.WithFields(fields).Debugf("Trying to load authentication credentials.") opts, err := registry.GetPullOptions(imageName) if err != nil { diff --git a/pkg/container/client_test.go b/pkg/container/client_test.go index 2f0157d..2487bb3 100644 --- a/pkg/container/client_test.go +++ b/pkg/container/client_test.go @@ -16,6 +16,7 @@ import ( . "github.com/onsi/gomega" . "github.com/onsi/gomega/types" + "context" "net/http" ) @@ -35,38 +36,47 @@ var _ = Describe("the client", func() { containerUnknown := *mockContainerWithImageName("unknown.repo/prefix/imagename:latest") containerKnown := *mockContainerWithImageName("docker.io/prefix/imagename:latest") - When("warn on head failure is set to \"always\"", func() { - c := newClientNoAPI(false, false, false, false, false, "always") + When(`warn on head failure is set to "always"`, func() { + c := dockerClient{ClientOptions: ClientOptions{WarnOnHeadFailed: WarnAlways}} It("should always return true", func() { Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeTrue()) Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue()) }) }) - When("warn on head failure is set to \"auto\"", func() { - c := newClientNoAPI(false, false, false, false, false, "auto") - It("should always return true", func() { + When(`warn on head failure is set to "auto"`, func() { + c := dockerClient{ClientOptions: ClientOptions{WarnOnHeadFailed: WarnAuto}} + It("should return false for unknown repos", func() { Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse()) }) - It("should", func() { + It("should return true for known repos", func() { Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue()) }) }) - When("warn on head failure is set to \"never\"", func() { - c := newClientNoAPI(false, false, false, false, false, "never") + When(`warn on head failure is set to "never"`, func() { + c := dockerClient{ClientOptions: ClientOptions{WarnOnHeadFailed: WarnNever}} It("should never return true", func() { Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse()) Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeFalse()) }) }) }) + When("pulling the latest image", func() { + When("the image consist of a pinned hash", func() { + It("should gracefully fail with a useful message", func() { + c := dockerClient{} + pinnedContainer := *mockContainerWithImageName("sha256:fa5269854a5e615e51a72b17ad3fd1e01268f278a6684c8ed3c5f0cdce3f230b") + c.PullImage(context.Background(), pinnedContainer) + }) + }) + }) When("listing containers", func() { When("no filter is provided", func() { It("should return all available containers", func() { mockServer.AppendHandlers(mocks.ListContainersHandler("running")) mockServer.AppendHandlers(mocks.GetContainerHandlers("watchtower", "running")...) client := dockerClient{ - api: docker, - pullImages: false, + api: docker, + ClientOptions: ClientOptions{PullImages: false}, } containers, err := client.ListContainers(filters.NoFilter) Expect(err).NotTo(HaveOccurred()) @@ -79,8 +89,8 @@ var _ = Describe("the client", func() { mockServer.AppendHandlers(mocks.GetContainerHandlers("watchtower", "running")...) filter := filters.FilterByNames([]string{"lollercoaster"}, filters.NoFilter) client := dockerClient{ - api: docker, - pullImages: false, + api: docker, + ClientOptions: ClientOptions{PullImages: false}, } containers, err := client.ListContainers(filter) Expect(err).NotTo(HaveOccurred()) @@ -92,8 +102,8 @@ var _ = Describe("the client", func() { mockServer.AppendHandlers(mocks.ListContainersHandler("running")) mockServer.AppendHandlers(mocks.GetContainerHandlers("watchtower", "running")...) client := dockerClient{ - api: docker, - pullImages: false, + api: docker, + ClientOptions: ClientOptions{PullImages: false}, } containers, err := client.ListContainers(filters.WatchtowerContainersFilter) Expect(err).NotTo(HaveOccurred()) @@ -105,9 +115,8 @@ var _ = Describe("the client", func() { mockServer.AppendHandlers(mocks.ListContainersHandler("running", "exited", "created")) mockServer.AppendHandlers(mocks.GetContainerHandlers("stopped", "watchtower", "running")...) client := dockerClient{ - api: docker, - pullImages: false, - includeStopped: true, + api: docker, + ClientOptions: ClientOptions{PullImages: false, IncludeStopped: true}, } containers, err := client.ListContainers(filters.NoFilter) Expect(err).NotTo(HaveOccurred()) @@ -119,9 +128,8 @@ var _ = Describe("the client", func() { mockServer.AppendHandlers(mocks.ListContainersHandler("running", "restarting")) mockServer.AppendHandlers(mocks.GetContainerHandlers("watchtower", "running", "restarting")...) client := dockerClient{ - api: docker, - pullImages: false, - includeRestarting: true, + api: docker, + ClientOptions: ClientOptions{PullImages: false, IncludeRestarting: true}, } containers, err := client.ListContainers(filters.NoFilter) Expect(err).NotTo(HaveOccurred()) @@ -133,9 +141,8 @@ var _ = Describe("the client", func() { mockServer.AppendHandlers(mocks.ListContainersHandler("running")) mockServer.AppendHandlers(mocks.GetContainerHandlers("watchtower", "running")...) client := dockerClient{ - api: docker, - pullImages: false, - includeRestarting: false, + api: docker, + ClientOptions: ClientOptions{PullImages: false, IncludeRestarting: false}, } containers, err := client.ListContainers(filters.NoFilter) Expect(err).NotTo(HaveOccurred()) @@ -147,8 +154,8 @@ var _ = Describe("the client", func() { When(`logging`, func() { It("should include container id field", func() { client := dockerClient{ - api: docker, - pullImages: false, + api: docker, + ClientOptions: ClientOptions{PullImages: false}, } // Capture logrus output in buffer diff --git a/pkg/container/container_test.go b/pkg/container/container_test.go index 1a4e956..6cd5c86 100644 --- a/pkg/container/container_test.go +++ b/pkg/container/container_test.go @@ -216,20 +216,20 @@ var _ = Describe("the container", func() { }) }) }) - + When("there is a pre or post update timeout", func() { - It("should return minute values", func() { - c = mockContainerWithLabels(map[string]string{ - "com.centurylinklabs.watchtower.lifecycle.pre-update-timeout": "3", - "com.centurylinklabs.watchtower.lifecycle.post-update-timeout": "5", - }) - preTimeout := c.PreUpdateTimeout() - Expect(preTimeout).To(Equal(3)) - postTimeout := c.PostUpdateTimeout() - Expect(postTimeout).To(Equal(5)) - }) - }) - + It("should return minute values", func() { + c = mockContainerWithLabels(map[string]string{ + "com.centurylinklabs.watchtower.lifecycle.pre-update-timeout": "3", + "com.centurylinklabs.watchtower.lifecycle.post-update-timeout": "5", + }) + preTimeout := c.PreUpdateTimeout() + Expect(preTimeout).To(Equal(3)) + postTimeout := c.PostUpdateTimeout() + Expect(postTimeout).To(Equal(5)) + }) + }) + }) }) @@ -282,15 +282,3 @@ func mockContainerWithLabels(labels map[string]string) *Container { } return NewContainer(&content, nil) } - -func newClientNoAPI(pullImages, includeStopped, reviveStopped, removeVolumes, includeRestarting bool, warnOnHeadFailed string) Client { - return dockerClient{ - api: nil, - pullImages: pullImages, - removeVolumes: removeVolumes, - includeStopped: includeStopped, - reviveStopped: reviveStopped, - includeRestarting: includeRestarting, - warnOnHeadFailed: warnOnHeadFailed, - } -}