From 86759d33bb047540b9c16e14e4eb7f0eeccb8613 Mon Sep 17 00:00:00 2001 From: Adrian Vladu Date: Mon, 19 Aug 2019 16:45:07 +0300 Subject: [PATCH] CloudStack: catch proper HTTP exceptions The exception to catch should be a valid Python exception instead of HTTPConnection. Also, switch the severity of the log mesages to debug instead of error, as the plugin continues to run. Change-Id: I57f314d00e2b201af9bfe40334c69fc972ea904b --- cloudbaseinit/metadata/services/cloudstack.py | 26 +++++-- .../metadata/services/test_cloudstack.py | 74 ++++++++++++++----- 2 files changed, 77 insertions(+), 23 deletions(-) diff --git a/cloudbaseinit/metadata/services/cloudstack.py b/cloudbaseinit/metadata/services/cloudstack.py index 360f0632..7e3a5235 100644 --- a/cloudbaseinit/metadata/services/cloudstack.py +++ b/cloudbaseinit/metadata/services/cloudstack.py @@ -173,9 +173,16 @@ class CloudStack(base.BaseHTTPMetadataService): for _ in range(CONF.retry_count): try: content = self._password_client(headers=headers).strip() - except http_client.HTTPConnection as exc: - LOG.error("Getting password failed: %s", exc) + except urllib.error.HTTPError as exc: + LOG.debug("Getting password failed: %s", exc.code) continue + except OSError as exc: + if exc.errno == 10061: + # Connection error + LOG.debug("Getting password failed due to a " + "connection failure.") + continue + raise if not content: LOG.warning("The Password Server did not have any " @@ -212,16 +219,23 @@ class CloudStack(base.BaseHTTPMetadataService): for _ in range(CONF.retry_count): try: content = self._password_client(headers=headers).strip() - except http_client.HTTPConnection as exc: - LOG.error("Removing password failed: %s", exc) + except urllib.error.HTTPError as exc: + LOG.debug("Removing password failed: %s", exc.code) continue + except OSError as exc: + if exc.errno == 10061: + # Connection error + LOG.debug("Removing password failed due to a " + "connection failure.") + continue + raise if content != BAD_REQUEST: LOG.info("The password was removed from the Password Server.") break else: - LOG.warning("Fail to remove the password from the " - "Password Server.") + LOG.error("Failed to remove the password from the " + "Password Server.") def get_admin_password(self): """Get the admin password from the Password Server. diff --git a/cloudbaseinit/tests/metadata/services/test_cloudstack.py b/cloudbaseinit/tests/metadata/services/test_cloudstack.py index b436fe39..d171d300 100644 --- a/cloudbaseinit/tests/metadata/services/test_cloudstack.py +++ b/cloudbaseinit/tests/metadata/services/test_cloudstack.py @@ -198,7 +198,8 @@ class CloudStackTest(unittest.TestCase): @mock.patch('cloudbaseinit.metadata.services.cloudstack.CloudStack' '._password_client') def test_get_password_fail(self, mock_password_client): - mock_password_client.side_effect = ["", cloudstack.BAD_REQUEST, + mock_password_client.side_effect = ["", + cloudstack.BAD_REQUEST, cloudstack.SAVED_PASSWORD] expected_output = [ ["Try to get password from the Password Server.", @@ -222,27 +223,66 @@ class CloudStackTest(unittest.TestCase): @mock.patch('cloudbaseinit.metadata.services.cloudstack.CloudStack' '._password_client') - def test_delete_password(self, mock_password_client): - mock_password_client.side_effect = [cloudstack.BAD_REQUEST, - cloudstack.SAVED_PASSWORD] + def test_get_password_exception(self, mock_password_client): + fake_http_error = urllib.error.HTTPError(url='127.0.0.1', code=404, + hdrs={}, fp=None, + msg='error') + fake_error = OSError(10061, "Connection error") + mock_password_client.side_effect = [fake_http_error, fake_error] expected_output = [ - 'Remove the password for this instance from the ' - 'Password Server.', - 'Fail to remove the password from the Password Server.', + ["Try to get password from the Password Server.", + "Getting password failed due to a connection failure."], - 'Remove the password for this instance from the ' - 'Password Server.', - 'The password was removed from the Password Server', + ["Try to get password from the Password Server.", + "Getting password failed: 404"], + ] + + for _ in range(2): + with testutils.LogSnatcher('cloudbaseinit.metadata.services.' + 'cloudstack') as snatcher: + self.assertIsNone(self._service._get_password()) + self.assertEqual(expected_output.pop(), snatcher.output) + + self.assertEqual(2, mock_password_client.call_count) + + @mock.patch('cloudbaseinit.metadata.services.cloudstack.CloudStack' + '._password_client') + def test_delete_password(self, mock_password_client): + fake_url_error = urllib.error.HTTPError(url='127.0.0.1', code=404, + hdrs={}, fp=None, + msg='error') + fake_connection_error = OSError(10061, "Connection error") + mock_password_client.side_effect = [cloudstack.SAVED_PASSWORD, + cloudstack.BAD_REQUEST, + fake_url_error, + fake_connection_error] + expected_output = [ + + ['Remove the password for this instance from the ' + 'Password Server.', + 'Removing password failed due to a connection failure.', + 'Failed to remove the password from the Password Server.'], + ['Remove the password for this instance from the ' + 'Password Server.', + 'Removing password failed: 404', + 'Failed to remove the password from the Password Server.'], + ['Remove the password for this instance from the ' + 'Password Server.', + 'Failed to remove the password from the Password Server.'], + ['Remove the password for this instance from the ' + 'Password Server.', + 'The password was removed from the Password Server.'], ] - with testutils.LogSnatcher('cloudbaseinit.metadata.services.' - 'cloudstack') as snatcher: - self.assertIsNone(self._service._delete_password()) - self.assertIsNone(self._service._delete_password()) - self.assertEqual(2, mock_password_client.call_count) - for expected, output in zip(expected_output, snatcher.output): - self.assertTrue(output.startswith(expected)) + expected_output_len = len(expected_output) + for _ in range(expected_output_len): + with testutils.LogSnatcher('cloudbaseinit.metadata.services.' + 'cloudstack') as snatcher: + self.assertIsNone(self._service._delete_password()) + self.assertEqual(expected_output.pop(), snatcher.output) + + self.assertEqual(expected_output_len, mock_password_client.call_count) @mock.patch('cloudbaseinit.metadata.services.cloudstack.CloudStack.' '_delete_password')