From 64d07c0b7fedf96c489772f7e6619fe112e4189a Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Thu, 14 Nov 2019 12:12:07 +0000 Subject: [PATCH] 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 | 5 ++++ ...ecreate-missing-pull-dba93327fd4c94c3.yaml | 6 +++++ tests/test_kolla_docker.py | 24 +++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 releasenotes/notes/fix-recreate-missing-pull-dba93327fd4c94c3.yaml diff --git a/ansible/library/kolla_docker.py b/ansible/library/kolla_docker.py index b9b1e724c2..249a3477f5 100644 --- a/ansible/library/kolla_docker.py +++ b/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() diff --git a/releasenotes/notes/fix-recreate-missing-pull-dba93327fd4c94c3.yaml b/releasenotes/notes/fix-recreate-missing-pull-dba93327fd4c94c3.yaml new file mode 100644 index 0000000000..26372bca42 --- /dev/null +++ b/releasenotes/notes/fix-recreate-missing-pull-dba93327fd4c94c3.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where a failure in pulling an image could lead to a + container being removed and not replaced. See `bug 1852572 + `__ for details. diff --git a/tests/test_kolla_docker.py b/tests/test_kolla_docker.py index 31dba4633e..8fad2aa82e 100644 --- a/tests/test_kolla_docker.py +++ b/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()