From 8b2a1145e47fa56703e17ee8bd94fd1d184bee30 Mon Sep 17 00:00:00 2001 From: Andrey Kurilin Date: Wed, 29 Mar 2017 20:05:30 +0300 Subject: [PATCH] [cleanup] Fix neutron resources Change-Id: I38cb050dfdc9d2017221f037a0d6487f89c39590 --- rally/plugins/openstack/cleanup/resources.py | 83 +++++++++++++++-- rally/plugins/openstack/wrappers/network.py | 27 ++++-- .../openstack/cleanup/test_resources.py | 90 ++++++++++++++++++- .../openstack/wrappers/test_network.py | 20 ++--- 4 files changed, 187 insertions(+), 33 deletions(-) diff --git a/rally/plugins/openstack/cleanup/resources.py b/rally/plugins/openstack/cleanup/resources.py index e2c15a24..43684829 100755 --- a/rally/plugins/openstack/cleanup/resources.py +++ b/rally/plugins/openstack/cleanup/resources.py @@ -379,20 +379,85 @@ class NeutronFloatingIP(NeutronMixin): @base.resource("neutron", "port", order=next(_neutron_order), tenant_resource=True) class NeutronPort(NeutronMixin): + # NOTE(andreykurilin): port is the kind of resource that can be created + # automatically. In this case it doesn't have name field which matches + # our resource name templates. But we still need to identify such + # resources, so let's do it by using parent resources. + + ROUTER_INTERFACE_OWNERS = ("network:router_interface", + "network:router_interface_distributed", + "network:ha_router_replicated_interface") + + ROUTER_GATEWAY_OWNER = "network:router_gateway" + + def __init__(self, *args, **kwargs): + super(NeutronPort, self).__init__(*args, **kwargs) + self._cache = {} + + def _get_resources(self, resource): + if resource not in self._cache: + resources = getattr(self._manager(), "list_%s" % resource)() + self._cache[resource] = [r for r in resources[resource] + if r["tenant_id"] == self.tenant_uuid] + return self._cache[resource] + + def list(self): + ports = self._get_resources("ports") + for port in ports: + if not port.get("name"): + parent_name = None + if (port["device_owner"] in self.ROUTER_INTERFACE_OWNERS or + port["device_owner"] == self.ROUTER_GATEWAY_OWNER): + # first case is a port created while adding an interface to + # the subnet + # second case is a port created while adding gateway for + # the network + port_router = [r for r in self._get_resources("routers") + if r["id"] == port["device_id"]] + if port_router: + parent_name = port_router[0]["name"] + # NOTE(andreykurilin): in case of existing network usage, + # there is no way to identify ports that was created + # automatically. + # FIXME(andreykurilin): find the way to filter ports created + # by rally + # elif port["device_owner"] == "network:dhcp": + # # port created while attaching a floating-ip to the VM + # if port.get("fixed_ips"): + # port_subnets = [] + # for fixedip in port["fixed_ips"]: + # port_subnets.extend( + # [sn for sn in self._get_resources("subnets") + # if sn["id"] == fixedip["subnet_id"]]) + # if port_subnets: + # parent_name = port_subnets[0]["name"] + + # NOTE(andreykurilin): the same case as for floating ips + # if not parent_name: + # port_net = [net for net in self._get_resources("networks") + # if net["id"] == port["network_id"]] + # if port_net: + # parent_name = port_net[0]["name"] + + if parent_name: + port["parent_name"] = parent_name + return ports def name(self): - # TODO(andreykurilin): return NoName instance only in case of "name" - # field is missed - return base.NoName(self._resource) + name = self.raw_resource.get("parent_name", + self.raw_resource.get("name", "")) + return name or base.NoName(self._resource) def delete(self): - if (self.raw_resource["device_owner"] in - ("network:router_interface", - "network:router_interface_distributed", - "network:ha_router_replicated_interface")): + device_owner = self.raw_resource["device_owner"] + if (device_owner in self.ROUTER_INTERFACE_OWNERS or + device_owner == self.ROUTER_GATEWAY_OWNER): + if device_owner == self.ROUTER_GATEWAY_OWNER: + self._manager().remove_gateway_router( + self.raw_resource["device_id"]) + self._manager().remove_interface_router( - self.raw_resource["device_id"], - {"port_id": self.raw_resource["id"]}) + self.raw_resource["device_id"], {"port_id": self.id()}) else: try: self._manager().delete_port(self.id()) diff --git a/rally/plugins/openstack/wrappers/network.py b/rally/plugins/openstack/wrappers/network.py index f28c41f4..a1c3e7d8 100644 --- a/rally/plugins/openstack/wrappers/network.py +++ b/rally/plugins/openstack/wrappers/network.py @@ -364,21 +364,30 @@ class NeutronWrapper(NetworkWrapper): for net_dhcp in net_dhcps: self.client.remove_network_from_dhcp_agent(net_dhcp["id"], network["id"]) - router_id = network["router_id"] - if router_id: - self.client.remove_gateway_router(router_id) - for subnet_id in network["subnets"]: - self.client.remove_interface_router(router_id, - {"subnet_id": subnet_id}) - self.client.delete_router(router_id) + + if network["router_id"]: + self.client.remove_gateway_router(network["router_id"]) for port in self.client.list_ports(network_id=network["id"])["ports"]: - self.client.delete_port(port["id"]) + if port["device_owner"] in ( + "network:router_interface", + "network:router_interface_distributed", + "network:ha_router_replicated_interface", + "network:router_gateway"): + self.client.remove_interface_router( + port["device_id"], {"port_id": port["id"]}) + else: + self.client.delete_port(port["id"]) for subnet_id in network["subnets"]: self._delete_subnet(subnet_id) - return self.client.delete_network(network["id"]) + responce = self.client.delete_network(network["id"]) + + if network["router_id"]: + self.client.delete_router(network["router_id"]) + + return responce def _delete_subnet(self, subnet_id): self.client.delete_subnet(subnet_id) diff --git a/tests/unit/plugins/openstack/cleanup/test_resources.py b/tests/unit/plugins/openstack/cleanup/test_resources.py index 59b79401..8bde337a 100755 --- a/tests/unit/plugins/openstack/cleanup/test_resources.py +++ b/tests/unit/plugins/openstack/cleanup/test_resources.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import copy + from boto import exception as boto_exception import ddt import mock @@ -472,17 +474,97 @@ class NeutronPortTestCase(test.TestCase): def test_name(self): raw_res = { - "device_owner": "network:router_interface", "id": "some_id", "device_id": "dev_id", - "name": "foo" } - user = mock.MagicMock() self.assertIsInstance( - resources.NeutronPort(resource=raw_res, user=user).name(), + resources.NeutronPort(resource=raw_res, + user=mock.MagicMock()).name(), resources.base.NoName) + raw_res["name"] = "foo" + self.assertEqual("foo", resources.NeutronPort( + resource=raw_res, user=mock.MagicMock()).name()) + + raw_res["parent_name"] = "bar" + self.assertEqual("bar", resources.NeutronPort( + resource=raw_res, user=mock.MagicMock()).name()) + + del raw_res["name"] + self.assertEqual("bar", resources.NeutronPort( + resource=raw_res, user=mock.MagicMock()).name()) + + def test_list(self): + + tenant_uuid = "uuuu-uuuu-iiii-dddd" + + ports = [ + # the case when 'name' is present, so 'device_owner' field is not + # required + {"tenant_id": tenant_uuid, "id": "id1", "name": "foo"}, + # 3 different cases when router_interface is an owner + {"tenant_id": tenant_uuid, "id": "id2", + "device_owner": "network:router_interface", + "device_id": "router-1"}, + {"tenant_id": tenant_uuid, "id": "id3", + "device_owner": "network:router_interface_distributed", + "device_id": "router-1"}, + {"tenant_id": tenant_uuid, "id": "id4", + "device_owner": "network:ha_router_replicated_interface", + "device_id": "router-2"}, + # the case when gateway router is an owner + {"tenant_id": tenant_uuid, "id": "id5", + "device_owner": "network:router_gateway", + "device_id": "router-3"}, + # the case when gateway router is an owner, but device_id is + # invalid + {"tenant_id": tenant_uuid, "id": "id6", + "device_owner": "network:router_gateway", + "device_id": "aaaa"}, + # the case when port was auto-created with floating-ip + {"tenant_id": tenant_uuid, "id": "id7", + "device_owner": "network:dhcp", + "device_id": "asdasdasd"}, + # the case when port is from another tenant + {"tenant_id": "wrong tenant", "id": "id8", "name": "foo"}, + # WTF port without any parent and name + {"tenant_id": tenant_uuid, "id": "id9", "device_owner": ""}, + ] + + routers = [ + {"id": "router-1", "name": "Router-1", "tenant_id": tenant_uuid}, + {"id": "router-2", "name": "Router-2", "tenant_id": tenant_uuid}, + {"id": "router-3", "name": "Router-3", "tenant_id": tenant_uuid}, + {"id": "router-4", "name": "Router-4", "tenant_id": tenant_uuid}, + {"id": "router-5", "name": "Router-5", "tenant_id": tenant_uuid}, + ] + + expected_ports = [] + for port in ports: + if port["tenant_id"] == tenant_uuid: + expected_ports.append(copy.deepcopy(port)) + if ("device_id" in port and + port["device_id"].startswith("router")): + expected_ports[-1]["parent_name"] = [ + r for r in routers + if r["id"] == port["device_id"]][0]["name"] + + class FakeNeutronClient(object): + + list_ports = mock.Mock() + list_routers = mock.Mock() + + neutron = FakeNeutronClient + neutron.list_ports.return_value = {"ports": ports} + neutron.list_routers.return_value = {"routers": routers} + + user = mock.Mock(neutron=neutron) + self.assertEqual(expected_ports, resources.NeutronPort( + user=user, tenant_uuid=tenant_uuid).list()) + neutron.list_ports.assert_called_once_with() + neutron.list_routers.assert_called_once_with() + @ddt.ddt class NeutronSecurityGroupTestCase(test.TestCase): diff --git a/tests/unit/plugins/openstack/wrappers/test_network.py b/tests/unit/plugins/openstack/wrappers/test_network.py index ca235f4a..bda26388 100644 --- a/tests/unit/plugins/openstack/wrappers/test_network.py +++ b/tests/unit/plugins/openstack/wrappers/test_network.py @@ -408,29 +408,27 @@ class NeutronWrapperTestCase(test.TestCase): service = self.get_wrapper() agents = ["foo_agent", "bar_agent"] subnets = ["foo_subnet", "bar_subnet"] - ports = ["foo_port", "bar_port"] + ports = [{"id": "foo_port", "device_owner": "network:router_interface", + "device_id": "rounttter"}, + {"id": "bar_port", "device_owner": "network:dhcp"}] service.client.list_dhcp_agent_hosting_networks.return_value = ( {"agents": [{"id": agent_id} for agent_id in agents]}) - service.client.list_ports.return_value = ( - {"ports": [{"id": port_id} for port_id in ports]}) + service.client.list_ports.return_value = ({"ports": ports}) service.client.delete_network.return_value = "foo_deleted" + result = service.delete_network( {"id": "foo_id", "router_id": "foo_router", "subnets": subnets, "lb_pools": []}) + self.assertEqual(result, "foo_deleted") self.assertEqual( service.client.remove_network_from_dhcp_agent.mock_calls, [mock.call(agent_id, "foo_id") for agent_id in agents]) self.assertEqual(service.client.remove_gateway_router.mock_calls, [mock.call("foo_router")]) - self.assertEqual( - service.client.remove_interface_router.mock_calls, - [mock.call("foo_router", {"subnet_id": subnet_id}) - for subnet_id in subnets]) - self.assertEqual(service.client.delete_router.mock_calls, - [mock.call("foo_router")]) - self.assertEqual(service.client.delete_port.mock_calls, - [mock.call(port_id) for port_id in ports]) + service.client.delete_port.assert_called_once_with(ports[1]["id"]) + service.client.remove_interface_router.assert_called_once_with( + ports[0]["device_id"], {"port_id": ports[0]["id"]}) self.assertEqual(service.client.delete_subnet.mock_calls, [mock.call(subnet_id) for subnet_id in subnets]) service.client.delete_network.assert_called_once_with("foo_id")