Allow 2 or more export IPs for LVM driver

Supporting dual IPv4/IPv6 backends will eventually require
multi-IP support in the first party backends. This change
enables multi-IP support (IPv4-only currently) in the LVM
driver.

Change-Id: Ib3594aa5d7751c829820fce830d87f6ceea6b049
Partial-Implements: blueprint support-ipv6-access

devstack/plugin.sh

@@ -781,7 +781,7 @@ function configure_samba {
 
         for backend_name in ${MANILA_ENABLED_BACKENDS//,/ }; do
             iniset $MANILA_CONF $backend_name driver_handles_share_servers False
-            iniset $MANILA_CONF $backend_name lvm_share_export_ip $MANILA_LVM_SHARE_EXPORT_IP
+            iniset $MANILA_CONF $backend_name lvm_share_export_ips $MANILA_LVM_SHARE_EXPORT_IPS
         done
         iniset $MANILA_CONF DEFAULT data_node_access_ip $HOST_IP
     fi

devstack/settings

@@ -152,7 +152,7 @@ MANILA_MNT_DIR=${MANILA_MNT_DIR:=$MANILA_STATE_PATH/mnt}
 SMB_CONF=${SMB_CONF:-/etc/samba/smb.conf}
 SMB_PRIVATE_DIR=${SMB_PRIVATE_DIR:-/var/lib/samba/private}
 CONFIGURE_BACKING_FILE=${CONFIGURE_BACKING_FILE:-"True"}
-MANILA_LVM_SHARE_EXPORT_IP=${MANILA_LVM_SHARE_EXPORT_IP:-$HOST_IP}
+MANILA_LVM_SHARE_EXPORT_IPS=${MANILA_LVM_SHARE_EXPORT_IPS:-$HOST_IP}
 
 # Options for replication
 MANILA_REPLICA_STATE_UPDATE_INTERVAL=${MANILA_REPLICA_STATE_UPDATE_INTERVAL:-300}

manila/share/drivers/helpers.py

@@ -76,9 +76,15 @@ class NASHelperBase(object):
 
     @staticmethod
     def _verify_server_has_public_address(server):
-        if 'public_address' not in server:
+        if 'public_address' in server:
+            pass
+        elif 'public_addresses' in server:
+            if not isinstance(server['public_addresses'], list):
+                raise exception.ManilaException(_("public_addresses must be "
+                                                  "a list"))
+        else:
             raise exception.ManilaException(
-                _("Can not get 'public_address' for generation of export."))
+                _("Can not get public_address(es) for generation of export."))
 
     def _get_export_location_template(self, export_location_or_path):
         """Returns template of export location.
@@ -97,24 +103,29 @@ class NASHelperBase(object):
             export_location_or_path)
         export_locations = []
 
+        if 'public_addresses' in server:
+            pairs = list(map(lambda addr: (addr, False),
+                             server['public_addresses']))
+        else:
+            pairs = [(server['public_address'], False)]
+
         # NOTE(vponomaryov):
         # Generic driver case: 'admin_ip' exists only in case of DHSS=True
         # mode and 'ip' exists in case of DHSS=False mode.
         # Use one of these for creation of export location for service needs.
-        # LVM driver will have only single export location.
         service_address = server.get("admin_ip", server.get("ip"))
-        for ip, is_admin in ((server['public_address'], False),
+        if service_address:
-                             (service_address, True)):
+            pairs.append((service_address, True))
-            if ip:
+        for ip, is_admin in pairs:
-                export_locations.append({
+            export_locations.append({
-                    "path": export_location_template % ip,
+                "path": export_location_template % ip,
-                    "is_admin_only": is_admin,
+                "is_admin_only": is_admin,
-                    "metadata": {
+                "metadata": {
-                        # TODO(vponomaryov): remove this fake metadata when
+                    # TODO(vponomaryov): remove this fake metadata when
-                        # proper appears.
+                    # proper appears.
-                        "export_location_metadata_example": "example",
+                    "export_location_metadata_example": "example",
-                    },
+                },
-                })
+            })
         return export_locations
 
     def get_share_path_by_export_location(self, server, export_location):

manila/share/drivers/lvm.py

@@ -40,7 +40,11 @@ share_opts = [
                default='$state_path/mnt',
                help='Base folder where exported shares are located.'),
     cfg.StrOpt('lvm_share_export_ip',
+               deprecated_for_removal=True,
+               deprecated_reason='Use lvm_share_export_ips instead.',
                help='IP to be added to export string.'),
+    cfg.ListOpt('lvm_share_export_ips',
+                help='List of IPs to export shares.'),
     cfg.IntOpt('lvm_share_mirrors',
                default=0,
                help='If set, create LVMs with multiple mirrors. Note that '
@@ -68,11 +72,19 @@ class LVMMixin(driver.ExecuteMixin):
                                  run_as_root=True)
         volume_groups = out.split()
         if self.configuration.lvm_share_volume_group not in volume_groups:
-            msg = (_("share volume group %s doesn't exist")
+            msg = (_("Share volume group %s doesn't exist.")
                    % self.configuration.lvm_share_volume_group)
             raise exception.InvalidParameterValue(err=msg)
-        if not self.configuration.lvm_share_export_ip:
+
-            msg = (_("lvm_share_export_ip isn't specified"))
+        if (self.configuration.lvm_share_export_ip and
+                self.configuration.lvm_share_export_ips):
+            msg = (_("Only one of lvm_share_export_ip or lvm_share_export_ips"
+                     " may be specified."))
+            raise exception.InvalidParameterValue(err=msg)
+        if not (self.configuration.lvm_share_export_ip or
+                self.configuration.lvm_share_export_ips):
+            msg = (_("Neither lvm_share_export_ip nor lvm_share_export_ips is"
+                     " specified."))
             raise exception.InvalidParameterValue(err=msg)
 
     def _allocate_container(self, share):
@@ -146,10 +158,15 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver):
         # Set of parameters used for compatibility with
         # Generic driver's helpers.
         self.share_server = {
-            'public_address': self.configuration.lvm_share_export_ip,
             'instance_id': self.backend_name,
             'lock_name': 'manila_lvm',
         }
+        if self.configuration.lvm_share_export_ip:
+            self.share_server['public_addresses'] = [
+                self.configuration.lvm_share_export_ip]
+        else:
+            self.share_server['public_addresses'] = (
+                self.configuration.lvm_share_export_ips)
 
     def _ssh_exec_as_root(self, server, command, check_exit_code=True):
         kwargs = {}

manila/tests/share/drivers/test_helpers.py

@@ -215,8 +215,8 @@ class NFSHelperTestCase(test.TestCase):
         self._helper._ssh_exec.assert_has_calls(
             [mock.call(self.server, mock.ANY) for i in range(1)])
 
-    @ddt.data('/foo/bar', '5.6.7.8:/bar/quuz', '5.6.7.88:/foo/quuz')
+    @ddt.data('/foo/bar', '5.6.7.8:/bar/quuz', '5.6.7.9:/foo/quuz')
-    def test_get_exports_for_share(self, export_location):
+    def test_get_exports_for_share_single_ip(self, export_location):
         server = dict(public_address='1.2.3.4')
 
         result = self._helper.get_exports_for_share(server, export_location)
@@ -229,6 +229,23 @@ class NFSHelperTestCase(test.TestCase):
         ]
         self.assertEqual(expected_export_locations, result)
 
+    @ddt.data('/foo/bar', '5.6.7.8:/bar/quuz', '5.6.7.9:/foo/quuz')
+    def test_get_exports_for_share_multi_ip(self, export_location):
+        server = dict(public_addresses=['1.2.3.4', '1.2.3.5'])
+
+        result = self._helper.get_exports_for_share(server, export_location)
+
+        path = export_location.split(':')[-1]
+        expected_export_locations = list(map(
+            lambda addr: {
+                "is_admin_only": False,
+                "path": "%s:%s" % (addr, path),
+                "metadata": {"export_location_metadata_example": "example"}
+            },
+            server['public_addresses'])
+        )
+        self.assertEqual(expected_export_locations, result)
+
     @ddt.data(
         {'public_address_with_suffix': 'foo'},
         {'with_prefix_public_address': 'bar'},

manila/tests/share/drivers/test_lvm.py

@@ -87,7 +87,7 @@ class LVMShareDriverTestCase(test.TestCase):
         self._context = context.get_admin_context()
 
         CONF.set_default('lvm_share_volume_group', 'fakevg')
-        CONF.set_default('lvm_share_export_ip', '10.0.0.1')
+        CONF.set_default('lvm_share_export_ips', ['10.0.0.1', '10.0.0.2'])
         CONF.set_default('driver_handles_share_servers', False)
         CONF.set_default('reserved_share_percentage', 50)
 
@@ -108,7 +108,7 @@ class LVMShareDriverTestCase(test.TestCase):
         self.access = fake_access()
         self.snapshot = fake_snapshot()
         self.server = {
-            'public_address': self.fake_conf.lvm_share_export_ip,
+            'public_addresses': self.fake_conf.lvm_share_export_ips,
             'instance_id': 'LVM',
             'lock_name': 'manila_lvm',
         }
@@ -148,13 +148,33 @@ class LVMShareDriverTestCase(test.TestCase):
         self.assertRaises(exception.InvalidParameterValue,
                           self._driver.check_for_setup_error)
 
-    def test_check_for_setup_error_no_export_ip(self):
+    def test_check_for_setup_error_deprecated_export_ip(self):
         def exec_runner(*ignore_args, **ignore_kwargs):
             return '\n   fake1\n   fakevg\n   fake2\n', ''
 
         fake_utils.fake_execute_set_repliers([('vgs --noheadings -o name',
                                                exec_runner)])
-        CONF.set_default('lvm_share_export_ip', None)
+        CONF.set_default('lvm_share_export_ip', CONF.lvm_share_export_ips[0])
+        CONF.set_default('lvm_share_export_ips', None)
+        self.assertIsNone(self._driver.check_for_setup_error())
+
+    def test_check_for_setup_error_no_export_ips(self):
+        def exec_runner(*ignore_args, **ignore_kwargs):
+            return '\n   fake1\n   fakevg\n   fake2\n', ''
+
+        fake_utils.fake_execute_set_repliers([('vgs --noheadings -o name',
+                                               exec_runner)])
+        CONF.set_default('lvm_share_export_ips', None)
+        self.assertRaises(exception.InvalidParameterValue,
+                          self._driver.check_for_setup_error)
+
+    def test_check_for_setup_error_both_export_ip_and_ips(self):
+        def exec_runner(*ignore_args, **ignore_kwargs):
+            return '\n   fake1\n   fakevg\n   fake2\n', ''
+
+        fake_utils.fake_execute_set_repliers([('vgs --noheadings -o name',
+                                               exec_runner)])
+        CONF.set_default('lvm_share_export_ip', CONF.lvm_share_export_ips[0])
         self.assertRaises(exception.InvalidParameterValue,
                           self._driver.check_for_setup_error)
 

releasenotes/notes/lvm-export-ips-5f73f30df94381d3.yaml

@@ -0,0 +1,15 @@
+---
+features:
+  - Added new config option 'lvm_share_export_ips' which
+    allows a list of IP addresses to use for export
+    locations for the LVM driver. Every share created
+    will be exported on every IP address. This new option
+    supercedes 'lvm_share_export_ip'.
+upgrade:
+  - After upgrading, rename lvm_share_export_ip to
+    lvm_share_export_ips in the manila.conf file to avoid
+    a deprecation warning. As long as the list remains a
+    single element, functionality is unchanged.
+deprecations:
+  - The 'lvm_share_export_ip' option is deprecated and
+    will be removed. Use 'lvm_share_export_ips' instead.