Repository: containrrr/watchtower (go)

Issue:
Schedule broken on latest dev build
**Describe the bug**

Running latest dev at time of this bug report, 2022-08-24. Setting the schedule no longer works and defaults to 24hr from container startup.

**To Reproduce**

start via docker compose:

```yaml
services:
  watchtower:
    image: ghcr.io/containrrr/watchtower:latest-dev
    container_name: watchtower
    network_mode: bridge
    environment:
      - WATCHTOWER_CLEANUP=true
      - WATCHTOWER_DEBUG=true
      - WATCHTOWER_INCLUDE_STOPPED=true
      - WATCHTOWER_NOTIFICATIONS=shoutrrr
      - WATCHTOWER_NOTIFICATION_URL=$url
      - WATCHTOWER_ROLLING_RESTART=true
      - WATCHTOWER_SCHEDULE=0 0 4 * * *
      - WATCHTOWER_NOTIFICATION_SKIP_TITLE=true
      - TZ=$TZ
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock
    labels:
      - com.centurylinklabs.watchtower.enable=false
    restart: unless-stopped
```
**Expected behavior**
<!-- A clear and concise description of what you expected to happen. -->

watchtower checks and updates containers at 4am daily

**Environment**:
 - Platform: debian
 - Architecture: x86_64
 - Docker version: 20.10.17 


<details>
 <summary><b> Logs from running watchtower with the <code>--debug</code> option </b></summary>

```
time="2022-08-24T16:45:44-07:00" level=debug msg="Sleeping for a second to ensure the docker api client has been properly initialized."
time="2022-08-24T16:45:45-07:00" level=debug msg="Making sure everything is sane before starting"
time="2022-08-24T16:45:45-07:00" level=debug msg="Retrieving running, stopped and exited containers"
time="2022-08-24T16:45:45-07:00" level=debug msg="Retrieving running, stopped and exited containers"
time="2022-08-24T16:45:45-07:00" level=debug msg="There are no additional watchtower containers"
time="2022-08-24T16:45:45-07:00" level=debug msg="Watchtower HTTP API skipped."
time="2022-08-24T16:45:45-07:00" level=info msg="Watchtower v1.4.0-30-g6b2ef10"
time="2022-08-24T16:45:45-07:00" level=info msg="Using notifications: discord"
time="2022-08-24T16:45:45-07:00" level=info msg="Checking all containers (except explicitly disabled with label)"
time="2022-08-24T16:45:45-07:00" level=info msg="Scheduling first run: 2022-08-25 16:45:45 -0700 PDT"
time="2022-08-24T16:45:45-07:00" level=info msg="Note that the first check will be performed in 23 hours, 59 minutes, 59 seconds"
```

</details>

**Additional context**
I updated the container manually in the past week. The previous dev version I updated from that worked was probably from july but I don't have the exact build.


Pull request description:
fix(flags): detect schedule set from env
Due to how viper is integrated with pflags, the change detection is broken for environment variables (as the current value becomes the new default). This fixes the "changed" logic for schedule/interval so that environment variables are taken into account.

Fixes #1362

Patch:
diff --git a/internal/flags/flags.go b/internal/flags/flags.go
index bae02a8..5abd69c 100644
--- a/internal/flags/flags.go
+++ b/internal/flags/flags.go
@@ -19,6 +19,8 @@ import (
 // use watchtower
 const DockerAPIMinVersion string = "1.25"
 
+var defaultInterval = int((time.Hour * 24).Seconds())
+
 // RegisterDockerFlags that are used directly by the docker api client
 func RegisterDockerFlags(rootCmd *cobra.Command) {
 	flags := rootCmd.PersistentFlags()
@@ -179,7 +181,7 @@ func RegisterSystemFlags(rootCmd *cobra.Command) {
 		"P",
 		viper.GetString("WATCHTOWER_PORCELAIN"),
 		`Write session results to stdout using a stable versioned format. Supported values: "v1"`)
-		
+
 }
 
 // RegisterNotificationFlags that are used by watchtower to send notifications
@@ -362,11 +364,10 @@ Should only be used for testing.`)
 
 // SetDefaults provides default values for environment variables
 func SetDefaults() {
-	day := (time.Hour * 24).Seconds()
 	viper.AutomaticEnv()
 	viper.SetDefault("DOCKER_HOST", "unix:///var/run/docker.sock")
 	viper.SetDefault("DOCKER_API_VERSION", DockerAPIMinVersion)
-	viper.SetDefault("WATCHTOWER_POLL_INTERVAL", day)
+	viper.SetDefault("WATCHTOWER_POLL_INTERVAL", defaultInterval)
 	viper.SetDefault("WATCHTOWER_TIMEOUT", time.Second*10)
 	viper.SetDefault("WATCHTOWER_NOTIFICATIONS", []string{})
 	viper.SetDefault("WATCHTOWER_NOTIFICATIONS_LEVEL", "info")
@@ -528,9 +529,9 @@ func ProcessFlagAliases(flags *pflag.FlagSet) {
 		log.Fatalf(`Failed to get flag: %v`, err)
 	}
 	if porcelain != "" {
-	    if porcelain != "v1" {
-	        log.Fatalf(`Unknown porcelain version %q. Supported values: "v1"`, porcelain)
-	    }
+		if porcelain != "v1" {
+			log.Fatalf(`Unknown porcelain version %q. Supported values: "v1"`, porcelain)
+		}
 		if err = appendFlagValue(flags, `notification-url`, `logger://`); err != nil {
 			log.Errorf(`Failed to set flag: %v`, err)
 		}
@@ -540,12 +541,23 @@ func ProcessFlagAliases(flags *pflag.FlagSet) {
 		setFlagIfDefault(flags, `notification-template`, tpl)
 	}
 
-	if flags.Changed(`interval`) && flags.Changed(`schedule`) {
+	scheduleChanged := flags.Changed(`schedule`)
+	intervalChanged := flags.Changed(`interval`)
+	// FIXME: snakeswap
+	// due to how viper is integrated by swapping the defaults for the flags, we need this hack:
+	if val, _ := flags.GetString(`schedule`); val != `` {
+		scheduleChanged = true
+	}
+	if val, _ := flags.GetInt(`interval`); val != defaultInterval {
+		intervalChanged = true
+	}
+
+	if intervalChanged && scheduleChanged {
 		log.Fatal(`Only schedule or interval can be defined, not both.`)
 	}
 
 	// update schedule flag to match interval if it's set, or to the default if none of them are
-	if flags.Changed(`interval`) || !flags.Changed(`schedule`) {
+	if intervalChanged || !scheduleChanged {
 		interval, _ := flags.GetInt(`interval`)
 		flags.Set(`schedule`, fmt.Sprintf(`@every %ds`, interval))
 	}


Test patch:
diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go
index 570ebf5..c7f0b80 100644
--- a/internal/flags/flags_test.go
+++ b/internal/flags/flags_test.go
@@ -143,8 +143,8 @@ func TestProcessFlagAliases(t *testing.T) {
 	RegisterNotificationFlags(cmd)
 
 	require.NoError(t, cmd.ParseFlags([]string{
-	    `--porcelain`, `v1`, 
-	    `--interval`, `10`,
+		`--porcelain`, `v1`,
+		`--interval`, `10`,
 	}))
 	flags := cmd.Flags()
 	ProcessFlagAliases(flags)
@@ -173,7 +173,7 @@ func TestProcessFlagAliasesSchedAndInterval(t *testing.T) {
 	RegisterSystemFlags(cmd)
 	RegisterNotificationFlags(cmd)
 
-	require.NoError(t, cmd.ParseFlags([]string{`--schedule`, `@now`, `--interval`, `10`}))
+	require.NoError(t, cmd.ParseFlags([]string{`--schedule`, `@hourly`, `--interval`, `10`}))
 	flags := cmd.Flags()
 
 	assert.PanicsWithValue(t, `FATAL`, func() {
@@ -181,6 +181,26 @@ func TestProcessFlagAliasesSchedAndInterval(t *testing.T) {
 	})
 }
 
+func TestProcessFlagAliasesScheduleFromEnvironment(t *testing.T) {
+	cmd := new(cobra.Command)
+
+	err := os.Setenv("WATCHTOWER_SCHEDULE", `@hourly`)
+	require.NoError(t, err)
+	defer os.Unsetenv("WATCHTOWER_SCHEDULE")
+
+	SetDefaults()
+	RegisterDockerFlags(cmd)
+	RegisterSystemFlags(cmd)
+	RegisterNotificationFlags(cmd)
+
+	require.NoError(t, cmd.ParseFlags([]string{}))
+	flags := cmd.Flags()
+	ProcessFlagAliases(flags)
+
+	sched, _ := flags.GetString(`schedule`)
+	assert.Equal(t, `@hourly`, sched)
+}
+
 func TestProcessFlagAliasesInvalidPorcelaineVersion(t *testing.T) {
 	logrus.StandardLogger().ExitFunc = func(_ int) { panic(`FATAL`) }
 	cmd := new(cobra.Command)