"keypair_id" should be existent when creating a baymodel

While creating the baymodel we must check existence of keypair-id,
So that we can avoid keypair NotFound error while creating the bay.

Change-Id: I3796503bac05a9aa1c9d155e93447a95fb567daf
Closes-Bug: #1476500

etc/magnum/magnum.conf.sample

@@ -35,7 +35,7 @@
 # (integer value)
 #periodic_interval_max = 60
 
-# Name of this node.  This can be an opaque identifier. It is not
+# Name of this node. This can be an opaque identifier. It is not
 # necessarily a hostname, FQDN, or IP address. However, the node name
 # must be valid within an AMQP key, and if using ZeroMQ, a valid
 # hostname, FQDN, or IP address. (string value)
@@ -46,7 +46,7 @@
 #
 
 # Print debugging output (set logging level to DEBUG instead of
-# default WARNING level). (boolean value)
+# default INFO level). (boolean value)
 #debug = false
 
 # If set to false, will disable INFO logging level, making WARNING the
@@ -88,8 +88,9 @@
 
 # (Optional) Enables or disables syslog rfc5424 format for logging. If
 # enabled, prefixes the MSG part of the syslog message with APP-NAME
-# (RFC5424). The format without the APP-NAME is deprecated in K, and
-# will be removed in M, along with this option. (boolean value)
+# (RFC5424). The format without the APP-NAME is deprecated in Kilo,
+# and will be removed in Mitaka, along with this option. (boolean
+# value)
 # This option is deprecated for removal.
 # Its value may be silently ignored in the future.
 #use_syslog_rfc_format = true
@@ -115,7 +116,7 @@
 #logging_exception_prefix = %(asctime)s.%(msecs)03d %(process)d ERROR %(name)s %(instance)s
 
 # List of logger=LEVEL pairs. (list value)
-#default_log_levels = amqp=WARN,amqplib=WARN,boto=WARN,qpid=WARN,sqlalchemy=WARN,suds=INFO,oslo.messaging=INFO,iso8601=WARN,requests.packages.urllib3.connectionpool=WARN,urllib3.connectionpool=WARN,websocket=WARN,requests.packages.urllib3.util.retry=WARN,urllib3.util.retry=WARN,keystonemiddleware=WARN,routes.middleware=WARN,stevedore=WARN
+#default_log_levels = amqp=WARN,amqplib=WARN,boto=WARN,qpid=WARN,sqlalchemy=WARN,suds=INFO,oslo.messaging=INFO,iso8601=WARN,requests.packages.urllib3.connectionpool=WARN,urllib3.connectionpool=WARN,websocket=WARN,requests.packages.urllib3.util.retry=WARN,urllib3.util.retry=WARN,keystonemiddleware=WARN,routes.middleware=WARN,stevedore=WARN,taskflow=WARN
 
 # Enables or disables publication of error events. (boolean value)
 #publish_errors = false
@@ -174,8 +175,9 @@
 # Heartbeat time-to-live. (integer value)
 #matchmaker_heartbeat_ttl = 600
 
-# Size of RPC thread pool. (integer value)
-#rpc_thread_pool_size = 64
+# Size of executor thread pool. (integer value)
+# Deprecated group/name - [DEFAULT]/rpc_thread_pool_size
+#executor_thread_pool_size = 64
 
 # The Drivers(s) to handle sending notifications. Possible values are
 # messaging, messagingv2, routing, log, test, noop (multi valued)
@@ -588,6 +590,9 @@
 # Verify HTTPS connections. (boolean value)
 #insecure = false
 
+# The region in which the identity server can be found. (string value)
+#region_name = <None>
+
 # Directory used to cache files related to PKI tokens. (string value)
 #signing_dir = <None>
 
@@ -772,6 +777,21 @@
 #ringfile = /etc/oslo/matchmaker_ring.json
 
 
+[nova_client]
+
+#
+# From magnum
+#
+
+# Region in Identity service catalog to use for communication with the
+# OpenStack service. (string value)
+#region_name = <None>
+
+# Type of endpoint in Identity service catalog to use for
+# communication with the OpenStack service. (string value)
+#endpoint_type = publicURL
+
+
 [oslo_concurrency]
 
 #
@@ -865,7 +885,8 @@
 # keep backward compatible at the same time. This option provides such
 # compatibility - it defaults to False in Liberty and can be turned on
 # for early adopters with a new installations or for testing. Please
-# note, that this option will be removed in M release. (boolean value)
+# note, that this option will be removed in the Mitaka release.
+# (boolean value)
 #send_single_reply = false
 
 # Qpid broker hostname. (string value)
@@ -940,7 +961,8 @@
 # keep backward compatible at the same time. This option provides such
 # compatibility - it defaults to False in Liberty and can be turned on
 # for early adopters with a new installations or for testing. Please
-# note, that this option will be removed in M release. (boolean value)
+# note, that this option will be removed in the Mitaka release.
+# (boolean value)
 #send_single_reply = false
 
 # SSL version to use (valid only if SSL enabled). Valid values are

magnum/api/controllers/v1/baymodel.py

@@ -15,6 +15,7 @@
 import datetime
 
 import glanceclient.exc
+import novaclient.exceptions as nova_exc
 import pecan
 from pecan import rest
 import wsme
@@ -282,6 +283,14 @@ class BayModelsController(rest.RestController):
         rpc_baymodel = api_utils.get_rpc_resource('BayModel', baymodel_ident)
         return BayModel.convert_with_links(rpc_baymodel)
 
+    def check_keypair_exists(self, context, keypair):
+        """Checks the existence of the keypair"""
+        cli = clients.OpenStackClients(context)
+        try:
+            cli.nova().keypairs.get(keypair)
+        except nova_exc.NotFound:
+            raise exception.KeyPairNotFound(keypair=keypair)
+
     @policy.enforce_wsgi("baymodel", "create")
     @expose.expose(BayModel, body=BayModel, status_code=201)
     def post(self, baymodel):
@@ -291,6 +300,7 @@ class BayModelsController(rest.RestController):
         """
         baymodel_dict = baymodel.as_dict()
         context = pecan.request.context
+        self.check_keypair_exists(context, baymodel_dict['keypair_id'])
         baymodel_dict['project_id'] = context.project_id
         baymodel_dict['user_id'] = context.user_id
         image_data = self._get_image_data(context, baymodel_dict['image_id'])

magnum/common/clients.py

@@ -15,6 +15,7 @@
 from barbicanclient import client as barbicanclient
 from glanceclient.v2 import client as glanceclient
 from heatclient.v1 import client as heatclient
+from novaclient.v2 import client as novaclient
 from oslo_config import cfg
 from oslo_log import log as logging
 
@@ -81,10 +82,22 @@ barbican_client_opts = [
                    'Type of endpoint in Identity service catalog to use '
                    'for communication with the OpenStack service.'))]
 
+nova_client_opts = [
+    cfg.StrOpt('region_name',
+               default=None,
+               help=_('Region in Identity service catalog to use for '
+                      'communication with the OpenStack service.')),
+    cfg.StrOpt('endpoint_type',
+               default='publicURL',
+               help=_(
+                   'Type of endpoint in Identity service catalog to use '
+                   'for communication with the OpenStack service.'))]
+
 cfg.CONF.register_opts(magnum_client_opts, group='magnum_client')
 cfg.CONF.register_opts(heat_client_opts, group='heat_client')
 cfg.CONF.register_opts(glance_client_opts, group='glance_client')
 cfg.CONF.register_opts(barbican_client_opts, group='barbican_client')
+cfg.CONF.register_opts(nova_client_opts, group='nova_client')
 
 
 class OpenStackClients(object):
@@ -96,6 +109,7 @@ class OpenStackClients(object):
         self._heat = None
         self._glance = None
         self._barbican = None
+        self._nova = None
 
     def url_for(self, **kwargs):
         return self.keystone().client.service_catalog.url_for(**kwargs)
@@ -187,3 +201,16 @@ class OpenStackClients(object):
                                                endpoint=endpoint)
 
         return self._barbican
+
+    @exception.wrap_keystone_exception
+    def nova(self):
+        if self._nova:
+            return self._nova
+        endpoint_type = self._get_client_option('nova', 'endpoint_type')
+        region_name = self._get_client_option('nova', 'region_name')
+        endpoint = self.url_for(service_type='compute',
+                                endpoint_type=endpoint_type,
+                                region_name=region_name)
+        self._nova = novaclient.Client(auth_token=self.auth_token)
+        self._nova.client.management_url = endpoint
+        return self._nova

magnum/common/exception.py

@@ -483,3 +483,7 @@ class CertificateStorageException(MagnumException):
 
 class CertificateValidationError(Invalid):
     message = _("Extension '%(extension)s' not allowed")
+
+
+class KeyPairNotFound(ResourceNotFound):
+    message = _("Unable to find keypair %(keypair)s.")

magnum/opts.py

@@ -51,6 +51,7 @@ def list_opts():
         ('heat_client', magnum.common.clients.heat_client_opts),
         ('glance_client', magnum.common.clients.glance_client_opts),
         ('barbican_client', magnum.common.clients.barbican_client_opts),
+        ('nova_client', magnum.common.clients.nova_client_opts),
         ('x509', magnum.common.x509.config.x509_opts),
         ('bay_heat', magnum.conductor.handlers.bay_conductor.bay_heat_opts),
         ('certificates',

magnum/tests/unit/api/controllers/v1/test_baymodel.py

@@ -27,6 +27,7 @@ from magnum.tests import base
 from magnum.tests.unit.api import base as api_base
 from magnum.tests.unit.api import utils as apiutils
 from magnum.tests.unit.objects import utils as obj_utils
+from novaclient import exceptions as nova_exc
 
 
 class TestBayModelObject(base.TestCase):
@@ -407,9 +408,13 @@ class TestPatch(api_base.FunctionalTest):
 class TestPost(api_base.FunctionalTest):
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
     @mock.patch('oslo_utils.timeutils.utcnow')
-    def test_create_baymodel(self, mock_utcnow, mock_image_data):
+    def test_create_baymodel(self, mock_utcnow,
+                             mock_keypair_exists, mock_image_data):
         bdict = apiutils.baymodel_post_data()
+        mock_keypair_exists.return_value = None
         test_time = datetime.datetime(2000, 1, 1, 0, 0)
         mock_utcnow.return_value = test_time
         mock_image_data.return_value = {'name': 'mock_name',
@@ -429,10 +434,14 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(test_time, return_created_at)
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
-    def test_create_baymodel_set_project_id_and_user_id(self, mock_image_data):
-
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_set_project_id_and_user_id(self,
+                                                        mock_keypair_exists,
+                                                        mock_image_data):
         with mock.patch.object(self.dbapi, 'create_baymodel',
                                wraps=self.dbapi.create_baymodel) as cc_mock:
+            mock_keypair_exists.return_value = None
             mock_image_data.return_value = {'name': 'mock_name',
                                             'os_distro': 'fedora-atomic'}
             bdict = apiutils.baymodel_post_data()
@@ -444,9 +453,14 @@ class TestPost(api_base.FunctionalTest):
                              self.context.user_id)
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
-    def test_create_baymodel_doesnt_contain_id(self, mock_image_data):
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_doesnt_contain_id(self,
+                                               mock_keypair_exists,
+                                               mock_image_data):
         with mock.patch.object(self.dbapi, 'create_baymodel',
                                wraps=self.dbapi.create_baymodel) as cc_mock:
+            mock_keypair_exists.return_value = None
             mock_image_data.return_value = {'name': 'mock_name',
                                             'os_distro': 'fedora-atomic'}
             bdict = apiutils.baymodel_post_data(image_id='my-image')
@@ -504,10 +518,14 @@ class TestPost(api_base.FunctionalTest):
         self._create_baymodel_raises_app_error(apiserver_port='not an int')
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
     def test_create_baymodel_with_docker_volume_size(self,
+                                                     mock_keypair_exists,
                                                      mock_image_data):
         with mock.patch.object(self.dbapi, 'create_baymodel',
                                wraps=self.dbapi.create_baymodel) as cc_mock:
+            mock_keypair_exists.return_value = None
             mock_image_data.return_value = {'name': 'mock_name',
                                             'os_distro': 'fedora-atomic'}
             bdict = apiutils.baymodel_post_data(docker_volume_size=99)
@@ -518,7 +536,12 @@ class TestPost(api_base.FunctionalTest):
             self.assertNotIn('id', cc_mock.call_args[0][0])
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
-    def test_create_baymodel_generate_uuid(self, mock_image_data):
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_generate_uuid(self,
+                                           mock_keypair_exists,
+                                           mock_image_data):
+        mock_keypair_exists.return_value = None
         mock_image_data.return_value = {'name': 'mock_name',
                                         'os_distro': 'fedora-atomic'}
         bdict = apiutils.baymodel_post_data()
@@ -529,7 +552,12 @@ class TestPost(api_base.FunctionalTest):
         self.assertTrue(utils.is_uuid_like(response.json['uuid']))
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
-    def test_create_baymodel_with_no_os_distro_image(self, mock_image_data):
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_with_no_os_distro_image(self,
+                                                     mock_keypair_exists,
+                                                     mock_image_data):
+        mock_keypair_exists.return_value = None
         mock_image_data.return_value = {'name': 'mock_name'}
         bdict = apiutils.baymodel_post_data()
         del bdict['uuid']
@@ -537,7 +565,12 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(404, response.status_int)
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
-    def test_create_baymodel_with_os_distro_image(self, mock_image_data):
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_with_os_distro_image(self,
+                                                  mock_keypair_exists,
+                                                  mock_image_data):
+        mock_keypair_exists.return_value = None
         mock_image_data.return_value = {'name': 'mock_name',
                                         'os_distro': 'fedora-atomic'}
         bdict = apiutils.baymodel_post_data()
@@ -546,7 +579,12 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(201, response.status_int)
 
     @mock.patch.object(openstack_client, 'glance')
-    def test_create_baymodel_with_image_name(self, mock_glance_client):
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_with_image_name(self,
+                                             mock_keypair_exists,
+                                             mock_glance_client):
+        mock_keypair_exists.return_value = None
         mock_images = [{'name': 'mock_name',
                        'os_distro': 'fedora-atomic'}]
         mock_glance = mock.MagicMock()
@@ -558,9 +596,13 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(201, response.status_int)
 
     @mock.patch.object(openstack_client, 'glance')
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
     def test_create_baymodel_with_no_exist_image_name(self,
+                                                      mock_keypair_exists,
                                                       mock_glance_client):
         mock_images = []
+        mock_keypair_exists.return_value = None
         mock_glance = mock.MagicMock()
         mock_glance.images.list.return_value = mock_images
         mock_glance_client.return_value = mock_glance
@@ -570,7 +612,12 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(404, response.status_int)
 
     @mock.patch.object(openstack_client, 'glance')
-    def test_create_baymodel_with_multi_image_name(self, mock_glance_client):
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_with_multi_image_name(self,
+                                                   mock_keypair_exists,
+                                                   mock_glance_client):
+        mock_keypair_exists.return_value = None
         mock_images = [{'name': 'mock_name',
                        'os_distro': 'fedora-atomic'},
                        {'name': 'mock_name',
@@ -596,7 +643,11 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(400, response.status_int)
 
     @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
-    def test_create_baymodel_with_dns(self, mock_image_data):
+    @mock.patch.object(api_baymodel.BayModelsController,
+                       'check_keypair_exists')
+    def test_create_baymodel_with_dns(self, mock_keypair_exists,
+                                      mock_image_data):
+        mock_keypair_exists.return_value = None
         mock_image_data.return_value = {'name': 'mock_name',
                                         'os_distro': 'fedora-atomic'}
         bdict = apiutils.baymodel_post_data()
@@ -605,6 +656,20 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(bdict['dns_nameserver'],
                          response.json['dns_nameserver'])
 
+    @mock.patch.object(openstack_client, 'nova')
+    @mock.patch.object(api_baymodel.BayModelsController, '_get_image_data')
+    def test_create_baymodel_with_no_exist_keypair(self,
+                                                   mock_image_data,
+                                                   mock_nova_client):
+        mock_nova = mock.MagicMock()
+        mock_nova.keypairs.get.side_effect = nova_exc.NotFound("Test")
+        mock_nova_client.return_value = mock_nova
+        mock_image_data.return_value = {'name': 'mock_name',
+                                        'os_distro': 'fedora-atomic'}
+        bdict = apiutils.baymodel_post_data()
+        response = self.post_json('/baymodels', bdict, expect_errors=True)
+        self.assertEqual(404, response.status_int)
+
 
 class TestDelete(api_base.FunctionalTest):
 

requirements.txt

@@ -39,6 +39,7 @@ pecan>=1.0.0
 python-barbicanclient>=3.3.0
 python-glanceclient>=0.18.0
 python-heatclient>=0.3.0
+python-novaclient>=2.28.1
 python-keystoneclient>=1.6.0
 requests>=2.5.2
 six>=1.9.0