From 44cf963d8ed4984604a6b8f0610cd56f354cd638 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 2 Jun 2023 12:42:54 +0100 Subject: [PATCH] Migrate 'availability zone list' to SDK This is a bit of an odd one. Because this is a "common" command, there are three clients in play here: a compute client, a block storage client, and a networking client. The networking aspects of this command are already using SDK though the tests were mistakenly using a fake version of the legacy neutron client (albeit one with the correct SDK properties). We were still using legacy clients for the other two. Correct the networking tests and migrate the compute and block storage aspects of the command. Change-Id: I292e1a712bca97e1270e8a97606e02a8a44c2954 Signed-off-by: Stephen Finucane --- openstackclient/common/availability_zone.py | 50 ++++++----- .../unit/common/test_availability_zone.py | 82 ++++++++----------- .../tests/unit/compute/v2/fakes.py | 39 ++++----- openstackclient/tests/unit/volume/v2/fakes.py | 43 ---------- openstackclient/tests/unit/volume/v3/fakes.py | 41 ++++++++++ 5 files changed, 113 insertions(+), 142 deletions(-) diff --git a/openstackclient/common/availability_zone.py b/openstackclient/common/availability_zone.py index 935f90c6c4..af6980f1db 100644 --- a/openstackclient/common/availability_zone.py +++ b/openstackclient/common/availability_zone.py @@ -26,27 +26,20 @@ from openstackclient.i18n import _ LOG = logging.getLogger(__name__) -def _xform_common_availability_zone(az, zone_info): - if hasattr(az, 'zoneState'): - zone_info['zone_status'] = ( - 'available' if az.zoneState['available'] else 'not available' - ) - if hasattr(az, 'zoneName'): - zone_info['zone_name'] = az.zoneName - - zone_info['zone_resource'] = '' - - def _xform_compute_availability_zone(az, include_extra): result = [] - zone_info = {} - _xform_common_availability_zone(az, zone_info) + zone_info = { + 'zone_name': az.name, + 'zone_status': ( + 'available' if az.state['available'] else 'not available' + ), + } if not include_extra: result.append(zone_info) return result - if hasattr(az, 'hosts') and az.hosts: + if az.hosts: for host, services in az.hosts.items(): host_info = copy.deepcopy(zone_info) host_info['host_name'] = host @@ -70,8 +63,12 @@ def _xform_compute_availability_zone(az, include_extra): def _xform_volume_availability_zone(az): result = [] - zone_info = {} - _xform_common_availability_zone(az, zone_info) + zone_info = { + 'zone_name': az.name, + 'zone_status': ( + 'available' if az.state['available'] else 'not available' + ), + } result.append(zone_info) return result @@ -79,11 +76,11 @@ def _xform_volume_availability_zone(az): def _xform_network_availability_zone(az): result = [] zone_info = {} - zone_info['zone_name'] = getattr(az, 'name', '') - zone_info['zone_status'] = getattr(az, 'state', '') + zone_info['zone_name'] = az.name + zone_info['zone_status'] = az.state if 'unavailable' == zone_info['zone_status']: zone_info['zone_status'] = 'not available' - zone_info['zone_resource'] = getattr(az, 'resource', '') + zone_info['zone_resource'] = az.resource result.append(zone_info) return result @@ -92,7 +89,7 @@ class ListAvailabilityZone(command.Lister): _description = _("List availability zones and their status") def get_parser(self, prog_name): - parser = super(ListAvailabilityZone, self).get_parser(prog_name) + parser = super().get_parser(prog_name) parser.add_argument( '--compute', action='store_true', @@ -120,26 +117,25 @@ class ListAvailabilityZone(command.Lister): return parser def _get_compute_availability_zones(self, parsed_args): - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute try: - data = compute_client.availability_zones.list() + data = compute_client.availability_zones(details=True) except nova_exceptions.Forbidden: # policy doesn't allow try: - data = compute_client.availability_zones.list(detailed=False) + data = compute_client.availability_zones(details=False) except Exception: raise - # Argh, the availability zones are not iterable... result = [] for zone in data: result += _xform_compute_availability_zone(zone, parsed_args.long) return result def _get_volume_availability_zones(self, parsed_args): - volume_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume data = [] try: - data = volume_client.availability_zones.list() + data = volume_client.availability_zones() except Exception as e: LOG.debug('Volume availability zone exception: %s', e) if parsed_args.volume: @@ -165,7 +161,7 @@ class ListAvailabilityZone(command.Lister): LOG.debug('Network availability zone exception: ', e) if parsed_args.network: message = _( - "Availability zones list not supported by " "Network API" + "Availability zones list not supported by Network API" ) LOG.warning(message) return [] diff --git a/openstackclient/tests/unit/common/test_availability_zone.py b/openstackclient/tests/unit/common/test_availability_zone.py index 2ee6e2c4ad..0d38ad41dd 100644 --- a/openstackclient/tests/unit/common/test_availability_zone.py +++ b/openstackclient/tests/unit/common/test_availability_zone.py @@ -15,24 +15,23 @@ from unittest import mock from openstackclient.common import availability_zone from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes -from openstackclient.tests.unit import fakes from openstackclient.tests.unit.network.v2 import fakes as network_fakes from openstackclient.tests.unit import utils -from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes +from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes def _build_compute_az_datalist(compute_az, long_datalist=False): datalist = () if not long_datalist: datalist = ( - compute_az.zoneName, + compute_az.name, 'available', ) else: for host, services in compute_az.hosts.items(): for service, state in services.items(): datalist += ( - compute_az.zoneName, + compute_az.name, 'available', '', host, @@ -46,12 +45,12 @@ def _build_volume_az_datalist(volume_az, long_datalist=False): datalist = () if not long_datalist: datalist = ( - volume_az.zoneName, + volume_az.name, 'available', ) else: datalist = ( - volume_az.zoneName, + volume_az.name, 'available', '', '', @@ -84,33 +83,20 @@ class TestAvailabilityZone(utils.TestCommand): def setUp(self): super().setUp() - compute_client = compute_fakes.FakeComputev2Client( - endpoint=fakes.AUTH_URL, - token=fakes.AUTH_TOKEN, - ) - self.app.client_manager.compute = compute_client + self.app.client_manager.sdk_connection = mock.Mock() - self.compute_azs_mock = compute_client.availability_zones - self.compute_azs_mock.reset_mock() + self.app.client_manager.sdk_connection.compute = mock.Mock() + self.compute_client = self.app.client_manager.sdk_connection.compute + self.compute_client.availability_zones = mock.Mock() - volume_client = volume_fakes.FakeVolumeClient( - endpoint=fakes.AUTH_URL, - token=fakes.AUTH_TOKEN, - ) - self.app.client_manager.volume = volume_client + self.app.client_manager.sdk_connection.volume = mock.Mock() + self.volume_client = self.app.client_manager.sdk_connection.volume + self.volume_client.availability_zones = mock.Mock() - self.volume_azs_mock = volume_client.availability_zones - self.volume_azs_mock.reset_mock() - - network_client = network_fakes.FakeNetworkV2Client( - endpoint=fakes.AUTH_URL, - token=fakes.AUTH_TOKEN, - ) - self.app.client_manager.network = network_client - - network_client.availability_zones = mock.Mock() - network_client.find_extension = mock.Mock() - self.network_azs_mock = network_client.availability_zones + self.app.client_manager.network = mock.Mock() + self.network_client = self.app.client_manager.network + self.network_client.availability_zones = mock.Mock() + self.network_client.find_extension = mock.Mock() class TestAvailabilityZoneList(TestAvailabilityZone): @@ -131,9 +117,9 @@ class TestAvailabilityZoneList(TestAvailabilityZone): def setUp(self): super().setUp() - self.compute_azs_mock.list.return_value = self.compute_azs - self.volume_azs_mock.list.return_value = self.volume_azs - self.network_azs_mock.return_value = self.network_azs + self.compute_client.availability_zones.return_value = self.compute_azs + self.volume_client.availability_zones.return_value = self.volume_azs + self.network_client.availability_zones.return_value = self.network_azs # Get the command object to test self.cmd = availability_zone.ListAvailabilityZone(self.app, None) @@ -148,9 +134,9 @@ class TestAvailabilityZoneList(TestAvailabilityZone): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.compute_azs_mock.list.assert_called_with() - self.volume_azs_mock.list.assert_called_with() - self.network_azs_mock.assert_called_with() + self.compute_client.availability_zones.assert_called_with(details=True) + self.volume_client.availability_zones.assert_called_with() + self.network_client.availability_zones.assert_called_with() self.assertEqual(self.short_columnslist, columns) datalist = () @@ -176,9 +162,9 @@ class TestAvailabilityZoneList(TestAvailabilityZone): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.compute_azs_mock.list.assert_called_with() - self.volume_azs_mock.list.assert_called_with() - self.network_azs_mock.assert_called_with() + self.compute_client.availability_zones.assert_called_with(details=True) + self.volume_client.availability_zones.assert_called_with() + self.network_client.availability_zones.assert_called_with() self.assertEqual(self.long_columnslist, columns) datalist = () @@ -210,9 +196,9 @@ class TestAvailabilityZoneList(TestAvailabilityZone): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.compute_azs_mock.list.assert_called_with() - self.volume_azs_mock.list.assert_not_called() - self.network_azs_mock.assert_not_called() + self.compute_client.availability_zones.assert_called_with(details=True) + self.volume_client.availability_zones.assert_not_called() + self.network_client.availability_zones.assert_not_called() self.assertEqual(self.short_columnslist, columns) datalist = () @@ -234,9 +220,9 @@ class TestAvailabilityZoneList(TestAvailabilityZone): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.compute_azs_mock.list.assert_not_called() - self.volume_azs_mock.list.assert_called_with() - self.network_azs_mock.assert_not_called() + self.compute_client.availability_zones.assert_not_called() + self.volume_client.availability_zones.assert_called_with() + self.network_client.availability_zones.assert_not_called() self.assertEqual(self.short_columnslist, columns) datalist = () @@ -258,9 +244,9 @@ class TestAvailabilityZoneList(TestAvailabilityZone): # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - self.compute_azs_mock.list.assert_not_called() - self.volume_azs_mock.list.assert_not_called() - self.network_azs_mock.assert_called_with() + self.compute_client.availability_zones.assert_not_called() + self.volume_client.availability_zones.assert_not_called() + self.network_client.availability_zones.assert_called_with() self.assertEqual(self.short_columnslist, columns) datalist = () diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index acee68e4e0..aac9104d30 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -20,6 +20,7 @@ import uuid from novaclient import api_versions from openstack.compute.v2 import aggregate as _aggregate +from openstack.compute.v2 import availability_zone as _availability_zone from openstack.compute.v2 import flavor as _flavor from openstack.compute.v2 import hypervisor as _hypervisor from openstack.compute.v2 import keypair as _keypair @@ -83,12 +84,6 @@ class FakeComputev2Client(object): self.agents = mock.Mock() self.agents.resource_class = fakes.FakeResource(None, {}) - self.aggregates = mock.Mock() - self.aggregates.resource_class = fakes.FakeResource(None, {}) - - self.availability_zones = mock.Mock() - self.availability_zones.resource_class = fakes.FakeResource(None, {}) - self.images = mock.Mock() self.images.resource_class = fakes.FakeResource(None, {}) @@ -783,36 +778,34 @@ def get_keypairs(keypairs=None, count=2): def create_one_availability_zone(attrs=None): """Create a fake AZ. - :param dict attrs: - A dictionary with all attributes - :return: - A FakeResource object with zoneName, zoneState, etc. + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.availability_zone.AvailabilityZone + object """ attrs = attrs or {} # Set default attributes. host_name = uuid.uuid4().hex service_name = uuid.uuid4().hex - service_updated_at = uuid.uuid4().hex - availability_zone = { - 'zoneName': uuid.uuid4().hex, - 'zoneState': {'available': True}, + availability_zone_info = { + 'name': uuid.uuid4().hex, + 'state': {'available': True}, 'hosts': { host_name: { service_name: { 'available': True, 'active': True, - 'updated_at': service_updated_at, + 'updated_at': '2023-01-01T00:00:00.000000', } } }, } # Overwrite default attributes. - availability_zone.update(attrs) + availability_zone_info.update(attrs) - availability_zone = fakes.FakeResource( - info=copy.deepcopy(availability_zone), loaded=True + availability_zone = _availability_zone.AvailabilityZone( + **availability_zone_info ) return availability_zone @@ -820,12 +813,10 @@ def create_one_availability_zone(attrs=None): def create_availability_zones(attrs=None, count=2): """Create multiple fake AZs. - :param dict attrs: - A dictionary with all attributes - :param int count: - The number of AZs to fake - :return: - A list of FakeResource objects faking the AZs + :param dict attrs: A dictionary with all attributes + :param int count: The number of availability zones to fake + :return: A list of fake + openstack.compute.v2.availability_zone.AvailabilityZone objects """ availability_zones = [] for i in range(0, count): diff --git a/openstackclient/tests/unit/volume/v2/fakes.py b/openstackclient/tests/unit/volume/v2/fakes.py index 81e3a8a24b..23f6ffe252 100644 --- a/openstackclient/tests/unit/volume/v2/fakes.py +++ b/openstackclient/tests/unit/volume/v2/fakes.py @@ -488,49 +488,6 @@ def get_volume_data(volume=None): return tuple(data_list) -def create_one_availability_zone(attrs=None): - """Create a fake AZ. - - :param dict attrs: - A dictionary with all attributes - :return: - A FakeResource object with zoneName, zoneState, etc. - """ - attrs = attrs or {} - - # Set default attributes. - availability_zone = { - 'zoneName': uuid.uuid4().hex, - 'zoneState': {'available': True}, - } - - # Overwrite default attributes. - availability_zone.update(attrs) - - availability_zone = fakes.FakeResource( - info=copy.deepcopy(availability_zone), loaded=True - ) - return availability_zone - - -def create_availability_zones(attrs=None, count=2): - """Create multiple fake AZs. - - :param dict attrs: - A dictionary with all attributes - :param int count: - The number of AZs to fake - :return: - A list of FakeResource objects faking the AZs - """ - availability_zones = [] - for i in range(0, count): - availability_zone = create_one_availability_zone(attrs) - availability_zones.append(availability_zone) - - return availability_zones - - def create_one_backup(attrs=None): """Create a fake backup. diff --git a/openstackclient/tests/unit/volume/v3/fakes.py b/openstackclient/tests/unit/volume/v3/fakes.py index eb7b7f9ab7..1fb3484df8 100644 --- a/openstackclient/tests/unit/volume/v3/fakes.py +++ b/openstackclient/tests/unit/volume/v3/fakes.py @@ -15,6 +15,7 @@ from unittest import mock import uuid from cinderclient import api_versions +from openstack.block_storage.v3 import availability_zone as _availability_zone from openstack.block_storage.v3 import block_storage_summary as _summary from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes @@ -75,6 +76,46 @@ create_one_volume = volume_v2_fakes.create_one_volume create_one_volume_type = volume_v2_fakes.create_one_volume_type +def create_one_availability_zone(attrs=None): + """Create a fake AZ. + + :param dict attrs: A dictionary with all attributes + :return: A fake + openstack.block_storage.v3.availability_zone.AvailabilityZone object + """ + attrs = attrs or {} + + # Set default attributes. + availability_zone_info = { + 'name': uuid.uuid4().hex, + 'state': {'available': True}, + } + + # Overwrite default attributes. + availability_zone_info.update(attrs) + + availability_zone = _availability_zone.AvailabilityZone( + **availability_zone_info + ) + return availability_zone + + +def create_availability_zones(attrs=None, count=2): + """Create multiple fake AZs. + + :param dict attrs: A dictionary with all attributes + :param int count: The number of availability zones to fake + :return: A list of fake + openstack.block_storage.v3.availability_zone.AvailabilityZone objects + """ + availability_zones = [] + for i in range(0, count): + availability_zone = create_one_availability_zone(attrs) + availability_zones.append(availability_zone) + + return availability_zones + + def create_one_cluster(attrs=None): """Create a fake service cluster.