service_instance: allow overriding the instance name

This uses the method _get_service_instance_name to generate the name
of the share service instance, instead of using directly the share
service id.

Note that it changes the default value of the
service_instance_name_template option to match the current default and
keep backwards compatibility.

Closes-Bug: #1945463
Change-Id: I3ee10f53315a37b47bd66eba082502b4209f4d95
Signed-off-by: David Caro <me@dcaro.es>

doc/source/configuration/tables/manila-generic.inc

@@ -52,7 +52,7 @@
      - (String) ID of flavor, that will be used for service instance creation. Only used if driver_handles_share_servers=True.
    * - ``service_instance_name_or_id`` = ``None``
      - (String) Name or ID of service instance in Nova to use for share exports. Used only when share servers handling is disabled.
-   * - ``service_instance_name_template`` = ``manila_service_instance_%s``
+   * - ``service_instance_name_template`` = ``%s``
      - (String) Name of service instance. Only used if driver_handles_share_servers=True.
    * - ``service_instance_network_helper_type`` = ``neutron``
      - (String) DEPRECATED: Used to select between neutron and nova helpers when driver_handles_share_servers=True. Obsolete. This option isn't used any longer because nova networking is no longer supported.

manila/share/drivers/service_instance.py

@@ -49,7 +49,7 @@ share_servers_handling_mode_opts = [
              "creation. Only used if driver_handles_share_servers=True."),
     cfg.StrOpt(
         "service_instance_name_template",
-        default="manila_service_instance_%s",
+        default="%s",
         help="Name of service instance. "
              "Only used if driver_handles_share_servers=True."),
     cfg.StrOpt(
@@ -306,7 +306,7 @@ class ServiceInstanceManager(object):
 
     def _get_service_instance_name(self, share_server_id):
         """Returns service vms name."""
-        if self.driver_config:
+        if self.driver_config and self.driver_config.config_group:
             # Make service instance name unique for multibackend installation
             name = "%s_%s" % (self.driver_config.config_group, share_server_id)
         else:
@@ -450,7 +450,8 @@ class ServiceInstanceManager(object):
         :returns: dict with service instance details
         :raises: exception.ServiceInstanceException
         """
-        instance_name = network_info['server_id']
+        instance_name = self._get_service_instance_name(
+            network_info['server_id'])
         server = self._create_service_instance(
             context, instance_name, network_info)
         instance_details = self._get_new_instance_details(server)

manila/tests/share/drivers/test_service_instance.py

@@ -506,6 +506,8 @@ class ServiceInstanceManagerTestCase(test.TestCase):
         expected_details = fake_server.copy()
         expected_details.pop('pk_path')
         expected_details['instance_id'] = expected_details.pop('id')
+        expected_instance_name = self._manager._get_service_instance_name(
+            fake_network_info['server_id'])
         self.mock_object(self._manager, '_create_service_instance',
                          mock.Mock(return_value=fake_server))
         self.mock_object(self._manager, '_check_server_availability')
@@ -515,7 +517,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
 
         self._manager._create_service_instance.assert_called_once_with(
             self._manager.admin_context,
-            fake_network_info['server_id'], fake_network_info)
+            expected_instance_name, fake_network_info)
         self._manager._check_server_availability.assert_called_once_with(
             expected_details)
         self.assertEqual(expected_details, result)
@@ -532,6 +534,8 @@ class ServiceInstanceManagerTestCase(test.TestCase):
         expected_details = fake_server.copy()
         expected_details.pop('pk_path')
         expected_details['instance_id'] = expected_details.pop('id')
+        expected_instance_name = self._manager._get_service_instance_name(
+            fake_network_info['server_id'])
         self.mock_object(self._manager, '_create_service_instance',
                          mock.Mock(return_value=fake_server))
         self.mock_object(self._manager, '_check_server_availability',
@@ -547,7 +551,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
             {'server_details': expected_details}, result.detail_data)
         self._manager._create_service_instance.assert_called_once_with(
             self._manager.admin_context,
-            fake_network_info['server_id'], fake_network_info)
+            expected_instance_name, fake_network_info)
         self._manager._check_server_availability.assert_called_once_with(
             expected_details)
 

releasenotes/notes/allow_override_instance_name-b730d106a9c32c83.yaml

@@ -0,0 +1,16 @@
+---
+prelude: >
+    Honor the service_instance_name_template option and allow specifying a
+    custom one.
+features:
+  - |
+    Honor the service_instance_name_template option and allow specifying a
+    custom one.
+upgrade:
+  - |
+    The option service_instance_name_template will start being honored, so
+    review the configuration and revert to the default if you don't want it to
+    be taken into account.
+fixes:
+  - |
+    The service_instance_name_template option was not being taken into account.