From 2979ccf226570d0c8459076f51cf844e21b4b3f1 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Mon, 23 Jan 2017 10:29:25 +0000 Subject: [PATCH] Ensure subports are deleted after container deletion There is a disaligned used of the device_owner field in Neutron and Kuryr which makes both of them changing the device_owner to different values, with the consequence that something the remaining value is the one set by Neutron, and thus making the filter to filter out the port to be removed, leaving it without being cleaned-up. This is a temporal fix that can be later enhanced by using tagging instead of device_owner to handle the filtering, once the tagging support is merged in Neutron. Change-Id: I42f2e69e6511469ff1ac03c5fec3492200a34e40 --- kuryr_libnetwork/controllers.py | 6 +- kuryr_libnetwork/tests/unit/base.py | 2 +- .../tests/unit/test_kuryr_ipam.py | 83 ++++++++++++++++++- 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/kuryr_libnetwork/controllers.py b/kuryr_libnetwork/controllers.py index 0f13c882..e5996b75 100644 --- a/kuryr_libnetwork/controllers.py +++ b/kuryr_libnetwork/controllers.py @@ -1514,11 +1514,13 @@ def ipam_release_address(): iface = ipaddress.ip_interface(six.text_type(rel_address)) rel_ip_address = six.text_type(iface.ip) try: - all_ports = app.neutron.list_ports(device_owner=lib_const.DEVICE_OWNER) + all_ports = app.neutron.list_ports() for port in all_ports['ports']: for tmp_subnet in subnets: if (port['fixed_ips'][0]['subnet_id'] == tmp_subnet['id'] and - port['fixed_ips'][0]['ip_address'] == rel_ip_address): + port['fixed_ips'][0]['ip_address'] == rel_ip_address and + port['name'] == utils.get_neutron_port_name( + port['device_id'])): app.neutron.delete_port(port['id']) except n_exceptions.NeutronClientException as ex: app.logger.error(_LE("Error happened while fetching " diff --git a/kuryr_libnetwork/tests/unit/base.py b/kuryr_libnetwork/tests/unit/base.py index c2ec584a..55aa91df 100644 --- a/kuryr_libnetwork/tests/unit/base.py +++ b/kuryr_libnetwork/tests/unit/base.py @@ -176,7 +176,7 @@ class TestKuryrBase(TestCase): "fixed_ips": [], "id": neutron_port_id, "security_groups": [], - "device_id": "" + "device_id": docker_endpoint_id } } diff --git a/kuryr_libnetwork/tests/unit/test_kuryr_ipam.py b/kuryr_libnetwork/tests/unit/test_kuryr_ipam.py index cafb3643..bff5108d 100644 --- a/kuryr_libnetwork/tests/unit/test_kuryr_ipam.py +++ b/kuryr_libnetwork/tests/unit/test_kuryr_ipam.py @@ -765,7 +765,86 @@ class TestKuryrIpam(base.TestKuryrBase): id=fake_kuryr_subnetpool_id) mock_list_subnets.assert_called_with( cidr=FAKE_IP4_CIDR) - mock_list_ports.assert_called_with( - device_owner=lib_const.DEVICE_OWNER) + mock_list_ports.assert_called() mock_delete_port.assert_called_with( fake_port['port']['id']) + + @mock.patch('kuryr_libnetwork.controllers.app.neutron.delete_port') + @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_ports') + @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets') + @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnetpools') + def test_ipam_driver_release_address_w_existing_port_w_same_ip_and_cidr( + self, mock_list_subnetpools, mock_list_subnets, mock_list_ports, + mock_delete_port): + # It checks only the kuryr port is removed even if the other port + # has the same IP and belongs to a subnet with the same cidr + # faking list_subnetpools + fake_kuryr_subnetpool_id = uuidutils.generate_uuid() + fake_name = str('-'.join(['kuryrPool', FAKE_IP4_CIDR])) + kuryr_subnetpools = self._get_fake_v4_subnetpools( + fake_kuryr_subnetpool_id, prefixes=[FAKE_IP4_CIDR], name=fake_name) + mock_list_subnetpools.return_value = kuryr_subnetpools + + # faking list_subnets + docker_network_id = lib_utils.get_hash() + docker_endpoint_id = lib_utils.get_hash() + no_kuryr_endpoint_id = lib_utils.get_hash() + subnet_v4_id = uuidutils.generate_uuid() + fake_v4_subnet = self._get_fake_v4_subnet( + docker_network_id, docker_endpoint_id, subnet_v4_id, + subnetpool_id=fake_kuryr_subnetpool_id, + cidr=FAKE_IP4_CIDR) + fake_subnet_response = { + 'subnets': [ + fake_v4_subnet['subnet'] + ] + } + mock_list_subnets.return_value = fake_subnet_response + + fake_ip4 = '10.0.0.5' + # faking list_ports and delete_port + neutron_network_id = uuidutils.generate_uuid() + fake_neutron_port_id = uuidutils.generate_uuid() + fake_port = base.TestKuryrBase._get_fake_port( + docker_endpoint_id, neutron_network_id, + fake_neutron_port_id, lib_const.PORT_STATUS_ACTIVE, + subnet_v4_id, + neutron_subnet_v4_address=fake_ip4, + device_owner=lib_const.DEVICE_OWNER) + fake_port_no_kuryr = base.TestKuryrBase._get_fake_port( + no_kuryr_endpoint_id, neutron_network_id, + fake_neutron_port_id, lib_const.PORT_STATUS_ACTIVE, + subnet_v4_id, + neutron_subnet_v4_address=fake_ip4) + fake_port_no_kuryr['port']['name'] = 'port0' + + fake_port['port']['fixed_ips'] = [ + {'subnet_id': subnet_v4_id, 'ip_address': fake_ip4} + ] + + fake_port_no_kuryr['port']['fixed_ips'] = [ + {'subnet_id': subnet_v4_id, 'ip_address': fake_ip4} + ] + + list_port_response = {'ports': [fake_port['port'], + fake_port_no_kuryr['port']]} + mock_list_ports.return_value = list_port_response + + mock_delete_port.return_value = {} + + fake_request = { + 'PoolID': fake_kuryr_subnetpool_id, + 'Address': fake_ip4 + } + response = self.app.post('/IpamDriver.ReleaseAddress', + content_type='application/json', + data=jsonutils.dumps(fake_request)) + + self.assertEqual(200, response.status_code) + mock_list_subnetpools.assert_called_with( + id=fake_kuryr_subnetpool_id) + mock_list_subnets.assert_called_with( + cidr=FAKE_IP4_CIDR) + mock_list_ports.assert_called() + mock_delete_port.assert_called_once_with( + fake_port['port']['id'])