diff --git a/rally/plugins/openstack/cleanup/base.py b/rally/plugins/openstack/cleanup/base.py index 55f7a9c3..883e4a14 100644 --- a/rally/plugins/openstack/cleanup/base.py +++ b/rally/plugins/openstack/cleanup/base.py @@ -32,6 +32,22 @@ CONF.register_group(cleanup_group) CONF.register_opts(CLEANUP_OPTS, cleanup_group) +# NOTE(andreykurilin): There are cases when there is no way to use any kind +# of "name" for resource as an identifier of alignment resource to the +# particular task run and even to Rally itself. Previously, we used empty +# strings as a workaround for name matching specific templates, but +# theoretically such behaviour can hide other cases when resource should have +# a name property, but it is missed. +# Let's use instances of specific class to return as a name of resources +# which do not have names at all. +class NoName(object): + def __init__(self, resource_type): + self.resource_type = resource_type + + def __repr__(self): + return "" % self.resource_type + + def resource(service, resource, order=0, admin_required=False, perform_for_admin_only=False, tenant_resource=False, max_attempts=3, timeout=CONF.cleanup.resource_deletion_timeout, diff --git a/rally/plugins/openstack/cleanup/manager.py b/rally/plugins/openstack/cleanup/manager.py index 1bdc6875..74b3810d 100644 --- a/rally/plugins/openstack/cleanup/manager.py +++ b/rally/plugins/openstack/cleanup/manager.py @@ -183,9 +183,10 @@ class SeekAndDestroy(object): user=self._get_cached_client(user), tenant_uuid=user and user["tenant_id"]) - if manager.name() == "" or rutils.name_matches_object( - manager.name(), *self.resource_classes, - task_id=self.task_id, exact=False): + if (isinstance(manager.name(), base.NoName) or + rutils.name_matches_object( + manager.name(), *self.resource_classes, + task_id=self.task_id, exact=False)): self._delete_single_resource(manager) def exterminate(self): diff --git a/rally/plugins/openstack/cleanup/resources.py b/rally/plugins/openstack/cleanup/resources.py index af85f7ed..4c2c7416 100755 --- a/rally/plugins/openstack/cleanup/resources.py +++ b/rally/plugins/openstack/cleanup/resources.py @@ -20,6 +20,7 @@ from oslo_config import cfg from saharaclient.api import base as saharaclient_base from rally.common import logging +from rally import consts from rally.plugins.openstack.cleanup import base from rally.plugins.openstack.services.identity import identity from rally.plugins.openstack.wrappers import glance as glance_wrapper @@ -317,7 +318,7 @@ class NeutronMixin(SynchronizedDeletion, base.ResourceManager): return self.raw_resource["id"] def name(self): - return self.raw_resource.get("name", "") + return self.raw_resource["name"] def delete(self): delete_method = getattr(self._manager(), "delete_%s" % self._resource) @@ -378,16 +379,33 @@ class NeutronV2Loadbalancer(NeutronLbaasV2Mixin): return False +# NOTE(andreykurilin): There are scenarios which uses unified way for creating +# and associating floating ips. They do not care about nova-net and neutron. +# We should clean floating IPs for them, but hardcoding "neutron.floatingip" +# cleanup resource should not work in case of Nova-Net. +# Since we are planning to abandon support of Nova-Network in next rally +# release, let's apply dirty workaround to handle all resources. @base.resource("neutron", "floatingip", order=next(_neutron_order), tenant_resource=True) class NeutronFloatingIP(NeutronMixin): - pass + def name(self): + return base.NoName(self._resource) + + def list(self): + if consts.ServiceType.NETWORK not in self.user.services(): + return [] + return super(NeutronFloatingIP, self).list() @base.resource("neutron", "port", order=next(_neutron_order), tenant_resource=True) class NeutronPort(NeutronMixin): + def name(self): + # TODO(andreykurilin): return NoName instance only in case of "name" + # field is missed + return base.NoName(self._resource) + def delete(self): if (self.raw_resource["device_owner"] in ("network:router_interface", diff --git a/rally/plugins/openstack/scenarios/nova/servers.py b/rally/plugins/openstack/scenarios/nova/servers.py index 57783da8..7d7eb3f4 100755 --- a/rally/plugins/openstack/scenarios/nova/servers.py +++ b/rally/plugins/openstack/scenarios/nova/servers.py @@ -813,7 +813,7 @@ class BootAndRebuildServer(utils.NovaScenario, cinder_utils.CinderScenario): @validation.required_services(consts.Service.NOVA) @validation.required_openstack(users=True) @validation.required_contexts("network") -@scenario.configure(context={"cleanup": ["nova"]}, +@scenario.configure(context={"cleanup": ["nova", "neutron.floatingip"]}, name="NovaServers.boot_and_associate_floating_ip") class BootAndAssociateFloatingIp(utils.NovaScenario, cinder_utils.CinderScenario): @@ -947,7 +947,7 @@ class BootServerFromVolumeSnapshot(utils.NovaScenario, @validation.required_services(consts.Service.NOVA) @validation.required_openstack(users=True) @validation.required_contexts("network") -@scenario.configure(context={"cleanup": ["nova"]}, +@scenario.configure(context={"cleanup": ["nova", "neutron.floatingip"]}, name="NovaServers.boot_server_associate_and" "_dissociate_floating_ip") class BootServerAssociateAndDissociateFloatingIP(utils.NovaScenario): diff --git a/tests/unit/plugins/openstack/cleanup/test_manager.py b/tests/unit/plugins/openstack/cleanup/test_manager.py index bfa7dd5a..04cc1069 100644 --- a/tests/unit/plugins/openstack/cleanup/test_manager.py +++ b/tests/unit/plugins/openstack/cleanup/test_manager.py @@ -328,6 +328,28 @@ class SeekAndDestroyTestCase(test.TestCase): mock__delete_single_resource.assert_called_once_with( mock_mgr.return_value) + @mock.patch("rally.common.utils.name_matches_object") + @mock.patch("%s.SeekAndDestroy._get_cached_client" % BASE) + @mock.patch("%s.SeekAndDestroy._delete_single_resource" % BASE) + def test__consumer_with_noname_resource(self, mock__delete_single_resource, + mock__get_cached_client, + mock_name_matches_object): + mock_mgr = mock.MagicMock(__name__="Test") + mock_mgr.return_value.name.return_value = True + task_id = "task_id" + mock_name_matches_object.return_value = False + + consumer = manager.SeekAndDestroy(mock_mgr, None, None, + task_id=task_id)._consumer + + consumer(None, (None, None, "res")) + self.assertFalse(mock__delete_single_resource.called) + + mock_mgr.return_value.name.return_value = base.NoName("foo") + consumer(None, (None, None, "res")) + mock__delete_single_resource.assert_called_once_with( + mock_mgr.return_value) + @mock.patch("%s.broker.run" % BASE) def test_exterminate(self, mock_broker_run): manager_cls = mock.MagicMock(_threads=5) diff --git a/tests/unit/plugins/openstack/cleanup/test_resources.py b/tests/unit/plugins/openstack/cleanup/test_resources.py index 48019339..d200fbe3 100755 --- a/tests/unit/plugins/openstack/cleanup/test_resources.py +++ b/tests/unit/plugins/openstack/cleanup/test_resources.py @@ -20,6 +20,7 @@ from neutronclient.common import exceptions as neutron_exceptions from novaclient import exceptions as nova_exc from watcherclient.common.apiclient import exceptions as watcher_exceptions +from rally import consts from rally.plugins.openstack.cleanup import resources from tests.unit import test @@ -439,6 +440,32 @@ class NeutronV2LoadbalancerTestCase(test.TestCase): neutron_lb.id()) +class NeutronFloatingIPTestCase(test.TestCase): + + def test_name(self): + fips = resources.NeutronFloatingIP({"name": "foo"}) + self.assertIsInstance(fips.name(), resources.base.NoName) + + def test_list(self): + fips = {"floatingips": [{"tenant_id": "foo", "id": "foo"}]} + + user = mock.MagicMock() + user.services.return_value = {} + user.neutron.return_value.list_floatingips.return_value = fips + + self.assertEqual( + [], resources.NeutronFloatingIP(user=user, + tenant_uuid="foo").list()) + self.assertFalse(user.neutron.return_value.list_floatingips.called) + + user.services.return_value = { + consts.ServiceType.NETWORK: consts.Service.NEUTRON} + self.assertEqual(fips["floatingips"], list( + resources.NeutronFloatingIP(user=user, tenant_uuid="foo").list())) + user.neutron.return_value.list_floatingips.assert_called_once_with( + tenant_id="foo") + + class NeutronPortTestCase(test.TestCase): def test_delete(self): @@ -472,6 +499,19 @@ class NeutronPortTestCase(test.TestCase): user.neutron().remove_interface_router.assert_called_once_with( raw_res["device_id"], {"port_id": raw_res["id"]}) + 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.base.NoName) + @ddt.ddt class NeutronSecurityGroupTestCase(test.TestCase):