diff --git a/manila/share/drivers/service_instance.py b/manila/share/drivers/service_instance.py index d0fbca7da1..1c129a2f74 100644 --- a/manila/share/drivers/service_instance.py +++ b/manila/share/drivers/service_instance.py @@ -39,7 +39,6 @@ from manila import utils LOG = log.getLogger(__name__) NEUTRON_NAME = "neutron" -NOVA_NAME = "nova" share_servers_handling_mode_opts = [ cfg.StrOpt( @@ -102,11 +101,6 @@ share_servers_handling_mode_opts = [ help="Attach share server directly to share network. " "Used only with Neutron and " "if driver_handles_share_servers=True."), - cfg.StrOpt( - "service_instance_network_helper_type", - default=NEUTRON_NAME, - help="Allowed values are %s. " % [NOVA_NAME, NEUTRON_NAME] + - "Only used if driver_handles_share_servers=True."), cfg.StrOpt( "admin_network_id", help="ID of neutron network used to communicate with admin network," @@ -183,22 +177,12 @@ class ServiceInstanceManager(object): return CONF.get(key) def _get_network_helper(self): - network_helper_type = ( - self.get_config_option( - "service_instance_network_helper_type").lower()) - if network_helper_type == NEUTRON_NAME: - return NeutronNetworkHelper(self) - elif network_helper_type == NOVA_NAME: - return NovaNetworkHelper(self) - else: - raise exception.ManilaException( - _("Wrong value '%(provided)s' for config opt " - "'service_instance_network_helper_type'. " - "Allowed values are %(allowed)s.") % dict( - provided=network_helper_type, - allowed=[NOVA_NAME, NEUTRON_NAME])) + # Historically, there were multiple types of network helper, + # but currently the only network helper type is Neutron. + return NeutronNetworkHelper(self) def __init__(self, driver_config=None): + super(ServiceInstanceManager, self).__init__() self.driver_config = driver_config @@ -558,32 +542,20 @@ class ServiceInstanceManager(object): security_group = self._get_or_create_security_group(context) if security_group: - if self.network_helper.NAME == NOVA_NAME: - # NOTE(vponomaryov): Nova-network allows to assign - # secgroups only by names. - sg_id = security_group.name - else: - sg_id = security_group.id + sg_id = security_group.id LOG.debug( "Adding security group '%(sg)s' to server '%(si)s'.", dict(sg=sg_id, si=service_instance["id"])) self.compute_api.add_security_group_to_server( context, service_instance["id"], sg_id) - if self.network_helper.NAME == NEUTRON_NAME: - ip = (network_data.get('service_port', - network_data.get( - 'admin_port'))['fixed_ips']) - service_instance['ip'] = ip[0]['ip_address'] - public_ip = (network_data.get( - 'public_port', network_data.get( - 'service_port'))['fixed_ips']) - service_instance['public_address'] = public_ip[0]['ip_address'] - else: - net_name = self.network_helper.get_network_name(network_info) - service_instance['ip'] = self._get_server_ip( - service_instance, net_name) - service_instance['public_address'] = service_instance['ip'] + ip = (network_data.get('service_port', + network_data.get( + 'admin_port'))['fixed_ips']) + service_instance['ip'] = ip[0]['ip_address'] + public_ip = (network_data.get('public_port', network_data.get( + 'service_port'))['fixed_ips']) + service_instance['public_address'] = public_ip[0]['ip_address'] except Exception as e: e.detail_data = {'server_details': fail_safe_data} @@ -1087,50 +1059,3 @@ class NeutronNetworkHelper(BaseNetworkhelper): for subnet_id in service_network['subnets']: subnets.append(self.neutron_api.get_subnet(subnet_id)) return subnets - - -class NovaNetworkHelper(BaseNetworkhelper): - """Nova network helper for Manila service instances. - - All security-group rules are applied to all interfaces of Nova VM - using Nova-network. In that case there is no need to create additional - service network. Only one thing should be satisfied - Manila host - should have access to all tenant networks. - This network helper does not create resources. - """ - - def __init__(self, service_instance_manager): - self.compute_api = service_instance_manager.compute_api - self.admin_context = service_instance_manager.admin_context - - @property - def NAME(self): - return NOVA_NAME - - def setup_network(self, network_info): - net = self._get_nova_network(network_info['nova_net_id']) - network_info['nics'] = [{'net-id': net['id']}] - return network_info - - def get_network_name(self, network_info): - """Returns name of network for service instance.""" - return self._get_nova_network(network_info['nova_net_id'])['label'] - - def teardown_network(self, server_details): - """Nothing to do. Placeholder.""" - - def setup_connectivity_with_service_instances(self): - """Nothing to do. Placeholder.""" - - def _get_nova_network(self, nova_network_id): - """Returns network to be used for service instance. - - :param nova_network_id: string with id of network. - :returns: dict -- network data as dict - :raises: exception.ManilaException - """ - if not nova_network_id: - raise exception.ManilaException( - _('Nova network for service instance is not provided.')) - net = self.compute_api.network_get(self.admin_context, nova_network_id) - return net diff --git a/manila/tests/share/drivers/test_service_instance.py b/manila/tests/share/drivers/test_service_instance.py index 3e6c4b0d51..b5ab8963ad 100644 --- a/manila/tests/share/drivers/test_service_instance.py +++ b/manila/tests/share/drivers/test_service_instance.py @@ -67,8 +67,6 @@ def fake_get_config_option(key): return '99.254.0.0/24' elif key == 'service_network_division_mask': return 27 - elif key == 'service_instance_network_helper_type': - return service_instance.NEUTRON_NAME elif key == 'service_network_name': return 'fake_service_network_name' elif key == 'interface_driver': @@ -97,7 +95,7 @@ class FakeNetworkHelper(service_instance.BaseNetworkhelper): @property def NAME(self): - return self.get_config_option("service_instance_network_helper_type") + return service_instance.NEUTRON_NAME def __init__(self, service_instance_manager): self.get_config_option = service_instance_manager.get_config_option @@ -131,8 +129,6 @@ class ServiceInstanceManagerTestCase(test.TestCase): service_instance.os.path, 'exists', mock.Mock(return_value=True)) self.mock_object(service_instance, 'NeutronNetworkHelper', mock.Mock(side_effect=FakeNetworkHelper)) - self.mock_object(service_instance, 'NovaNetworkHelper', - mock.Mock(side_effect=FakeNetworkHelper)) self._manager = service_instance.ServiceInstanceManager(self.config) self._manager._execute = mock.Mock(return_value=('', '')) self.mock_object(time, 'sleep') @@ -159,64 +155,23 @@ class ServiceInstanceManagerTestCase(test.TestCase): result = self._manager.get_config_option('service_instance_user') self.assertEqual(username, result) - def test_get_nova_network_helper(self): - # Mock it again, because one of these was called in setUp method. - self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') - config_data = dict(DEFAULT=dict( - service_instance_user='fake_username', - driver_handles_share_servers=True, - service_instance_network_helper_type=service_instance.NOVA_NAME)) - with test_utils.create_temp_config_with_opts(config_data): - self._manager = service_instance.ServiceInstanceManager() - self._manager.network_helper - service_instance.NovaNetworkHelper.assert_called_once_with( - self._manager) - self.assertFalse(service_instance.NeutronNetworkHelper.called) - def test_get_neutron_network_helper(self): - # Mock it again, because one of these was called in setUp method. + # Mock it again, because it was called in setUp method. self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') - config_data = dict(DEFAULT=dict( - service_instance_user='fake_username', - driver_handles_share_servers=True, - service_instance_network_helper_type=service_instance.NEUTRON_NAME) - ) + config_data = dict(DEFAULT=dict(service_instance_user='fake_username', + driver_handles_share_servers=True)) + with test_utils.create_temp_config_with_opts(config_data): self._manager = service_instance.ServiceInstanceManager() self._manager.network_helper service_instance.NeutronNetworkHelper.assert_called_once_with( self._manager) - self.assertFalse(service_instance.NovaNetworkHelper.called) - - @ddt.data( - None, '', 'fake', service_instance.NOVA_NAME + '_as_prefix', - service_instance.NEUTRON_NAME + '_as_prefix', - 'as_suffix_' + service_instance.NOVA_NAME, - 'as_suffix_' + service_instance.NEUTRON_NAME) - def test_get_fake_network_helper(self, value): - # Mock it again, because one of these was called in setUp method. - self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') - config_data = dict(DEFAULT=dict( - service_instance_user='fake_username', - driver_handles_share_servers=True, - service_instance_network_helper_type=value)) - with test_utils.create_temp_config_with_opts(config_data): - manager = service_instance.ServiceInstanceManager() - self.assertRaises(exception.ManilaException, - lambda: manager.network_helper) - self.assertFalse(service_instance.NeutronNetworkHelper.called) - self.assertFalse(service_instance.NovaNetworkHelper.called) def test_init_with_driver_config_and_handling_of_share_servers(self): self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') config_data = dict(CUSTOM=dict( driver_handles_share_servers=True, - service_instance_user='fake_user', - service_instance_network_helper_type=service_instance.NOVA_NAME)) + service_instance_user='fake_user')) opts = service_instance.common_opts + driver.share_opts with test_utils.create_temp_config_with_opts(config_data): self.config = configuration.Configuration(opts, 'CUSTOM') @@ -226,12 +181,10 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.get_config_option("driver_handles_share_servers")) self.assertIsNotNone(self._manager.driver_config) self.assertTrue(hasattr(self._manager, 'network_helper')) - self.assertTrue(service_instance.NovaNetworkHelper.called) - self.assertFalse(service_instance.NeutronNetworkHelper.called) + self.assertTrue(service_instance.NeutronNetworkHelper.called) def test_init_with_driver_config_and_wo_handling_of_share_servers(self): self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') config_data = dict(CUSTOM=dict( driver_handles_share_servers=False, service_instance_user='fake_user')) @@ -242,28 +195,23 @@ class ServiceInstanceManagerTestCase(test.TestCase): self.config) self.assertIsNotNone(self._manager.driver_config) self.assertFalse(hasattr(self._manager, 'network_helper')) - self.assertFalse(service_instance.NovaNetworkHelper.called) self.assertFalse(service_instance.NeutronNetworkHelper.called) def test_init_with_common_config_and_handling_of_share_servers(self): self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') config_data = dict(DEFAULT=dict( service_instance_user='fake_username', - driver_handles_share_servers=True, - service_instance_network_helper_type=service_instance.NOVA_NAME)) + driver_handles_share_servers=True)) with test_utils.create_temp_config_with_opts(config_data): self._manager = service_instance.ServiceInstanceManager() self.assertTrue( self._manager.get_config_option("driver_handles_share_servers")) self.assertIsNone(self._manager.driver_config) self.assertTrue(hasattr(self._manager, 'network_helper')) - self.assertTrue(service_instance.NovaNetworkHelper.called) - self.assertFalse(service_instance.NeutronNetworkHelper.called) + self.assertTrue(service_instance.NeutronNetworkHelper.called) def test_init_with_common_config_and_wo_handling_of_share_servers(self): self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') config_data = dict(DEFAULT=dict( service_instance_user='fake_username', driver_handles_share_servers=False)) @@ -274,7 +222,6 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.get_config_option("driver_handles_share_servers")) self.assertIsNone(self._manager.driver_config) self.assertFalse(hasattr(self._manager, 'network_helper')) - self.assertFalse(service_instance.NovaNetworkHelper.called) self.assertFalse(service_instance.NeutronNetworkHelper.called) def test_no_service_user_defined(self): @@ -290,11 +237,9 @@ class ServiceInstanceManagerTestCase(test.TestCase): def test_get_service_instance_name_using_driver_config(self): fake_server_id = 'fake_share_server_id_%s' % self.id() self.mock_object(service_instance, 'NeutronNetworkHelper') - self.mock_object(service_instance, 'NovaNetworkHelper') config_data = dict(CUSTOM=dict( driver_handles_share_servers=True, - service_instance_user='fake_user', - service_instance_network_helper_type=service_instance.NOVA_NAME)) + service_instance_user='fake_user')) opts = service_instance.common_opts + driver.share_opts with test_utils.create_temp_config_with_opts(config_data): self.config = configuration.Configuration(opts, 'CUSTOM') @@ -310,14 +255,12 @@ class ServiceInstanceManagerTestCase(test.TestCase): self.assertTrue( self._manager.get_config_option("driver_handles_share_servers")) self.assertTrue(hasattr(self._manager, 'network_helper')) - self.assertTrue(service_instance.NovaNetworkHelper.called) - self.assertFalse(service_instance.NeutronNetworkHelper.called) + self.assertTrue(service_instance.NeutronNetworkHelper.called) def test_get_service_instance_name_using_default_config(self): fake_server_id = 'fake_share_server_id_%s' % self.id() config_data = dict(CUSTOM=dict( - service_instance_user='fake_user', - service_instance_network_helper_type=service_instance.NOVA_NAME)) + service_instance_user='fake_user')) with test_utils.create_temp_config_with_opts(config_data): self._manager = service_instance.ServiceInstanceManager() result = self._manager._get_service_instance_name(fake_server_id) @@ -943,16 +886,12 @@ class ServiceInstanceManagerTestCase(test.TestCase): self.assertTrue( self._manager.compute_api.server_get_by_name_or_id.called) - @ddt.data(service_instance.NOVA_NAME, service_instance.NEUTRON_NAME) - def test___create_service_instance_with_sg_success(self, helper_type): + def test___create_service_instance_with_sg_success(self): self.mock_object(service_instance, 'NeutronNetworkHelper', mock.Mock(side_effect=FakeNetworkHelper)) - self.mock_object(service_instance, 'NovaNetworkHelper', - mock.Mock(side_effect=FakeNetworkHelper)) config_data = dict(DEFAULT=dict( driver_handles_share_servers=True, - service_instance_user='fake_user', - service_instance_network_helper_type=helper_type)) + service_instance_user='fake_user')) with test_utils.create_temp_config_with_opts(config_data): self._manager = service_instance.ServiceInstanceManager() @@ -965,19 +904,17 @@ class ServiceInstanceManagerTestCase(test.TestCase): instance_name = 'fake_instance_name' network_info = dict() network_data = {'nics': ['fake_nic1', 'fake_nic2']} - if helper_type == service_instance.NEUTRON_NAME: - network_data['router'] = dict(id='fake_router_id') + network_data['router'] = dict(id='fake_router_id') server_get = dict( id='fakeid', status='ACTIVE', networks={net_name: [ip_address]}) - if helper_type == service_instance.NEUTRON_NAME: - network_data.update(dict( - router_id='fake_router_id', subnet_id='fake_subnet_id', - public_port=dict(id='fake_public_port', - fixed_ips=[dict(ip_address=ip_address)]), - service_port=dict(id='fake_service_port', - fixed_ips=[{'ip_address': ip_address}]), - admin_port={'id': 'fake_admin_port', - 'fixed_ips': [{'ip_address': ip_address}]})) + network_data.update(dict( + router_id='fake_router_id', subnet_id='fake_subnet_id', + public_port=dict(id='fake_public_port', + fixed_ips=[dict(ip_address=ip_address)]), + service_port=dict(id='fake_service_port', + fixed_ips=[{'ip_address': ip_address}]), + admin_port={'id': 'fake_admin_port', + 'fixed_ips': [{'ip_address': ip_address}]})) self.mock_object(service_instance.time, 'time', mock.Mock(return_value=5)) self.mock_object(self._manager.network_helper, 'setup_network', @@ -1005,13 +942,13 @@ class ServiceInstanceManagerTestCase(test.TestCase): 'subnet_id': network_data.get('subnet_id'), 'instance_id': server_get['id'], 'ip': ip_address, - 'networks': server_get['networks']} - if helper_type == service_instance.NEUTRON_NAME: - expected['router_id'] = network_data['router']['id'] - expected['public_port_id'] = 'fake_public_port' - expected['service_port_id'] = 'fake_service_port' - expected['admin_port_id'] = 'fake_admin_port' - expected['admin_ip'] = 'fake_ip_address' + 'networks': server_get['networks'], + 'router_id': network_data['router']['id'], + 'public_port_id': 'fake_public_port', + 'service_port_id': 'fake_service_port', + 'admin_port_id': 'fake_admin_port', + 'admin_ip': 'fake_ip_address', + } result = self._manager._create_service_instance( self._manager.admin_context, instance_name, network_info) @@ -1033,26 +970,17 @@ class ServiceInstanceManagerTestCase(test.TestCase): availability_zone=service_instance.CONF.storage_availability_zone) self._manager.compute_api.server_get.assert_called_once_with( self._manager.admin_context, server_create['id']) - if helper_type == service_instance.NEUTRON_NAME: - self._manager.compute_api.add_security_group_to_server.\ - assert_called_once_with( - self._manager.admin_context, server_get['id'], sg.id) - self._manager.network_helper.get_network_name.assert_has_calls([]) - else: - self._manager.compute_api.add_security_group_to_server.\ - assert_called_once_with( - self._manager.admin_context, server_get['id'], sg.name) - self._manager.network_helper.get_network_name.\ - assert_called_once_with(network_info) + self._manager.compute_api.add_security_group_to_server.\ + assert_called_once_with( + self._manager.admin_context, server_get['id'], sg.id) + self._manager.network_helper.get_network_name.assert_has_calls([]) def test___create_service_instance_neutron_no_admin_ip(self): self.mock_object(service_instance, 'NeutronNetworkHelper', mock.Mock(side_effect=FakeNetworkHelper)) config_data = {'DEFAULT': { 'driver_handles_share_servers': True, - 'service_instance_user': 'fake_user', - 'service_instance_network_helper_type': ( - service_instance.NEUTRON_NAME)}} + 'service_instance_user': 'fake_user'}} with test_utils.create_temp_config_with_opts(config_data): self._manager = service_instance.ServiceInstanceManager() @@ -2333,65 +2261,3 @@ class NeutronNetworkHelperTestCase(test.TestCase): instance.service_network_id) instance.neutron_api.get_subnet.assert_has_calls([ mock.call(subnet_id1), mock.call(subnet_id2)]) - - -@ddt.ddt -class NovaNetworkHelperTestCase(test.TestCase): - """Tests Nova network helper for service instance.""" - - def setUp(self): - super(NovaNetworkHelperTestCase, self).setUp() - self.fake_manager = FakeServiceInstance() - - def test_init(self): - instance = service_instance.NovaNetworkHelper(self.fake_manager) - self.assertEqual(service_instance.NOVA_NAME, instance.NAME) - self.assertIsNone(instance.teardown_network('fake')) - self.assertIsNone( - instance.setup_connectivity_with_service_instances()) - - def test_get_network_name(self): - network_info = dict(nova_net_id='fake_nova_net_id') - network = dict(label='fake_network') - instance = service_instance.NovaNetworkHelper(self.fake_manager) - self.mock_object(instance.compute_api, 'network_get', - mock.Mock(return_value=network)) - - result = instance.get_network_name(network_info) - - self.assertEqual(network['label'], result) - instance.compute_api.network_get.assert_called_once_with( - instance.admin_context, network_info['nova_net_id']) - - @ddt.data(None, [], {}, '') - def test_get_network_name_invalid(self, net_name): - network_info = dict(nova_net_id=net_name) - instance = service_instance.NovaNetworkHelper(self.fake_manager) - - self.assertRaises( - exception.ManilaException, instance.get_network_name, network_info) - - def test_setup_network(self): - network_info = dict(nova_net_id='fake_nova_net_id') - network = dict(label='fake_network', id='fake_network_id', - gateway='fake_gateway_ip') - instance = service_instance.NovaNetworkHelper(self.fake_manager) - self.mock_object(instance.compute_api, 'network_get', - mock.Mock(return_value=network)) - expected = { - 'nova_net_id': network_info['nova_net_id'], - 'nics': [{'net-id': network['id']}]} - - result = instance.setup_network(network_info) - - self.assertEqual(expected, result) - instance.compute_api.network_get.assert_called_once_with( - instance.admin_context, network_info['nova_net_id']) - - @ddt.data(None, [], {}, '') - def test_setup_network_invalid(self, net_name): - network_info = dict(nova_net_id=net_name) - instance = service_instance.NovaNetworkHelper(self.fake_manager) - - self.assertRaises( - exception.ManilaException, instance.get_network_name, network_info) diff --git a/releasenotes/notes/remove-nova-net-support-from-service-instance-module-dd7559803fa01d45.yaml b/releasenotes/notes/remove-nova-net-support-from-service-instance-module-dd7559803fa01d45.yaml new file mode 100644 index 0000000000..11949e8880 --- /dev/null +++ b/releasenotes/notes/remove-nova-net-support-from-service-instance-module-dd7559803fa01d45.yaml @@ -0,0 +1,5 @@ +--- +upgrade: + - Removed nova net support from the service instance module since + legacy nova networking was deprecated in Newton and is no longer + supported in regular deployments in Ocata.