feat: check container config before update (#925)

* feat: check container config before restart
* fix: only skip when hostconfig and config differ
* fix: update test mocks to not fail tests
* test: add verify config tests

cmd/root.go

@@ -269,6 +269,9 @@ func writeStartupMessage(c *cobra.Command, sched time.Time, filtering string) {
 		}
 
 		log.Info("Watchtower ", version, "\n", notifs, "\n", filtering, "\n", schedMessage)
+		if log.IsLevelEnabled(log.TraceLevel) {
+			log.Warn("trace level enabled: log will include sensitive information as credentials and tokens")
+		}
 	}
 }
 
@@ -330,8 +333,10 @@ func runUpdatesWithNotifications(filter t.Filter) *metrics.Metric {
 	}
 	metricResults, err := actions.Update(client, updateParams)
 	if err != nil {
-		log.Println(err)
+		log.Error(err)
 	}
 	notifier.SendNotification()
+	log.Debugf("Session done: %v scanned, %v updated, %v failed",
+		metricResults.Scanned, metricResults.Updated, metricResults.Failed)
 	return metricResults
 }

internal/actions/mocks/container.go

@@ -16,10 +16,13 @@ func CreateMockContainer(id string, name string, image string, created time.Time
 			Image:   image,
 			Name:    name,
 			Created: created.String(),
+			HostConfig: &container2.HostConfig{
+				PortBindings: map[nat.Port][]nat.PortBinding{},
+			},
 		},
 		Config: &container2.Config{
-			Image:  image,
-			Labels: make(map[string]string),
+			Image:        image,
+			Labels:       make(map[string]string),
 			ExposedPorts: map[nat.Port]struct{}{},
 		},
 	}

internal/actions/update.go

@@ -1,7 +1,6 @@
 package actions
 
 import (
-	"errors"
 	"github.com/containrrr/watchtower/internal/util"
 	"github.com/containrrr/watchtower/pkg/container"
 	"github.com/containrrr/watchtower/pkg/lifecycle"
@@ -33,11 +32,23 @@ func Update(client container.Client, params types.UpdateParams) (*metrics2.Metri
 
 	for i, targetContainer := range containers {
 		stale, err := client.IsContainerStale(targetContainer)
-		if stale && !params.NoRestart && !params.MonitorOnly && !targetContainer.IsMonitorOnly() && !targetContainer.HasImageInfo() {
-			err = errors.New("no available image info")
+		shouldUpdate := stale && !params.NoRestart && !params.MonitorOnly && !targetContainer.IsMonitorOnly()
+		if err == nil && shouldUpdate {
+			// Check to make sure we have all the necessary information for recreating the container
+			err = targetContainer.VerifyConfiguration()
+			// If the image information is incomplete and trace logging is enabled, log it for further diagnosis
+			if err != nil && log.IsLevelEnabled(log.TraceLevel) {
+				imageInfo := targetContainer.ImageInfo()
+				log.Tracef("Image info: %#v", imageInfo)
+				log.Tracef("Container info: %#v", targetContainer.ContainerInfo())
+				if imageInfo != nil {
+					log.Tracef("Image config: %#v", imageInfo.Config)
+				}
+			}
 		}
+
 		if err != nil {
-			log.Infof("Unable to update container %q: %v. Proceeding to next.", containers[i].Name(), err)
+			log.Infof("Unable to update container %q: %v. Proceeding to next.", targetContainer.Name(), err)
 			stale = false
 			staleCheckFailed++
 			metric.Failed++

pkg/container/container.go

@@ -258,3 +258,32 @@ func (c Container) HasImageInfo() bool {
 func (c Container) ImageInfo() *types.ImageInspect {
 	return c.imageInfo
 }
+
+// VerifyConfiguration checks the container and image configurations for nil references to make sure
+// that the container can be recreated once deleted
+func (c Container) VerifyConfiguration() error {
+	if c.imageInfo == nil {
+		return errorNoImageInfo
+	}
+
+	containerInfo := c.ContainerInfo()
+	if containerInfo == nil {
+		return errorInvalidConfig
+	}
+
+	containerConfig := containerInfo.Config
+	if containerConfig == nil {
+		return errorInvalidConfig
+	}
+
+	hostConfig := containerInfo.HostConfig
+	if hostConfig == nil {
+		return errorInvalidConfig
+	}
+
+	if len(hostConfig.PortBindings) > 0 && containerConfig.ExposedPorts == nil {
+		return errorNoExposedPorts
+	}
+
+	return nil
+}

pkg/container/container_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	cli "github.com/docker/docker/client"
+	"github.com/docker/go-connections/nat"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -32,14 +33,14 @@ var _ = Describe("the container", func() {
 			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")
+				c := newClientNoAPI(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")
+				c := newClientNoAPI(false, false, false, false, false, "auto")
 				It("should always return true", func() {
 					Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
 				})
@@ -48,7 +49,7 @@ var _ = Describe("the container", func() {
 				})
 			})
 			When("warn on head failure is set to \"never\"", func() {
-				c := NewClient(false, false, false, false, false, "never")
+				c := newClientNoAPI(false, false, false, false, false, "never")
 				It("should never return true", func() {
 					Expect(c.WarnOnHeadPullFailed(containerUnknown)).To(BeFalse())
 					Expect(c.WarnOnHeadPullFailed(containerKnown)).To(BeFalse())
@@ -130,6 +131,63 @@ var _ = Describe("the container", func() {
 			})
 		})
 	})
+	Describe("VerifyConfiguration", func() {
+		When("verifying a container with no image info", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.imageInfo = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorNoImageInfo))
+			})
+		})
+		When("verifying a container with no container info", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.containerInfo = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorInvalidConfig))
+			})
+		})
+		When("verifying a container with no config", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.containerInfo.Config = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorInvalidConfig))
+			})
+		})
+		When("verifying a container with no host config", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings()
+				c.containerInfo.HostConfig = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorInvalidConfig))
+			})
+		})
+		When("verifying a container with no port bindings", func() {
+			It("should not return an error", func() {
+				c := mockContainerWithPortBindings()
+				err := c.VerifyConfiguration()
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
+		When("verifying a container with port bindings, but no exposed ports", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings("80/tcp")
+				c.containerInfo.Config.ExposedPorts = nil
+				err := c.VerifyConfiguration()
+				Expect(err).To(Equal(errorNoExposedPorts))
+			})
+		})
+		When("verifying a container with port bindings and exposed ports is non-nil", func() {
+			It("should return an error", func() {
+				c := mockContainerWithPortBindings("80/tcp")
+				c.containerInfo.Config.ExposedPorts = map[nat.Port]struct{}{"80/tcp": {}}
+				err := c.VerifyConfiguration()
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
+	})
 	When("asked for metadata", func() {
 		var c *Container
 		BeforeEach(func() {
@@ -281,10 +339,23 @@ var _ = Describe("the container", func() {
 	})
 })
 
+func mockContainerWithPortBindings(portBindingSources ...string) *Container {
+	mockContainer := mockContainerWithLabels(nil)
+	mockContainer.imageInfo = &types.ImageInspect{}
+	hostConfig := &container.HostConfig{
+		PortBindings: nat.PortMap{},
+	}
+	for _, pbs := range portBindingSources {
+		hostConfig.PortBindings[nat.Port(pbs)] = []nat.PortBinding{}
+	}
+	mockContainer.containerInfo.HostConfig = hostConfig
+	return mockContainer
+}
+
 func mockContainerWithImageName(name string) *Container {
-	container := mockContainerWithLabels(nil)
-	container.containerInfo.Config.Image = name
-	return container
+	mockContainer := mockContainerWithLabels(nil)
+	mockContainer.containerInfo.Config.Image = name
+	return mockContainer
 }
 
 func mockContainerWithLinks(links []string) *Container {
@@ -317,3 +388,15 @@ func mockContainerWithLabels(labels map[string]string) *Container {
 	}
 	return NewContainer(&content, nil)
 }
+
+func newClientNoAPI(pullImages, includeStopped, reviveStopped, removeVolumes, includeRestarting bool, warnOnHeadFailed string) Client {
+	return dockerClient{
+		api:               nil,
+		pullImages:        pullImages,
+		removeVolumes:     removeVolumes,
+		includeStopped:    includeStopped,
+		reviveStopped:     reviveStopped,
+		includeRestarting: includeRestarting,
+		warnOnHeadFailed:  warnOnHeadFailed,
+	}
+}

pkg/container/errors.go

@@ -0,0 +1,7 @@
+package container
+
+import "errors"
+
+var errorNoImageInfo = errors.New("no available image info")
+var errorNoExposedPorts = errors.New("exposed ports does not match port bindings")
+var errorInvalidConfig = errors.New("container configuration missing or invalid")