From d6f46d544fd757fdc811d738422f85e64a6dc09b Mon Sep 17 00:00:00 2001 From: Marcin Juszkiewicz Date: Mon, 14 Oct 2019 04:36:48 -0700 Subject: [PATCH] handle push error properly If there was some error during pushing then Kolla was greeting with "all went fine" message anyway: INFO:kolla.common.utils.aodh-api:Trying to push the image ERROR:kolla.common.utils.aodh-api:Get http://10.101.16.1:5000/v2/: dial tcp 10.101.16.1:5000: connect: no route to host INFO:kolla.common.utils.aodh-api:Pushed successfully This patch changes that. Now if there is an error during push then proper exception is raised to PushTask and image is marked with PUSH_ERROR status. This way at the end of build it is easy to spot which images did not got pushed to registry: INFO:kolla.common.utils:=========================== INFO:kolla.common.utils:Images that failed to build INFO:kolla.common.utils:=========================== ERROR:kolla.common.utils:base Failed with status: push_error ERROR:kolla.common.utils:nova-api Failed with status: push_error ERROR:kolla.common.utils:nova-base Failed with status: push_error ERROR:kolla.common.utils:nova-compute Failed with status: push_error ERROR:kolla.common.utils:nova-compute-ironic Failed with status: push_error ERROR:kolla.common.utils:nova-conductor Failed with status: push_error Closes-Bug: #1848019 Change-Id: Id2ab97bf4c0dc3423268a0ea435b56f4a65f7196 (cherry picked from commit 52cac09d3d0b8dcdbc8d68a0bcc8092008220309) --- kolla/image/build.py | 11 +++++++++-- kolla/tests/test_build.py | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/kolla/image/build.py b/kolla/image/build.py index 2e88061049..5fd2cde0ce 100755 --- a/kolla/image/build.py +++ b/kolla/image/build.py @@ -317,6 +317,11 @@ class PushIntoQueueTask(task.Task): self.success = True +class PushError(Exception): + """Raised when there is a problem with pushing image to repository.""" + pass + + class PushTask(DockerTask): """Task that pushes an image to a docker repository.""" @@ -340,6 +345,9 @@ class PushTask(DockerTask): ' have the correct privileges to run Docker' ' (root)') image.status = STATUS_CONNECTION_ERROR + except PushError as exception: + self.logger.error(exception) + image.status = STATUS_PUSH_ERROR except Exception: self.logger.exception('Unknown error when pushing') image.status = STATUS_PUSH_ERROR @@ -364,8 +372,7 @@ class PushTask(DockerTask): if 'stream' in response: self.logger.info(response['stream']) elif 'errorDetail' in response: - image.status = STATUS_ERROR - self.logger.error(response['errorDetail']['message']) + raise PushError(response['errorDetail']['message']) # Reset any previous errors. image.status = STATUS_BUILT diff --git a/kolla/tests/test_build.py b/kolla/tests/test_build.py index b0ce009a9a..f9b2deab0b 100644 --- a/kolla/tests/test_build.py +++ b/kolla/tests/test_build.py @@ -83,6 +83,7 @@ class TasksTest(base.TestCase): @mock.patch.dict(os.environ, clear=True) @mock.patch('docker.APIClient') def test_push_image_failure(self, mock_client): + """failure on connecting Docker API""" self.dc = mock_client mock_client().push.side_effect = Exception pusher = build.PushTask(self.conf, self.image) @@ -96,6 +97,7 @@ class TasksTest(base.TestCase): @mock.patch.dict(os.environ, clear=True) @mock.patch('docker.APIClient') def test_push_image_failure_retry(self, mock_client): + """failure on connecting Docker API, success on retry""" self.dc = mock_client mock_client().push.side_effect = [Exception, []] pusher = build.PushTask(self.conf, self.image) @@ -112,6 +114,44 @@ class TasksTest(base.TestCase): self.assertTrue(pusher.success) self.assertEqual(build.STATUS_BUILT, self.image.status) + @mock.patch('docker.version', '3.0.0') + @mock.patch.dict(os.environ, clear=True) + @mock.patch('docker.APIClient') + def test_push_image_failure_error(self, mock_client): + """Docker connected, failure to push""" + self.dc = mock_client + mock_client().push.return_value = [{'errorDetail': {'message': + 'mock push fail'}}] + pusher = build.PushTask(self.conf, self.image) + pusher.run() + mock_client().push.assert_called_once_with( + self.image.canonical_name, decode=True, stream=True) + self.assertFalse(pusher.success) + self.assertEqual(build.STATUS_PUSH_ERROR, self.image.status) + + @mock.patch('docker.version', '3.0.0') + @mock.patch.dict(os.environ, clear=True) + @mock.patch('docker.APIClient') + def test_push_image_failure_error_retry(self, mock_client): + """Docker connected, failure to push, success on retry""" + self.dc = mock_client + mock_client().push.return_value = [{'errorDetail': {'message': + 'mock push fail'}}] + pusher = build.PushTask(self.conf, self.image) + pusher.run() + mock_client().push.assert_called_once_with( + self.image.canonical_name, decode=True, stream=True) + self.assertFalse(pusher.success) + self.assertEqual(build.STATUS_PUSH_ERROR, self.image.status) + + # Try again, this time without exception. + mock_client().push.return_value = [{'stream': 'mock push passes'}] + pusher.reset() + pusher.run() + self.assertEqual(2, mock_client().push.call_count) + self.assertTrue(pusher.success) + self.assertEqual(build.STATUS_BUILT, self.image.status) + @mock.patch.dict(os.environ, clear=True) @mock.patch('docker.APIClient') def test_build_image(self, mock_client):