From 394c693e359ed4f19cc2f7d975b1f9ee5500b7f6 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Thu, 21 Feb 2013 15:09:29 -0500 Subject: Make allocate_for_instance() return only info about ports allocated Previously, the compute manager was filtering the results based on the port id that the user requested. However, if they do not pass one (which is valid) then we can not do the filter and we fail. This makes the allocation function do the filtering, which should not affect the normal use case where it is only called once, since all the ports returned were allocated at that time. The manager filter behavior is thus removed and a test is added to verify that the allocate method does the right thing the second time around. This fixes the fallout found after fixing the actual cause of bug 1131264 Change-Id: I8f3af96051268503596007491af365e8ce28f5b3 --- nova/compute/manager.py | 12 +++++++----- nova/network/quantumv2/api.py | 11 +++++++++-- nova/tests/network/test_quantumv2.py | 14 ++++++++++++-- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 99b97e921..942cc7f5c 100755 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2763,13 +2763,15 @@ class ComputeManager(manager.SchedulerDependentManager): network_info = self.network_api.allocate_port_for_instance( context, instance, port_id, network_id, requested_ip, self.conductor_api) + if len(network_info) != 1: + LOG.error(_('allocate_port_for_instance returned %(port)s ports') % + dict(ports=len(network_info))) + raise exception.InterfaceAttachFailed(instance=instance) image_meta = _get_image_meta(context, instance['image_ref']) legacy_net_info = self._legacy_nw_info(network_info) - for (network, mapping) in legacy_net_info: - if mapping['vif_uuid'] == port_id: - self.driver.attach_interface(instance, image_meta, - [(network, mapping)]) - return (network, mapping) + (network, mapping) = legacy_net_info[0] + self.driver.attach_interface(instance, image_meta, legacy_net_info) + return legacy_net_info[0] def detach_interface(self, context, instance, port_id): """Detach an network adapter from an instance.""" diff --git a/nova/network/quantumv2/api.py b/nova/network/quantumv2/api.py index 50e38eebd..6f0515ce0 100644 --- a/nova/network/quantumv2/api.py +++ b/nova/network/quantumv2/api.py @@ -279,8 +279,15 @@ class API(base.Base): self.trigger_security_group_members_refresh(context, instance) self.trigger_instance_add_security_group_refresh(context, instance) - return self.get_instance_nw_info(context, instance, networks=nets, - conductor_api=kwargs.get('conductor_api')) + nw_info = self.get_instance_nw_info(context, instance, networks=nets, + conductor_api=kwargs.get('conductor_api')) + # NOTE(danms): Only return info about ports we created in this run. + # In the initial allocation case, this will be everything we created, + # and in later runs will only be what was created that time. Thus, + # this only affects the attach case, not the original use for this + # method. + return network_model.NetworkInfo([port for port in nw_info + if port['id'] in created_port_ids]) def _refresh_quantum_extensions_cache(self): if (not self.last_quantum_extension_sync or diff --git a/nova/tests/network/test_quantumv2.py b/nova/tests/network/test_quantumv2.py index a168e451f..47798e70d 100644 --- a/nova/tests/network/test_quantumv2.py +++ b/nova/tests/network/test_quantumv2.py @@ -227,6 +227,7 @@ class TestQuantumv2(test.TestCase): 'port_id': self.port_data2[1]['id'], 'fixed_ip_address': fixed_ip_address, 'router_id': 'router_id1'} + self._returned_nw_info = [] def tearDown(self): self.addCleanup(CONF.reset) @@ -457,13 +458,14 @@ class TestQuantumv2(test.TestCase): api.get_instance_nw_info(mox.IgnoreArg(), self.instance, networks=nets, - conductor_api=mox.IgnoreArg()).AndReturn(None) + conductor_api=mox.IgnoreArg()).AndReturn( + self._returned_nw_info) self.mox.ReplayAll() return api def _allocate_for_instance(self, net_idx=1, **kwargs): api = self._stub_allocate_for_instance(net_idx, **kwargs) - api.allocate_for_instance(self.context, self.instance, **kwargs) + return api.allocate_for_instance(self.context, self.instance, **kwargs) def test_allocate_for_instance_1(self): # Allocate one port in one network env. @@ -646,6 +648,14 @@ class TestQuantumv2(test.TestCase): self.context, self.instance, requested_networks=[(None, None, None)]) + def test_allocate_for_instance_second_time(self): + # Make sure that allocate_for_instance only returns ports that it + # allocated during _that_ run. + new_port = {'id': 'fake'} + self._returned_nw_info = self.port_data1 + [new_port] + nw_info = self._allocate_for_instance() + self.assertEqual(nw_info, [new_port]) + def _deallocate_for_instance(self, number): port_data = number == 1 and self.port_data1 or self.port_data2 self.moxed_client.list_ports( -- cgit