neutron: destroy VIFs if allocating ports fails

Following on 92a388a1e3 we also
need to rollback VirtualInterfaces that we create if allocation
fails since a reschedule with a pre-existing port will fail
allocation with a duplicate VIF constraint error.

A new test is added for the rollback section being changed
since there isn't an obvious existing test that this could be
worked into.

Change-Id: I04ca4fff319912ce2a8dc07308a3aec1c24dc58c
Closes-Bug: #1603197
This commit is contained in:
Matt Riedemann 2016-07-14 16:51:35 -04:00
parent f6f4003dfd
commit 7a8f1369c5
2 changed files with 61 additions and 0 deletions

View File

@ -785,6 +785,7 @@ class API(base_api.NetworkAPI):
created_port_ids = []
ports_in_requested_order = []
nets_in_requested_order = []
created_vifs = [] # this list is for cleanups if we fail
for request, created_port_id in requests_and_created_ports:
vifobj = objects.VirtualInterface(context)
vifobj.instance_uuid = instance.uuid
@ -836,6 +837,7 @@ class API(base_api.NetworkAPI):
updated_port['id'])
vifobj.uuid = port_id
vifobj.create()
created_vifs.append(vifobj)
if not created_port_id:
# only add if update worked and port create not called
@ -850,6 +852,8 @@ class API(base_api.NetworkAPI):
preexisting_port_ids,
neutron, port_client)
self._delete_ports(neutron, instance, created_port_ids)
for vif in created_vifs:
vif.destroy()
return (nets_in_requested_order, ports_in_requested_order,
preexisting_port_ids, created_port_ids)

View File

@ -3991,6 +3991,63 @@ class TestNeutronv2WithMock(test.TestCase):
self.assertEqual(expected_floating.obj_to_primitive(),
actual_obj.obj_to_primitive())
@mock.patch('nova.network.neutronv2.api.API._has_port_binding_extension',
return_value=True)
@mock.patch('nova.network.neutronv2.api.API.'
'_populate_neutron_extension_values')
@mock.patch('nova.network.neutronv2.api.API._update_port',
# called twice, fails on the 2nd call and triggers the cleanup
side_effect=(mock.MagicMock(),
exception.PortInUse(
port_id=uuids.created_port_id)))
@mock.patch.object(objects.VirtualInterface, 'create')
@mock.patch.object(objects.VirtualInterface, 'destroy')
@mock.patch('nova.network.neutronv2.api.API._unbind_ports')
@mock.patch('nova.network.neutronv2.api.API._delete_ports')
def test_update_ports_for_instance_fails_rollback_ports_and_vifs(self,
mock_delete_ports,
mock_unbind_ports,
mock_vif_destroy,
mock_vif_create,
mock_update_port,
mock_populate_ext_values,
mock_has_port_binding_extension):
"""Makes sure we rollback ports and VIFs if we fail updating ports"""
instance = fake_instance.fake_instance_obj(self.context)
ntrn = mock.Mock(spec='neutronclient.v2_0.client.Client')
# we have two requests, one with a preexisting port and one where nova
# created the port (on the same network)
requests_and_created_ports = [
(objects.NetworkRequest(network_id=uuids.network_id,
port_id=uuids.preexisting_port_id),
None), # None means Nova didn't create this port
(objects.NetworkRequest(network_id=uuids.network_id,
port_id=uuids.created_port_id),
uuids.created_port_id),
]
network = {'id': uuids.network_id}
nets = {uuids.network_id: network}
self.assertRaises(exception.PortInUse,
self.api._update_ports_for_instance,
self.context, instance, ntrn, ntrn,
requests_and_created_ports, nets, bind_host_id=None,
dhcp_opts=None, available_macs=None)
# assert the calls
mock_update_port.assert_has_calls([
mock.call(ntrn, instance, uuids.preexisting_port_id, mock.ANY),
mock.call(ntrn, instance, uuids.created_port_id, mock.ANY)
])
# we only got to create one vif since the 2nd _update_port call fails
mock_vif_create.assert_called_once_with()
# we only destroy one vif since we only created one
mock_vif_destroy.assert_called_once_with()
# we unbind the pre-existing port
mock_unbind_ports.assert_called_once_with(
self.context, [uuids.preexisting_port_id], ntrn, ntrn)
# we delete the created port
mock_delete_ports.assert_called_once_with(
ntrn, instance, [uuids.created_port_id])
class TestNeutronv2ModuleMethods(test.NoDBTestCase):