Merge "Check config when checking the containers"

3 files changed, 149 insertions, 1 deletions

diff --git a/ansible/library/kolla_docker.py b/ansible/library/kolla_docker.py
index 22bd78b55..f5903ac53 100644
--- a/ansible/library/kolla_docker.py
+++ b/ansible/library/kolla_docker.py
@@ -264,6 +264,9 @@ EXAMPLES = '''
 '''
 
 
+COMPARE_CONFIG_CMD = ['/usr/local/bin/kolla_set_configs', '--check']
+
+
 def get_docker_client():
     return docker.APIClient
 
@@ -348,7 +351,9 @@ class DockerWorker(object):
 
     def compare_container(self):
         container = self.check_container()
-        if not container or self.check_container_differs():
+        if (not container or
+                self.check_container_differs() or
+                self.compare_config()):
             self.changed = True
         return self.changed
 
@@ -614,6 +619,43 @@ class DockerWorker(object):
             if current_healthcheck:
                 return True
 
+    def compare_config(self):
+        try:
+            job = self.dc.exec_create(
+                self.params['name'],
+                COMPARE_CONFIG_CMD,
+                user='root',
+            )
+            output = self.dc.exec_start(job)
+            exec_inspect = self.dc.exec_inspect(job)
+        except docker.errors.APIError as e:
+            # NOTE(yoctozepto): If we have a client error, then the container
+            # cannot be used for config check (e.g., is restarting, or stopped
+            # in the mean time) - assume config is stale = return True.
+            # Else, propagate the server error back.
+            if e.is_client_error():
+                return True
+            else:
+                raise
+        # Exit codes:
+        # 0: not changed
+        # 1: changed
+        # 137: abrupt exit -> changed
+        # else: error
+        if exec_inspect['ExitCode'] == 0:
+            return False
+        elif exec_inspect['ExitCode'] == 1:
+            return True
+        elif exec_inspect['ExitCode'] == 137:
+            # NOTE(yoctozepto): This is Docker's command exit due to container
+            # exit. It means the container is unstable so we are better off
+            # marking it as requiring a restart due to config update.
+            return True
+        else:
+            raise Exception('Failed to compare container configuration: '
+                            'ExitCode: %s Message: %s' %
+                            (exec_inspect['ExitCode'], output))
+
     def parse_image(self):
         full_image = self.params.get('image')
 
diff --git a/releasenotes/notes/bug-1848775-b0625b7586adac96.yaml b/releasenotes/notes/bug-1848775-b0625b7586adac96.yaml
new file mode 100644
index 000000000..16c9c41d7
--- /dev/null
+++ b/releasenotes/notes/bug-1848775-b0625b7586adac96.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixes an issue where configuration in containers could become stale.
+    This prevented containers with updated configuration from being
+    restarted, e.g., if the ``kolla-ansible genconfig`` and
+    ``kolla-ansible deploy-containers`` commands were used together.
+    `LP#1848775 <https://launchpad.net/bugs/1848775>`__
diff --git a/tests/test_kolla_docker.py b/tests/test_kolla_docker.py
index e681ff3d4..8443451f7 100644
--- a/tests/test_kolla_docker.py
+++ b/tests/test_kolla_docker.py
@@ -721,6 +721,104 @@ class TestImage(base.BaseTestCase):
         self.dw.dc.images.assert_called_once_with()
         self.assertTrue(return_data)
 
+    def test_compare_config_unchanged(self):
+        self.dw = get_DockerWorker(FAKE_DATA['params'])
+        job = mock.MagicMock()
+        self.dw.dc.exec_create.return_value = job
+        self.dw.dc.exec_start.return_value = 'fake output'
+        self.dw.dc.exec_inspect.return_value = {'ExitCode': 0}
+        return_data = self.dw.compare_config()
+        self.dw.dc.exec_create.assert_called_once_with(
+            FAKE_DATA['params']['name'],
+            kd.COMPARE_CONFIG_CMD,
+            user='root')
+        self.dw.dc.exec_start.assert_called_once_with(job)
+        self.dw.dc.exec_inspect.assert_called_once_with(job)
+        self.assertFalse(return_data)
+
+    def test_compare_config_changed(self):
+        self.dw = get_DockerWorker(FAKE_DATA['params'])
+        job = mock.MagicMock()
+        self.dw.dc.exec_create.return_value = job
+        self.dw.dc.exec_start.return_value = 'fake output'
+        self.dw.dc.exec_inspect.return_value = {'ExitCode': 1}
+        return_data = self.dw.compare_config()
+        self.dw.dc.exec_create.assert_called_once_with(
+            FAKE_DATA['params']['name'],
+            kd.COMPARE_CONFIG_CMD,
+            user='root')
+        self.dw.dc.exec_start.assert_called_once_with(job)
+        self.dw.dc.exec_inspect.assert_called_once_with(job)
+        self.assertTrue(return_data)
+
+    def test_compare_config_changed_container_exited(self):
+        self.dw = get_DockerWorker(FAKE_DATA['params'])
+        job = mock.MagicMock()
+        self.dw.dc.exec_create.return_value = job
+        self.dw.dc.exec_start.return_value = 'fake output'
+        self.dw.dc.exec_inspect.return_value = {'ExitCode': 137}
+        return_data = self.dw.compare_config()
+        self.dw.dc.exec_create.assert_called_once_with(
+            FAKE_DATA['params']['name'],
+            kd.COMPARE_CONFIG_CMD,
+            user='root')
+        self.dw.dc.exec_start.assert_called_once_with(job)
+        self.dw.dc.exec_inspect.assert_called_once_with(job)
+        self.assertTrue(return_data)
+
+    def test_compare_config_changed_client_failure(self):
+        self.dw = get_DockerWorker(FAKE_DATA['params'])
+        job = mock.MagicMock()
+        self.dw.dc.exec_create.return_value = job
+        self.dw.dc.exec_start.return_value = 'fake output'
+        failure_response = mock.MagicMock()
+        failure_response.status_code = 409  # any client error should do here
+        self.dw.dc.exec_inspect.side_effect = docker_error.APIError(
+            message="foo",
+            response=failure_response,
+        )
+        return_data = self.dw.compare_config()
+        self.dw.dc.exec_create.assert_called_once_with(
+            FAKE_DATA['params']['name'],
+            kd.COMPARE_CONFIG_CMD,
+            user='root')
+        self.dw.dc.exec_start.assert_called_once_with(job)
+        self.dw.dc.exec_inspect.assert_called_once_with(job)
+        self.assertTrue(return_data)
+
+    def test_compare_config_error(self):
+        self.dw = get_DockerWorker(FAKE_DATA['params'])
+        job = mock.MagicMock()
+        self.dw.dc.exec_create.return_value = job
+        self.dw.dc.exec_start.return_value = 'fake output'
+        self.dw.dc.exec_inspect.return_value = {'ExitCode': -1}
+        self.assertRaises(Exception, self.dw.compare_config)  # noqa: H202
+        self.dw.dc.exec_create.assert_called_once_with(
+            FAKE_DATA['params']['name'],
+            kd.COMPARE_CONFIG_CMD,
+            user='root')
+        self.dw.dc.exec_start.assert_called_once_with(job)
+        self.dw.dc.exec_inspect.assert_called_once_with(job)
+
+    def test_compare_config_error_server_failure(self):
+        self.dw = get_DockerWorker(FAKE_DATA['params'])
+        job = mock.MagicMock()
+        self.dw.dc.exec_create.return_value = job
+        self.dw.dc.exec_start.return_value = 'fake output'
+        failure_response = mock.MagicMock()
+        failure_response.status_code = 500  # any server error should do here
+        self.dw.dc.exec_inspect.side_effect = docker_error.APIError(
+            message="foo",
+            response=failure_response,
+        )
+        self.assertRaises(docker_error.APIError, self.dw.compare_config)
+        self.dw.dc.exec_create.assert_called_once_with(
+            FAKE_DATA['params']['name'],
+            kd.COMPARE_CONFIG_CMD,
+            user='root')
+        self.dw.dc.exec_start.assert_called_once_with(job)
+        self.dw.dc.exec_inspect.assert_called_once_with(job)
+
     def test_get_image_id_not_exists(self):
         self.dw = get_DockerWorker(
             {'image': 'myregistrydomain.com:5000/ubuntu:16.04'})