diff --git a/ansible/library/kolla_docker.py b/ansible/library/kolla_docker.py index c76392eda6..57313007a2 100644 --- a/ansible/library/kolla_docker.py +++ b/ansible/library/kolla_docker.py @@ -252,6 +252,9 @@ EXAMPLES = ''' ''' +COMPARE_CONFIG_CMD = ['/usr/local/bin/kolla_set_configs', '--check'] + + def get_docker_client(): return docker.APIClient @@ -333,7 +336,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 @@ -583,6 +588,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 0000000000..16c9c41d7b --- /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 `__ diff --git a/tests/test_kolla_docker.py b/tests/test_kolla_docker.py index 29db3111f4..aab0f21533 100644 --- a/tests/test_kolla_docker.py +++ b/tests/test_kolla_docker.py @@ -709,6 +709,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'})