From f3a56f98c41a5269c16dfcd868ebb7fe9a522ccc Mon Sep 17 00:00:00 2001 From: "OTSUKA, Yuanying" Date: Tue, 22 Mar 2016 12:22:05 +0900 Subject: [PATCH] Release certs/trust when creating bay is failed Certificates and trust/trustee should be released correctly when creating bay is failed. This fixes it. Change-Id: Ic784a57ef751526123898f00447ebe8fac650d3e Closes-Bug: #1560308 --- magnum/common/exception.py | 8 ++ magnum/conductor/handlers/bay_conductor.py | 10 +- .../conductor/handlers/common/cert_manager.py | 26 +++-- .../handlers/common/trust_manager.py | 27 +++-- .../handlers/common/test_cert_manager.py | 11 ++ .../handlers/common/test_trust_manager.py | 11 ++ .../conductor/handlers/test_bay_conductor.py | 104 ++++++++++++++---- 7 files changed, 152 insertions(+), 45 deletions(-) diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 9f06fe469e..c807eea3c6 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -569,3 +569,11 @@ class QuotaAlreadyExists(Conflict): class RegionsListFailed(MagnumException): message = _("Failed to list regions.") + + +class TrusteeOrTrustToBayFailed(MagnumException): + message = _("Failed to create trustee or trust for Bay: %(bay_uuid)s") + + +class CertificatesToBayFailed(MagnumException): + message = _("Failed to create certificates for Bay: %(bay_uuid)s") diff --git a/magnum/conductor/handlers/bay_conductor.py b/magnum/conductor/handlers/bay_conductor.py index 6a5180250a..9164aa78b9 100644 --- a/magnum/conductor/handlers/bay_conductor.py +++ b/magnum/conductor/handlers/bay_conductor.py @@ -132,12 +132,14 @@ class Handler(object): cert_manager.generate_certificates_to_bay(bay) created_stack = _create_stack(context, osc, bay, bay_create_timeout) - except exc.HTTPBadRequest as e: + except Exception as e: cert_manager.delete_certificates_from_bay(bay) trust_manager.delete_trustee_and_trust(osc, bay) - raise exception.InvalidParameterValue(message=six.text_type(e)) - except Exception: - raise + + if isinstance(e, exc.HTTPBadRequest): + e = exception.InvalidParameterValue(message=six.text_type(e)) + + raise e bay.stack_id = created_stack['stack']['id'] bay.status = bay_status.CREATE_IN_PROGRESS diff --git a/magnum/conductor/handlers/common/cert_manager.py b/magnum/conductor/handlers/common/cert_manager.py index 9eb5665c59..952dd09166 100644 --- a/magnum/conductor/handlers/common/cert_manager.py +++ b/magnum/conductor/handlers/common/cert_manager.py @@ -18,8 +18,10 @@ from oslo_log import log as logging import six from magnum.common import cert_manager +from magnum.common import exception from magnum.common import short_id from magnum.common.x509 import operations as x509 +from magnum.i18n import _LE from magnum.i18n import _LW CONDUCTOR_CLIENT_NAME = six.u('Magnum-Conductor') @@ -87,15 +89,22 @@ def generate_certificates_to_bay(bay): :param bay: The bay to set CA cert and magnum client cert :returns: CA cert uuid and magnum client cert uuid """ - issuer_name = _get_issuer_name(bay) + try: + issuer_name = _get_issuer_name(bay) - LOG.debug('Start to generate certificates: %s', issuer_name) + LOG.debug('Start to generate certificates: %s', issuer_name) - ca_cert_ref, ca_cert, ca_password = _generate_ca_cert(issuer_name) - magnum_cert_ref = _generate_client_cert(issuer_name, ca_cert, ca_password) + ca_cert_ref, ca_cert, ca_password = _generate_ca_cert(issuer_name) + magnum_cert_ref = _generate_client_cert(issuer_name, + ca_cert, + ca_password) - bay.ca_cert_ref = ca_cert_ref - bay.magnum_cert_ref = magnum_cert_ref + bay.ca_cert_ref = ca_cert_ref + bay.magnum_cert_ref = magnum_cert_ref + except Exception: + LOG.exception(_LE('Failed to generate certificates for Bay: %s'), + bay.uuid) + raise exception.CertificatesToBayFailed(bay_uuid=bay.uuid) def get_bay_ca_certificate(bay): @@ -153,10 +162,11 @@ def delete_certificates_from_bay(bay): :param bay: The bay which has certs """ - for cert_ref in [bay.ca_cert_ref, bay.magnum_cert_ref]: + for cert_ref in ['ca_cert_ref', 'magnum_cert_ref']: try: + cert_ref = getattr(bay, cert_ref, None) if cert_ref: cert_manager.get_backend().CertManager.delete_cert( cert_ref, resource_ref=bay.uuid) except Exception: - LOG.warning(_LW("Deleting cert is failed: %s"), cert_ref) + LOG.warning(_LW("Deleting certs is failed for Bay %s"), bay.uuid) diff --git a/magnum/conductor/handlers/common/trust_manager.py b/magnum/conductor/handlers/common/trust_manager.py index 92ea458d76..12b6bdf3f2 100644 --- a/magnum/conductor/handlers/common/trust_manager.py +++ b/magnum/conductor/handlers/common/trust_manager.py @@ -13,7 +13,9 @@ from oslo_config import cfg from oslo_log import log as logging +from magnum.common import exception from magnum.common import utils +from magnum.i18n import _LE CONF = cfg.CONF CONF.import_opt('trustee_domain_id', 'magnum.common.keystone', @@ -23,16 +25,21 @@ LOG = logging.getLogger(__name__) def create_trustee_and_trust(osc, bay): - password = utils.generate_password(length=18) - trustee = osc.keystone().create_trustee( - bay.uuid, - password, - CONF.trust.trustee_domain_id) - bay.trustee_username = trustee.name - bay.trustee_user_id = trustee.id - bay.trustee_password = password - trust = osc.keystone().create_trust(trustee.id) - bay.trust_id = trust.id + try: + password = utils.generate_password(length=18) + trustee = osc.keystone().create_trustee( + bay.uuid, + password, + CONF.trust.trustee_domain_id) + bay.trustee_username = trustee.name + bay.trustee_user_id = trustee.id + bay.trustee_password = password + trust = osc.keystone().create_trust(trustee.id) + bay.trust_id = trust.id + except Exception: + LOG.exception(_LE('Failed to create trustee and trust for Bay: %s'), + bay.uuid) + raise exception.TrusteeOrTrustToBayFailed(bay_uuid=bay.uuid) def delete_trustee_and_trust(osc, bay): diff --git a/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py b/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py index 41573ae954..7593d075e5 100644 --- a/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py +++ b/magnum/tests/unit/conductor/handlers/common/test_cert_manager.py @@ -14,6 +14,7 @@ import mock +from magnum.common import exception from magnum.conductor.handlers.common import cert_manager from magnum.tests import base @@ -152,6 +153,16 @@ class CertManagerTestCase(base.BaseTestCase): mock_generate_ca_cert, mock_generate_client_cert) + @mock.patch('magnum.conductor.handlers.common.cert_manager.' + '_get_issuer_name') + def test_generate_certificates_with_error(self, mock_get_issuer_name): + mock_bay = mock.MagicMock() + mock_get_issuer_name.side_effect = exception.MagnumException() + + self.assertRaises(exception.CertificatesToBayFailed, + cert_manager.generate_certificates_to_bay, + mock_bay) + @mock.patch('magnum.common.x509.operations.sign') def test_sign_node_certificate(self, mock_x509_sign): mock_bay = mock.MagicMock() diff --git a/magnum/tests/unit/conductor/handlers/common/test_trust_manager.py b/magnum/tests/unit/conductor/handlers/common/test_trust_manager.py index 31b2a98d51..211eef58a8 100644 --- a/magnum/tests/unit/conductor/handlers/common/test_trust_manager.py +++ b/magnum/tests/unit/conductor/handlers/common/test_trust_manager.py @@ -16,6 +16,7 @@ import mock from mock import patch from oslo_config import fixture +from magnum.common import exception from magnum.conductor.handlers.common import trust_manager from magnum.tests import base @@ -67,6 +68,16 @@ class TrustManagerTestCase(base.BaseTestCase): self.assertEqual(mock_password, mock_bay.trustee_password) self.assertEqual(mock_trust.id, mock_bay.trust_id) + @patch('magnum.common.utils.generate_password') + def test_create_trustee_and_trust_with_error(self, mock_generate_password): + mock_bay = mock.MagicMock() + mock_generate_password.side_effect = exception.MagnumException() + + self.assertRaises(exception.TrusteeOrTrustToBayFailed, + trust_manager.create_trustee_and_trust, + self.osc, + mock_bay) + def test_delete_trustee_and_trust(self): mock_bay = mock.MagicMock() mock_bay.trust_id = 'trust_id' diff --git a/magnum/tests/unit/conductor/handlers/test_bay_conductor.py b/magnum/tests/unit/conductor/handlers/test_bay_conductor.py index 3618b4e7dc..ca6d5b8ed5 100644 --- a/magnum/tests/unit/conductor/handlers/test_bay_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_bay_conductor.py @@ -191,6 +191,39 @@ class TestHandler(db_base.DbTestCase): mock_trust_manager.create_trustee_and_trust.assert_called_once_with( osc, self.bay) + def _test_create_failed(self, + mock_openstack_client_class, + mock_cert_manager, + mock_trust_manager, + expected_exception, + is_create_cert_called=True, + is_create_trust_called=True): + osc = mock.MagicMock() + mock_openstack_client_class.return_value = osc + timeout = 15 + + self.assertRaises( + expected_exception, + self.handler.bay_create, + self.context, + self.bay, timeout + ) + + gctb = mock_cert_manager.generate_certificates_to_bay + if is_create_cert_called: + gctb.assert_called_once_with(self.bay) + else: + gctb.assert_not_called() + ctat = mock_trust_manager.create_trustee_and_trust + if is_create_trust_called: + ctat.assert_called_once_with(osc, self.bay) + else: + ctat.assert_not_called() + + mock_cert_manager.delete_certificates_from_bay(self.bay) + mock_trust_manager.delete_trustee_and_trust.assert_called_once_with( + osc, self.bay) + @patch('magnum.conductor.handlers.bay_conductor.trust_manager') @patch('magnum.conductor.handlers.bay_conductor.cert_manager') @patch('magnum.conductor.handlers.bay_conductor._create_stack') @@ -199,20 +232,49 @@ class TestHandler(db_base.DbTestCase): mock_create_stack, mock_cert_manager, mock_trust_manager): - osc = mock.MagicMock() - mock_openstack_client_class.return_value = osc mock_create_stack.side_effect = exc.HTTPBadRequest - timeout = 15 - self.assertRaises(exception.InvalidParameterValue, - self.handler.bay_create, self.context, - self.bay, timeout) - mock_cert_manager.generate_certificates_to_bay.assert_called_once_with( - self.bay) - mock_cert_manager.delete_certificates_from_bay(self.bay) - mock_trust_manager.create_trustee_and_trust.assert_called_once_with( - osc, self.bay) - mock_trust_manager.delete_trustee_and_trust.assert_called_once_with( - osc, self.bay) + + self._test_create_failed( + mock_openstack_client_class, + mock_cert_manager, + mock_trust_manager, + exception.InvalidParameterValue + ) + + @patch('magnum.conductor.handlers.bay_conductor.trust_manager') + @patch('magnum.conductor.handlers.bay_conductor.cert_manager') + @patch('magnum.common.clients.OpenStackClients') + def test_create_with_cert_failed(self, mock_openstack_client_class, + mock_cert_manager, + mock_trust_manager): + e = exception.CertificatesToBayFailed(bay_uuid='uuid') + mock_cert_manager.generate_certificates_to_bay.side_effect = e + + self._test_create_failed( + mock_openstack_client_class, + mock_cert_manager, + mock_trust_manager, + exception.CertificatesToBayFailed + ) + + @patch('magnum.conductor.handlers.bay_conductor.trust_manager') + @patch('magnum.conductor.handlers.bay_conductor.cert_manager') + @patch('magnum.conductor.handlers.bay_conductor._create_stack') + @patch('magnum.common.clients.OpenStackClients') + def test_create_with_trust_failed(self, mock_openstack_client_class, + mock_create_stack, + mock_cert_manager, + mock_trust_manager): + e = exception.TrusteeOrTrustToBayFailed(bay_uuid='uuid') + mock_trust_manager.create_trustee_and_trust.side_effect = e + + self._test_create_failed( + mock_openstack_client_class, + mock_cert_manager, + mock_trust_manager, + exception.TrusteeOrTrustToBayFailed, + False + ) @patch('magnum.conductor.handlers.bay_conductor.trust_manager') @patch('magnum.conductor.handlers.bay_conductor.cert_manager') @@ -225,9 +287,6 @@ class TestHandler(db_base.DbTestCase): mock_create_stack, mock_cert_manager, mock_trust_manager): - timeout = 15 - osc = mock.MagicMock() - mock_openstack_client_class.return_value = osc test_uuid = uuid.uuid4() mock_uuid.uuid4.return_value = test_uuid error_message = six.u("""Invalid stack name 测试集群-zoyh253geukk @@ -235,13 +294,12 @@ class TestHandler(db_base.DbTestCase): characters, must start with alpha""") mock_create_stack.side_effect = exc.HTTPBadRequest(error_message) - self.assertRaises(exception.InvalidParameterValue, - self.handler.bay_create, self.context, - self.bay, timeout) - mock_cert_manager.generate_certificates_to_bay.assert_called_once_with( - self.bay) - mock_trust_manager.create_trustee_and_trust.assert_called_once_with( - osc, self.bay) + self._test_create_failed( + mock_openstack_client_class, + mock_cert_manager, + mock_trust_manager, + exception.InvalidParameterValue + ) @patch('magnum.common.clients.OpenStackClients') def test_bay_delete(self, mock_openstack_client_class):