Extract port create from allocate_for_instance

This pushes the port create loop into a helper function, so we are
slowly pulling apart the create and update stages of
allocate_for_instance.

This change also adds unit tests for that function, isolated from the
rest of the allocate_for_instance logic.

Because we now call all port creates, before doing the port updates,
some of the existing unit tests had to be updated.

blueprint prep-for-network-aware-scheduling

Change-Id: Ia12aa98296b96c1948b162efd1b3a6b717dd3ade
This commit is contained in:
John Garbutt 2016-06-15 10:15:51 +01:00
parent 809289cfe5
commit 9c45d6d033
2 changed files with 261 additions and 96 deletions
nova
network/neutronv2
tests/unit/network

@ -305,8 +305,8 @@ class API(base_api.NetworkAPI):
instance=instance)
return port
except neutron_client_exc.MacAddressInUseClient:
mac_address = port_req_body['port']['mac_address']
network_id = port_req_body['port']['network_id']
mac_address = port_req_body['port'].get('mac_address')
network_id = port_req_body['port'].get('network_id')
LOG.warning(_LW('Neutron error: MAC address %(mac)s is already '
'in use on network %(network)s.'),
{'mac': mac_address, 'network': network_id},
@ -595,6 +595,69 @@ class API(base_api.NetworkAPI):
return {net['id']: net for net in nets}
def _create_ports_for_instance(self, context, instance, ordered_networks,
nets, neutron, security_group_ids):
"""Create port for network_requests that don't have a port_id
:param context: The request context.
:param instance: nova.objects.instance.Instance object.
:param ordered_networks: objects.NetworkRequestList in requested order
:param nets: a dict of network_id to networks returned from neutron
:param neutron: neutronclient using built from users request context
:param security_group_ids: a list of security_groups to go to neutron
:returns a list of pairs (NetworkRequest, created_port_uuid)
"""
created_port_ids = []
requests_and_created_ports = []
for request in ordered_networks:
network = nets.get(request.network_id)
# if network_id did not pass validate_networks() and not available
# here then skip it safely not continuing with a None Network
if not network:
continue
try:
port_security_enabled = network.get(
'port_security_enabled', True)
if port_security_enabled:
if not network.get('subnets'):
# Neutron can't apply security groups to a port
# for a network without L3 assignments.
LOG.debug('Network with port security enabled does '
'not have subnets so security groups '
'cannot be applied: %s',
network, instance=instance)
raise exception.SecurityGroupCannotBeApplied()
else:
if security_group_ids:
# We don't want to apply security groups on port
# for a network defined with
# 'port_security_enabled=False'.
LOG.debug('Network has port security disabled so '
'security groups cannot be applied: %s',
network, instance=instance)
raise exception.SecurityGroupCannotBeApplied()
created_port_id = None
if not request.port_id:
# create minimal port, if port not already created by user
created_port = self._create_port_minimal(
neutron, instance, request.network_id,
request.address, security_group_ids)
created_port_id = created_port['id']
created_port_ids.append(created_port_id)
requests_and_created_ports.append((
request, created_port_id))
except Exception:
with excutils.save_and_reraise_exception():
if created_port_ids:
self._delete_ports(
neutron, instance, created_port_ids)
return requests_and_created_ports
def allocate_for_instance(self, context, instance, **kwargs):
"""Allocate network resources for the instance.
@ -637,11 +700,22 @@ class API(base_api.NetworkAPI):
LOG.debug("No network configured", instance=instance)
return network_model.NetworkInfo([])
#
# Create any ports that might be required,
# after validating requested security groups
#
security_groups = self._clean_security_groups(
kwargs.get('security_groups', []))
security_group_ids = self._process_security_groups(
instance, neutron, security_groups)
requests_and_created_ports = self._create_ports_for_instance(
context, instance, ordered_networks, nets, neutron,
security_group_ids)
#
# Update existing and newly created ports
#
dhcp_opts = kwargs.get('dhcp_options', None)
bind_host_id = kwargs.get('bind_host_id')
@ -664,7 +738,7 @@ class API(base_api.NetworkAPI):
created_port_ids = []
ports_in_requested_order = []
nets_in_requested_order = []
for request in ordered_networks:
for request, created_port_id in requests_and_created_ports:
vifobj = objects.VirtualInterface(context)
vifobj.instance_uuid = instance.uuid
vifobj.tag = request.tag if 'tag' in request else None
@ -677,25 +751,6 @@ class API(base_api.NetworkAPI):
nets_in_requested_order.append(network)
port_security_enabled = network.get('port_security_enabled', True)
if port_security_enabled:
if not network.get('subnets'):
# Neutron can't apply security groups to a port
# for a network without L3 assignments.
LOG.debug('Network with port security enabled does not '
'have subnets so security groups cannot be '
'applied: %s', network, instance=instance)
raise exception.SecurityGroupCannotBeApplied()
else:
if security_group_ids:
# We don't want to apply security groups on port
# for a network defined with
# 'port_security_enabled=False'.
LOG.debug('Network has port security disabled so security '
'groups cannot be applied: %s', network,
instance=instance)
raise exception.SecurityGroupCannotBeApplied()
zone = 'compute:%s' % instance.availability_zone
port_req_body = {'port': {'device_id': instance.uuid,
'device_owner': zone}}
@ -711,24 +766,16 @@ class API(base_api.NetworkAPI):
if dhcp_opts is not None:
port_req_body['port']['extra_dhcp_opts'] = dhcp_opts
if not request.port_id:
# create minimal port, if port not already created by user
# NOTE(johngarbutt) doing this extra hop so we can extract
# this step later on
created_port = self._create_port_minimal(
port_client, instance, request.network_id,
request.address, security_group_ids)
created_port_id = created_port['id']
created_port_ids.append(created_port_id)
ports_in_requested_order.append(created_port_id)
vifobj.uuid = created_port_id
if created_port_id:
port_id = created_port_id
created_port_ids.append(port_id)
else:
ports_in_requested_order.append(request.port_id)
vifobj.uuid = request.port_id
port_id = request.port_id
ports_in_requested_order.append(port_id)
# After port is created, update other bits
updated_port = self._update_port(
port_client, instance, vifobj.uuid, port_req_body)
port_client, instance, port_id, port_req_body)
# NOTE(danms): The virtual_interfaces table enforces global
# uniqueness on MAC addresses, which clearly does not match
@ -740,12 +787,12 @@ class API(base_api.NetworkAPI):
# for longer than that of course.
vifobj.address = '%s/%s' % (updated_port['mac_address'],
updated_port['id'])
vifobj.uuid = port_id
vifobj.create()
# only add to preexisting_port_ids after update success
# we use request.port_id to check if is preexisting
if request.port_id:
preexisting_port_ids.append(request.port_id)
if not created_port_id:
# only add if update worked and port create not called
preexisting_port_ids.append(port_id)
self._update_port_dns_name(context, instance, network,
ports_in_requested_order[-1],

@ -437,13 +437,16 @@ class TestNeutronv2Base(test.TestCase):
self.mox.ReplayAll()
return api
has_portbinding = self._stub_allocate_for_instance_port_binding(
api, kwargs.get('portbinding'), has_dns_extension)
if kwargs.get('_break') == 'post_list_extensions':
self.mox.ReplayAll()
return api
self._stub_allocate_for_instance_create_port(
ordered_networks, fixed_ips, nets)
has_portbinding = self._stub_allocate_for_instance_port_binding(
api, kwargs.get('portbinding'), has_dns_extension)
preexisting_port_ids = []
ports_in_requested_net_order = []
nets_in_requested_net_order = []
@ -492,26 +495,14 @@ class TestNeutronv2Base(test.TestCase):
port_req_body['port']['extra_dhcp_opts'] = dhcp_options
if not request.port_id:
port_req_body_create = {'port': {}}
request.address = fixed_ips.get(request.network_id)
if request.address:
port_req_body_create['port']['fixed_ips'] = [
{'ip_address': str(request.address)}]
port_req_body_create['port']['network_id'] = \
request.network_id
port_req_body_create['port']['admin_state_up'] = True
port_req_body_create['port']['tenant_id'] = \
self.instance.project_id
port_id = "fake"
update_port_res = {'port': {
'id': port_id,
'mac_address': 'fakemac%i' % index}}
ports_in_requested_net_order.append(port_id)
if kwargs.get('_break') == 'mac' + request.network_id:
self.mox.ReplayAll()
return api
res_port = {'port': {'id': 'fake',
'mac_address': 'fakemac%i' % index}}
self.moxed_client.create_port(
MyComparator(port_req_body_create)).AndReturn(res_port)
ports_in_requested_net_order.append(res_port['port']['id'])
port_id = "fake"
update_port_res = res_port
else:
ports_in_requested_net_order.append(request.port_id)
preexisting_port_ids.append(request.port_id)
@ -651,6 +642,33 @@ class TestNeutronv2Base(test.TestCase):
self.moxed_client.list_networks(
**mox_list_params).AndReturn({'networks': []})
def _stub_allocate_for_instance_create_port(self, ordered_networks,
fixed_ips, nets):
for request in ordered_networks:
if not request.port_id:
# Check network is available, skip if not
network = None
for net in nets:
if net['id'] == request.network_id:
network = net
break
if network is None:
continue
port_req_body_create = {'port': {}}
request.address = fixed_ips.get(request.network_id)
if request.address:
port_req_body_create['port']['fixed_ips'] = [
{'ip_address': str(request.address)}]
port_req_body_create['port']['network_id'] = \
request.network_id
port_req_body_create['port']['admin_state_up'] = True
port_req_body_create['port']['tenant_id'] = \
self.instance.project_id
res_port = {'port': {'id': "fake"}}
self.moxed_client.create_port(
MyComparator(port_req_body_create)).AndReturn(res_port)
def _verify_nw_info(self, nw_inf, index=0):
id_suffix = index + 1
self.assertEqual('10.0.%s.2' % id_suffix,
@ -1151,7 +1169,7 @@ class TestNeutronv2(TestNeutronv2Base):
requested_networks[0].tag = 'foo'
self._allocate_for_instance(net_idx=2,
requested_networks=requested_networks)
self.assertEqual(3, len(self._vifs_created))
self.assertEqual(2, len(self._vifs_created))
# NOTE(danms) nets3[2] is chosen above as one that won't validate,
# so we never actually run create() on the VIF.
vifs_really_created = [vif for vif in self._vifs_created
@ -1220,11 +1238,13 @@ class TestNeutronv2(TestNeutronv2Base):
nwinfo = api.allocate_for_instance(self.context, self.instance)
self.assertEqual(0, len(nwinfo))
@mock.patch('nova.network.neutronv2.api.API._get_preexisting_port_ids')
@mock.patch(
'nova.network.neutronv2.api.API._populate_neutron_extension_values')
@mock.patch('nova.network.neutronv2.api.API._has_port_binding_extension')
@mock.patch('nova.network.neutronv2.api.API._create_ports_for_instance')
@mock.patch('nova.network.neutronv2.api.API._unbind_ports')
def test_allocate_for_instance_ex1(self,
mock_unbind,
mock_preexisting):
def test_allocate_for_instance_ex1(self, mock_unbind, mock_create_ports,
mock_has_port_binding, mock_populate):
"""verify we will delete created ports
if we fail to allocate all net resources.
@ -1233,18 +1253,17 @@ class TestNeutronv2(TestNeutronv2Base):
"""
self.instance = fake_instance.fake_instance_obj(self.context,
**self.instance)
mock_preexisting.return_value = []
api = neutronapi.API()
self.mox.StubOutWithMock(api, '_populate_neutron_extension_values')
self.mox.StubOutWithMock(api, '_has_port_binding_extension')
api._has_port_binding_extension(mox.IgnoreArg(),
neutron=self.moxed_client,
refresh_cache=True).AndReturn(False)
requested_networks = objects.NetworkRequestList(
objects=[objects.NetworkRequest(network_id=net['id'])
for net in (self.nets2[0], self.nets2[1])])
self.moxed_client.list_networks(
id=['my_netid1', 'my_netid2']).AndReturn({'networks': self.nets2})
mock_has_port_binding.return_value = False
mock_create_ports.return_value = [
(request, ('portid_' + request.network_id))
for request in requested_networks
]
index = 0
for network in self.nets2:
binding_port_req_body = {
@ -1260,29 +1279,24 @@ class TestNeutronv2(TestNeutronv2Base):
'tenant_id': self.instance.project_id,
},
}
port_create_body = copy.deepcopy(port_req_body)
port_req_body['port'].update(binding_port_req_body['port'])
port_id = 'portid_' + network['id']
port = {'id': port_id, 'mac_address': 'foo'}
api._populate_neutron_extension_values(self.context,
self.instance, None, binding_port_req_body, network=network,
neutron=self.moxed_client, bind_host_id=None).AndReturn(None)
if index == 0:
self.moxed_client.create_port(
MyComparator(port_create_body)).AndReturn({'port': port})
self.moxed_client.update_port(port_id,
MyComparator(binding_port_req_body)).AndReturn(
{'port': port})
else:
NeutronOverQuota = exceptions.OverQuotaClient()
self.moxed_client.create_port(
MyComparator(port_create_body)).AndRaise(
NeutronOverQuota = exceptions.MacAddressInUseClient()
self.moxed_client.update_port(port_id,
MyComparator(binding_port_req_body)).AndRaise(
NeutronOverQuota)
index += 1
self.moxed_client.delete_port('portid_' + self.nets2[0]['id'])
self.moxed_client.delete_port('portid_' + self.nets2[1]['id'])
self.mox.ReplayAll()
self.assertRaises(exception.PortLimitExceeded,
self.assertRaises(exception.PortInUse,
api.allocate_for_instance,
self.context, self.instance,
requested_networks=requested_networks)
@ -1299,22 +1313,11 @@ class TestNeutronv2(TestNeutronv2Base):
self.instance = fake_instance.fake_instance_obj(self.context,
**self.instance)
api = neutronapi.API()
self.mox.StubOutWithMock(api, '_populate_neutron_extension_values')
self.mox.StubOutWithMock(api, '_has_port_binding_extension')
api._has_port_binding_extension(mox.IgnoreArg(),
neutron=self.moxed_client,
refresh_cache=True).AndReturn(False)
requested_networks = objects.NetworkRequestList(
objects=[objects.NetworkRequest(network_id=net['id'])
for net in (self.nets2[0], self.nets2[1])])
self.moxed_client.list_networks(
id=['my_netid1', 'my_netid2']).AndReturn({'networks': self.nets2})
binding_port_req_body = {
'port': {
'device_id': self.instance.uuid,
'device_owner': 'compute:nova',
},
}
port_req_body = {
'port': {
'network_id': self.nets2[0]['id'],
@ -1323,10 +1326,6 @@ class TestNeutronv2(TestNeutronv2Base):
'tenant_id': self.instance.project_id,
},
}
api._populate_neutron_extension_values(self.context,
self.instance, None, binding_port_req_body,
network=self.nets2[0], neutron=self.moxed_client,
bind_host_id=None).AndReturn(None)
self.moxed_client.create_port(
MyComparator(port_req_body)).AndRaise(
Exception("fail to create port"))
@ -4495,6 +4494,125 @@ class TestAllocateForInstanceHelpers(test.NoDBTestCase):
exception.NetworkAmbiguous,
[{'id': "net1"}, {'id': "net2"}])
def test_create_ports_for_instance_no_security(self):
api = neutronapi.API()
ordered_networks = [objects.NetworkRequest(network_id=uuids.net)]
nets = {uuids.net: {"id": uuids.net, "port_security_enabled": False}}
mock_client = mock.Mock()
mock_client.create_port.return_value = {"port": {"id": uuids.port}}
result = api._create_ports_for_instance(self.context, self.instance,
ordered_networks, nets, mock_client, None)
self.assertEqual([(ordered_networks[0], uuids.port)], result)
mock_client.create_port.assert_called_once_with(
{'port': {
'network_id': uuids.net, 'tenant_id': uuids.tenant_id,
'admin_state_up': True}})
def test_create_ports_for_instance_with_security_groups(self):
api = neutronapi.API()
ordered_networks = [objects.NetworkRequest(network_id=uuids.net)]
nets = {uuids.net: {"id": uuids.net, "subnets": [uuids.subnet]}}
mock_client = mock.Mock()
mock_client.create_port.return_value = {"port": {"id": uuids.port}}
security_groups = [uuids.sg]
result = api._create_ports_for_instance(self.context, self.instance,
ordered_networks, nets, mock_client, security_groups)
self.assertEqual([(ordered_networks[0], uuids.port)], result)
mock_client.create_port.assert_called_once_with(
{'port': {
'network_id': uuids.net, 'tenant_id': uuids.tenant_id,
'admin_state_up': True, 'security_groups': security_groups}})
def test_create_ports_for_instance_with_cleanup_after_pc_failure(self):
api = neutronapi.API()
ordered_networks = [
objects.NetworkRequest(network_id=uuids.net1),
objects.NetworkRequest(network_id=uuids.net2),
objects.NetworkRequest(network_id=uuids.net3),
objects.NetworkRequest(network_id=uuids.net4)
]
nets = {
uuids.net1: {"id": uuids.net1, "port_security_enabled": False},
uuids.net2: {"id": uuids.net2, "port_security_enabled": False},
uuids.net3: {"id": uuids.net3, "port_security_enabled": False},
uuids.net3: {"id": uuids.net4, "port_security_enabled": False}
}
error = exception.PortLimitExceeded()
mock_client = mock.Mock()
mock_client.create_port.side_effect = [
{"port": {"id": uuids.port1}},
{"port": {"id": uuids.port2}},
error
]
self.assertRaises(exception.PortLimitExceeded,
api._create_ports_for_instance,
self.context, self.instance, ordered_networks, nets,
mock_client, None)
self.assertEqual([mock.call(uuids.port1), mock.call(uuids.port2)],
mock_client.delete_port.call_args_list)
self.assertEqual(3, mock_client.create_port.call_count)
def test_create_ports_for_instance_with_cleanup_after_sg_failure(self):
api = neutronapi.API()
ordered_networks = [
objects.NetworkRequest(network_id=uuids.net1),
objects.NetworkRequest(network_id=uuids.net2),
objects.NetworkRequest(network_id=uuids.net3)
]
nets = {
uuids.net1: {"id": uuids.net1, "port_security_enabled": False},
uuids.net2: {"id": uuids.net2, "port_security_enabled": False},
uuids.net3: {"id": uuids.net3, "port_security_enabled": True}
}
mock_client = mock.Mock()
mock_client.create_port.side_effect = [
{"port": {"id": uuids.port1}},
{"port": {"id": uuids.port2}}
]
self.assertRaises(exception.SecurityGroupCannotBeApplied,
api._create_ports_for_instance,
self.context, self.instance, ordered_networks, nets,
mock_client, None)
self.assertEqual([mock.call(uuids.port1), mock.call(uuids.port2)],
mock_client.delete_port.call_args_list)
self.assertEqual(2, mock_client.create_port.call_count)
def test_create_ports_for_instance_raises_subnets_missing(self):
api = neutronapi.API()
ordered_networks = [objects.NetworkRequest(network_id=uuids.net)]
nets = {uuids.net: {"id": uuids.net, "port_security_enabled": True}}
mock_client = mock.Mock()
self.assertRaises(exception.SecurityGroupCannotBeApplied,
api._create_ports_for_instance,
self.context, self.instance,
ordered_networks, nets, mock_client, None)
self.assertFalse(mock_client.create_port.called)
def test_create_ports_for_instance_raises_security_off(self):
api = neutronapi.API()
ordered_networks = [objects.NetworkRequest(network_id=uuids.net)]
nets = {uuids.net: {
"id": uuids.net,
"port_security_enabled": False}}
mock_client = mock.Mock()
self.assertRaises(exception.SecurityGroupCannotBeApplied,
api._create_ports_for_instance,
self.context, self.instance,
ordered_networks, nets, mock_client, [uuids.sg])
self.assertFalse(mock_client.create_port.called)
class TestNeutronv2NeutronHostnameDNS(TestNeutronv2Base):
def setUp(self):