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 <stephenfin@redhat.com>

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 = {}
+    zone_info = {
-    _xform_common_availability_zone(az, 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 = {}
+    zone_info = {
-    _xform_common_availability_zone(az, 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_name'] = az.name
-    zone_info['zone_status'] = getattr(az, 'state', '')
+    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 []

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(
+        self.app.client_manager.sdk_connection = mock.Mock()
-            endpoint=fakes.AUTH_URL,
-            token=fakes.AUTH_TOKEN,
-        )
-        self.app.client_manager.compute = compute_client
 
-        self.compute_azs_mock = compute_client.availability_zones
+        self.app.client_manager.sdk_connection.compute = mock.Mock()
-        self.compute_azs_mock.reset_mock()
+        self.compute_client = self.app.client_manager.sdk_connection.compute
+        self.compute_client.availability_zones = mock.Mock()
 
-        volume_client = volume_fakes.FakeVolumeClient(
+        self.app.client_manager.sdk_connection.volume = mock.Mock()
-            endpoint=fakes.AUTH_URL,
+        self.volume_client = self.app.client_manager.sdk_connection.volume
-            token=fakes.AUTH_TOKEN,
+        self.volume_client.availability_zones = mock.Mock()
-        )
-        self.app.client_manager.volume = volume_client
 
-        self.volume_azs_mock = volume_client.availability_zones
+        self.app.client_manager.network = mock.Mock()
-        self.volume_azs_mock.reset_mock()
+        self.network_client = self.app.client_manager.network
-
+        self.network_client.availability_zones = mock.Mock()
-        network_client = network_fakes.FakeNetworkV2Client(
+        self.network_client.find_extension = mock.Mock()
-            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
 
 
 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.compute_client.availability_zones.return_value = self.compute_azs
-        self.volume_azs_mock.list.return_value = self.volume_azs
+        self.volume_client.availability_zones.return_value = self.volume_azs
-        self.network_azs_mock.return_value = self.network_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.compute_client.availability_zones.assert_called_with(details=True)
-        self.volume_azs_mock.list.assert_called_with()
+        self.volume_client.availability_zones.assert_called_with()
-        self.network_azs_mock.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.compute_client.availability_zones.assert_called_with(details=True)
-        self.volume_azs_mock.list.assert_called_with()
+        self.volume_client.availability_zones.assert_called_with()
-        self.network_azs_mock.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.compute_client.availability_zones.assert_called_with(details=True)
-        self.volume_azs_mock.list.assert_not_called()
+        self.volume_client.availability_zones.assert_not_called()
-        self.network_azs_mock.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.compute_client.availability_zones.assert_not_called()
-        self.volume_azs_mock.list.assert_called_with()
+        self.volume_client.availability_zones.assert_called_with()
-        self.network_azs_mock.assert_not_called()
+        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.compute_client.availability_zones.assert_not_called()
-        self.volume_azs_mock.list.assert_not_called()
+        self.volume_client.availability_zones.assert_not_called()
-        self.network_azs_mock.assert_called_with()
+        self.network_client.availability_zones.assert_called_with()
 
         self.assertEqual(self.short_columnslist, columns)
         datalist = ()

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:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A fake openstack.compute.v2.availability_zone.AvailabilityZone
-    :return:
+        object
-        A FakeResource object with zoneName, zoneState, etc.
     """
     attrs = attrs or {}
 
     # Set default attributes.
     host_name = uuid.uuid4().hex
     service_name = uuid.uuid4().hex
-    service_updated_at = uuid.uuid4().hex
+    availability_zone_info = {
-    availability_zone = {
+        'name': uuid.uuid4().hex,
-        'zoneName': uuid.uuid4().hex,
+        'state': {'available': True},
-        'zoneState': {'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(
+    availability_zone = _availability_zone.AvailabilityZone(
-        info=copy.deepcopy(availability_zone), loaded=True
+        **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:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :param int count: The number of availability zones to fake
-    :param int count:
+    :return: A list of fake
-        The number of AZs to fake
+        openstack.compute.v2.availability_zone.AvailabilityZone objects
-    :return:
-        A list of FakeResource objects faking the AZs
     """
     availability_zones = []
     for i in range(0, count):

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.
 

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.