From b12da559b6a40fba4e4431d74372b5e350047525 Mon Sep 17 00:00:00 2001 From: Gary Kotton Date: Wed, 30 Oct 2013 04:21:53 -0700 Subject: [PATCH] Fix bug for neutron network-name There may be cases when the network-name is not updated correctly due to the requested networks not being owned by the tenant. In the case when there is no matched network we use the details from the used port. A failed deletion of a port will raise an exception. If the port is not found no exception will be raised. Co-authored-by: Evgeny Fedoruk Change-Id: Ie28e88ec9a9c7a180410ae5e57f84b1f653bbffc Closes-Bug: #1246258 --- nova/network/neutronv2/api.py | 30 ++++++++++---- nova/tests/network/test_neutronv2.py | 61 ++++++++++++++++++++-------- 2 files changed, 66 insertions(+), 25 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 75965676cef5..de8785a3e17d 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -386,7 +386,8 @@ class API(base.Base): LOG.debug(_('deallocate_for_instance() for %s'), instance['display_name']) search_opts = {'device_id': instance['uuid']} - data = neutronv2.get_client(context).list_ports(**search_opts) + neutron = neutronv2.get_client(context) + data = neutron.list_ports(**search_opts) ports = [port['id'] for port in data.get('ports', [])] requested_networks = kwargs.get('requested_networks') or {} @@ -395,10 +396,14 @@ class API(base.Base): for port in ports: try: - neutronv2.get_client(context).delete_port(port) - except Exception: - LOG.exception(_("Failed to delete neutron port %(portid)s") - % {'portid': port}) + neutron.delete_port(port) + except neutronv2.exceptions.NeutronClientException as e: + if e.status_code == 404: + LOG.warning(_("Port %s does not exist"), port) + else: + with excutils.save_and_reraise_exception(): + LOG.exception(_("Failed to delete neutron port %s"), + port) def allocate_port_for_instance(self, context, instance, port_id, network_id=None, requested_ip=None): @@ -946,12 +951,18 @@ class API(base.Base): return subnets def _nw_info_build_network(self, port, networks, subnets): - # NOTE(danms): This loop can't fail to find a network since we - # filtered ports to only the ones matching networks in our parent + network_name = None for net in networks: if port['network_id'] == net['id']: network_name = net['name'] + tenant_id = net['tenant_id'] break + else: + tenant_id = port['tenant_id'] + LOG.warning(_("Network %(id)s not matched with the tenants " + "network! The ports tenant %(tenant_id)s will be " + "used."), + {'id': port['network_id'], 'tenant_id': tenant_id}) bridge = None ovs_interfaceid = None @@ -975,7 +986,7 @@ class API(base.Base): bridge=bridge, injected=CONF.flat_injected, label=network_name, - tenant_id=net['tenant_id'] + tenant_id=tenant_id ) network['subnets'] = subnets port_profile = port.get('binding:profile') @@ -1008,7 +1019,8 @@ class API(base.Base): if networks is None: net_ids = [iface['network']['id'] for iface in ifaces] networks = self._get_available_networks(context, - instance['project_id']) + instance['project_id'], + net_ids) # ensure ports are in preferred network order, and filter out # those not attached to one of the provided list of networks diff --git a/nova/tests/network/test_neutronv2.py b/nova/tests/network/test_neutronv2.py index 2afbdfefa75e..8445f37c1cde 100644 --- a/nova/tests/network/test_neutronv2.py +++ b/nova/tests/network/test_neutronv2.py @@ -415,7 +415,7 @@ class TestNeutronv2Base(test.TestCase): self.instance['uuid'], mox.IgnoreArg()) port_data = number == 1 and self.port_data1 or self.port_data2 - + nets = number == 1 and self.nets1 or self.nets2 net_info_cache = [] for port in port_data: net_info_cache.append({"network": {"id": port['network_id']}}) @@ -428,12 +428,10 @@ class TestNeutronv2Base(test.TestCase): tenant_id=self.instance['project_id'], device_id=self.instance['uuid']).AndReturn( {'ports': port_data}) + net_ids = [port['network_id'] for port in port_data] nets = number == 1 and self.nets1 or self.nets2 self.moxed_client.list_networks( - tenant_id=self.instance['project_id'], - shared=False).AndReturn({'networks': nets}) - self.moxed_client.list_networks( - shared=True).AndReturn({'networks': []}) + id=net_ids).AndReturn({'networks': nets}) for i in xrange(1, number + 1): float_data = number == 1 and self.float_data1 or self.float_data2 for ip in port_data[i - 1]['fixed_ips']: @@ -580,11 +578,8 @@ class TestNeutronv2(TestNeutronv2Base): device_id=self.instance['uuid']).AndReturn( {'ports': self.port_data3}) self.moxed_client.list_networks( - shared=False, - tenant_id=self.instance['project_id']).AndReturn( + id=[self.port_data1[0]['network_id']]).AndReturn( {'networks': self.nets1}) - self.moxed_client.list_networks( - shared=True).AndReturn({'networks': []}) neutronv2.get_client(mox.IgnoreArg(), admin=True).MultipleTimes().AndReturn( self.moxed_client) @@ -884,7 +879,7 @@ class TestNeutronv2(TestNeutronv2Base): self.moxed_client.list_ports( device_id=self.instance['uuid']).AndReturn( {'ports': port_data}) - for port in port_data: + for port in reversed(port_data): self.moxed_client.delete_port(port['id']) self.mox.ReplayAll() @@ -900,8 +895,25 @@ class TestNeutronv2(TestNeutronv2Base): # Test to deallocate in two ports env. self._deallocate_for_instance(2) + def test_deallocate_for_instance_port_not_found(self): + port_data = self.port_data1 + self.moxed_client.list_ports( + device_id=self.instance['uuid']).AndReturn( + {'ports': port_data}) + + NeutronNotFound = neutronv2.exceptions.NeutronClientException( + status_code=404) + for port in reversed(port_data): + self.moxed_client.delete_port(port['id']).AndRaise( + NeutronNotFound) + self.mox.ReplayAll() + + api = neutronapi.API() + api.deallocate_for_instance(self.context, self.instance) + def _test_deallocate_port_for_instance(self, number): port_data = number == 1 and self.port_data1 or self.port_data2 + nets = number == 1 and self.nets1 or self.nets2 self.moxed_client.delete_port(port_data[0]['id']) net_info_cache = [] @@ -920,12 +932,9 @@ class TestNeutronv2(TestNeutronv2Base): {'ports': port_data[1:]}) neutronv2.get_client(mox.IgnoreArg()).MultipleTimes().AndReturn( self.moxed_client) - self.moxed_client.list_networks( - tenant_id=self.instance['project_id'], - shared=False).AndReturn( - {'networks': [self.nets2[1]]}) - self.moxed_client.list_networks(shared=True).AndReturn( - {'networks': []}) + net_ids = [port['network_id'] for port in port_data] + self.moxed_client.list_networks(id=net_ids).AndReturn( + {'networks': nets}) float_data = number == 1 and self.float_data1 or self.float_data2 for data in port_data[1:]: for ip in data['fixed_ips']: @@ -1755,6 +1764,26 @@ class TestNeutronv2(TestNeutronv2Base): self.assertNotIn('should_create_bridge', net) self.assertIsNone(iid) + def test_nw_info_build_no_match(self): + fake_port = { + 'fixed_ips': [{'ip_address': '1.1.1.1'}], + 'id': 'port-id', + 'network_id': 'net-id1', + 'tenant_id': 'tenant', + 'binding:vif_type': model.VIF_TYPE_OVS, + } + fake_subnets = [model.Subnet(cidr='1.0.0.0/8')] + fake_nets = [{'id': 'net-id2', 'name': 'foo', 'tenant_id': 'tenant'}] + api = neutronapi.API() + self.mox.ReplayAll() + neutronv2.get_client('fake') + net, iid = api._nw_info_build_network(fake_port, fake_nets, + fake_subnets) + self.assertEqual(fake_subnets, net['subnets']) + self.assertEqual('net-id1', net['id']) + self.assertEqual('net-id1', net['id']) + self.assertEqual('tenant', net['meta']['tenant_id']) + def test_build_network_info_model(self): api = neutronapi.API() fake_inst = {'project_id': 'fake', 'uuid': 'uuid',