Check config when checking the containers

The proposed approach allows for checking whether config
files are current, e.g. cases when the deployment was aborted after
config files were generated but before they were injected into the
containers which lead to old config staying in containers.

After this patch we can do:
  kolla-ansible genconfig
  kolla-ansible deploy-containers
and it would do what we expected rather than being a noop
in the second part.

We also lose the need to have notifies
and whens in config and handler sections respectively.
This is optimised in a separate patch.

Future work:
- optimise for large files
  - could we get away with comparing timestamps and sizes?
    container's should have a newer timestamp due to copy,
    could also preserve it

Change-Id: I1d26e48e1958f13b854d8afded4bfba5021a2dec
Closes-Bug: #1848775
Depends-On: https://review.opendev.org/c/openstack/kolla/+/773257
Co-Authored-By: Mark Goddard <mark@stackhpc.com>
(cherry picked from commit c3afbd3c5e)
This commit is contained in:
Radosław Piliszek 2020-07-31 17:54:52 +02:00
parent b6d8eefb8e
commit 767f1e4b23
3 changed files with 149 additions and 1 deletions

View File

@ -264,6 +264,9 @@ EXAMPLES = '''
''' '''
COMPARE_CONFIG_CMD = ['/usr/local/bin/kolla_set_configs', '--check']
def get_docker_client(): def get_docker_client():
return docker.APIClient return docker.APIClient
@ -348,7 +351,9 @@ class DockerWorker(object):
def compare_container(self): def compare_container(self):
container = self.check_container() 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 self.changed = True
return self.changed return self.changed
@ -614,6 +619,43 @@ class DockerWorker(object):
if current_healthcheck: if current_healthcheck:
return True 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): def parse_image(self):
full_image = self.params.get('image') full_image = self.params.get('image')

View File

@ -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>`__

View File

@ -721,6 +721,104 @@ class TestImage(base.BaseTestCase):
self.dw.dc.images.assert_called_once_with() self.dw.dc.images.assert_called_once_with()
self.assertTrue(return_data) 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): def test_get_image_id_not_exists(self):
self.dw = get_DockerWorker( self.dw = get_DockerWorker(
{'image': 'myregistrydomain.com:5000/ubuntu:16.04'}) {'image': 'myregistrydomain.com:5000/ubuntu:16.04'})