diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3e94bfdb..a80214f5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -24,6 +24,8 @@ Added * Support for specifying microversions for Cinder service. * Support Python 3.11 +* ``nova_servers_list_page_size`` configuration option to limit the size of + pages during listing Nova Servers Removed ~~~~~~~ diff --git a/rally_openstack/common/cfg/nova.py b/rally_openstack/common/cfg/nova.py index 5c07dbf7..88eabbaa 100644 --- a/rally_openstack/common/cfg/nova.py +++ b/rally_openstack/common/cfg/nova.py @@ -16,6 +16,12 @@ from rally.common import cfg OPTS = {"openstack": [ + cfg.IntOpt( + "nova_servers_list_page_size", + default=None, + min=1, + help="Page size to use while listing servers" + ), # prepoll delay, timeout, poll interval # "start": (0, 300, 1) cfg.FloatOpt("nova_server_start_prepoll_delay", diff --git a/rally_openstack/task/cleanup/resources.py b/rally_openstack/task/cleanup/resources.py index f54f0720..0ddbd93c 100644 --- a/rally_openstack/task/cleanup/resources.py +++ b/rally_openstack/task/cleanup/resources.py @@ -22,6 +22,7 @@ from rally_openstack.common.services.image import glance_v2 from rally_openstack.common.services.image import image from rally_openstack.common.services.network import neutron from rally_openstack.task.cleanup import base +from rally_openstack.task.scenarios.nova import utils as nova_utils CONF = cfg.CONF @@ -144,33 +145,13 @@ _nova_order = get_order(200) class NovaServer(base.ResourceManager): def list(self): """List all servers.""" - from novaclient import exceptions as nova_exc - - nc = self._manager() - - all_servers = [] - - marker = None - bad_markers = [] - while True: - try: - servers = nc.list(marker=marker) - except nova_exc.BadRequest as e: - if f"marker [{marker}] not found" not in e.message: - raise - LOG.error(f"Ignoring '{e.message}' error to fetch more " - f"servers for cleanup.") - bad_markers.append(marker) - servers = [] - else: - bad_markers = [] - - all_servers.extend(servers) - if not servers and (not bad_markers - or len(bad_markers) == len(all_servers)): - break - marker = all_servers[-(len(bad_markers) + 1)].id - return all_servers + clients = (self._admin_required and self.admin or self.user) + nc = getattr(clients, self._service)() + return nova_utils.list_servers( + nc, + # we need details to get locked states + detailed=True + ) def delete(self): if getattr(self.raw_resource, "OS-EXT-STS:locked", False): diff --git a/rally_openstack/task/scenarios/nova/utils.py b/rally_openstack/task/scenarios/nova/utils.py index ba62d5bb..9107d262 100644 --- a/rally_openstack/task/scenarios/nova/utils.py +++ b/rally_openstack/task/scenarios/nova/utils.py @@ -30,6 +30,64 @@ CONF = cfg.CONF LOG = logging.getLogger(__file__) +def list_servers(client, detailed=True): + """List nova servers with pagination + + :param client: novaclient instance + :param detailed: Whether to request detailed server view or not + """ + from novaclient import exceptions as nova_exc + + base_url = f"/servers{'/detail' if detailed else ''}" + + all_servers = [] + all_ids = [] + + last_success_marker = None + marker = None + + bad_markers_count = 0 + + while True: + filters = {} + + if marker: + filters["marker"] = marker + if CONF.openstack.nova_servers_list_page_size: + filters["limit"] = CONF.openstack.nova_servers_list_page_size + + try: + servers = client.servers._list( + f"{base_url}", response_key="servers", + filters=filters + ) + except nova_exc.BadRequest as e: + if f"marker [{marker}] not found" not in e.message: + raise + LOG.error(f"Ignoring '{e.message}' error to fetch more " + f"servers for cleanup.") + bad_markers_count += 1 + + if bad_markers_count == len(all_servers): + break + marker = all_servers[-(bad_markers_count + 1)].id + if marker == last_success_marker: + break + continue + + bad_markers_count = 0 + last_success_marker = marker + marker = servers[-1].id if servers else None + + all_servers.extend(s for s in servers if s.id not in all_ids) + all_ids.extend(s.id for s in servers) + + if not servers: + break + + return all_servers + + class NovaScenario(neutron_utils.NeutronBaseScenario, scenario.OpenStackScenario): """Base class for Nova scenarios with basic atomic actions.""" @@ -37,7 +95,7 @@ class NovaScenario(neutron_utils.NeutronBaseScenario, @atomic.action_timer("nova.list_servers") def _list_servers(self, detailed=True): """Returns user servers list.""" - return self.clients("nova").servers.list(detailed) + return list_servers(self.clients("nova"), detailed=detailed) def _pick_random_nic(self): """Choose one network from existing ones.""" diff --git a/tests/unit/task/cleanup/test_resources.py b/tests/unit/task/cleanup/test_resources.py index 0cb10daa..17d6ae67 100644 --- a/tests/unit/task/cleanup/test_resources.py +++ b/tests/unit/task/cleanup/test_resources.py @@ -15,7 +15,6 @@ import copy from unittest import mock -import uuid import ddt from neutronclient.common import exceptions as neutron_exceptions @@ -77,42 +76,22 @@ class MagnumMixinTestCase(test.TestCase): class NovaServerTestCase(test.TestCase): - def test_list(self): - class Server(object): - def __init__(self, id=None): - self.id = id or str(uuid.uuid4()) + @mock.patch(BASE + ".nova_utils.list_servers") + def test_list(self, mock_list_servers): - manager_cls = mock.Mock() - manager = manager_cls.return_value + user_clients = mock.Mock() + admin_clients = mock.Mock() - server1 = Server(id=1) - server2 = Server(id=2) - server3 = Server(id=3) - server4 = Server(id=4) - - manager.list.side_effect = ( - [server1, server2, server3], - # simulate marker error - nova_exc.BadRequest(code=400, message="marker [3] not found"), - nova_exc.BadRequest(code=400, message="marker [2] not found"), - # valid response - [server4], - # fetching is completed - [] + servers_res = resources.NovaServer( + admin=admin_clients, user=user_clients ) - servers_res = resources.NovaServer() - servers_res._manager = manager_cls - self.assertEqual( - [server1, server2, server3, server4], + mock_list_servers.return_value, servers_res.list() ) - - self.assertEqual( - [mock.call(marker=m) for m in (None, 3, 2, 1, 4)], - manager.list.call_args_list - ) + mock_list_servers.assert_called_once_with(user_clients.nova(), + detailed=True) def test_delete(self): server = resources.NovaServer() diff --git a/tests/unit/task/scenarios/nova/test_utils.py b/tests/unit/task/scenarios/nova/test_utils.py index 2cd23abe..11763a94 100644 --- a/tests/unit/task/scenarios/nova/test_utils.py +++ b/tests/unit/task/scenarios/nova/test_utils.py @@ -16,6 +16,7 @@ from unittest import mock import ddt +from novaclient import exceptions as nova_exc from rally.common import cfg from rally import exceptions as rally_exceptions @@ -50,7 +51,7 @@ class NovaScenarioTestCase(test.ScenarioTestCase): def test__list_servers(self): servers_list = [] - self.clients("nova").servers.list.return_value = servers_list + self.clients("nova").servers._list.return_value = servers_list nova_scenario = utils.NovaScenario(self.context) return_servers_list = nova_scenario._list_servers(True) self.assertEqual(servers_list, return_servers_list) @@ -1286,3 +1287,96 @@ class NovaScenarioTestCase(test.ScenarioTestCase): "fake_aggregate", fake_metadata) self._test_atomic_action_timer(nova_scenario.atomic_actions(), "nova.aggregate_set_metadata") + + def test_list_servers(self): + + class ServersManager(object): + def __init__(self): + self._list = mock.Mock() + + class NovaClient(object): + def __init__(self): + self.servers = ServersManager() + + s1 = fakes.FakeServer() + s2 = fakes.FakeServer() + s3 = fakes.FakeServer() + s4 = fakes.FakeServer() + s5 = fakes.FakeServer() + s6 = fakes.FakeServer() + + # case #1 pagination ends with bad marker error + nc = NovaClient() + + nc.servers._list.side_effect = ( + [s1, s2, s3], + [s4, s5], + nova_exc.BadRequest(400, f"marker [{s5.id}] not found"), + [s5, s6], + nova_exc.BadRequest(400, f"marker [{s6.id}] not found"), + nova_exc.BadRequest(400, f"marker [{s5.id}] not found") + ) + self.assertEqual([s1, s2, s3, s4, s5, s6], utils.list_servers(nc)) + + self.assertEqual( + [ + mock.call( + "/servers/detail", + response_key="servers", + filters={} + ), + mock.call( + "/servers/detail", + response_key="servers", + filters={"marker": s3.id} + ), + mock.call( + "/servers/detail", + response_key="servers", + filters={"marker": s5.id} + ), + mock.call( + "/servers/detail", + response_key="servers", + filters={"marker": s4.id} + ), + mock.call( + "/servers/detail", + response_key="servers", + filters={"marker": s6.id} + ), + mock.call( + "/servers/detail", + response_key="servers", + filters={"marker": s5.id} + ) + ], + nc.servers._list.call_args_list + ) + + # case #2 pagination ends without any errors + limit + CONF.set_override("nova_servers_list_page_size", 2, "openstack") + nc = NovaClient() + + nc.servers._list.side_effect = ( + [s1, s2, s3], + [s4, s5], + [] + ) + self.assertEqual([s1, s2, s3, s4, s5], + utils.list_servers(nc, detailed=False)) + + self.assertEqual( + [ + mock.call("/servers", + response_key="servers", + filters={"limit": 2}), + mock.call("/servers", + response_key="servers", + filters={"marker": s3.id, "limit": 2}), + mock.call("/servers", + response_key="servers", + filters={"marker": s5.id, "limit": 2}) + ], + nc.servers._list.call_args_list + )