From 42db1e34a7aafa14073d47b02c0753b20ecbbb9e Mon Sep 17 00:00:00 2001 From: Ben Swartzlander Date: Thu, 30 Nov 2017 11:01:44 -0500 Subject: [PATCH] Simplify the way drivers report support for ipv6 Driver shouldn't have to report support for ipv6 in two places. Drivers that assert ipv6_implemented=True just need to implement get_configured_ip_versions. Likewise ipv4 support is computed by the same method. Closes-bug: #1734127 Change-Id: I382767918a65b91e99ac1e604304ad01fac332e6 --- manila/network/__init__.py | 15 +-- manila/share/driver.py | 71 +++++++------- manila/share/drivers/lvm.py | 4 +- .../dataontap/cluster_mode/drv_multi_svm.py | 4 +- .../dataontap/cluster_mode/drv_single_svm.py | 4 +- .../dataontap/cluster_mode/lib_multi_svm.py | 2 +- .../dataontap/cluster_mode/lib_single_svm.py | 2 +- manila/tests/share/drivers/test_lvm.py | 6 +- manila/tests/share/test_driver.py | 94 +++++++++---------- manila/tests/test_network.py | 18 ++-- .../notes/bug-1734127-a239d022bef4a002.yaml | 4 + 11 files changed, 106 insertions(+), 118 deletions(-) create mode 100644 releasenotes/notes/bug-1734127-a239d022bef4a002.yaml diff --git a/manila/network/__init__.py b/manila/network/__init__.py index d3369dfbbe..31613ba812 100644 --- a/manila/network/__init__.py +++ b/manila/network/__init__.py @@ -106,15 +106,16 @@ class NetworkBaseAPI(db_base.Base): pass @property - def enabled_ip_version(self): - if not hasattr(self, '_enabled_ip_version'): + def enabled_ip_versions(self): + if not hasattr(self, '_enabled_ip_versions'): + self._enabled_ip_versions = set() if self.configuration.network_plugin_ipv6_enabled: - self._enabled_ip_version = 6 - elif self.configuration.network_plugin_ipv4_enabled: - self._enabled_ip_version = 4 - else: + self._enabled_ip_versions.add(6) + if self.configuration.network_plugin_ipv4_enabled: + self._enabled_ip_versions.add(4) + if not self._enabled_ip_versions: msg = _("Either 'network_plugin_ipv4_enabled' or " "'network_plugin_ipv6_enabled' " "should be configured to 'True'.") raise exception.NetworkBadConfigurationException(reason=msg) - return self._enabled_ip_version + return self._enabled_ip_versions diff --git a/manila/share/driver.py b/manila/share/driver.py index 0ba3850016..e3eee93e92 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -242,7 +242,7 @@ class ShareDriver(object): self.configuration = kwargs.get('configuration', None) self.initialized = False self._stats = {} - self.ip_version = None + self.ip_versions = None self.ipv6_implemented = False self.pools = [] @@ -1111,7 +1111,6 @@ class ShareDriver(object): replication_domain=self.replication_domain, filter_function=self.get_filter_function(), goodness_function=self.get_goodness_function(), - ipv4_support=True, ) if isinstance(data, dict): common.update(data) @@ -2447,13 +2446,20 @@ class ShareDriver(object): "shares created on it.") return [] - def get_configured_ip_version(self): - """"Get Configured IP versions when DHSS is false. + def get_configured_ip_versions(self): + """"Get allowed IP versions. The supported versions are returned with list, possible - values are: [4], [6] or [4, 6] - Each driver could override the method to return the IP version - which represents its self configuration. + values are: [4], [6], or [4, 6] + + Drivers that assert ipv6_implemented = True must override + this method. If the returned list includes 4, then shares + created by this driver must have an IPv4 export location. + If the list includes 6, then shares created by the driver + must have an IPv6 export location. + + Drivers should check that their storage controller actually + has IPv4/IPv6 enabled and configured properly. """ # For drivers that haven't implemented IPv6, assume legacy behavior @@ -2467,40 +2473,29 @@ class ShareDriver(object): When DHSS is true, the capabilities are determined by driver and configured network plugin. - When DHSS is false, the capabilities are determined by driver and its - configuration. + When DHSS is false, the capabilities are determined by driver + only. :param data: the capability dictionary :returns: capability data """ - ipv4_support = data.get('ipv4_support', False) - ipv6_support = data.get('ipv6_support', False) - if self.ip_version is None: - if self.driver_handles_share_servers: - user_network_version = self.network_api.enabled_ip_version - if self.admin_network_api: - if (user_network_version == - self.admin_network_api.enabled_ip_version): - self.ip_version = user_network_version - else: - LOG.warning("The enabled IP version for the admin " - "network plugin is different from " - "that of user network plugin, this " - "may lead to the backend never being " - "chosen by the scheduler when ip " - "version is specified in the share " - "type.") - else: - self.ip_version = user_network_version - else: - self.ip_version = self.get_configured_ip_version() + self.ip_versions = self.get_configured_ip_versions() + if isinstance(self.ip_versions, list): + self.ip_versions = set(self.ip_versions) + else: + self.ip_versions = set(list(self.ip_versions)) - if not isinstance(self.ip_version, list): - self.ip_version = [self.ip_version] - - data['ipv4_support'] = (4 in self.ip_version) and ipv4_support - data['ipv6_support'] = (6 in self.ip_version) and ipv6_support - if not (data['ipv4_support'] or data['ipv6_support']): - LOG.error("Backend %s capabilities 'ipv4_support' " - "and 'ipv6_support' are both False.", + if not self.ip_versions: + LOG.error("Backend %s supports neither IPv4 nor IPv6.", data['share_backend_name']) + + if self.driver_handles_share_servers: + network_versions = self.network_api.enabled_ip_versions + self.ip_versions = self.ip_versions & network_versions + if not self.ip_versions: + LOG.error("The enabled IP version of the network plugin is " + "not compatible with the version supported by " + "backend %s.", data['share_backend_name']) + + data['ipv4_support'] = (4 in self.ip_versions) + data['ipv6_support'] = (6 in self.ip_versions) return data diff --git a/manila/share/drivers/lvm.py b/manila/share/drivers/lvm.py index 45c11c18f0..e2fc3e695e 100644 --- a/manila/share/drivers/lvm.py +++ b/manila/share/drivers/lvm.py @@ -216,7 +216,6 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): 'mount_snapshot_support': True, 'driver_name': 'LVMShareDriver', 'pools': self.get_share_server_pools(), - 'ipv6_support': True } super(LVMShareDriver, self)._update_share_stats(data) @@ -432,8 +431,7 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): super(LVMShareDriver, self).delete_snapshot(context, snapshot, share_server) - def get_configured_ip_version(self): - """"Get Configured IP versions when DHSS is false.""" + def get_configured_ip_versions(self): if self.configured_ip_version is None: try: self.configured_ip_version = [] diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py index 03a07ba218..0b66068415 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py @@ -235,5 +235,5 @@ class NetAppCmodeMultiSvmShareDriver(driver.ShareDriver): fallback_create, share_server) - def get_configured_ip_version(self): - return self.library.get_configured_ip_version() + def get_configured_ip_versions(self): + return self.library.get_configured_ip_versions() diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py index 46e8392ae1..f46b75c820 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py @@ -251,5 +251,5 @@ class NetAppCmodeSingleSvmShareDriver(driver.ShareDriver): fallback_create, share_server) - def get_configured_ip_version(self): - return self.library.get_configured_ip_version() + def get_configured_ip_versions(self): + return self.library.get_configured_ip_versions() diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py index f9bd64004d..a3e08e50c1 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py @@ -385,7 +385,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary( except exception.NetAppException: LOG.exception("Deleting Vserver VLAN failed.") - def get_configured_ip_version(self): + def get_configured_ip_versions(self): versions = [4] options = self._client.get_net_options() if options['ipv6-enabled']: diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py index e5acb5a126..41c8aece3a 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py @@ -148,7 +148,7 @@ class NetAppCmodeSingleSVMFileStorageLibrary( return 0 @na_utils.trace - def get_configured_ip_version(self): + def get_configured_ip_versions(self): ipv4 = False ipv6 = False vserver_client = self._get_api_client(vserver=self._vserver) diff --git a/manila/tests/share/drivers/test_lvm.py b/manila/tests/share/drivers/test_lvm.py index 4dc2fc6f30..525c6410d0 100644 --- a/manila/tests/share/drivers/test_lvm.py +++ b/manila/tests/share/drivers/test_lvm.py @@ -407,16 +407,16 @@ class LVMShareDriverTestCase(test.TestCase): (None, ['1001::1001'], [6]), (None, ['1.1.1.0'], [4]), (None, ['1001::1001/129', '1.1.1.0'], False)) @ddt.unpack - def test_get_configured_ip_version( + def test_get_configured_ip_versions( self, configured_ip, configured_ips, configured_ip_version): CONF.set_default('lvm_share_export_ip', configured_ip) CONF.set_default('lvm_share_export_ips', configured_ips) if configured_ip_version: self.assertEqual(configured_ip_version, - self._driver.get_configured_ip_version()) + self._driver.get_configured_ip_versions()) else: self.assertRaises(exception.InvalidInput, - self._driver.get_configured_ip_version) + self._driver.get_configured_ip_versions) def test_mount_device(self): mount_path = self._get_mount_path(self.share) diff --git a/manila/tests/share/test_driver.py b/manila/tests/share/test_driver.py index 4a390850da..086de24309 100644 --- a/manila/tests/share/test_driver.py +++ b/manila/tests/share/test_driver.py @@ -1085,67 +1085,57 @@ class ShareDriverTestCase(test.TestCase): 'fake_context', 'fake_snapshot', ['r1', 'r2'], [], []) - @ddt.data({'capability': (True, True), - 'user_admin_networks': [[4], [4]], + @ddt.data({'user_networks': set([4]), 'conf': [4], 'expected': {'ipv4': True, 'ipv6': False}}, - {'capability': (True, True), - 'user_admin_networks': [[6], [6]], - 'expected': {'ipv4': False, 'ipv6': True}}, - {'capability': (False, False), - 'user_admin_networks': [[4], [4]], + {'user_networks': set([6]), 'conf': [4], 'expected': {'ipv4': False, 'ipv6': False}}, - {'capability': (True, True), - 'user_admin_networks': [[4], [6]], - 'expected': {'ipv4': False, 'ipv6': False}}, - {'capability': (False, False), - 'user_admin_networks': [[6], [4]], - 'expected': {'ipv4': False, 'ipv6': False}},) - @ddt.unpack - def test_add_ip_version_capability_if_dhss_true(self, capability, - user_admin_networks, - expected): - share_driver = self._instantiate_share_driver(None, True) - version = PropertyMock(side_effect=user_admin_networks) - type(share_driver.network_api).enabled_ip_version = version - data = {'share_backend_name': 'fake_backend', - 'ipv4_support': capability[0], - 'ipv6_support': capability[1]} - - result = share_driver.add_ip_version_capability(data) - - self.assertIsNotNone(result['ipv4_support']) - self.assertEqual(expected['ipv4'], result['ipv4_support']) - self.assertIsNotNone(result['ipv6_support']) - self.assertEqual(expected['ipv6'], result['ipv6_support']) - - @ddt.data({'capability': (True, False), - 'conf': [4], + {'user_networks': set([4, 6]), 'conf': [4], 'expected': {'ipv4': True, 'ipv6': False}}, - {'capability': (True, True), - 'conf': [6], - 'expected': {'ipv4': False, 'ipv6': True}}, - {'capability': (False, False), - 'conf': [4], + {'user_networks': set([4]), 'conf': [6], 'expected': {'ipv4': False, 'ipv6': False}}, - {'capability': (False, True), - 'conf': [4], - 'expected': {'ipv4': False, 'ipv6': False}}, - {'capability': (False, True), - 'conf': [6], + {'user_networks': set([6]), 'conf': [6], 'expected': {'ipv4': False, 'ipv6': True}}, - {'capability': (True, True), - 'conf': [4, 6], + {'user_networks': set([4, 6]), 'conf': [6], + 'expected': {'ipv4': False, 'ipv6': True}}, + {'user_networks': set([4]), 'conf': [4, 6], + 'expected': {'ipv4': True, 'ipv6': False}}, + {'user_networks': set([6]), 'conf': [4, 6], + 'expected': {'ipv4': False, 'ipv6': True}}, + {'user_networks': set([4, 6]), 'conf': [4, 6], 'expected': {'ipv4': True, 'ipv6': True}}, ) @ddt.unpack - def test_add_ip_version_capability_if_dhss_false(self, capability, - conf, expected): - share_driver = self._instantiate_share_driver(None, False) - self.mock_object(share_driver, 'get_configured_ip_version', + def test_add_ip_version_capability_if_dhss_true(self, + user_networks, + conf, + expected): + share_driver = self._instantiate_share_driver(None, True) + self.mock_object(share_driver, 'get_configured_ip_versions', mock.Mock(return_value=conf)) - data = {'share_backend_name': 'fake_backend', - 'ipv4_support': capability[0], - 'ipv6_support': capability[1]} + versions = PropertyMock(return_value=user_networks) + type(share_driver.network_api).enabled_ip_versions = versions + data = {'share_backend_name': 'fake_backend'} + + result = share_driver.add_ip_version_capability(data) + + self.assertIsNotNone(result['ipv4_support']) + self.assertEqual(expected['ipv4'], result['ipv4_support']) + self.assertIsNotNone(result['ipv6_support']) + self.assertEqual(expected['ipv6'], result['ipv6_support']) + + @ddt.data({'conf': [4], + 'expected': {'ipv4': True, 'ipv6': False}}, + {'conf': [6], + 'expected': {'ipv4': False, 'ipv6': True}}, + {'conf': [4, 6], + 'expected': {'ipv4': True, 'ipv6': True}}, + ) + @ddt.unpack + def test_add_ip_version_capability_if_dhss_false(self, conf, expected): + share_driver = self._instantiate_share_driver(None, False) + self.mock_object(share_driver, 'get_configured_ip_versions', + mock.Mock(return_value=conf)) + data = {'share_backend_name': 'fake_backend'} result = share_driver.add_ip_version_capability(data) self.assertIsNotNone(result['ipv4_support']) diff --git a/manila/tests/test_network.py b/manila/tests/test_network.py index b3b03b2adb..e4ece45ea0 100644 --- a/manila/tests/test_network.py +++ b/manila/tests/test_network.py @@ -129,12 +129,12 @@ class NetworkBaseAPITestCase(test.TestCase): exception.NetworkBadConfigurationException, result._verify_share_network, 'foo_id', None) - @ddt.data((True, False, 6), (False, True, 4), - (True, True, 6), (None, None, False)) + @ddt.data((True, False, set([6])), (False, True, set([4])), + (True, True, set([4, 6])), (False, False, set())) @ddt.unpack - def test_enabled_ip_version(self, network_plugin_ipv6_enabled, - network_plugin_ipv4_enabled, - enable_ip_version): + def test_enabled_ip_versions(self, network_plugin_ipv6_enabled, + network_plugin_ipv4_enabled, + enable_ip_versions): class FakeNetworkAPI(network.NetworkBaseAPI): def allocate_network(self, *args, **kwargs): pass @@ -149,9 +149,9 @@ class NetworkBaseAPITestCase(test.TestCase): result = FakeNetworkAPI() - if enable_ip_version: - self.assertTrue(hasattr(result, 'enabled_ip_version')) - self.assertEqual(enable_ip_version, result.enabled_ip_version) + if enable_ip_versions: + self.assertTrue(hasattr(result, 'enabled_ip_versions')) + self.assertEqual(enable_ip_versions, result.enabled_ip_versions) else: self.assertRaises(exception.NetworkBadConfigurationException, - getattr, result, 'enabled_ip_version') + getattr, result, 'enabled_ip_versions') diff --git a/releasenotes/notes/bug-1734127-a239d022bef4a002.yaml b/releasenotes/notes/bug-1734127-a239d022bef4a002.yaml new file mode 100644 index 0000000000..8ce6b3b304 --- /dev/null +++ b/releasenotes/notes/bug-1734127-a239d022bef4a002.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixed logic in driver base class that determines whether + IPv6 is supported at runtime.