From 64d07c0b7fedf96c489772f7e6619fe112e4189a Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
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
+ <https://bugs.launchpad.net/kolla-ansible/+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()