Switch list server volume to sdk

Ritvik Vinodkumar · stephenfin · commit d47e4320059f · 2022-12-15T17:39:16.000Z
Switch the server volume list command from novaclient to SDK.
Modified functional test for server add/remove volume.

Change-Id: I5b4ab7d0275aec2e02451c5371319ac350af6a5f
diff --git a/openstackclient/compute/v2/server_volume.py b/openstackclient/compute/v2/server_volume.py
@@ -15,6 +15,7 @@
 """Compute v2 Server action implementations"""
 
 from novaclient import api_versions
+from openstack import utils as sdk_utils
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -34,53 +35,47 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
+        compute_client = self.app.client_manager.sdk_connection.compute
 
-        compute_client = self.app.client_manager.compute
-
-        server = utils.find_resource(
-            compute_client.servers,
+        server = compute_client.find_server(
             parsed_args.server,
+            ignore_missing=False,
         )
-
-        volumes = compute_client.volumes.get_server_volumes(server.id)
+        volumes = compute_client.volume_attachments(server)
 
         columns = ()
         column_headers = ()
 
-        if compute_client.api_version < api_versions.APIVersion('2.89'):
+        if not sdk_utils.supports_microversion(compute_client, '2.89'):
             columns += ('id',)
             column_headers += ('ID',)
 
         columns += (
             'device',
-            'serverId',
-            'volumeId',
+            'server_id',
+            'volume_id',
         )
         column_headers += (
             'Device',
             'Server ID',
             'Volume ID',
         )
 
-        if compute_client.api_version >= api_versions.APIVersion('2.70'):
+        if sdk_utils.supports_microversion(compute_client, '2.70'):
             columns += ('tag',)
             column_headers += ('Tag',)
 
-        if compute_client.api_version >= api_versions.APIVersion('2.79'):
+        if sdk_utils.supports_microversion(compute_client, '2.79'):
             columns += ('delete_on_termination',)
             column_headers += ('Delete On Termination?',)
 
-        if compute_client.api_version >= api_versions.APIVersion('2.89'):
-            columns += ('attachment_id', 'bdm_uuid')
+        if sdk_utils.supports_microversion(compute_client, '2.89'):
+            columns += ('attachment_id', 'bdm_id')
             column_headers += ('Attachment ID', 'BlockDeviceMapping UUID')
 
         return (
             column_headers,
-            (
-                utils.get_item_properties(
-                    s, columns, mixed_case_fields=('serverId', 'volumeId')
-                ) for s in volumes
-            ),
+            (utils.get_item_properties(s, columns) for s in volumes),
         )
 
 
diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py
@@ -1291,18 +1291,15 @@ def test_server_add_remove_volume(self):
             parse_output=True,
         )
 
-        self.assertIsNotNone(cmd_output['ID'])
         self.assertEqual(server_id, cmd_output['Server ID'])
         self.assertEqual(volume_id, cmd_output['Volume ID'])
-        volume_attachment_id = cmd_output['ID']
 
         cmd_output = self.openstack(
             'server volume list ' +
             server_name,
             parse_output=True,
         )
 
-        self.assertEqual(volume_attachment_id, cmd_output[0]['ID'])
         self.assertEqual(server_id, cmd_output[0]['Server ID'])
         self.assertEqual(volume_id, cmd_output[0]['Volume ID'])
 
diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py
@@ -1609,122 +1609,49 @@ def create_one_server_migration(attrs=None, methods=None):
         return migration
 
 
-class FakeVolumeAttachment(object):
-    """Fake one or more volume attachments (BDMs)."""
+def create_one_volume_attachment(attrs=None):
+    """Create a fake volume attachment.
 
-    @staticmethod
-    def create_one_volume_attachment(attrs=None, methods=None):
-        """Create a fake volume attachment.
-
-        :param dict attrs:
-            A dictionary with all attributes
-        :param dict methods:
-            A dictionary with all methods
-        :return:
-            A FakeResource object, with id, device, and so on
-        """
-        attrs = attrs or {}
-        methods = methods or {}
-
-        # Set default attributes.
-        volume_attachment_info = {
-            "id": uuid.uuid4().hex,
-            "device": "/dev/sdb",
-            "serverId": uuid.uuid4().hex,
-            "volumeId": uuid.uuid4().hex,
-            # introduced in API microversion 2.70
-            "tag": "foo",
-            # introduced in API microversion 2.79
-            "delete_on_termination": True,
-            # introduced in API microversion 2.89
-            "attachment_id": uuid.uuid4().hex,
-            "bdm_uuid": uuid.uuid4().hex
-        }
-
-        # Overwrite default attributes.
-        volume_attachment_info.update(attrs)
-
-        volume_attachment = fakes.FakeResource(
-            info=copy.deepcopy(volume_attachment_info),
-            methods=methods,
-            loaded=True)
-        return volume_attachment
-
-    @staticmethod
-    def create_volume_attachments(attrs=None, methods=None, count=2):
-        """Create multiple fake volume attachments (BDMs).
-
-        :param dict attrs:
-            A dictionary with all attributes
-        :param dict methods:
-            A dictionary with all methods
-        :param int count:
-            The number of volume attachments to fake
-        :return:
-            A list of FakeResource objects faking the volume attachments.
-        """
-        volume_attachments = []
-        for i in range(0, count):
-            volume_attachments.append(
-                FakeVolumeAttachment.create_one_volume_attachment(
-                    attrs, methods))
-
-        return volume_attachments
-
-    @staticmethod
-    def create_one_sdk_volume_attachment(attrs=None, methods=None):
-        """Create a fake sdk VolumeAttachment.
+    :param dict attrs: A dictionary with all attributes
+    :return: A fake openstack.compute.v2.volume_attachment.VolumeAttachment
+        object
+    """
+    attrs = attrs or {}
 
-        :param dict attrs:
-            A dictionary with all attributes
-        :param dict methods:
-            A dictionary with all methods
-        :return:
-            A fake VolumeAttachment object, with id, device, and so on
-        """
-        attrs = attrs or {}
-        methods = methods or {}
+    # Set default attributes.
+    volume_attachment_info = {
+        "id": uuid.uuid4().hex,
+        "device": "/dev/sdb",
+        "server_id": uuid.uuid4().hex,
+        "volume_id": uuid.uuid4().hex,
+        # introduced in API microversion 2.70
+        "tag": "foo",
+        # introduced in API microversion 2.79
+        "delete_on_termination": True,
+        # introduced in API microversion 2.89
+        "attachment_id": uuid.uuid4().hex,
+        "bdm_id": uuid.uuid4().hex,
+    }
 
-        # Set default attributes.
-        volume_attachment_info = {
-            "id": uuid.uuid4().hex,
-            "device": "/dev/sdb",
-            "server_id": uuid.uuid4().hex,
-            "volume_id": uuid.uuid4().hex,
-            # introduced in API microversion 2.70
-            "tag": "foo",
-            # introduced in API microversion 2.79
-            "delete_on_termination": True,
-            # introduced in API microversion 2.89
-            "attachment_id": uuid.uuid4().hex,
-            "bdm_uuid": uuid.uuid4().hex
-        }
+    # Overwrite default attributes.
+    volume_attachment_info.update(attrs)
 
-        # Overwrite default attributes.
-        volume_attachment_info.update(attrs)
+    return volume_attachment.VolumeAttachment(**volume_attachment_info)
 
-        return volume_attachment.VolumeAttachment(**volume_attachment_info)
 
-    @staticmethod
-    def create_sdk_volume_attachments(attrs=None, methods=None, count=2):
-        """Create multiple fake VolumeAttachment objects (BDMs).
+def create_volume_attachments(attrs=None, count=2):
+    """Create multiple fake volume attachments.
 
-        :param dict attrs:
-            A dictionary with all attributes
-        :param dict methods:
-            A dictionary with all methods
-        :param int count:
-            The number of volume attachments to fake
-        :return:
-            A list of VolumeAttachment objects faking the volume attachments.
-        """
-        volume_attachments = []
-        for i in range(0, count):
-            volume_attachments.append(
-                FakeVolumeAttachment.create_one_sdk_volume_attachment(
-                    attrs, methods))
+    :param dict attrs: A dictionary with all attributes
+    :param int count: The number of volume attachments to fake
+    :return: A list of fake
+        openstack.compute.v2.volume_attachment.VolumeAttachment objects
+    """
+    volume_attachments = []
+    for i in range(0, count):
+        volume_attachments.append(create_one_volume_attachment(attrs))
 
-        return volume_attachments
+    return volume_attachments
 
 
 def create_one_hypervisor(attrs=None):
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -933,8 +933,7 @@ def setUp(self):
             'volume_id': self.volumes[0].id,
         }
         self.volume_attachment = \
-            compute_fakes.FakeVolumeAttachment.\
-            create_one_sdk_volume_attachment(attrs=attrs)
+            compute_fakes.create_one_volume_attachment(attrs=attrs)
 
         self.sdk_client.create_volume_attachment.return_value = \
             self.volume_attachment
diff --git a/openstackclient/tests/unit/compute/v2/test_server_volume.py b/openstackclient/tests/unit/compute/v2/test_server_volume.py
diff --git a/releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml b/releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml

Original file line number	Diff line number	Diff line change
`@@ -933,8 +933,7 @@ def setUp(self):`
`933`	`933`	`'volume_id': self.volumes[0].id,`
`934`	`934`	`}`
`935`	`935`	`self.volume_attachment = \`
`936`		`- compute_fakes.FakeVolumeAttachment.\`
`937`		`- create_one_sdk_volume_attachment(attrs=attrs)`
	`936`	`+ compute_fakes.create_one_volume_attachment(attrs=attrs)`
`938`	`937`
`939`	`938`	`self.sdk_client.create_volume_attachment.return_value = \`
`940`	`939`	`self.volume_attachment`