Merge pull request #173 from v2tec/filters

Filters refactoring and always exclude disabled containers

README.md

@@ -129,9 +129,21 @@ docker run -d --label=com.centurylinklabs.watchtower.stop-signal=SIGHUP someimag
 
 ## Selectively Watching Containers
 
-By default, watchtower will watch all containers.
-However, sometimes only some containers should be updated.
-If you need to selectively watch containers, pass the --label-enable flag on startup and set the *com.centurylinklabs.watchtower.enable* label with a value of true for the containers you want to watch.
+By default, watchtower will watch all containers. However, sometimes only some containers should be updated.
+
+If you need to exclude some containers, set the *com.centurylinklabs.watchtower.enable* label to `false`.
+
+```docker
+LABEL com.centurylinklabs.watchtower.enable="false"
+```
+
+Or, it can be specified as part of the `docker run` command line:
+
+```bash
+docker run -d --label=com.centurylinklabs.watchtower.enable=false someimage
+```
+
+If you need to only include only some containers, pass the --label-enable flag on startup and set the *com.centurylinklabs.watchtower.enable* label with a value of true for the containers you want to watch.
 
 ```docker
 LABEL com.centurylinklabs.watchtower.enable="true"

actions/check.go

@@ -6,14 +6,12 @@ import (
 	"github.com/v2tec/watchtower/container"
 )
 
-func watchtowerContainersFilter(c container.Container) bool { return c.IsWatchtower() }
-
 // CheckPrereqs will ensure that there are not multiple instances of the
 // watchtower running simultaneously. If multiple watchtower containers are
 // detected, this function will stop and remove all but the most recently
 // started container.
 func CheckPrereqs(client container.Client, cleanup bool) error {
-	containers, err := client.ListContainers(watchtowerContainersFilter)
+	containers, err := client.ListContainers(container.WatchtowerContainersFilter)
 	if err != nil {
 		return err
 	}

actions/update.go

@@ -12,31 +12,14 @@ var (
 	letters  = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
 )
 
-func allContainersFilter(container.Container) bool { return true }
-
-func containerFilter(names []string) container.Filter {
-	if len(names) == 0 {
-		return allContainersFilter
-	}
-
-	return func(c container.Container) bool {
-		for _, name := range names {
-			if (name == c.Name()) || (name == c.Name()[1:]) {
-				return true
-			}
-		}
-		return false
-	}
-}
-
 // Update looks at the running Docker containers to see if any of the images
 // used to start those containers have been updated. If a change is detected in
 // any of the images, the associated containers are stopped and restarted with
 // the new image.
-func Update(client container.Client, names []string, cleanup bool, noRestart bool, timeout time.Duration) error {
+func Update(client container.Client, filter container.Filter, cleanup bool, noRestart bool, timeout time.Duration) error {
 	log.Debug("Checking containers for updated images")
 
-	containers, err := client.ListContainers(containerFilter(names))
+	containers, err := client.ListContainers(filter)
 	if err != nil {
 		return err
 	}

container/client.go

@@ -5,10 +5,10 @@ import (
 	"io/ioutil"
 	"time"
 
-	log "github.com/sirupsen/logrus"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/network"
 	dockerclient "github.com/docker/docker/client"
+	log "github.com/sirupsen/logrus"
 	"golang.org/x/net/context"
 )
 
@@ -16,10 +16,6 @@ const (
 	defaultStopSignal = "SIGTERM"
 )
 
-// A Filter is a prototype for a function that can be used to filter the
-// results from a call to the ListContainers() method on the Client.
-type Filter func(Container) bool
-
 // A Client is the interface through which watchtower interacts with the
 // Docker API.
 type Client interface {
@@ -37,20 +33,19 @@ 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, enableLabel bool) Client {
+func NewClient(pullImages bool) Client {
 	cli, err := dockerclient.NewEnvClient()
 
 	if err != nil {
 		log.Fatalf("Error instantiating Docker client: %s", err)
 	}
 
-	return dockerClient{api: cli, pullImages: pullImages, enableLabel: enableLabel}
+	return dockerClient{api: cli, pullImages: pullImages}
 }
 
 type dockerClient struct {
-	api         *dockerclient.Client
-	pullImages  bool
-	enableLabel bool
+	api        *dockerclient.Client
+	pullImages bool
 }
 
 func (client dockerClient) ListContainers(fn Filter) ([]Container, error) {
@@ -62,6 +57,7 @@ func (client dockerClient) ListContainers(fn Filter) ([]Container, error) {
 	runningContainers, err := client.api.ContainerList(
 		bg,
 		types.ContainerListOptions{})
+
 	if err != nil {
 		return nil, err
 	}
@@ -79,15 +75,6 @@ func (client dockerClient) ListContainers(fn Filter) ([]Container, error) {
 
 		c := Container{containerInfo: &containerInfo, imageInfo: &imageInfo}
 
-		if client.enableLabel {
-			// If label filtering is enabled, containers should only be enabled
-			// if the label is specifically set to true.
-			enabledLabel, ok := c.Enabled()
-			if !ok || !enabledLabel {
-				continue
-			}
-		}
-
 		if fn(c) {
 			cs = append(cs, c)
 		}
@@ -229,10 +216,9 @@ func (client dockerClient) IsContainerStale(c Container) (bool, error) {
 	if newImageInfo.ID != oldImageInfo.ID {
 		log.Infof("Found new %s image (%s)", imageName, newImageInfo.ID)
 		return true, nil
-	} else {
-		log.Debugf("No new images found for %s", c.Name())
 	}
 
+	log.Debugf("No new images found for %s", c.Name())
 	return false, nil
 }
 

container/container.go

@@ -155,7 +155,7 @@ func (c Container) runtimeConfig() *dockercontainer.Config {
 	config.Volumes = structMapSubtract(config.Volumes, imageConfig.Volumes)
 
 	// subtract ports exposed in image from container
-	for k, _ := range config.ExposedPorts {
+	for k := range config.ExposedPorts {
 		if _, ok := imageConfig.ExposedPorts[k]; ok {
 			delete(config.ExposedPorts, k)
 		}

container/filters.go

@@ -0,0 +1,75 @@
+package container
+
+// A Filter is a prototype for a function that can be used to filter the
+// results from a call to the ListContainers() method on the Client.
+type Filter func(FilterableContainer) bool
+
+// A FilterableContainer is the interface which is used to filter
+// containers.
+type FilterableContainer interface {
+	Name() string
+	IsWatchtower() bool
+	Enabled() (bool, bool)
+}
+
+// WatchtowerContainersFilter filters only watchtower containers
+func WatchtowerContainersFilter(c FilterableContainer) bool { return c.IsWatchtower() }
+
+// Filter no containers and returns all
+func noFilter(FilterableContainer) bool { return true }
+
+// Filters containers which don't have a specified name
+func filterByNames(names []string, baseFilter Filter) Filter {
+	if len(names) == 0 {
+		return baseFilter
+	}
+
+	return func(c FilterableContainer) bool {
+		for _, name := range names {
+			if (name == c.Name()) || (name == c.Name()[1:]) {
+				return baseFilter(c)
+			}
+		}
+		return false
+	}
+}
+
+// Filters out containers that don't have the 'enableLabel'
+func filterByEnableLabel(baseFilter Filter) Filter {
+	return func(c FilterableContainer) bool {
+		// If label filtering is enabled, containers should only be considered
+		// if the label is specifically set.
+		_, ok := c.Enabled()
+		if !ok {
+			return false
+		}
+
+		return baseFilter(c)
+	}
+}
+
+// Filters out containers that have a 'enableLabel' and is set to disable.
+func filterByDisabledLabel(baseFilter Filter) Filter {
+	return func(c FilterableContainer) bool {
+		enabledLabel, ok := c.Enabled()
+		if ok && !enabledLabel {
+			// If the label has been set and it demands a disable
+			return false
+		}
+
+		return baseFilter(c)
+	}
+}
+
+// BuildFilter creates the needed filter of containers
+func BuildFilter(names []string, enableLabel bool) Filter {
+	filter := noFilter
+	filter = filterByNames(names, filter)
+	if enableLabel {
+		// If label filtering is enabled, containers should only be considered
+		// if the label is specifically set.
+		filter = filterByEnableLabel(filter)
+	}
+	filter = filterByDisabledLabel(filter)
+	return filter
+}

container/filters_test.go

@@ -0,0 +1,153 @@
+package container
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/v2tec/watchtower/container/mocks"
+)
+
+func TestWatchtowerContainersFilter(t *testing.T) {
+	container := new(mocks.FilterableContainer)
+
+	container.On("IsWatchtower").Return(true)
+
+	assert.True(t, WatchtowerContainersFilter(container))
+
+	container.AssertExpectations(t)
+}
+
+func TestNoFilter(t *testing.T) {
+	container := new(mocks.FilterableContainer)
+
+	assert.True(t, noFilter(container))
+
+	container.AssertExpectations(t)
+}
+
+func TestFilterByNames(t *testing.T) {
+	var names []string
+
+	filter := filterByNames(names, nil)
+	assert.Nil(t, filter)
+
+	names = append(names, "test")
+
+	filter = filterByNames(names, noFilter)
+	assert.NotNil(t, filter)
+
+	container := new(mocks.FilterableContainer)
+	container.On("Name").Return("test")
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Name").Return("NoTest")
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+}
+
+func TestFilterByEnableLabel(t *testing.T) {
+	filter := filterByEnableLabel(noFilter)
+	assert.NotNil(t, filter)
+
+	container := new(mocks.FilterableContainer)
+	container.On("Enabled").Return(true, true)
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Enabled").Return(false, true)
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Enabled").Return(false, false)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+}
+
+func TestFilterByDisabledLabel(t *testing.T) {
+	filter := filterByDisabledLabel(noFilter)
+	assert.NotNil(t, filter)
+
+	container := new(mocks.FilterableContainer)
+	container.On("Enabled").Return(true, true)
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Enabled").Return(false, true)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Enabled").Return(false, false)
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+}
+
+func TestBuildFilter(t *testing.T) {
+	var names []string
+	names = append(names, "test")
+
+	filter := BuildFilter(names, false)
+
+	container := new(mocks.FilterableContainer)
+	container.On("Name").Return("Invalid")
+	container.On("Enabled").Return(false, false)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Name").Return("test")
+	container.On("Enabled").Return(false, false)
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Name").Return("Invalid")
+	container.On("Enabled").Return(true, true)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Name").Return("test")
+	container.On("Enabled").Return(true, true)
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Enabled").Return(false, true)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+}
+
+func TestBuildFilterEnableLabel(t *testing.T) {
+	var names []string
+	names = append(names, "test")
+
+	filter := BuildFilter(names, true)
+
+	container := new(mocks.FilterableContainer)
+	container.On("Enabled").Return(false, false)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Name").Return("Invalid")
+	container.On("Enabled").Twice().Return(true, true)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Name").Return("test")
+	container.On("Enabled").Twice().Return(true, true)
+	assert.True(t, filter(container))
+	container.AssertExpectations(t)
+
+	container = new(mocks.FilterableContainer)
+	container.On("Enabled").Return(false, true)
+	assert.False(t, filter(container))
+	container.AssertExpectations(t)
+}

container/mockclient/mock.go

@@ -1,42 +0,0 @@
-package mockclient
-
-import (
-	"time"
-
-	"github.com/stretchr/testify/mock"
-	"github.com/v2tec/watchtower/container"
-)
-
-type MockClient struct {
-	mock.Mock
-}
-
-func (m *MockClient) ListContainers(cf container.Filter) ([]container.Container, error) {
-	args := m.Called(cf)
-	return args.Get(0).([]container.Container), args.Error(1)
-}
-
-func (m *MockClient) StopContainer(c container.Container, timeout time.Duration) error {
-	args := m.Called(c, timeout)
-	return args.Error(0)
-}
-
-func (m *MockClient) StartContainer(c container.Container) error {
-	args := m.Called(c)
-	return args.Error(0)
-}
-
-func (m *MockClient) RenameContainer(c container.Container, name string) error {
-	args := m.Called(c, name)
-	return args.Error(0)
-}
-
-func (m *MockClient) IsContainerStale(c container.Container) (bool, error) {
-	args := m.Called(c)
-	return args.Bool(0), args.Error(1)
-}
-
-func (m *MockClient) RemoveImage(c container.Container) error {
-	args := m.Called(c)
-	return args.Error(0)
-}

container/mockclient/mock_test.go

@@ -1,17 +0,0 @@
-package mockclient
-
-import (
-	"reflect"
-	"testing"
-
-	"github.com/v2tec/watchtower/container"
-)
-
-func TestMockInterface(t *testing.T) {
-	iface := reflect.TypeOf((*container.Client)(nil)).Elem()
-	mock := &MockClient{}
-
-	if !reflect.TypeOf(mock).Implements(iface) {
-		t.Fatalf("Mock does not implement the Client interface")
-	}
-}

container/mocks/FilterableContainer.go

@@ -0,0 +1,57 @@
+package mocks
+
+import mock "github.com/stretchr/testify/mock"
+
+// FilterableContainer is an autogenerated mock type for the FilterableContainer type
+type FilterableContainer struct {
+	mock.Mock
+}
+
+// Enabled provides a mock function with given fields:
+func (_m *FilterableContainer) Enabled() (bool, bool) {
+	ret := _m.Called()
+
+	var r0 bool
+	if rf, ok := ret.Get(0).(func() bool); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	var r1 bool
+	if rf, ok := ret.Get(1).(func() bool); ok {
+		r1 = rf()
+	} else {
+		r1 = ret.Get(1).(bool)
+	}
+
+	return r0, r1
+}
+
+// IsWatchtower provides a mock function with given fields:
+func (_m *FilterableContainer) IsWatchtower() bool {
+	ret := _m.Called()
+
+	var r0 bool
+	if rf, ok := ret.Get(0).(func() bool); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Get(0).(bool)
+	}
+
+	return r0
+}
+
+// Name provides a mock function with given fields:
+func (_m *FilterableContainer) Name() string {
+	ret := _m.Called()
+
+	var r0 string
+	if rf, ok := ret.Get(0).(func() string); ok {
+		r0 = rf()
+	} else {
+		r0 = ret.Get(0).(string)
+	}
+
+	return r0
+}

container/trust.go

@@ -5,20 +5,18 @@ import (
 	"os"
 	"strings"
 
-	log "github.com/sirupsen/logrus"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/reference"
 	"github.com/docker/docker/cli/command"
 	"github.com/docker/docker/cliconfig"
 	"github.com/docker/docker/cliconfig/configfile"
 	"github.com/docker/docker/cliconfig/credentials"
+	log "github.com/sirupsen/logrus"
 )
 
-/**
- * Return an encoded auth config for the given registry
- * loaded from environment variables or docker config
- * as available in that order
- */
+// EncodedAuth returns an encoded auth config for the given registry
+// loaded from environment variables or docker config
+// as available in that order
 func EncodedAuth(ref string) (string, error) {
 	auth, err := EncodedEnvAuth(ref)
 	if err != nil {
@@ -27,11 +25,9 @@ func EncodedAuth(ref string) (string, error) {
 	return auth, err
 }
 
-/*
- * Return an encoded auth config for the given registry
- * loaded from environment variables
- * Returns an error if authentication environment variables have not been set
- */
+// EncodedEnvAuth returns an encoded auth config for the given registry
+// loaded from environment variables
+// Returns an error if authentication environment variables have not been set
 func EncodedEnvAuth(ref string) (string, error) {
 	username := os.Getenv("REPO_USER")
 	password := os.Getenv("REPO_PASS")
@@ -42,17 +38,14 @@ func EncodedEnvAuth(ref string) (string, error) {
 		}
 		log.Debugf("Loaded auth credentials %s for %s", auth, ref)
 		return EncodeAuth(auth)
-	} else {
-		return "", errors.New("Registry auth environment variables (REPO_USER, REPO_PASS) not set")
 	}
+	return "", errors.New("Registry auth environment variables (REPO_USER, REPO_PASS) not set")
 }
 
-/*
- * Return an encoded auth config for the given registry
- * loaded from the docker config
- * Returns an empty string if credentials cannot be found for the referenced server
- * The docker config must be mounted on the container
- */
+// EncodedConfigAuth returns an encoded auth config for the given registry
+// loaded from the docker config
+// Returns an empty string if credentials cannot be found for the referenced server
+// The docker config must be mounted on the container
 func EncodedConfigAuth(ref string) (string, error) {
 	server, err := ParseServerAddress(ref)
 	configDir := os.Getenv("DOCKER_CONFIG")
@@ -92,18 +85,14 @@ func CredentialsStore(configFile configfile.ConfigFile) credentials.Store {
 	return credentials.NewFileStore(&configFile)
 }
 
-/*
- * Base64 encode an AuthConfig struct for transmission over HTTP
- */
+// EncodeAuth Base64 encode an AuthConfig struct for transmission over HTTP
 func EncodeAuth(auth types.AuthConfig) (string, error) {
 	return command.EncodeAuthToBase64(auth)
 }
 
-/**
- * This function will be invoked if an AuthConfig is rejected
- * It could be used to return a new value for the "X-Registry-Auth" authentication header,
- * but there's no point trying again with the same value as used in AuthConfig
- */
+// DefaultAuthHandler will be invoked if an AuthConfig is rejected
+// It could be used to return a new value for the "X-Registry-Auth" authentication header,
+// but there's no point trying again with the same value as used in AuthConfig
 func DefaultAuthHandler() (string, error) {
 	log.Debug("Authentication request was rejected. Trying again without authentication")
 	return "", nil

main.go

@@ -29,6 +29,7 @@ var (
 	scheduleSpec string
 	cleanup      bool
 	noRestart    bool
+	enableLabel  bool
 	notifier     *notifications.Notifier
 	timeout		 time.Duration
 )
@@ -189,6 +190,7 @@ func before(c *cli.Context) error {
 	if timeout < 0 {
 		log.Fatal("Please specify a positive value for timeout value.")
 	}
+	enableLabel = c.GlobalBool("label-enable")
 
 	// configure environment vars for client
 	err := envConfig(c)
@@ -196,7 +198,7 @@ func before(c *cli.Context) error {
 		return err
 	}
 
-	client = container.NewClient(!c.GlobalBool("no-pull"), c.GlobalBool("label-enable"))
+	client = container.NewClient(!c.GlobalBool("no-pull"))
 	notifier = notifications.NewNotifier(c)
 
 	return nil
@@ -209,6 +211,8 @@ func start(c *cli.Context) error {
 		log.Fatal(err)
 	}
 
+	filter := container.BuildFilter(names, enableLabel)
+
 	tryLockSem := make(chan bool, 1)
 	tryLockSem <- true
 
@@ -220,7 +224,7 @@ func start(c *cli.Context) error {
 			case v := <-tryLockSem:
 				defer func() { tryLockSem <- v }()
 				notifier.StartNotification()
-				if err := actions.Update(client, names, cleanup, noRestart, timeout); err != nil {
+				if err := actions.Update(client, filter, cleanup, noRestart, timeout); err != nil {
 					log.Println(err)
 				}
 				notifier.SendNotification()