Attempt to pull image before stopping and removing container

* Deploy services using kolla-ansible deploy
* Reconfigure the image for one or more services to use an invalid
* config
* Deploy/reconfigure services using kolla-ansible reconfigure

The invalid config could be a wrong docker registry, wrong image name,
wrong tag, etc.

The restart handler for the service fails, and the old container is
left running.

The restart handler for the service fails, and the old container is
stopped and removed. This leaves the service in a broken state.

This change fixes the issue by pulling the image if necessary prior to
stopping and removing the container.

Change-Id: I85b2a1b224d4c4d85c32c4922a2cd2c41171a1dc
Closes-Bug: #1852572

ansible/library/kolla_docker.py

@@ -742,6 +742,11 @@ class DockerWorker(object):
         # If config_strategy is COPY_ONCE or container's parameters are
         # changed, try to start a new one.
         if config_strategy == 'COPY_ONCE' or self.check_container_differs():
+            # NOTE(mgoddard): Pull the image if necessary before stopping the
+            # container, otherwise a failure to pull the image will leave the
+            # container stopped.
+            if not self.check_image():
+                self.pull_image()
             self.stop_container()
             self.remove_container()
             self.start_container()

releasenotes/notes/fix-recreate-missing-pull-dba93327fd4c94c3.yaml

+---
+fixes:
+  - |
+    Fixes an issue where a failure in pulling an image could lead to a
+    container being removed and not replaced. See `bug 1852572
+    <https://bugs.launchpad.net/kolla-ansible/+bug/1852572>`__ for details.

tests/test_kolla_docker.py

@@ -545,12 +545,15 @@ class TestContainer(base.BaseTestCase):
             'environment': dict(KOLLA_CONFIG_STRATEGY='COPY_ALWAYS')})
         self.dw.check_container = mock.Mock(
             return_value=self.fake_data['containers'][0])
+        self.dw.check_image = mock.Mock(
+            return_value=self.fake_data['images'][0])
         self.dw.start_container = mock.Mock()
         self.dw.remove_container = mock.Mock()
         self.dw.check_container_differs = mock.Mock(return_value=True)
 
         self.dw.recreate_or_restart_container()
 
+        self.dw.check_image.assert_called_once_with()
         self.dw.remove_container.assert_called_once_with()
         self.dw.start_container.assert_called_once_with()
 
@@ -559,11 +562,32 @@ class TestContainer(base.BaseTestCase):
             'environment': dict(KOLLA_CONFIG_STRATEGY='COPY_ONCE')})
         self.dw.check_container = mock.Mock(
             return_value=self.fake_data['containers'][0])
+        self.dw.check_image = mock.Mock(
+            return_value=self.fake_data['images'][0])
         self.dw.start_container = mock.Mock()
         self.dw.remove_container = mock.Mock()
 
         self.dw.recreate_or_restart_container()
 
+        self.dw.check_image.assert_called_once_with()
+        self.dw.remove_container.assert_called_once_with()
+        self.dw.start_container.assert_called_once_with()
+
+    def test_recreate_or_restart_container_pull_before_stop(self):
+        # Testing fix for https://launchpad.net/bugs/1852572.
+        self.dw = get_DockerWorker({
+            'environment': dict(KOLLA_CONFIG_STRATEGY='COPY_ONCE')})
+        self.dw.check_container = mock.Mock(
+            return_value=self.fake_data['containers'][0])
+        self.dw.check_image = mock.Mock(return_value=None)
+        self.dw.pull_image = mock.Mock()
+        self.dw.start_container = mock.Mock()
+        self.dw.remove_container = mock.Mock()
+
+        self.dw.recreate_or_restart_container()
+
+        self.dw.check_image.assert_called_once_with()
+        self.dw.pull_image.assert_called_once_with()
         self.dw.remove_container.assert_called_once_with()
         self.dw.start_container.assert_called_once_with()