From 1957339df302e2da75e0dbe78b5d566194ab2c08 Mon Sep 17 00:00:00 2001 From: Aaron Rosen Date: Fri, 25 Oct 2013 15:32:01 -0700 Subject: [PATCH] Fix interface-attach removes existing interfaces from db The following commit 394c693e359ed4f19cc2f7d975b1f9ee5500b7f6 changed allocate_port_for_instance() to only return the ports that were created rather than all of the ports on the instance which broke the attach-interface code. This patch fixes this issue by remove the sync decorators from: allocate_for_instance, allocate_port_for_instance, and deallocate_port_for_instance to just _build_instance_nw_info which is called in all these cases instead. Closes-bug: #1223859 Change-Id: I66eb0c0ab926e0a8d1e2c9cfe1f7fd579ea3aa27 --- nova/network/neutronv2/api.py | 29 ++++++++-- .../contrib/test_neutron_security_groups.py | 1 + nova/tests/network/test_neutronv2.py | 56 ++++++++++++++++++- 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index bddc9a9280b5..fe105aee1a5e 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -191,7 +191,6 @@ class API(base.Base): raise exception.PortLimitExceeded() raise - @refresh_cache def allocate_for_instance(self, context, instance, **kwargs): """Allocate network resources for the instance. @@ -416,14 +415,12 @@ class API(base.Base): LOG.exception(_("Failed to delete neutron port %(portid)s") % {'portid': port}) - @refresh_cache def allocate_port_for_instance(self, context, instance, port_id, network_id=None, requested_ip=None): """Allocate a port for the instance.""" return self.allocate_for_instance(context, instance, requested_networks=[(network_id, requested_ip, port_id)]) - @refresh_cache def deallocate_port_for_instance(self, context, instance, port_id): """Remove a specified port from the instance. @@ -454,6 +451,7 @@ class API(base.Base): update_cells=False) return result + @refresh_cache def _get_instance_nw_info(self, context, instance, networks=None): LOG.debug(_('get_instance_nw_info() for %s'), instance['display_name']) @@ -969,11 +967,15 @@ class API(base.Base): return network, ovs_interfaceid def _build_network_info_model(self, context, instance, networks=None): + # Note(arosen): on interface-attach networks only contains the + # network that the interface is being attached to. + search_opts = {'tenant_id': instance['project_id'], 'device_id': instance['uuid'], } client = neutronv2.get_client(context, admin=True) data = client.list_ports(**search_opts) ports = data.get('ports', []) + nw_info = network_model.NetworkInfo() if networks is None: # retrieve networks from info_cache to get correct nic order network_cache = self.conductor_api.instance_get_by_uuid( @@ -986,12 +988,31 @@ class API(base.Base): # ensure ports are in preferred network order, and filter out # those not attached to one of the provided list of networks else: + + # Unfortunately, this is sometimes in unicode and sometimes not + if isinstance(instance['info_cache']['network_info'], unicode): + ifaces = jsonutils.loads( + instance['info_cache']['network_info']) + else: + ifaces = instance['info_cache']['network_info'] + + # Include existing interfaces so they are not removed from the db. + # Needed when interfaces are added to existing instances. + for iface in ifaces: + nw_info.append(network_model.VIF( + id=iface['id'], + address=iface['address'], + network=iface['network'], + type=iface['type'], + ovs_interfaceid=iface['ovs_interfaceid'], + devname=iface['devname'])) + net_ids = [n['id'] for n in networks] + ports = [port for port in ports if port['network_id'] in net_ids] _ensure_requested_network_ordering(lambda x: x['network_id'], ports, net_ids) - nw_info = network_model.NetworkInfo() for port in ports: network_IPs = self._nw_info_get_ips(client, port) subnets = self._nw_info_get_subnets(context, port, network_IPs) diff --git a/nova/tests/api/openstack/compute/contrib/test_neutron_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_neutron_security_groups.py index d901aa37f6ec..d8225cdaa73c 100644 --- a/nova/tests/api/openstack/compute/contrib/test_neutron_security_groups.py +++ b/nova/tests/api/openstack/compute/contrib/test_neutron_security_groups.py @@ -164,6 +164,7 @@ class TestNeutronSecurityGroups( self._create_network() fake_instance = {'project_id': 'fake_tenant', 'availability_zone': 'zone_one', + 'info_cache': {'network_info': []}, 'security_groups': [], 'uuid': str(uuid.uuid4()), 'display_name': 'test_instance'} diff --git a/nova/tests/network/test_neutronv2.py b/nova/tests/network/test_neutronv2.py index 8d83112b3c6a..fe261f0e1c09 100644 --- a/nova/tests/network/test_neutronv2.py +++ b/nova/tests/network/test_neutronv2.py @@ -512,11 +512,64 @@ class TestNeutronv2(TestNeutronv2Base): admin=True).MultipleTimes().AndReturn( self.moxed_client) self.mox.ReplayAll() + self.instance['info_cache'] = {'network_info': []} nw_inf = api.get_instance_nw_info(self.context, self.instance, networks=self.nets1) self._verify_nw_info(nw_inf, 0) + def test_get_instance_nw_info_with_nets_and_info_cache(self): + # This tests that adding an interface to an instance does not + # remove the first instance from the instance. + api = neutronapi.API() + self.mox.StubOutWithMock(api.db, 'instance_info_cache_update') + api.db.instance_info_cache_update( + mox.IgnoreArg(), + self.instance['uuid'], mox.IgnoreArg()) + self.moxed_client.list_ports( + tenant_id=self.instance['project_id'], + device_id=self.instance['uuid']).AndReturn( + {'ports': self.port_data1}) + port_data = self.port_data1 + for ip in port_data[0]['fixed_ips']: + self.moxed_client.list_floatingips( + fixed_ip_address=ip['ip_address'], + port_id=port_data[0]['id']).AndReturn( + {'floatingips': self.float_data1}) + self.moxed_client.list_subnets( + id=mox.SameElementsAs(['my_subid1'])).AndReturn( + {'subnets': self.subnet_data1}) + self.moxed_client.list_ports( + network_id='my_netid1', + device_owner='network:dhcp').AndReturn( + {'ports': self.dhcp_port_data1}) + neutronv2.get_client(mox.IgnoreArg(), + admin=True).MultipleTimes().AndReturn( + self.moxed_client) + self.mox.ReplayAll() + network_model = model.Network(id='network_id', + bridge='br-int', + injected='injected', + label='fake_network', + tenant_name='fake_tenant') + + self.instance['info_cache'] = { + 'network_info': [{'id': 'port_id', + 'address': 'mac_address', + 'network': network_model, + 'type': 'ovs', + 'ovs_interfaceid': 'ovs_interfaceid', + 'devname': 'devname'}]} + nw_inf = api.get_instance_nw_info(self.context, + self.instance, + networks=self.nets1) + self.assertEqual(2, len(nw_inf)) + for k, v in self.instance['info_cache']['network_info'][0].iteritems(): + self.assertEqual(nw_inf[0][k], v) + # remove first inf and verify that the second interface is correct + del nw_inf[0] + self._verify_nw_info(nw_inf, 0) + def test_get_instance_nw_info_without_subnet(self): # Test get instance_nw_info for a port without subnet. api = neutronapi.API() @@ -1554,7 +1607,8 @@ class TestNeutronv2(TestNeutronv2Base): def test_build_network_info_model(self): api = neutronapi.API() - fake_inst = {'project_id': 'fake', 'uuid': 'uuid'} + fake_inst = {'project_id': 'fake', 'uuid': 'uuid', + 'info_cache': {'network_info': []}} fake_ports = [ {'id': 'port0', 'network_id': 'net-id',