Skip to content

Commit 540a363

Browse files
Zuulopenstack-gerrit
authored andcommitted
Merge "Check config when checking the containers" into stable/victoria
2 parents d7d7ede + 7cc41b3 commit 540a363

File tree

3 files changed

+149
-1
lines changed

3 files changed

+149
-1
lines changed

ansible/library/kolla_docker.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,9 @@
247247
'''
248248

249249

250+
COMPARE_CONFIG_CMD = ['/usr/local/bin/kolla_set_configs', '--check']
251+
252+
250253
def get_docker_client():
251254
return docker.APIClient
252255

@@ -328,7 +331,9 @@ def get_container_info(self):
328331

329332
def compare_container(self):
330333
container = self.check_container()
331-
if not container or self.check_container_differs():
334+
if (not container or
335+
self.check_container_differs() or
336+
self.compare_config()):
332337
self.changed = True
333338
return self.changed
334339

@@ -566,6 +571,43 @@ def compare_healthcheck(self, container_info):
566571
if current_healthcheck:
567572
return True
568573

574+
def compare_config(self):
575+
try:
576+
job = self.dc.exec_create(
577+
self.params['name'],
578+
COMPARE_CONFIG_CMD,
579+
user='root',
580+
)
581+
output = self.dc.exec_start(job)
582+
exec_inspect = self.dc.exec_inspect(job)
583+
except docker.errors.APIError as e:
584+
# NOTE(yoctozepto): If we have a client error, then the container
585+
# cannot be used for config check (e.g., is restarting, or stopped
586+
# in the mean time) - assume config is stale = return True.
587+
# Else, propagate the server error back.
588+
if e.is_client_error():
589+
return True
590+
else:
591+
raise
592+
# Exit codes:
593+
# 0: not changed
594+
# 1: changed
595+
# 137: abrupt exit -> changed
596+
# else: error
597+
if exec_inspect['ExitCode'] == 0:
598+
return False
599+
elif exec_inspect['ExitCode'] == 1:
600+
return True
601+
elif exec_inspect['ExitCode'] == 137:
602+
# NOTE(yoctozepto): This is Docker's command exit due to container
603+
# exit. It means the container is unstable so we are better off
604+
# marking it as requiring a restart due to config update.
605+
return True
606+
else:
607+
raise Exception('Failed to compare container configuration: '
608+
'ExitCode: %s Message: %s' %
609+
(exec_inspect['ExitCode'], output))
610+
569611
def parse_image(self):
570612
full_image = self.params.get('image')
571613

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
fixes:
3+
- |
4+
Fixes an issue where configuration in containers could become stale.
5+
This prevented containers with updated configuration from being
6+
restarted, e.g., if the ``kolla-ansible genconfig`` and
7+
``kolla-ansible deploy-containers`` commands were used together.
8+
`LP#1848775 <https://launchpad.net/bugs/1848775>`__

tests/test_kolla_docker.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,104 @@ def test_compare_image(self):
688688
self.dw.dc.images.assert_called_once_with()
689689
self.assertTrue(return_data)
690690

691+
def test_compare_config_unchanged(self):
692+
self.dw = get_DockerWorker(FAKE_DATA['params'])
693+
job = mock.MagicMock()
694+
self.dw.dc.exec_create.return_value = job
695+
self.dw.dc.exec_start.return_value = 'fake output'
696+
self.dw.dc.exec_inspect.return_value = {'ExitCode': 0}
697+
return_data = self.dw.compare_config()
698+
self.dw.dc.exec_create.assert_called_once_with(
699+
FAKE_DATA['params']['name'],
700+
kd.COMPARE_CONFIG_CMD,
701+
user='root')
702+
self.dw.dc.exec_start.assert_called_once_with(job)
703+
self.dw.dc.exec_inspect.assert_called_once_with(job)
704+
self.assertFalse(return_data)
705+
706+
def test_compare_config_changed(self):
707+
self.dw = get_DockerWorker(FAKE_DATA['params'])
708+
job = mock.MagicMock()
709+
self.dw.dc.exec_create.return_value = job
710+
self.dw.dc.exec_start.return_value = 'fake output'
711+
self.dw.dc.exec_inspect.return_value = {'ExitCode': 1}
712+
return_data = self.dw.compare_config()
713+
self.dw.dc.exec_create.assert_called_once_with(
714+
FAKE_DATA['params']['name'],
715+
kd.COMPARE_CONFIG_CMD,
716+
user='root')
717+
self.dw.dc.exec_start.assert_called_once_with(job)
718+
self.dw.dc.exec_inspect.assert_called_once_with(job)
719+
self.assertTrue(return_data)
720+
721+
def test_compare_config_changed_container_exited(self):
722+
self.dw = get_DockerWorker(FAKE_DATA['params'])
723+
job = mock.MagicMock()
724+
self.dw.dc.exec_create.return_value = job
725+
self.dw.dc.exec_start.return_value = 'fake output'
726+
self.dw.dc.exec_inspect.return_value = {'ExitCode': 137}
727+
return_data = self.dw.compare_config()
728+
self.dw.dc.exec_create.assert_called_once_with(
729+
FAKE_DATA['params']['name'],
730+
kd.COMPARE_CONFIG_CMD,
731+
user='root')
732+
self.dw.dc.exec_start.assert_called_once_with(job)
733+
self.dw.dc.exec_inspect.assert_called_once_with(job)
734+
self.assertTrue(return_data)
735+
736+
def test_compare_config_changed_client_failure(self):
737+
self.dw = get_DockerWorker(FAKE_DATA['params'])
738+
job = mock.MagicMock()
739+
self.dw.dc.exec_create.return_value = job
740+
self.dw.dc.exec_start.return_value = 'fake output'
741+
failure_response = mock.MagicMock()
742+
failure_response.status_code = 409 # any client error should do here
743+
self.dw.dc.exec_inspect.side_effect = docker_error.APIError(
744+
message="foo",
745+
response=failure_response,
746+
)
747+
return_data = self.dw.compare_config()
748+
self.dw.dc.exec_create.assert_called_once_with(
749+
FAKE_DATA['params']['name'],
750+
kd.COMPARE_CONFIG_CMD,
751+
user='root')
752+
self.dw.dc.exec_start.assert_called_once_with(job)
753+
self.dw.dc.exec_inspect.assert_called_once_with(job)
754+
self.assertTrue(return_data)
755+
756+
def test_compare_config_error(self):
757+
self.dw = get_DockerWorker(FAKE_DATA['params'])
758+
job = mock.MagicMock()
759+
self.dw.dc.exec_create.return_value = job
760+
self.dw.dc.exec_start.return_value = 'fake output'
761+
self.dw.dc.exec_inspect.return_value = {'ExitCode': -1}
762+
self.assertRaises(Exception, self.dw.compare_config) # noqa: H202
763+
self.dw.dc.exec_create.assert_called_once_with(
764+
FAKE_DATA['params']['name'],
765+
kd.COMPARE_CONFIG_CMD,
766+
user='root')
767+
self.dw.dc.exec_start.assert_called_once_with(job)
768+
self.dw.dc.exec_inspect.assert_called_once_with(job)
769+
770+
def test_compare_config_error_server_failure(self):
771+
self.dw = get_DockerWorker(FAKE_DATA['params'])
772+
job = mock.MagicMock()
773+
self.dw.dc.exec_create.return_value = job
774+
self.dw.dc.exec_start.return_value = 'fake output'
775+
failure_response = mock.MagicMock()
776+
failure_response.status_code = 500 # any server error should do here
777+
self.dw.dc.exec_inspect.side_effect = docker_error.APIError(
778+
message="foo",
779+
response=failure_response,
780+
)
781+
self.assertRaises(docker_error.APIError, self.dw.compare_config)
782+
self.dw.dc.exec_create.assert_called_once_with(
783+
FAKE_DATA['params']['name'],
784+
kd.COMPARE_CONFIG_CMD,
785+
user='root')
786+
self.dw.dc.exec_start.assert_called_once_with(job)
787+
self.dw.dc.exec_inspect.assert_called_once_with(job)
788+
691789
def test_get_image_id_not_exists(self):
692790
self.dw = get_DockerWorker(
693791
{'image': 'myregistrydomain.com:5000/ubuntu:16.04'})

0 commit comments

Comments
 (0)