0003-Release-certs-trust-when-creating-bay-is-failed.patch 15.1 KB
From fed47e7484687a0e7c75fff0669426951ece9365 Mon Sep 17 00:00:00 2001
From: "OTSUKA, Yuanying" <yuanying@fraction.jp>
Date: Tue, 22 Mar 2016 12:22:05 +0900
Subject: [PATCH 3/8] 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 +-
 magnum/conductor/handlers/common/cert_manager.py   |  26 ++++--
 magnum/conductor/handlers/common/trust_manager.py  |  27 ++++--
 .../conductor/handlers/common/test_cert_manager.py |  11 +++
 .../handlers/common/test_trust_manager.py          |  11 +++
 .../unit/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 9f06fe4..c807eea 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 6a51802..9164aa7 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 9eb5665..952dd09 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 92ea458..12b6bdf 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 41573ae..7593d07 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 31b2a98..211eef5 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 3618b4e..ca6d5b8 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):
-- 
2.5.5