Merge pull request #480 from containrrr/feature/367

Allow overriding, or even disabling, the pre-update timeout

docs/lifecycle-hooks.md

@@ -46,6 +46,19 @@ docker run -d \
   --label=com.centurylinklabs.watchtower.lifecycle.post-check="/send-heartbeat.sh" \
 ```
 
+### Timeouts
+The timeout for all lifecycle commands is 60 seconds. After that, a timeout will
+occur, forcing Watchtower to continue the update loop.
+
+#### Pre-update timeouts
+
+For the `pre-update` lifecycle command, it is possible to override this timeout to
+allow the script to finish before forcefully killing it. This is done by adding the
+label `com.centurylinklabs.watchtower.lifecycle.pre-update-timeout` followed by
+the timeout expressed in minutes.
+
+If the label value is explicitly set to `0`, the timeout will be disabled.  
+
 ### Execution failure
 
 The failure of a command to execute, identified by an exit code different than

go.sum

@@ -61,6 +61,7 @@ github.com/docker/distribution v2.7.1+incompatible h1:a5mlkVzth6W5A4fOsS3D2EO5BU
 github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
 github.com/docker/docker v0.0.0-20190404075923-dbe4a30928d4 h1:34LfsqlE2kEvmGP9qbRoPvOWkmluYGzmlvWVTzwvT0A=
 github.com/docker/docker v0.0.0-20190404075923-dbe4a30928d4/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
+github.com/docker/docker v1.13.1 h1:IkZjBSIc8hBjLpqeAbeE5mca5mNgeatLHBy3GO78BWo=
 github.com/docker/docker-credential-helpers v0.6.1 h1:Dq4iIfcM7cNtddhLVWe9h4QDjsi4OER3Z8voPu/I52g=
 github.com/docker/docker-credential-helpers v0.6.1/go.mod h1:WRaJzqw3CTB9bk10avuGsjVBZsD05qeibJ1/TYlvc0Y=
 github.com/docker/go v1.5.1-1 h1:hr4w35acWBPhGBXlzPoHpmZ/ygPjnmFVxGxxGnMyP7k=
@@ -284,6 +285,7 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190522155817-f3200d17e092 h1:4QSRKanuywn15aTZvI/mIDEgPQpswuFndXpOj3rKEco=
 golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
+golang.org/x/net v0.0.0-20191116160921-f9c825593386 h1:ktbWvQrW08Txdxno1PiDpSxPXG6ndGsfnJjRRtkM0LQ=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

internal/actions/actions_suite_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/containrrr/watchtower/pkg/container/mocks"
 
 	cli "github.com/docker/docker/client"
+	"github.com/docker/docker/api/types"
 
 	. "github.com/containrrr/watchtower/internal/actions/mocks"
 	. "github.com/onsi/ginkgo"
@@ -132,3 +133,14 @@ var _ = Describe("the actions package", func() {
 	})
 })
 
+func createMockContainer(id string, name string, image string, created time.Time) container.Container {
+	content := types.ContainerJSON{
+		ContainerJSONBase: &types.ContainerJSONBase{
+			ID:      id,
+			Image:   image,
+			Name:    name,
+			Created: created.String(),
+		},
+	}
+	return *container.NewContainer(&content, nil)
+}

internal/actions/mocks/client.go

@@ -73,7 +73,7 @@ func (client MockClient) GetContainer(containerID string) (container.Container,
 }
 
 // ExecuteCommand is a mock method
-func (client MockClient) ExecuteCommand(containerID string, command string) error {
+func (client MockClient) ExecuteCommand(containerID string, command string, timeout int) error {
 	return nil
 }
 

internal/actions/update.go

@@ -74,8 +74,11 @@ func stopStaleContainer(container container.Container, client container.Client,
 		return
 	}
 	if params.LifecycleHooks {
-		lifecycle.ExecutePreUpdateCommand(client, container)
-
+		if err := lifecycle.ExecutePreUpdateCommand(client, container); err != nil {
+			log.Error(err)
+			log.Info("Skipping container as the pre-update command failed")
+			return
+		}
 	}
 	
 	if err := client.StopContainer(container, params.Timeout); err != nil {

internal/flags/flags.go

@@ -30,12 +30,14 @@ func RegisterSystemFlags(rootCmd *cobra.Command) {
 		viper.GetInt("WATCHTOWER_POLL_INTERVAL"),
 		"poll interval (in seconds)")
 
-	flags.StringP("schedule",
+	flags.StringP(
+		"schedule",
 		"s",
 		viper.GetString("WATCHTOWER_SCHEDULE"),
 		"the cron expression which defines when to update")
 
-	flags.DurationP("stop-timeout",
+	flags.DurationP(
+		"stop-timeout",
 		"t",
 		viper.GetDuration("WATCHTOWER_TIMEOUT"),
 		"timeout before a container is forcefully stopped")

pkg/container/client.go

@@ -29,9 +29,8 @@ type Client interface {
 	StartContainer(Container) (string, error)
 	RenameContainer(Container, string) error
 	IsContainerStale(Container) (bool, error)
-	ExecuteCommand(containerID string, command string) error
+	ExecuteCommand(containerID string, command string, timeout int) error
 	RemoveImageByID(string) error
-
 }
 
 // NewClient returns a new Client instance which can be used to interact with
@@ -301,7 +300,7 @@ func (client dockerClient) RemoveImageByID(id string) error {
 	return err
 }
 
-func (client dockerClient) ExecuteCommand(containerID string, command string) error {
+func (client dockerClient) ExecuteCommand(containerID string, command string, timeout int) error {
 	bg := context.Background()
 
 	// Create the exec
@@ -331,7 +330,7 @@ func (client dockerClient) ExecuteCommand(containerID string, command string) er
 		return err
 	}
 
-	var execOutput string
+	var output string
 	if attachErr == nil {
 		defer response.Close()
 		var writer bytes.Buffer
@@ -339,26 +338,56 @@ func (client dockerClient) ExecuteCommand(containerID string, command string) er
 		if err != nil {
 			log.Error(err)
 		} else if written > 0 {
-			execOutput = strings.TrimSpace(writer.String())
+			output = strings.TrimSpace(writer.String())
 		}
 	}
 
 	// Inspect the exec to get the exit code and print a message if the
 	// exit code is not success.
-	execInspect, err := client.api.ContainerExecInspect(bg, exec.ID)
+	err = client.waitForExecOrTimeout(bg, exec.ID, output, timeout)
 	if err != nil {
 		return err
 	}
 
-	if execInspect.ExitCode > 0 {
-		log.Errorf("Command exited with code %v.", execInspect.ExitCode)
-		log.Error(execOutput)
+	return nil
+}
+
+func (client dockerClient) waitForExecOrTimeout(bg context.Context, ID string, execOutput string, timeout int) error {
+	var ctx context.Context
+	var cancel context.CancelFunc
+
+	if timeout > 0 {
+		ctx, cancel = context.WithTimeout(bg, time.Duration(timeout)*time.Minute)
+		defer cancel()
 	} else {
+		ctx = bg
+	}
+
+	for {
+		execInspect, err := client.api.ContainerExecInspect(ctx, ID)
+
+		log.WithFields(log.Fields{
+			"exit-code": execInspect.ExitCode,
+			"exec-id":   execInspect.ExecID,
+			"running":   execInspect.Running,
+		}).Debug("Awaiting timeout or completion")
+
+		if err != nil {
+			return err
+		}
+		if execInspect.Running == true {
+			time.Sleep(1 * time.Second)
+			continue
+		}
 		if len(execOutput) > 0 {
 			log.Infof("Command output:\n%v", execOutput)
 		}
+		if execInspect.ExitCode > 0 {
+			log.Errorf("Command exited with code %v.", execInspect.ExitCode)
+			log.Error(execOutput)
+		}
+		break
 	}
-
 	return nil
 }
 
@@ -377,7 +406,6 @@ func (client dockerClient) waitForStopOrTimeout(c Container, waitTime time.Durat
 				return nil
 			}
 		}
-
 		time.Sleep(1 * time.Second)
 	}
 }

pkg/container/container.go

@@ -2,10 +2,11 @@ package container
 
 import (
 	"fmt"
-	"github.com/containrrr/watchtower/internal/util"
 	"strconv"
 	"strings"
 
+	"github.com/containrrr/watchtower/internal/util"
+
 	"github.com/docker/docker/api/types"
 	dockercontainer "github.com/docker/docker/api/types/container"
 )
@@ -118,6 +119,25 @@ func (c Container) IsWatchtower() bool {
 	return ContainsWatchtowerLabel(c.containerInfo.Config.Labels)
 }
 
+// PreUpdateTimeout checks whether a container has a specific timeout set
+// for how long the pre-update command is allowed to run. This value is expressed
+// either as an integer, in minutes, or as 0 which will allow the command/script
+// to run indefinitely. Users should be cautious with the 0 option, as that
+// could result in watchtower waiting forever.
+func (c Container) PreUpdateTimeout() int {
+	var minutes int
+	var err error
+
+	val := c.getLabelValueOrEmpty(preUpdateTimeoutLabel)
+
+	minutes, err = strconv.Atoi(val)
+	if err != nil || val == "" {
+		return 1
+	}
+
+	return minutes
+}
+
 // StopSignal returns the custom stop signal (if any) that is encoded in the
 // container's metadata. If the container has not specified a custom stop
 // signal, the empty string "" is returned.

pkg/container/metadata.go

@@ -1,14 +1,15 @@
 package container
 
 const (
-	watchtowerLabel = "com.centurylinklabs.watchtower"
-	signalLabel     = "com.centurylinklabs.watchtower.stop-signal"
-	enableLabel     = "com.centurylinklabs.watchtower.enable"
-	zodiacLabel     = "com.centurylinklabs.zodiac.original-image"
-	preCheckLabel   = "com.centurylinklabs.watchtower.lifecycle.pre-check"
-	postCheckLabel  = "com.centurylinklabs.watchtower.lifecycle.post-check"
-	preUpdateLabel  = "com.centurylinklabs.watchtower.lifecycle.pre-update"
-	postUpdateLabel = "com.centurylinklabs.watchtower.lifecycle.post-update"
+	watchtowerLabel       = "com.centurylinklabs.watchtower"
+	signalLabel           = "com.centurylinklabs.watchtower.stop-signal"
+	enableLabel           = "com.centurylinklabs.watchtower.enable"
+	zodiacLabel           = "com.centurylinklabs.zodiac.original-image"
+	preCheckLabel         = "com.centurylinklabs.watchtower.lifecycle.pre-check"
+	postCheckLabel        = "com.centurylinklabs.watchtower.lifecycle.post-check"
+	preUpdateLabel        = "com.centurylinklabs.watchtower.lifecycle.pre-update"
+	postUpdateLabel       = "com.centurylinklabs.watchtower.lifecycle.post-update"
+	preUpdateTimeoutLabel = "com.centurylinklabs.watchtower.lifecycle.pre-update-timeout"
 )
 
 // GetLifecyclePreCheckCommand returns the pre-check command set in the container metadata or an empty string

pkg/lifecycle/lifecycle.go

@@ -37,7 +37,7 @@ func ExecutePreCheckCommand(client container.Client, container container.Contain
 	}
 
 	log.Info("Executing pre-check command.")
-	if err := client.ExecuteCommand(container.ID(), command); err != nil {
+	if err := client.ExecuteCommand(container.ID(), command, 1); err != nil {
 		log.Error(err)
 	}
 }
@@ -51,24 +51,22 @@ func ExecutePostCheckCommand(client container.Client, container container.Contai
 	}
 
 	log.Info("Executing post-check command.")
-	if err := client.ExecuteCommand(container.ID(), command); err != nil {
+	if err := client.ExecuteCommand(container.ID(), command, 1); err != nil {
 		log.Error(err)
 	}
 }
 
 // ExecutePreUpdateCommand tries to run the pre-update lifecycle hook for a single container.
-func ExecutePreUpdateCommand(client container.Client, container container.Container) {
-
+func ExecutePreUpdateCommand(client container.Client, container container.Container) error {
+	timeout := container.PreUpdateTimeout()
 	command := container.GetLifecyclePreUpdateCommand()
 	if len(command) == 0 {
 		log.Debug("No pre-update command supplied. Skipping")
-		return
+		return nil
 	}
 
 	log.Info("Executing pre-update command.")
-	if err := client.ExecuteCommand(container.ID(), command); err != nil {
-		log.Error(err)
-	}
+	return client.ExecuteCommand(container.ID(), command, timeout)
 }
 
 // ExecutePostUpdateCommand tries to run the post-update lifecycle hook for a single container.
@@ -86,7 +84,7 @@ func ExecutePostUpdateCommand(client container.Client, newContainerID string) {
 	}
 
 	log.Info("Executing post-update command.")
-	if err := client.ExecuteCommand(newContainerID, command); err != nil {
+	if err := client.ExecuteCommand(newContainerID, command, 1); err != nil {
 		log.Error(err)
 	}
 }

pkg/notifications/notifier.go

@@ -27,8 +27,10 @@ func NewNotifier(c *cobra.Command) *Notifier {
 	acceptedLogLevels := slackrus.LevelThreshold(logLevel)
 
 	// Parse types and create notifiers.
-	types, _ := f.GetStringSlice("notifications")
-
+	types, err := f.GetStringSlice("notifications")
+	if err != nil {
+		log.WithField("could not read notifications argument", log.Fields{ "Error": err }).Fatal()
+	}
 	for _, t := range types {
 		var tn ty.Notifier
 		switch t {