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

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")