From 3755978ef78cbfb9fec6a071390fcd6530f78717 Mon Sep 17 00:00:00 2001 From: Ben Swartzlander Date: Wed, 21 Jun 2017 14:51:06 -0400 Subject: [PATCH] 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 | 2 +- devstack/settings | 2 +- manila/share/drivers/helpers.py | 41 ++++++++++++------- manila/share/drivers/lvm.py | 25 +++++++++-- manila/tests/share/drivers/test_helpers.py | 21 +++++++++- manila/tests/share/drivers/test_lvm.py | 28 +++++++++++-- .../lvm-export-ips-5f73f30df94381d3.yaml | 15 +++++++ 7 files changed, 107 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/lvm-export-ips-5f73f30df94381d3.yaml diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 67f399d61a..68036c612b 100755 --- a/devstack/plugin.sh +++ b/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 diff --git a/devstack/settings b/devstack/settings index 42f1a3f253..8e06673bb6 100644 --- a/devstack/settings +++ b/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} diff --git a/manila/share/drivers/helpers.py b/manila/share/drivers/helpers.py index d82cf8f5e6..0e37443600 100644 --- a/manila/share/drivers/helpers.py +++ b/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), - (service_address, True)): - if ip: - export_locations.append({ - "path": export_location_template % ip, - "is_admin_only": is_admin, - "metadata": { - # TODO(vponomaryov): remove this fake metadata when - # proper appears. - "export_location_metadata_example": "example", - }, - }) + if service_address: + pairs.append((service_address, True)) + for ip, is_admin in pairs: + export_locations.append({ + "path": export_location_template % ip, + "is_admin_only": is_admin, + "metadata": { + # TODO(vponomaryov): remove this fake metadata when + # proper appears. + "export_location_metadata_example": "example", + }, + }) return export_locations def get_share_path_by_export_location(self, server, export_location): diff --git a/manila/share/drivers/lvm.py b/manila/share/drivers/lvm.py index 598952f411..d86b1261c5 100644 --- a/manila/share/drivers/lvm.py +++ b/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 = {} diff --git a/manila/tests/share/drivers/test_helpers.py b/manila/tests/share/drivers/test_helpers.py index c3cd1a242b..404640dda1 100644 --- a/manila/tests/share/drivers/test_helpers.py +++ b/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') - def test_get_exports_for_share(self, export_location): + @ddt.data('/foo/bar', '5.6.7.8:/bar/quuz', '5.6.7.9:/foo/quuz') + 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'}, diff --git a/manila/tests/share/drivers/test_lvm.py b/manila/tests/share/drivers/test_lvm.py index 1acdad3857..e70a6ea204 100644 --- a/manila/tests/share/drivers/test_lvm.py +++ b/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) diff --git a/releasenotes/notes/lvm-export-ips-5f73f30df94381d3.yaml b/releasenotes/notes/lvm-export-ips-5f73f30df94381d3.yaml new file mode 100644 index 0000000000..20b2b54dfa --- /dev/null +++ b/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.