From b194775ebeaf75a88e6b0b9623aa602cb8c32e0c Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Tue, 28 Mar 2017 19:19:50 +0800 Subject: [PATCH] Delete internal ports when detach them Make sure only to delete internal ports when user wants to detach them, and just do detach for external ports. Change-Id: I09e686d4e1603c6a8b388772eee900f06b1e775d Closes-Bug: #1676821 --- .../openstack/nova/server_network_mixin.py | 15 ++-- heat/tests/openstack/nova/test_server.py | 69 +++++++------------ 2 files changed, 36 insertions(+), 48 deletions(-) diff --git a/heat/engine/resources/openstack/nova/server_network_mixin.py b/heat/engine/resources/openstack/nova/server_network_mixin.py index 6cd51b8ac..88c5778f1 100644 --- a/heat/engine/resources/openstack/nova/server_network_mixin.py +++ b/heat/engine/resources/openstack/nova/server_network_mixin.py @@ -361,13 +361,20 @@ class ServerNetworkMixin(object): # according to nova interface-detach command detached port # will be deleted + inter_port_data = self._data_get_ports() + inter_port_ids = [p['id'] for p in inter_port_data] for net in old_nets: - if net.get(self.NETWORK_PORT): - remove_ports.append(net.get(self.NETWORK_PORT)) - if self.data().get('internal_ports'): + port_id = net.get(self.NETWORK_PORT) + # we can't match the port for some user case, like: + # the internal port was detached in nova first, then + # user update template to detach this nic. The internal + # port will remains till we delete the server resource. + if port_id: + remove_ports.append(port_id) + if port_id in inter_port_ids: # if we have internal port with such id, remove it # instantly. - self._delete_internal_port(net.get(self.NETWORK_PORT)) + self._delete_internal_port(port_id) if net.get(self.NETWORK_FLOATING_IP): self._floating_ip_disassociate( net.get(self.NETWORK_FLOATING_IP)) diff --git a/heat/tests/openstack/nova/test_server.py b/heat/tests/openstack/nova/test_server.py index b24612acf..9d1c77945 100644 --- a/heat/tests/openstack/nova/test_server.py +++ b/heat/tests/openstack/nova/test_server.py @@ -3997,7 +3997,7 @@ class ServersTest(common.HeatTestCase): delete_swift_object.assert_called_once_with() -class ServerInternalPortTest(common.HeatTestCase): +class ServerInternalPortTest(ServersTest): def setUp(self): super(ServerInternalPortTest, self).setUp() self.resolve = self.patchobject(neutron.NeutronClientPlugin, @@ -4008,14 +4008,6 @@ class ServerInternalPortTest(common.HeatTestCase): 'delete_port') self.port_show = self.patchobject(neutronclient.Client, 'show_port') - self.patchobject(resource.Resource, 'is_using_neutron', - return_value=True) - - def flavor_side_effect(*args): - return 2 if args[0] == 'm1.small' else 1 - - def image_side_effect(*args): - return 2 if args[0] == 'F17-x86_64-gold' else 1 def neutron_side_effect(*args): if args[0] == 'subnet': @@ -4025,10 +4017,6 @@ class ServerInternalPortTest(common.HeatTestCase): if args[0] == 'port': return '12345' - self.patchobject(nova.NovaClientPlugin, 'find_flavor_by_name_or_id', - side_effect=flavor_side_effect) - self.patchobject(glance.GlanceClientPlugin, 'find_image_by_name_or_id', - side_effect=image_side_effect) self.resolve.side_effect = neutron_side_effect def _return_template_stack_and_rsrc_defn(self, stack_name, temp): @@ -4249,54 +4237,47 @@ class ServerInternalPortTest(common.HeatTestCase): - network: 4321 subnet: 1234 fixed_ip: 127.0.0.1 - - network: 8765 - subnet: 5678 - fixed_ip: 127.0.0.2 + - port: 3344 """ t, stack, server = self._return_template_stack_and_rsrc_defn('test', tmpl) - - # NOTE(prazumovsky): this method update old_net and new_net with - # interfaces' ports. Because of uselessness of checking this method, - # we can afford to give port as part of calculate_networks args. - self.patchobject(server, 'update_networks_matching_iface_port') - - server._data = {'internal_ports': '[{"id": "1122"}]'} - self.port_create.return_value = {'port': {'id': '5566'}} + data_mock = self.patchobject(server, '_data_get_ports') + data_mock.side_effect = [[{"id": "1122"}], [{"id": "1122"}], []] + self.port_create.return_value = {'port': {'id': '7788'}} data_set = self.patchobject(resource.Resource, 'data_set') - self.resolve.side_effect = ['0912', '9021'] old_net = [{'network': '4321', 'subnet': '1234', - 'fixed_ip': '127.0.0.1', - 'port': '1122'}, - {'network': '8765', + 'fixed_ip': '127.0.0.1'}, + {'port': '3344'}] + + new_net = [{'port': '3344'}, + {'port': '5566'}, + {'network': '4321', 'subnet': '5678', - 'fixed_ip': '127.0.0.2', - 'port': '3344'}] + 'fixed_ip': '10.0.0.1'} + ] + interfaces = [ + self.create_fake_iface('1122', '4321', '127.0.0.1'), + self.create_fake_iface('3344', '4321', '10.0.0.2'), + ] - new_net = [{'network': '8765', - 'subnet': '5678', - 'fixed_ip': '127.0.0.2', - 'port': '3344'}, - {'network': '0912', - 'subnet': '9021', - 'fixed_ip': '127.0.0.1'}] - - server.calculate_networks(old_net, new_net, []) + server.calculate_networks(old_net, new_net, interfaces) + # we can only delete the port 1122, + # port 3344 is external port, cant delete it self.port_delete.assert_called_once_with('1122') self.port_create.assert_called_once_with( - {'port': {'name': 'server-port-0', - 'network_id': '0912', - 'fixed_ips': [{'subnet_id': '9021', - 'ip_address': '127.0.0.1'}]}}) + {'port': {'name': 'server-port-1', + 'network_id': '4321', + 'fixed_ips': [{'subnet_id': '5678', + 'ip_address': '10.0.0.1'}]}}) self.assertEqual(2, data_set.call_count) data_set.assert_has_calls(( mock.call('internal_ports', '[]'), - mock.call('internal_ports', '[{"id": "1122"}, {"id": "5566"}]'))) + mock.call('internal_ports', '[{"id": "7788"}]'))) def test_calculate_networks_nova_with_fipa(self): tmpl = """