fix: gracefully skip pinned images (#1277)

* move client args to opts struct
* gracefully skip pinned images
* replace newClientNoAPI with literals

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(
+	client = container.NewClient(container.ClientOptions{
-		!noPull,
+		PullImages:        !noPull,
-		includeStopped,
+		IncludeStopped:    includeStopped,
-		reviveStopped,
+		ReviveStopped:     reviveStopped,
-		removeVolumes,
+		RemoveVolumes:     removeVolumes,
-		includeRestarting,
+		IncludeRestarting: includeRestarting,
-		warnOnHeadPullFailed,
+		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.")
 	}

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,
+		api:           cli,
-		pullImages:        pullImages,
+		ClientOptions: opts,
-		removeVolumes:     removeVolumes,
-		includeStopped:    includeStopped,
-		reviveStopped:     reviveStopped,
-		includeRestarting: includeRestarting,
-		warnOnHeadFailed:  warnOnHeadFailed,
 	}
 }
 
+// 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
+	api sdkClient.CommonAPIClient
-	pullImages        bool
+	ClientOptions
-	removeVolumes     bool
-	includeStopped    bool
-	reviveStopped     bool
-	includeRestarting bool
-	warnOnHeadFailed  string
 }
 
 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 {

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() {
+		When(`warn on head failure is set to "always"`, func() {
-			c := newClientNoAPI(false, false, false, false, false, "always")
+			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() {
+		When(`warn on head failure is set to "auto"`, func() {
-			c := newClientNoAPI(false, false, false, false, false, "auto")
+			c := dockerClient{ClientOptions: ClientOptions{WarnOnHeadFailed: WarnAuto}}
-			It("should always return true", func() {
+			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() {
+		When(`warn on head failure is set to "never"`, func() {
-			c := newClientNoAPI(false, false, false, false, false, "never")
+			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,
+					api:           docker,
-					pullImages: false,
+					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,
+					api:           docker,
-					pullImages: false,
+					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,
+					api:           docker,
-					pullImages: false,
+					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,
+					api:           docker,
-					pullImages:     false,
+					ClientOptions: ClientOptions{PullImages: false, IncludeStopped: true},
-					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,
+					api:           docker,
-					pullImages:        false,
+					ClientOptions: ClientOptions{PullImages: false, IncludeRestarting: true},
-					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,
+					api:           docker,
-					pullImages:        false,
+					ClientOptions: ClientOptions{PullImages: false, IncludeRestarting: 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,
+					api:           docker,
-					pullImages: false,
+					ClientOptions: ClientOptions{PullImages: false},
 				}
 
 				// Capture logrus output in buffer

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() {
+			It("should return minute values", func() {
- 					c = mockContainerWithLabels(map[string]string{
+				c = mockContainerWithLabels(map[string]string{
- 						"com.centurylinklabs.watchtower.lifecycle.pre-update-timeout":  "3",
+					"com.centurylinklabs.watchtower.lifecycle.pre-update-timeout":  "3",
- 						"com.centurylinklabs.watchtower.lifecycle.post-update-timeout": "5",
+					"com.centurylinklabs.watchtower.lifecycle.post-update-timeout": "5",
- 					})
+				})
- 					preTimeout := c.PreUpdateTimeout()
+				preTimeout := c.PreUpdateTimeout()
- 					Expect(preTimeout).To(Equal(3))
+				Expect(preTimeout).To(Equal(3))
- 					postTimeout := c.PostUpdateTimeout()
+				postTimeout := c.PostUpdateTimeout()
- 					Expect(postTimeout).To(Equal(5))
+				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,
-	}
-}