feat: make head pull failure warning toggleable (#912)

* feat: make head pull failure warning toggleable

* expect prometheus tests to go through EVENTUALLY

* wait for queue to be empty before checking test conditions

* clean up new head failure toggle

* fixup! clean up new head failure toggle

* test: add registry tests

* test: add warn on head failure tests

* fix client interface and make tests hit more lines

* make all tests use NewClient instead of creating a struct pointer

* fix lint issues

Co-authored-by: Simon Aronsson <[email protected]>

cmd/root.go

@@ -132,6 +132,7 @@ func PreRun(cmd *cobra.Command, _ []string) {
 	includeRestarting, _ := f.GetBool("include-restarting")
 	reviveStopped, _ := f.GetBool("revive-stopped")
 	removeVolumes, _ := f.GetBool("remove-volumes")
+	warnOnHeadPullFailed, _ := f.GetString("warn-on-head-failure")
 
 	if monitorOnly && noPull {
 		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.")
@@ -143,6 +144,7 @@ func PreRun(cmd *cobra.Command, _ []string) {
 		reviveStopped,
 		removeVolumes,
 		includeRestarting,
+		warnOnHeadPullFailed,
 	)
 
 	notifier = notifications.NewNotifier(cmd)

internal/actions/mocks/client.go

@@ -82,3 +82,8 @@ func (client MockClient) ExecuteCommand(containerID string, command string, time
 func (client MockClient) IsContainerStale(c container.Container) (bool, error) {
 	return true, nil
 }
+
+// WarnOnHeadPullFailed is always true for the mock client
+func (client MockClient) WarnOnHeadPullFailed(c container.Container) bool {
+	return true
+}

internal/flags/flags.go

@@ -307,6 +307,12 @@ Should only be used for testing.`)
 		"",
 		viper.GetStringSlice("WATCHTOWER_NOTIFICATION_URL"),
 		"The shoutrrr URL to send notifications to")
+
+	flags.String(
+		"warn-on-head-failure",
+		viper.GetString("WATCHTOWER_WARN_ON_HEAD_FAILURE"),
+		"When to warn about HEAD pull requests failing. Possible values: always, auto or never")
+
 }
 
 // SetDefaults provides default values for environment variables

pkg/api/metrics/metrics_test.go

@@ -37,25 +37,26 @@ func getWithToken(c http.Client, url string) (*http.Response, error) {
 var _ = Describe("the metrics", func() {
 	httpAPI := api.New(Token)
 	m := metricsAPI.New()
+
 	httpAPI.RegisterHandler(m.Path, m.Handle)
 	httpAPI.Start(false)
 
-	// We should likely split this into multiple tests, but as prometheus requires a restart of the binary
-	// to reset the metrics and gauges, we'll just do it all at once.
-
 	It("should serve metrics", func() {
 		metric := &metrics.Metric{
 			Scanned: 4,
 			Updated: 3,
 			Failed:  1,
 		}
 		metrics.RegisterScan(metric)
+		Eventually(metrics.Default().QueueIsEmpty).Should(BeTrue())
+
 		c := http.Client{}
+
 		res, err := getWithToken(c, "http://localhost:8080/v1/metrics")
+		Expect(err).ToNot(HaveOccurred())
 
-		Expect(err).NotTo(HaveOccurred())
 		contents, err := ioutil.ReadAll(res.Body)
-
+		Expect(err).ToNot(HaveOccurred())
 		Expect(string(contents)).To(ContainSubstring("watchtower_containers_updated 3"))
 		Expect(string(contents)).To(ContainSubstring("watchtower_containers_failed 1"))
 		Expect(string(contents)).To(ContainSubstring("watchtower_containers_scanned 4"))
@@ -65,11 +66,13 @@ var _ = Describe("the metrics", func() {
 		for i := 0; i < 3; i++ {
 			metrics.RegisterScan(nil)
 		}
+		Eventually(metrics.Default().QueueIsEmpty).Should(BeTrue())
 
 		res, err = getWithToken(c, "http://localhost:8080/v1/metrics")
-		Expect(err).NotTo(HaveOccurred())
-		contents, err = ioutil.ReadAll(res.Body)
+		Expect(err).ToNot(HaveOccurred())
 
+		contents, err = ioutil.ReadAll(res.Body)
+		Expect(err).ToNot(HaveOccurred())
 		Expect(string(contents)).To(ContainSubstring("watchtower_scans_total 4"))
 		Expect(string(contents)).To(ContainSubstring("watchtower_scans_skipped 3"))
 	})

pkg/container/client.go

@@ -33,6 +33,7 @@ type Client interface {
 	IsContainerStale(Container) (bool, error)
 	ExecuteCommand(containerID string, command string, timeout int) error
 	RemoveImageByID(string) error
+	WarnOnHeadPullFailed(container Container) bool
 }
 
 // NewClient returns a new Client instance which can be used to interact with
@@ -41,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 bool, includeStopped bool, reviveStopped bool, removeVolumes bool, includeRestarting bool) Client {
+func NewClient(pullImages, includeStopped, reviveStopped, removeVolumes, includeRestarting bool, warnOnHeadFailed string) Client {
 	cli, err := sdkClient.NewClientWithOpts(sdkClient.FromEnv)
 
 	if err != nil {
@@ -55,6 +56,7 @@ func NewClient(pullImages bool, includeStopped bool, reviveStopped bool, removeV
 		includeStopped:    includeStopped,
 		reviveStopped:     reviveStopped,
 		includeRestarting: includeRestarting,
+		warnOnHeadFailed:  warnOnHeadFailed,
 	}
 }
 
@@ -65,6 +67,18 @@ type dockerClient struct {
 	includeStopped    bool
 	reviveStopped     bool
 	includeRestarting bool
+	warnOnHeadFailed  string
+}
+
+func (client dockerClient) WarnOnHeadPullFailed(container Container) bool {
+	if client.warnOnHeadFailed == "always" {
+		return true
+	}
+	if client.warnOnHeadFailed == "never" {
+		return false
+	}
+
+	return registry.WarnOnAPIConsumption(container)
 }
 
 func (client dockerClient) ListContainers(fn t.Filter) ([]Container, error) {
@@ -275,6 +289,8 @@ func (client dockerClient) HasNewImage(ctx context.Context, container Container)
 	return true, nil
 }
 
+// PullImage pulls the latest image for the supplied container, optionally skipping if it's digest can be confirmed
+// to match the one that the registry reports via a HEAD request
 func (client dockerClient) PullImage(ctx context.Context, container Container) error {
 	containerName := container.Name()
 	imageName := container.ImageName()
@@ -297,12 +313,12 @@ func (client dockerClient) PullImage(ctx context.Context, container Container) e
 	log.WithFields(fields).Debugf("Checking if pull is needed")
 
 	if match, err := digest.CompareDigest(container, opts.RegistryAuth); err != nil {
-		if registry.WarnOnAPIConsumption(container) {
-			log.WithFields(fields).Warning("Could not do a head request, falling back to regular pull.")
-		} else {
-			log.Debug("Could not do a head request, falling back to regular pull.")
+		headLevel := log.DebugLevel
+		if client.WarnOnHeadPullFailed(container) {
+			headLevel = log.WarnLevel
 		}
-		log.Debugf("Reason: %s", err.Error())
+		log.WithFields(fields).Logf(headLevel, "Could not do a head request for %q, falling back to regular pull.", imageName)
+		log.WithFields(fields).Log(headLevel, "Reason: ", err)
 	} else if match {
 		log.Debug("No pull needed. Skipping image.")
 		return nil

pkg/container/container_test.go

@@ -1,8 +1,6 @@
 package container
 
 import (
-	"testing"
-
 	"github.com/containrrr/watchtower/pkg/container/mocks"
 	"github.com/containrrr/watchtower/pkg/filters"
 	"github.com/docker/docker/api/types"
@@ -12,11 +10,6 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestContainer(t *testing.T) {
-	RegisterFailHandler(Fail)
-	RunSpecs(t, "Container Suite")
-}
-
 var _ = Describe("the container", func() {
 	Describe("the client", func() {
 		var docker *cli.Client
@@ -34,6 +27,35 @@ var _ = Describe("the container", func() {
 		It("should return a client for the api", func() {
 			Expect(client).NotTo(BeNil())
 		})
+		Describe("WarnOnHeadPullFailed", 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 := NewClient(false, false, false, false, false, "always")
+				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 := NewClient(false, false, false, false, false, "auto")
+				It("should always return true", func() {
+					Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
+				})
+				It("should", func() {
+					Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeTrue())
+				})
+			})
+			When("warn on head failure is set to \"never\"", func() {
+				c := NewClient(false, false, false, false, false, "never")
+				It("should never return true", func() {
+					Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
+					Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeFalse())
+				})
+			})
+		})
+
 		When("listing containers without any filter", func() {
 			It("should return all available containers", func() {
 				containers, err := client.ListContainers(filters.NoFilter)

pkg/metrics/metrics.go

@@ -24,6 +24,11 @@ type Metrics struct {
 	skipped prometheus.Counter
 }
 
+// QueueIsEmpty checks whether any messages are enqueued in the channel
+func (metrics *Metrics) QueueIsEmpty() bool {
+	return len(metrics.channel) == 0
+}
+
 // Register registers metrics for an executed scan
 func (metrics *Metrics) Register(metric *Metric) {
 	metrics.channel <- metric

pkg/registry/digest/digest.go

@@ -95,7 +95,11 @@ func GetDigest(url string, token string) (string, error) {
 	defer res.Body.Close()
 
 	if res.StatusCode != 200 {
-		return "", fmt.Errorf("registry responded to head request with %v", res)
+		wwwAuthHeader := res.Header.Get("www-authenticate")
+		if wwwAuthHeader == "" {
+			wwwAuthHeader = "not present"
+		}
+		return "", fmt.Errorf("registry responded to head request with %q, auth: %q", res.Status, wwwAuthHeader)
 	}
 	return res.Header.Get(ContentDigestHeader), nil
 }

pkg/registry/registry_suite_test.go

@@ -0,0 +1,13 @@
+package registry_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestRegistry(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Registry Suite")
+}

pkg/registry/registry_test.go

@@ -0,0 +1,45 @@
+package registry_test
+
+import (
+	"github.com/containrrr/watchtower/internal/actions/mocks"
+	unit "github.com/containrrr/watchtower/pkg/registry"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"time"
+)
+
+var _ = Describe("Registry", func() {
+	Describe("WarnOnAPIConsumption", func() {
+		When("Given a container with an image from ghcr.io", func() {
+			It("should want to warn", func() {
+				Expect(testContainerWithImage("ghcr.io/containrrr/watchtower")).To(BeTrue())
+			})
+		})
+		When("Given a container with an image implicitly from dockerhub", func() {
+			It("should want to warn", func() {
+				Expect(testContainerWithImage("docker:latest")).To(BeTrue())
+			})
+		})
+		When("Given a container with an image explicitly from dockerhub", func() {
+			It("should want to warn", func() {
+				Expect(testContainerWithImage("registry-1.docker.io/docker:latest")).To(BeTrue())
+				Expect(testContainerWithImage("index.docker.io/docker:latest")).To(BeTrue())
+				Expect(testContainerWithImage("docker.io/docker:latest")).To(BeTrue())
+			})
+
+		})
+		When("Given a container with an image from some other registry", func() {
+			It("should not want to warn", func() {
+				Expect(testContainerWithImage("docker.fsf.org/docker:latest")).To(BeFalse())
+				Expect(testContainerWithImage("altavista.com/docker:latest")).To(BeFalse())
+				Expect(testContainerWithImage("gitlab.com/docker:latest")).To(BeFalse())
+			})
+		})
+	})
+})
+
+func testContainerWithImage(imageName string) bool {
+	container := mocks.CreateMockContainer("", "", imageName, time.Now())
+	return unit.WarnOnAPIConsumption(container)
+}

pkg/registry/trust_test.go

@@ -4,14 +4,8 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"os"
-	"testing"
 )
 
-func TestTrust(t *testing.T) {
-	RegisterFailHandler(Fail)
-	RunSpecs(t, "Trust Suite")
-}
-
 var _ = Describe("Testing with Ginkgo", func() {
 	It("encoded env auth_ should return an error if repo envs are unset", func() {
 		_ = os.Unsetenv("REPO_USER")

scripts/codecov.sh

@@ -0,0 +1,6 @@
+#!/usr/bin/env bash
+
+go test -v -coverprofile coverage.out -covermode atomic ./...
+
+# Requires CODECOV_TOKEN to be set
+bash <(curl -s https://codecov.io/bash)