Merge "Make [cinder]/catalog_info no longer require a service_name"

nova/conf/cinder.py

@@ -23,11 +23,14 @@ cinder_group = cfg.OptGroup(
 
 cinder_opts = [
     cfg.StrOpt('catalog_info',
-            default='volumev3:cinderv3:publicURL',
+            default='volumev3::publicURL',
-            regex='(\w+):(\w+):(.*?)',
+            regex='(\w+):(\w*):(.*?)',
             help="""
 Info to match when looking for cinder in the service catalog.
 
+The ``<service_name>`` is optional and omitted by default since it should
+not be necessary in most deployments.
+
 Possible values:
 
 * Format is separated values of the form:

nova/tests/unit/test_cinder.py

@@ -233,3 +233,22 @@ class CinderV3TestCase(BaseCinderTestCase, test.NoDBTestCase):
         volume = self.api.get(self.context, '5678')
         self.assertIn('attachments', volume)
         self.assertEqual(exp_volume_attachment_2, volume['attachments'])
+
+    def test_create_client_with_no_service_name(self):
+        """Tests that service_name is not required and not passed through
+        when constructing the cinder client Client object if it's not
+        configured.
+        """
+        self.flags(catalog_info='volumev3::public', group='cinder')
+        with mock.patch('cinderclient.client.Client') as mock_client:
+            # We don't use self.create_client() because that has additional
+            # assertions that we don't care about in this test. We just care
+            # about how the client is created, not what is returned.
+            cinder.cinderclient(self.context)
+        self.assertEqual(1, len(mock_client.call_args_list))
+        call_kwargs = mock_client.call_args_list[0][1]
+        # Make sure service_type and interface are passed through.
+        self.assertEqual('volumev3', call_kwargs['service_type'])
+        self.assertEqual('public', call_kwargs['interface'])
+        # And service_name is not passed through.
+        self.assertNotIn('service_name', call_kwargs)

nova/volume/cinder.py

@@ -200,9 +200,11 @@ def _get_cinderclient_parameters(context):
     service_type, service_name, interface = CONF.cinder.catalog_info.split(':')
 
     service_parameters = {'service_type': service_type,
-                          'service_name': service_name,
                           'interface': interface,
                           'region_name': CONF.cinder.os_region_name}
+    # Only include the service_name if it's provided.
+    if service_name:
+        service_parameters['service_name'] = service_name
 
     if CONF.cinder.endpoint_template:
         url = CONF.cinder.endpoint_template % context.to_dict()

releasenotes/notes/bug-1803627-cinder-catalog-info-service-name-optional-fa673ad29fb762ea.yaml

@@ -0,0 +1,11 @@
+---
+other:
+  - |
+    The ``[cinder]/catalog_info`` default value is changed such that the
+    ``service_name`` portion of the value is no longer set and is also
+    no longer required. Since looking up the cinder endpoint in the service
+    catalog should only need the endpoint type (``volumev3`` by default) and
+    interface (``publicURL`` by default), the service name is dropped and only
+    provided during endpoint lookup if configured.
+    See `bug 1803627 <https://bugs.launchpad.net/nova/+bug/1803627>`_ for
+    details.