feat: add a label take precedence argument (#1754)

Co-authored-by: nils måsén <nils@piksel.se>

cmd/root.go

@@ -28,17 +28,18 @@ import (
 )
 
 var (
-	client         container.Client
+	client          container.Client
-	scheduleSpec   string
+	scheduleSpec    string
-	cleanup        bool
+	cleanup         bool
-	noRestart      bool
+	noRestart       bool
-	monitorOnly    bool
+	monitorOnly     bool
-	enableLabel    bool
+	enableLabel     bool
-	notifier       t.Notifier
+	notifier        t.Notifier
-	timeout        time.Duration
+	timeout         time.Duration
-	lifecycleHooks bool
+	lifecycleHooks  bool
-	rollingRestart bool
+	rollingRestart  bool
-	scope          string
+	scope           string
+	labelPrecedence bool
 )
 
 var rootCmd = NewRootCommand()
@@ -109,6 +110,7 @@ func PreRun(cmd *cobra.Command, _ []string) {
 	lifecycleHooks, _ = f.GetBool("enable-lifecycle-hooks")
 	rollingRestart, _ = f.GetBool("rolling-restart")
 	scope, _ = f.GetString("scope")
+	labelPrecedence, _ = f.GetBool("label-take-precedence")
 
 	if scope != "" {
 		log.Debugf(`Using scope %q`, scope)
@@ -359,13 +361,14 @@ func runUpgradesOnSchedule(c *cobra.Command, filter t.Filter, filtering string,
 func runUpdatesWithNotifications(filter t.Filter) *metrics.Metric {
 	notifier.StartNotification()
 	updateParams := t.UpdateParams{
-		Filter:         filter,
+		Filter:          filter,
-		Cleanup:        cleanup,
+		Cleanup:         cleanup,
-		NoRestart:      noRestart,
+		NoRestart:       noRestart,
-		Timeout:        timeout,
+		Timeout:         timeout,
-		MonitorOnly:    monitorOnly,
+		MonitorOnly:     monitorOnly,
-		LifecycleHooks: lifecycleHooks,
+		LifecycleHooks:  lifecycleHooks,
-		RollingRestart: rollingRestart,
+		RollingRestart:  rollingRestart,
+		LabelPrecedence: labelPrecedence,
 	}
 	result, err := actions.Update(client, updateParams)
 	if err != nil {

docs/arguments.md

@@ -205,7 +205,7 @@ Environment Variable: WATCHTOWER_POLL_INTERVAL
 ```
 
 ## Filter by enable label
-Update containers that have a `com.centurylinklabs.watchtower.enable` label set to true.
+Monitor and update containers that have a `com.centurylinklabs.watchtower.enable` label set to true.
 
 ```text
             Argument: --label-enable
@@ -215,7 +215,7 @@ Environment Variable: WATCHTOWER_LABEL_ENABLE
 ```
 
 ## Filter by disable label
-__Do not__ update containers that have `com.centurylinklabs.watchtower.enable` label set to false and 
+__Do not__ Monitor and update containers that have `com.centurylinklabs.watchtower.enable` label set to false and 
 no `--label-enable` argument is passed. Note that only one or the other (targeting by enable label) can be 
 used at the same time to target containers.
 
@@ -238,6 +238,19 @@ Environment Variable: WATCHTOWER_MONITOR_ONLY
 
 Note that monitor-only can also be specified on a per-container basis with the `com.centurylinklabs.watchtower.monitor-only` label set on those containers.
 
+See [With label taking precedence over arguments](#With-label-taking-precedence-over-arguments) for behavior when both argument and label are set
+
+## With label taking precedence over arguments
+
+By default, arguments will take precedence over labels. This means that if you set `WATCHTOWER_MONITOR_ONLY` to true or use `--monitor-only`, a container with `com.centurylinklabs.watchtower.monitor-only` set to false will not be updated. If you set `WATCHTOWER_LABEL_TAKE_PRECEDENCE` to true or use `--label-take-precedence`, then the container will also be updated. This also apply to the no pull option. if you set `WATCHTOWER_NO_PULL` to true or use `--no-pull`, a container with `com.centurylinklabs.watchtower.no-pull` set to false will not pull the new image. If you set `WATCHTOWER_LABEL_TAKE_PRECEDENCE` to true or use `--label-take-precedence`, then the container will pull image
+
+```text
+            Argument: --label-take-precedence
+Environment Variable: WATCHTOWER_LABEL_TAKE_PRECEDENCE
+                Type: Boolean
+             Default: false
+```
+
 ## Without restarting containers
 Do not restart containers after updating. This option can be useful when the start of the containers
 is managed by an external system such as systemd.
@@ -264,6 +277,8 @@ Environment Variable: WATCHTOWER_NO_PULL
 Note that no-pull can also be specified on a per-container basis with the
 `com.centurylinklabs.watchtower.no-pull` label set on those containers.
 
+See [With label taking precedence over arguments](#With-label-taking-precedence-over-arguments) for behavior when both argument and label are set
+
 ## Without sending a startup message
 Do not send a message after watchtower started. Otherwise there will be an info-level notification.
 

internal/actions/mocks/client.go

@@ -86,7 +86,7 @@ func (client MockClient) ExecuteCommand(_ t.ContainerID, command string, _ int)
 }
 
 // IsContainerStale is true if not explicitly stated in TestData for the mock client
-func (client MockClient) IsContainerStale(cont t.Container) (bool, t.ImageID, error) {
+func (client MockClient) IsContainerStale(cont t.Container, params t.UpdateParams) (bool, t.ImageID, error) {
 	stale, found := client.TestData.Staleness[cont.Name()]
 	if !found {
 		stale = true

internal/actions/update.go

@@ -33,8 +33,8 @@ func Update(client container.Client, params types.UpdateParams) (types.Report, e
 	staleCheckFailed := 0
 
 	for i, targetContainer := range containers {
-		stale, newestImage, err := client.IsContainerStale(targetContainer)
+		stale, newestImage, err := client.IsContainerStale(targetContainer, params)
-		shouldUpdate := stale && !params.NoRestart && !params.MonitorOnly && !targetContainer.IsMonitorOnly()
+		shouldUpdate := stale && !params.NoRestart && !targetContainer.IsMonitorOnly(params)
 		if err == nil && shouldUpdate {
 			// Check to make sure we have all the necessary information for recreating the container
 			err = targetContainer.VerifyConfiguration()
@@ -72,12 +72,10 @@ func Update(client container.Client, params types.UpdateParams) (types.Report, e
 	UpdateImplicitRestart(containers)
 
 	var containersToUpdate []types.Container
-	if !params.MonitorOnly {
+	for _, c := range containers {
-		for _, c := range containers {
+		if !c.IsMonitorOnly(params) {
-			if !c.IsMonitorOnly() {
+			containersToUpdate = append(containersToUpdate, c)
-				containersToUpdate = append(containersToUpdate, c)
+			progress.MarkForUpdate(c.ID())
-				progress.MarkForUpdate(c.ID())
-			}
 		}
 	}
 

internal/actions/update_test.go

@@ -178,12 +178,84 @@ var _ = Describe("the update action", func() {
 					false,
 					false,
 				)
-				_, err := actions.Update(client, types.UpdateParams{MonitorOnly: true})
+				_, err := actions.Update(client, types.UpdateParams{Cleanup: true, MonitorOnly: true})
 				Expect(err).NotTo(HaveOccurred())
 				Expect(client.TestData.TriedToRemoveImageCount).To(Equal(0))
 			})
-		})
+			When("watchtower has been instructed to have label take precedence", func() {
+				It("it should update containers when monitor only is set to false", func() {
+					client := CreateMockClient(
+						&TestData{
+							//NameOfContainerToKeep: "test-container-02",
+							Containers: []types.Container{
+								CreateMockContainerWithConfig(
+									"test-container-02",
+									"test-container-02",
+									"fake-image2:latest",
+									false,
+									false,
+									time.Now(),
+									&dockerContainer.Config{
+										Labels: map[string]string{
+											"com.centurylinklabs.watchtower.monitor-only": "false",
+										},
+									}),
+							},
+						},
+						false,
+						false,
+					)
+					_, err := actions.Update(client, types.UpdateParams{Cleanup: true, MonitorOnly: true, LabelPrecedence: true})
+					Expect(err).NotTo(HaveOccurred())
+					Expect(client.TestData.TriedToRemoveImageCount).To(Equal(1))
+				})
+				It("it should update not containers when monitor only is set to true", func() {
+					client := CreateMockClient(
+						&TestData{
+							//NameOfContainerToKeep: "test-container-02",
+							Containers: []types.Container{
+								CreateMockContainerWithConfig(
+									"test-container-02",
+									"test-container-02",
+									"fake-image2:latest",
+									false,
+									false,
+									time.Now(),
+									&dockerContainer.Config{
+										Labels: map[string]string{
+											"com.centurylinklabs.watchtower.monitor-only": "true",
+										},
+									}),
+							},
+						},
+						false,
+						false,
+					)
+					_, err := actions.Update(client, types.UpdateParams{Cleanup: true, MonitorOnly: true, LabelPrecedence: true})
+					Expect(err).NotTo(HaveOccurred())
+					Expect(client.TestData.TriedToRemoveImageCount).To(Equal(0))
+				})
+				It("it should update not containers when monitor only is not set", func() {
+					client := CreateMockClient(
+						&TestData{
+							Containers: []types.Container{
+								CreateMockContainer(
+									"test-container-01",
+									"test-container-01",
+									"fake-image:latest",
+									time.Now()),
+							},
+						},
+						false,
+						false,
+					)
+					_, err := actions.Update(client, types.UpdateParams{Cleanup: true, MonitorOnly: true, LabelPrecedence: true})
+					Expect(err).NotTo(HaveOccurred())
+					Expect(client.TestData.TriedToRemoveImageCount).To(Equal(0))
+				})
 
+			})
+		})
 	})
 
 	When("watchtower has been instructed to run lifecycle hooks", func() {

internal/flags/flags.go

@@ -185,6 +185,12 @@ func RegisterSystemFlags(rootCmd *cobra.Command) {
 		"log-level",
 		envString("WATCHTOWER_LOG_LEVEL"),
 		"The maximum log level that will be written to STDERR. Possible values: panic, fatal, error, warn, info, debug or trace")
+
+	flags.BoolP(
+		"label-take-precedence",
+		"",
+		viper.GetBool("WATCHTOWER_LABEL_TAKE_PRECEDENCE"),
+		"Label applied to containers take precedence over arguments")
 }
 
 // RegisterNotificationFlags that are used by watchtower to send notifications

pkg/container/client.go

@@ -30,7 +30,7 @@ type Client interface {
 	StopContainer(t.Container, time.Duration) error
 	StartContainer(t.Container) (t.ContainerID, error)
 	RenameContainer(t.Container, string) error
-	IsContainerStale(t.Container) (stale bool, latestImage t.ImageID, err error)
+	IsContainerStale(t.Container, t.UpdateParams) (stale bool, latestImage t.ImageID, err error)
 	ExecuteCommand(containerID t.ContainerID, command string, timeout int) (SkipUpdate bool, err error)
 	RemoveImageByID(t.ImageID) error
 	WarnOnHeadPullFailed(container t.Container) bool
@@ -308,10 +308,10 @@ func (client dockerClient) RenameContainer(c t.Container, newName string) error
 	return client.api.ContainerRename(bg, string(c.ID()), newName)
 }
 
-func (client dockerClient) IsContainerStale(container t.Container) (stale bool, latestImage t.ImageID, err error) {
+func (client dockerClient) IsContainerStale(container t.Container, params t.UpdateParams) (stale bool, latestImage t.ImageID, err error) {
 	ctx := context.Background()
 
-	if !client.PullImages || container.IsNoPull() {
+	if container.IsNoPull(params) {
 		log.Debugf("Skipping image pull.")
 	} else if err := client.PullImage(ctx, container); err != nil {
 		return false, container.SafeImageID(), err

pkg/container/container.go

@@ -2,12 +2,14 @@
 package container
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
 
 	"github.com/containrrr/watchtower/internal/util"
 	wt "github.com/containrrr/watchtower/pkg/types"
+	"github.com/sirupsen/logrus"
 
 	"github.com/docker/docker/api/types"
 	dockercontainer "github.com/docker/docker/api/types/container"
@@ -129,36 +131,31 @@ func (c Container) Enabled() (bool, bool) {
 	return parsedBool, true
 }
 
-// IsMonitorOnly returns the value of the monitor-only label. If the label
+// IsMonitorOnly returns whether the container should only be monitored based on values of
-// is not set then false is returned.
+// the monitor-only label, the monitor-only argument and the label-take-precedence argument.
-func (c Container) IsMonitorOnly() bool {
+func (c Container) IsMonitorOnly(params wt.UpdateParams) bool {
-	rawBool, ok := c.getLabelValue(monitorOnlyLabel)
+	return c.getContainerOrGlobalBool(params.MonitorOnly, monitorOnlyLabel, params.LabelPrecedence)
-	if !ok {
-		return false
-	}
-
-	parsedBool, err := strconv.ParseBool(rawBool)
-	if err != nil {
-		return false
-	}
-
-	return parsedBool
 }
 
-// IsNoPull returns the value of the no-pull label. If the label is not set
+// IsNoPull returns whether the image should be pulled based on values of
-// then false is returned.
+// the no-pull label, the no-pull argument and the label-take-precedence argument.
-func (c Container) IsNoPull() bool {
+func (c Container) IsNoPull(params wt.UpdateParams) bool {
-	rawBool, ok := c.getLabelValue(noPullLabel)
+	return c.getContainerOrGlobalBool(params.NoPull, noPullLabel, params.LabelPrecedence)
-	if !ok {
+}
-		return false
-	}
 
-	parsedBool, err := strconv.ParseBool(rawBool)
+func (c Container) getContainerOrGlobalBool(globalVal bool, label string, contPrecedence bool) bool {
-	if err != nil {
+	if contVal, err := c.getBoolLabelValue(label); err != nil {
-		return false
+		if !errors.Is(err, errorLabelNotFound) {
+			logrus.WithField("error", err).WithField("label", label).Warn("Failed to parse label value")
+		}
+		return globalVal
+	} else {
+		if contPrecedence {
+			return contVal
+		} else {
+			return contVal || globalVal
+		}
 	}
-
-	return parsedBool
 }
 
 // Scope returns the value of the scope UID label and if the label

pkg/container/container_test.go

@@ -1,6 +1,7 @@
 package container
 
 import (
+	"github.com/containrrr/watchtower/pkg/types"
 	"github.com/docker/go-connections/nat"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -215,34 +216,72 @@ var _ = Describe("the container", func() {
 		})
 
 		When("checking no-pull label", func() {
-			When("no-pull label is true", func() {
+			When("no-pull argument is not set", func() {
-				c := MockContainer(WithLabels(map[string]string{
+				When("no-pull label is true", func() {
-					"com.centurylinklabs.watchtower.no-pull": "true",
+					c := MockContainer(WithLabels(map[string]string{
-				}))
+						"com.centurylinklabs.watchtower.no-pull": "true",
-				It("should return true", func() {
+					}))
-					Expect(c.IsNoPull()).To(Equal(true))
+					It("should return true", func() {
+						Expect(c.IsNoPull(types.UpdateParams{})).To(Equal(true))
+					})
 				})
-			})
+				When("no-pull label is false", func() {
-			When("no-pull label is false", func() {
+					c := MockContainer(WithLabels(map[string]string{
-				c := MockContainer(WithLabels(map[string]string{
+						"com.centurylinklabs.watchtower.no-pull": "false",
-					"com.centurylinklabs.watchtower.no-pull": "false",
+					}))
-				}))
+					It("should return false", func() {
-				It("should return false", func() {
+						Expect(c.IsNoPull(types.UpdateParams{})).To(Equal(false))
-					Expect(c.IsNoPull()).To(Equal(false))
+					})
 				})
-			})
+				When("no-pull label is set to an invalid value", func() {
-			When("no-pull label is set to an invalid value", func() {
+					c := MockContainer(WithLabels(map[string]string{
-				c := MockContainer(WithLabels(map[string]string{
+						"com.centurylinklabs.watchtower.no-pull": "maybe",
-					"com.centurylinklabs.watchtower.no-pull": "maybe",
+					}))
-				}))
+					It("should return false", func() {
-				It("should return false", func() {
+						Expect(c.IsNoPull(types.UpdateParams{})).To(Equal(false))
-					Expect(c.IsNoPull()).To(Equal(false))
+					})
+				})
+				When("no-pull label is unset", func() {
+					c = MockContainer(WithLabels(map[string]string{}))
+					It("should return false", func() {
+						Expect(c.IsNoPull(types.UpdateParams{})).To(Equal(false))
+					})
 				})
 			})
-			When("no-pull label is unset", func() {
+			When("no-pull argument is set to true", func() {
-				c = MockContainer(WithLabels(map[string]string{}))
+				When("no-pull label is true", func() {
-				It("should return false", func() {
+					c := MockContainer(WithLabels(map[string]string{
-					Expect(c.IsNoPull()).To(Equal(false))
+						"com.centurylinklabs.watchtower.no-pull": "true",
+					}))
+					It("should return true", func() {
+						Expect(c.IsNoPull(types.UpdateParams{NoPull: true})).To(Equal(true))
+					})
+				})
+				When("no-pull label is false", func() {
+					c := MockContainer(WithLabels(map[string]string{
+						"com.centurylinklabs.watchtower.no-pull": "false",
+					}))
+					It("should return true", func() {
+						Expect(c.IsNoPull(types.UpdateParams{NoPull: true})).To(Equal(true))
+					})
+				})
+				When("label-take-precedence argument is set to true", func() {
+					When("no-pull label is true", func() {
+						c := MockContainer(WithLabels(map[string]string{
+							"com.centurylinklabs.watchtower.no-pull": "true",
+						}))
+						It("should return true", func() {
+							Expect(c.IsNoPull(types.UpdateParams{LabelPrecedence: true, NoPull: true})).To(Equal(true))
+						})
+					})
+					When("no-pull label is false", func() {
+						c := MockContainer(WithLabels(map[string]string{
+							"com.centurylinklabs.watchtower.no-pull": "false",
+						}))
+						It("should return false", func() {
+							Expect(c.IsNoPull(types.UpdateParams{LabelPrecedence: true, NoPull: true})).To(Equal(false))
+						})
+					})
 				})
 			})
 		})

pkg/container/errors.go

@@ -5,3 +5,4 @@ import "errors"
 var errorNoImageInfo = errors.New("no available image info")
 var errorNoContainerInfo = errors.New("no available container info")
 var errorInvalidConfig = errors.New("container configuration missing or invalid")
+var errorLabelNotFound = errors.New("label was not found in container")

pkg/container/metadata.go

@@ -1,5 +1,7 @@
 package container
 
+import "strconv"
+
 const (
 	watchtowerLabel        = "com.centurylinklabs.watchtower"
 	signalLabel            = "com.centurylinklabs.watchtower.stop-signal"
@@ -55,3 +57,11 @@ func (c Container) getLabelValue(label string) (string, bool) {
 	val, ok := c.containerInfo.Config.Labels[label]
 	return val, ok
 }
+
+func (c Container) getBoolLabelValue(label string) (bool, error) {
+	if strVal, ok := c.containerInfo.Config.Labels[label]; ok {
+		value, err := strconv.ParseBool(strVal)
+		return value, err
+	}
+	return false, errorLabelNotFound
+}

pkg/types/container.go

@@ -52,7 +52,7 @@ type Container interface {
 	SafeImageID() ImageID
 	ImageName() string
 	Enabled() (bool, bool)
-	IsMonitorOnly() bool
+	IsMonitorOnly(UpdateParams) bool
 	Scope() (string, bool)
 	Links() []string
 	ToRestart() bool
@@ -67,7 +67,7 @@ type Container interface {
 	VerifyConfiguration() error
 	SetStale(bool)
 	IsStale() bool
-	IsNoPull() bool
+	IsNoPull(UpdateParams) bool
 	SetLinkedToRestarting(bool)
 	IsLinkedToRestarting() bool
 	PreUpdateTimeout() int

pkg/types/update_params.go

@@ -6,11 +6,13 @@ import (
 
 // UpdateParams contains all different options available to alter the behavior of the Update func
 type UpdateParams struct {
-	Filter         Filter
+	Filter          Filter
-	Cleanup        bool
+	Cleanup         bool
-	NoRestart      bool
+	NoRestart       bool
-	Timeout        time.Duration
+	Timeout         time.Duration
-	MonitorOnly    bool
+	MonitorOnly     bool
-	LifecycleHooks bool
+	NoPull			bool
-	RollingRestart bool
+	LifecycleHooks  bool
+	RollingRestart  bool
+	LabelPrecedence bool
 }