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
This commit is contained in:
parent
88d26e52d3
commit
52cac09d3d
|
@ -321,6 +321,11 @@ class PushIntoQueueTask(task.Task):
|
||||||
self.success = True
|
self.success = True
|
||||||
|
|
||||||
|
|
||||||
|
class PushError(Exception):
|
||||||
|
"""Raised when there is a problem with pushing image to repository."""
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
class PushTask(DockerTask):
|
class PushTask(DockerTask):
|
||||||
"""Task that pushes an image to a docker repository."""
|
"""Task that pushes an image to a docker repository."""
|
||||||
|
|
||||||
|
@ -344,6 +349,9 @@ class PushTask(DockerTask):
|
||||||
' have the correct privileges to run Docker'
|
' have the correct privileges to run Docker'
|
||||||
' (root)')
|
' (root)')
|
||||||
image.status = STATUS_CONNECTION_ERROR
|
image.status = STATUS_CONNECTION_ERROR
|
||||||
|
except PushError as exception:
|
||||||
|
self.logger.error(exception)
|
||||||
|
image.status = STATUS_PUSH_ERROR
|
||||||
except Exception:
|
except Exception:
|
||||||
self.logger.exception('Unknown error when pushing')
|
self.logger.exception('Unknown error when pushing')
|
||||||
image.status = STATUS_PUSH_ERROR
|
image.status = STATUS_PUSH_ERROR
|
||||||
|
@ -368,8 +376,7 @@ class PushTask(DockerTask):
|
||||||
if 'stream' in response:
|
if 'stream' in response:
|
||||||
self.logger.info(response['stream'])
|
self.logger.info(response['stream'])
|
||||||
elif 'errorDetail' in response:
|
elif 'errorDetail' in response:
|
||||||
image.status = STATUS_ERROR
|
raise PushError(response['errorDetail']['message'])
|
||||||
self.logger.error(response['errorDetail']['message'])
|
|
||||||
|
|
||||||
# Reset any previous errors.
|
# Reset any previous errors.
|
||||||
image.status = STATUS_BUILT
|
image.status = STATUS_BUILT
|
||||||
|
|
|
@ -83,6 +83,7 @@ class TasksTest(base.TestCase):
|
||||||
@mock.patch.dict(os.environ, clear=True)
|
@mock.patch.dict(os.environ, clear=True)
|
||||||
@mock.patch('docker.APIClient')
|
@mock.patch('docker.APIClient')
|
||||||
def test_push_image_failure(self, mock_client):
|
def test_push_image_failure(self, mock_client):
|
||||||
|
"""failure on connecting Docker API"""
|
||||||
self.dc = mock_client
|
self.dc = mock_client
|
||||||
mock_client().push.side_effect = Exception
|
mock_client().push.side_effect = Exception
|
||||||
pusher = build.PushTask(self.conf, self.image)
|
pusher = build.PushTask(self.conf, self.image)
|
||||||
|
@ -96,6 +97,7 @@ class TasksTest(base.TestCase):
|
||||||
@mock.patch.dict(os.environ, clear=True)
|
@mock.patch.dict(os.environ, clear=True)
|
||||||
@mock.patch('docker.APIClient')
|
@mock.patch('docker.APIClient')
|
||||||
def test_push_image_failure_retry(self, mock_client):
|
def test_push_image_failure_retry(self, mock_client):
|
||||||
|
"""failure on connecting Docker API, success on retry"""
|
||||||
self.dc = mock_client
|
self.dc = mock_client
|
||||||
mock_client().push.side_effect = [Exception, []]
|
mock_client().push.side_effect = [Exception, []]
|
||||||
pusher = build.PushTask(self.conf, self.image)
|
pusher = build.PushTask(self.conf, self.image)
|
||||||
|
@ -112,6 +114,44 @@ class TasksTest(base.TestCase):
|
||||||
self.assertTrue(pusher.success)
|
self.assertTrue(pusher.success)
|
||||||
self.assertEqual(build.STATUS_BUILT, self.image.status)
|
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.dict(os.environ, clear=True)
|
||||||
@mock.patch('docker.APIClient')
|
@mock.patch('docker.APIClient')
|
||||||
def test_build_image(self, mock_client):
|
def test_build_image(self, mock_client):
|
||||||
|
|
Loading…
Reference in New Issue