openstack
/
manila
Code Issues Proposed changes

Do not use router for service instance with direct connect 

For Nova VMs we have two possible approaches for network access:
1) using router
2) using direct connection and two net interfaces.

Now code of "service_instance" module requires tenant network to be attached
to public router in both cases, but should do it only in first case.

Remove dependency on router when config option
'connect_share_server_to_tenant_network' is set to True.

Change-Id: Ib515c0b64b500712c5682c03fa69377eed8e038d
Closes-Bug: #1410246
This commit is contained in:
Valeriy Ponomaryov 2015-01-19 13:04:10 +02:00
parent 4fcce39981
commit efb4f1cdf5
3 changed files with 56 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
manila/share/drivers/generic.py
View File

@ -603,7 +603,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
instance_id)
self.service_instance_manager.delete_service_instance(
self.admin_context, instance_id, server_details['subnet_id'],
server_details['router_id'])
server_details.get('router_id'))
class NASHelperBase(object):

97
manila/share/drivers/service_instance.py
View File

@ -300,21 +300,20 @@ class ServiceInstanceManager(object):
:returns: dict with service instance details
:raises: exception.ServiceInstanceException
"""
server = self._create_service_instance(context,
instance_name,
neutron_net_id,
neutron_subnet_id)
server = self._create_service_instance(
context, instance_name, neutron_net_id, neutron_subnet_id)
instance_details = {'instance_id': server['id'],
'ip': server['ip'],
'pk_path': server['pk_path'],
'subnet_id': server['subnet_id'],
'router_id': server['router_id'],
'password': self.get_config_option(
'service_instance_password'),
'username': self.get_config_option(
'service_instance_user'),
'public_address': server['public_address']}
instance_details = {
'instance_id': server['id'],
'ip': server['ip'],
'pk_path': server['pk_path'],
'subnet_id': server['subnet_id'],
'password': self.get_config_option('service_instance_password'),
'username': self.get_config_option('service_instance_user'),
'public_address': server['public_address'],
}
if 'router_id' in server:
instance_details['router_id'] = server['router_id']
for key in ('password', 'pk_path'):
if not instance_details[key]:
instance_details.pop(key)
@ -445,16 +444,12 @@ class ServiceInstanceManager(object):
context,
service_instance["id"], security_group.id)
router, service_subnet, service_port = (
network_data['router'], network_data['service_subnet'],
network_data['service_port']
)
service_instance['ip'] = self._get_server_ip(service_instance)
service_instance['pk_path'] = key_path
service_instance['router_id'] = router['id']
service_instance['subnet_id'] = service_subnet['id']
service_instance['port_id'] = service_port['id']
if 'router' in network_data and 'id' in network_data['router']:
service_instance['router_id'] = network_data['router']['id']
service_instance['subnet_id'] = network_data['service_subnet']['id']
service_instance['port_id'] = network_data['service_port']['id']
try:
public_ip = network_data['public_port']
@ -494,48 +489,44 @@ class ServiceInstanceManager(object):
"""Sets up network for service vm."""
network_data = dict()
subnet_name = ('service_subnet_for_handling_of_share_server_for_'
'tenant_subnet_%s' % neutron_subnet_id)
# We get/create subnet for service instance that is routed to
# subnet provided in args.
subnet_name = "routed_to_%s" % neutron_subnet_id
service_subnet = self._get_service_subnet(subnet_name)
if not service_subnet:
service_subnet = self.neutron_api.subnet_create(
network_data['service_subnet'] = self._get_service_subnet(subnet_name)
if not network_data['service_subnet']:
network_data['service_subnet'] = self.neutron_api.subnet_create(
self.service_tenant_id,
self.service_network_id,
subnet_name,
self._get_cidr_for_subnet()
)
self._get_cidr_for_subnet())
network_data['service_subnet'] = service_subnet
network_data['router'] = router = self._get_private_router(
neutron_net_id, neutron_subnet_id)
try:
self.neutron_api.router_add_interface(router['id'],
service_subnet['id'])
except exception.NetworkException as e:
if e.kwargs['code'] != 400:
raise
LOG.debug('Subnet %(subnet_id)s is already attached to the '
'router %(router_id)s.',
{'subnet_id': service_subnet['id'],
'router_id': router['id']})
if not self.connect_share_server_to_tenant_network:
network_data['router'] = self._get_private_router(
neutron_net_id, neutron_subnet_id)
try:
self.neutron_api.router_add_interface(
network_data['router']['id'],
network_data['service_subnet']['id'])
except exception.NetworkException as e:
if e.kwargs['code'] != 400:
raise
LOG.debug('Subnet %(subnet_id)s is already attached to the '
'router %(router_id)s.',
{'subnet_id': network_data['service_subnet']['id'],
'router_id': network_data['router']['id']})
network_data['service_port'] = self.neutron_api.create_port(
self.service_tenant_id, self.service_network_id,
subnet_id=service_subnet['id'], device_owner='manila')
self.service_tenant_id,
self.service_network_id,
subnet_id=network_data['service_subnet']['id'],
device_owner='manila')
network_data['ports'] = [network_data['service_port']]
if self.connect_share_server_to_tenant_network:
network_data['public_port'] = self.neutron_api.create_port(
self.service_tenant_id, neutron_net_id,
subnet_id=neutron_subnet_id, device_owner='manila')
network_data['ports'] = ports = list()
ports.append(network_data['service_port'])
try:
ports.append(network_data['public_port'])
except KeyError:
pass
network_data['ports'].append(network_data['public_port'])
return network_data
@ -671,7 +662,7 @@ class ServiceInstanceManager(object):
raise exception.ServiceInstanceException(_('No available cidrs.'))
def delete_service_instance(self, context, instance_id, subnet_id,
router_id):
router_id=None):
"""Removes share infrastructure.
Deletes service vm and subnet, associated to share network.

32
manila/tests/share/drivers/test_service_instance.py
View File

@ -611,7 +611,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
'fake-neutron-net',
'fake-neutron-subnet')
def test_setup_network_for_instance0(self):
def test_setup_network_for_instance_using_router(self):
fake_service_net = fake_network.FakeNetwork(subnets=[])
fake_service_subnet = fake_network.\
FakeSubnet(name=self.share['share_network_id'])
@ -646,7 +646,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
self._manager.neutron_api.subnet_create.assert_called_once_with(
self._manager.service_tenant_id,
self._manager.service_network_id,
'routed_to_fake-subnet',
mock.ANY,
'fake_cidr')
self._manager.neutron_api.create_port.assert_called_once_with(
self._manager.service_tenant_id,
@ -660,11 +660,10 @@ class ServiceInstanceManagerTestCase(test.TestCase):
self.assertNotIn('public_port', network_data)
self.assertEqual(network_data.get('ports'), [fake_port])
def test_setup_network_for_instance1(self):
def test_setup_network_for_instance_direct_connection(self):
fake_service_net = fake_network.FakeNetwork(subnets=[])
fake_service_subnet = fake_network. \
FakeSubnet(name=self.share['share_network_id'])
fake_router = fake_network.FakeRouter()
fake_service_subnet = fake_network.FakeSubnet(
name=self.share['share_network_id'])
fake_ports = [
fake_network.FakePort(fixed_ips=[{'ip_address': '1.2.3.4'}]),
fake_network.FakePort(fixed_ips=[{'ip_address': '5.6.7.8'}]),
@ -675,28 +674,20 @@ class ServiceInstanceManagerTestCase(test.TestCase):
mock.Mock(return_value=fake_service_net))
self.stubs.Set(self._manager.neutron_api, 'subnet_create',
mock.Mock(return_value=fake_service_subnet))
self.stubs.Set(self._manager, '_get_private_router',
mock.Mock(return_value=fake_router))
self.stubs.Set(self._manager.neutron_api, 'router_add_interface',
mock.Mock())
self.stubs.Set(self._manager.neutron_api, 'create_port',
mock.Mock(side_effect=fake_ports))
self.stubs.Set(self._manager, '_get_cidr_for_subnet',
mock.Mock(return_value='fake_cidr'))
network_data = self._manager._setup_network_for_instance('fake-net',
'fake-subnet')
network_data = self._manager._setup_network_for_instance(
'fake-net', 'fake-subnet')
self._manager.neutron_api.get_network. \
assert_called_once_with(self._manager.service_network_id)
self._manager._get_private_router. \
assert_called_once_with('fake-net', 'fake-subnet')
self._manager.neutron_api.router_add_interface. \
assert_called_once_with('fake_router_id', 'fake_subnet_id')
self._manager.neutron_api.get_network.assert_called_once_with(
self._manager.service_network_id)
self._manager.neutron_api.subnet_create.assert_called_once_with(
self._manager.service_tenant_id,
self._manager.service_network_id,
'routed_to_fake-subnet',
mock.ANY,
'fake_cidr')
self.assertEqual(self._manager.neutron_api.create_port.call_args_list,
[((self._manager.service_tenant_id,
@ -708,9 +699,8 @@ class ServiceInstanceManagerTestCase(test.TestCase):
{'subnet_id': 'fake-subnet',
'device_owner': 'manila'})])
self._manager._get_cidr_for_subnet.assert_called_once_with()
self.assertIs(network_data.get('service_subnet'), fake_service_subnet)
self.assertIs(network_data.get('router'), fake_router)
self.assertIs(network_data.get('router'), None)
self.assertIs(network_data.get('service_port'), fake_ports[0])
self.assertIs(network_data.get('public_port'), fake_ports[1])
self.assertEqual(network_data.get('ports'), fake_ports)