From c7d465a221ced77427d33295b8dc79409b7c85a0 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 7 Mar 2025 14:16:02 +0000 Subject: [PATCH] volume: Migrate 'volume show' to SDK Change-Id: Ibd9d7a62c2500a1f31aa2d3d13ac7e8bad4e6964 Signed-off-by: Stephen Finucane --- .../tests/functional/volume/v2/test_volume.py | 2 +- .../tests/functional/volume/v3/test_volume.py | 2 +- .../tests/unit/volume/v2/test_volume.py | 132 +++++++++++----- .../tests/unit/volume/v3/test_volume.py | 141 +++++++++++++----- openstackclient/volume/v2/volume.py | 21 +-- openstackclient/volume/v3/volume.py | 21 +-- 6 files changed, 211 insertions(+), 108 deletions(-) diff --git a/openstackclient/tests/functional/volume/v2/test_volume.py b/openstackclient/tests/functional/volume/v2/test_volume.py index 4667858751..32b60dfaac 100644 --- a/openstackclient/tests/functional/volume/v2/test_volume.py +++ b/openstackclient/tests/functional/volume/v2/test_volume.py @@ -171,7 +171,7 @@ class VolumeTests(common.BaseVolumeTests): cmd_output["volume_image_metadata"], ) self.assertEqual( - 'true', + True, cmd_output["bootable"], ) diff --git a/openstackclient/tests/functional/volume/v3/test_volume.py b/openstackclient/tests/functional/volume/v3/test_volume.py index d4cd0ac633..07a7959167 100644 --- a/openstackclient/tests/functional/volume/v3/test_volume.py +++ b/openstackclient/tests/functional/volume/v3/test_volume.py @@ -172,7 +172,7 @@ class VolumeTests(common.BaseVolumeTests): cmd_output["volume_image_metadata"], ) self.assertEqual( - 'true', + True, cmd_output["bootable"], ) diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index 5b81bdf7a2..b68020fa95 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -12,6 +12,7 @@ # under the License. from unittest import mock +import uuid from openstack.block_storage.v2 import snapshot as _snapshot from openstack.block_storage.v2 import volume as _volume @@ -1622,42 +1623,83 @@ class TestVolumeSet(TestVolume): self.assertIsNone(result) -class TestVolumeShow(TestVolume): +class TestVolumeShow(volume_fakes.TestVolume): def setUp(self): super().setUp() - self._volume = volume_fakes.create_one_volume() - self.volumes_mock.get.return_value = self._volume - # Get the command object to test + self.volume = sdk_fakes.generate_fake_resource(_volume.Volume) + self.volume_sdk_client.find_volume.return_value = self.volume + + self.columns = ( + 'attachments', + 'availability_zone', + 'bootable', + 'consistencygroup_id', + 'created_at', + 'description', + 'encrypted', + 'id', + 'multiattach', + 'name', + 'os-vol-host-attr:host', + 'os-vol-mig-status-attr:migstat', + 'os-vol-mig-status-attr:name_id', + 'os-vol-tenant-attr:tenant_id', + 'os-volume-replication:driver_data', + 'os-volume-replication:extended_status', + 'properties', + 'replication_status', + 'size', + 'snapshot_id', + 'source_volid', + 'status', + 'type', + 'updated_at', + 'user_id', + 'volume_image_metadata', + ) + self.data = ( + self.volume.attachments, + self.volume.availability_zone, + self.volume.is_bootable, + self.volume.consistency_group_id, + self.volume.created_at, + self.volume.description, + self.volume.is_encrypted, + self.volume.id, + self.volume.is_multiattach, + self.volume.name, + self.volume.host, + self.volume.migration_status, + self.volume.migration_id, + self.volume.project_id, + self.volume.replication_driver_data, + self.volume.extended_replication_status, + format_columns.DictColumn(self.volume.metadata), + self.volume.replication_status, + self.volume.size, + self.volume.snapshot_id, + self.volume.source_volume_id, + self.volume.status, + self.volume.volume_type, + self.volume.updated_at, + self.volume.user_id, + self.volume.volume_image_metadata, + ) + self.cmd = volume.ShowVolume(self.app, None) def test_volume_show(self): - arglist = [self._volume.id] - verifylist = [("volume", self._volume.id)] + arglist = [self.volume.id] + verifylist = [("volume", self.volume.id)] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.volumes_mock.get.assert_called_with(self._volume.id) - self.assertEqual( - tuple(sorted(self._volume.keys())), - columns, - ) - self.assertTupleEqual( - ( - self._volume.attachments, - self._volume.availability_zone, - self._volume.bootable, - self._volume.description, - self._volume.id, - self._volume.name, - format_columns.DictColumn(self._volume.metadata), - self._volume.size, - self._volume.snapshot_id, - self._volume.status, - self._volume.volume_type, - ), - data, + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) + self.volume_sdk_client.find_volume.assert_called_with( + self.volume.id, ignore_missing=False ) @@ -1748,31 +1790,47 @@ class TestVolumeUnset(TestVolume): ) -class TestColumns(TestVolume): +class TestColumns(volume_fakes.TestVolume): def test_attachments_column_without_server_cache(self): - _volume = volume_fakes.create_one_volume() - server_id = _volume.attachments[0]['server_id'] - device = _volume.attachments[0]['device'] + vol = sdk_fakes.generate_fake_resource( + _volume.Volume, + attachments=[ + { + 'device': '/dev/' + uuid.uuid4().hex, + 'server_id': uuid.uuid4().hex, + }, + ], + ) + server_id = vol.attachments[0]['server_id'] + device = vol.attachments[0]['device'] - col = volume.AttachmentsColumn(_volume.attachments, {}) + col = volume.AttachmentsColumn(vol.attachments, {}) self.assertEqual( f'Attached to {server_id} on {device} ', col.human_readable(), ) - self.assertEqual(_volume.attachments, col.machine_readable()) + self.assertEqual(vol.attachments, col.machine_readable()) def test_attachments_column_with_server_cache(self): - _volume = volume_fakes.create_one_volume() + vol = sdk_fakes.generate_fake_resource( + _volume.Volume, + attachments=[ + { + 'device': '/dev/' + uuid.uuid4().hex, + 'server_id': uuid.uuid4().hex, + }, + ], + ) - server_id = _volume.attachments[0]['server_id'] - device = _volume.attachments[0]['device'] + server_id = vol.attachments[0]['server_id'] + device = vol.attachments[0]['device'] fake_server = mock.Mock() fake_server.name = 'fake-server-name' server_cache = {server_id: fake_server} - col = volume.AttachmentsColumn(_volume.attachments, server_cache) + col = volume.AttachmentsColumn(vol.attachments, server_cache) self.assertEqual( 'Attached to {} on {} '.format('fake-server-name', device), col.human_readable(), ) - self.assertEqual(_volume.attachments, col.machine_readable()) + self.assertEqual(vol.attachments, col.machine_readable()) diff --git a/openstackclient/tests/unit/volume/v3/test_volume.py b/openstackclient/tests/unit/volume/v3/test_volume.py index 764958af6f..5fb35a067e 100644 --- a/openstackclient/tests/unit/volume/v3/test_volume.py +++ b/openstackclient/tests/unit/volume/v3/test_volume.py @@ -13,6 +13,7 @@ import copy from unittest import mock +import uuid from openstack.block_storage.v3 import backup as _backup from openstack.block_storage.v3 import block_storage_summary as _summary @@ -2004,41 +2005,91 @@ class TestVolumeShow(volume_fakes.TestVolume): def setUp(self): super().setUp() - self.volumes_mock = self.volume_client.volumes - self.volumes_mock.reset_mock() + self.volume = sdk_fakes.generate_fake_resource(_volume.Volume) + self.volume_sdk_client.find_volume.return_value = self.volume + + self.columns = ( + 'attachments', + 'availability_zone', + 'bootable', + 'cluster_name', + 'consistencygroup_id', + 'consumes_quota', + 'created_at', + 'description', + 'encrypted', + 'encryption_key_id', + 'group_id', + 'id', + 'multiattach', + 'name', + 'os-vol-host-attr:host', + 'os-vol-mig-status-attr:migstat', + 'os-vol-mig-status-attr:name_id', + 'os-vol-tenant-attr:tenant_id', + 'properties', + 'provider_id', + 'replication_status', + 'service_uuid', + 'shared_targets', + 'size', + 'snapshot_id', + 'source_volid', + 'status', + 'type', + 'updated_at', + 'user_id', + 'volume_image_metadata', + 'volume_type_id', + ) + self.data = ( + self.volume.attachments, + self.volume.availability_zone, + self.volume.is_bootable, + self.volume.cluster_name, + self.volume.consistency_group_id, + self.volume.consumes_quota, + self.volume.created_at, + self.volume.description, + self.volume.is_encrypted, + self.volume.encryption_key_id, + self.volume.group_id, + self.volume.id, + self.volume.is_multiattach, + self.volume.name, + self.volume.host, + self.volume.migration_status, + self.volume.migration_id, + self.volume.project_id, + format_columns.DictColumn(self.volume.metadata), + self.volume.provider_id, + self.volume.replication_status, + self.volume.service_uuid, + self.volume.shared_targets, + self.volume.size, + self.volume.snapshot_id, + self.volume.source_volume_id, + self.volume.status, + self.volume.volume_type, + self.volume.updated_at, + self.volume.user_id, + self.volume.volume_image_metadata, + self.volume.volume_type_id, + ) - self._volume = volume_fakes.create_one_volume() - self.volumes_mock.get.return_value = self._volume - # Get the command object to test self.cmd = volume.ShowVolume(self.app, None) def test_volume_show(self): - arglist = [self._volume.id] - verifylist = [("volume", self._volume.id)] + arglist = [self.volume.id] + verifylist = [("volume", self.volume.id)] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.volumes_mock.get.assert_called_with(self._volume.id) - self.assertEqual( - tuple(sorted(self._volume.keys())), - columns, - ) - self.assertTupleEqual( - ( - self._volume.attachments, - self._volume.availability_zone, - self._volume.bootable, - self._volume.description, - self._volume.id, - self._volume.name, - format_columns.DictColumn(self._volume.metadata), - self._volume.size, - self._volume.snapshot_id, - self._volume.status, - self._volume.volume_type, - ), - data, + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) + self.volume_sdk_client.find_volume.assert_called_with( + self.volume.id, ignore_missing=False ) @@ -2284,29 +2335,45 @@ class TestVolumeRevertToSnapshot(volume_fakes.TestVolume): class TestColumns(volume_fakes.TestVolume): def test_attachments_column_without_server_cache(self): - _volume = volume_fakes.create_one_volume() - server_id = _volume.attachments[0]['server_id'] - device = _volume.attachments[0]['device'] + vol = sdk_fakes.generate_fake_resource( + _volume.Volume, + attachments=[ + { + 'device': '/dev/' + uuid.uuid4().hex, + 'server_id': uuid.uuid4().hex, + }, + ], + ) + server_id = vol.attachments[0]['server_id'] + device = vol.attachments[0]['device'] - col = volume.AttachmentsColumn(_volume.attachments, {}) + col = volume.AttachmentsColumn(vol.attachments, {}) self.assertEqual( f'Attached to {server_id} on {device} ', col.human_readable(), ) - self.assertEqual(_volume.attachments, col.machine_readable()) + self.assertEqual(vol.attachments, col.machine_readable()) def test_attachments_column_with_server_cache(self): - _volume = volume_fakes.create_one_volume() + vol = sdk_fakes.generate_fake_resource( + _volume.Volume, + attachments=[ + { + 'device': '/dev/' + uuid.uuid4().hex, + 'server_id': uuid.uuid4().hex, + }, + ], + ) - server_id = _volume.attachments[0]['server_id'] - device = _volume.attachments[0]['device'] + server_id = vol.attachments[0]['server_id'] + device = vol.attachments[0]['device'] fake_server = mock.Mock() fake_server.name = 'fake-server-name' server_cache = {server_id: fake_server} - col = volume.AttachmentsColumn(_volume.attachments, server_cache) + col = volume.AttachmentsColumn(vol.attachments, server_cache) self.assertEqual( 'Attached to {} on {} '.format('fake-server-name', device), col.human_readable(), ) - self.assertEqual(_volume.attachments, col.machine_readable()) + self.assertEqual(vol.attachments, col.machine_readable()) diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index 9881172d6c..fa76d776eb 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -955,24 +955,13 @@ class ShowVolume(command.ShowOne): return parser def take_action(self, parsed_args): - volume_client = self.app.client_manager.volume - volume = utils.find_resource(volume_client.volumes, parsed_args.volume) - - # Special mapping for columns to make the output easier to read: - # 'metadata' --> 'properties' - # 'volume_type' --> 'type' - volume._info.update( - { - 'properties': format_columns.DictColumn( - volume._info.pop('metadata') - ), - 'type': volume._info.pop('volume_type'), - }, + volume_client = self.app.client_manager.sdk_connection.volume + volume = volume_client.find_volume( + parsed_args.volume, ignore_missing=False ) - # Remove key links from being displayed - volume._info.pop("links", None) - return zip(*sorted(volume._info.items())) + data = _format_volume(volume) + return zip(*sorted(data.items())) class UnsetVolume(command.Command): diff --git a/openstackclient/volume/v3/volume.py b/openstackclient/volume/v3/volume.py index 2ecc11ba6d..54dd5c7547 100644 --- a/openstackclient/volume/v3/volume.py +++ b/openstackclient/volume/v3/volume.py @@ -1116,24 +1116,13 @@ class ShowVolume(command.ShowOne): return parser def take_action(self, parsed_args): - volume_client = self.app.client_manager.volume - volume = utils.find_resource(volume_client.volumes, parsed_args.volume) - - # Special mapping for columns to make the output easier to read: - # 'metadata' --> 'properties' - # 'volume_type' --> 'type' - volume._info.update( - { - 'properties': format_columns.DictColumn( - volume._info.pop('metadata') - ), - 'type': volume._info.pop('volume_type'), - }, + volume_client = self.app.client_manager.sdk_connection.volume + volume = volume_client.find_volume( + parsed_args.volume, ignore_missing=False ) - # Remove key links from being displayed - volume._info.pop("links", None) - return zip(*sorted(volume._info.items())) + data = _format_volume(volume) + return zip(*sorted(data.items())) class UnsetVolume(command.Command):