diff --git a/openstackclient/tests/unit/volume/v2/test_volume_snapshot.py b/openstackclient/tests/unit/volume/v2/test_volume_snapshot.py index fc26e53f35..dc605be599 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume_snapshot.py +++ b/openstackclient/tests/unit/volume/v2/test_volume_snapshot.py @@ -14,6 +14,7 @@ from unittest import mock from openstack.block_storage.v2 import snapshot as _snapshot +from openstack.block_storage.v3 import volume as _volume from openstack import exceptions as sdk_exceptions from openstack.test import fakes as sdk_fakes from osc_lib.cli import format_columns @@ -37,7 +38,7 @@ class TestVolumeSnapshot(volume_fakes.TestVolume): self.project_mock.reset_mock() -class TestVolumeSnapshotCreate(TestVolumeSnapshot): +class TestVolumeSnapshotCreate(volume_fakes.TestVolume): columns = ( 'created_at', 'description', @@ -52,69 +53,71 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): def setUp(self): super().setUp() - self.volume = volume_fakes.create_one_volume() - self.new_snapshot = volume_fakes.create_one_snapshot( - attrs={'volume_id': self.volume.id} + self.volume = sdk_fakes.generate_fake_resource(_volume.Volume) + self.volume_sdk_client.find_volume.return_value = self.volume + self.snapshot = sdk_fakes.generate_fake_resource( + _snapshot.Snapshot, volume_id=self.volume.id ) + self.volume_sdk_client.create_snapshot.return_value = self.snapshot + self.volume_sdk_client.manage_snapshot.return_value = self.snapshot self.data = ( - self.new_snapshot.created_at, - self.new_snapshot.description, - self.new_snapshot.id, - self.new_snapshot.name, - format_columns.DictColumn(self.new_snapshot.metadata), - self.new_snapshot.size, - self.new_snapshot.status, - self.new_snapshot.volume_id, + self.snapshot.created_at, + self.snapshot.description, + self.snapshot.id, + self.snapshot.name, + format_columns.DictColumn(self.snapshot.metadata), + self.snapshot.size, + self.snapshot.status, + self.snapshot.volume_id, ) - self.volumes_mock.get.return_value = self.volume - self.snapshots_mock.create.return_value = self.new_snapshot - self.snapshots_mock.manage.return_value = self.new_snapshot - # Get the command object to test self.cmd = volume_snapshot.CreateVolumeSnapshot(self.app, None) def test_snapshot_create(self): arglist = [ "--volume", - self.new_snapshot.volume_id, + self.snapshot.volume_id, "--description", - self.new_snapshot.description, + self.snapshot.description, "--force", '--property', 'Alpha=a', '--property', 'Beta=b', - self.new_snapshot.name, + self.snapshot.name, ] verifylist = [ - ("volume", self.new_snapshot.volume_id), - ("description", self.new_snapshot.description), + ("volume", self.snapshot.volume_id), + ("description", self.snapshot.description), ("force", True), - ('property', {'Alpha': 'a', 'Beta': 'b'}), - ("snapshot_name", self.new_snapshot.name), + ('properties', {'Alpha': 'a', 'Beta': 'b'}), + ("snapshot_name", self.snapshot.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.snapshots_mock.create.assert_called_with( - self.new_snapshot.volume_id, - force=True, - name=self.new_snapshot.name, - description=self.new_snapshot.description, - metadata={'Alpha': 'a', 'Beta': 'b'}, - ) self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) + self.volume_sdk_client.find_volume.assert_called_once_with( + self.snapshot.volume_id, ignore_missing=False + ) + self.volume_sdk_client.create_snapshot.assert_called_with( + volume_id=self.snapshot.volume_id, + force=True, + name=self.snapshot.name, + description=self.snapshot.description, + metadata={'Alpha': 'a', 'Beta': 'b'}, + ) def test_snapshot_create_without_name(self): arglist = [ "--volume", - self.new_snapshot.volume_id, + self.snapshot.volume_id, ] verifylist = [ - ("volume", self.new_snapshot.volume_id), + ("volume", self.snapshot.volume_id), ] self.assertRaises( test_utils.ParserException, @@ -127,29 +130,31 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): def test_snapshot_create_without_volume(self): arglist = [ "--description", - self.new_snapshot.description, + self.snapshot.description, "--force", - self.new_snapshot.name, + self.snapshot.name, ] verifylist = [ - ("description", self.new_snapshot.description), + ("description", self.snapshot.description), ("force", True), - ("snapshot_name", self.new_snapshot.name), + ("snapshot_name", self.snapshot.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.volumes_mock.get.assert_called_once_with(self.new_snapshot.name) - self.snapshots_mock.create.assert_called_once_with( - self.new_snapshot.volume_id, - force=True, - name=self.new_snapshot.name, - description=self.new_snapshot.description, - metadata=None, - ) self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) + self.volume_sdk_client.find_volume.assert_called_once_with( + self.snapshot.name, ignore_missing=False + ) + self.volume_sdk_client.create_snapshot.assert_called_once_with( + volume_id=self.snapshot.volume_id, + force=True, + name=self.snapshot.name, + description=self.snapshot.description, + metadata=None, + ) def test_snapshot_create_with_remote_source(self): arglist = [ @@ -158,8 +163,8 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): '--remote-source', 'source-id=test_source_id', '--volume', - self.new_snapshot.volume_id, - self.new_snapshot.name, + self.snapshot.volume_id, + self.snapshot.name, ] ref_dict = { 'source-name': 'test_source_name', @@ -167,23 +172,26 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): } verifylist = [ ('remote_source', ref_dict), - ('volume', self.new_snapshot.volume_id), - ("snapshot_name", self.new_snapshot.name), + ('volume', self.snapshot.volume_id), + ("snapshot_name", self.snapshot.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.snapshots_mock.manage.assert_called_with( - volume_id=self.new_snapshot.volume_id, + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) + self.volume_sdk_client.find_volume.assert_called_once_with( + self.snapshot.volume_id, ignore_missing=False + ) + self.volume_sdk_client.manage_snapshot.assert_called_with( + volume_id=self.snapshot.volume_id, ref=ref_dict, - name=self.new_snapshot.name, + name=self.snapshot.name, description=None, metadata=None, ) - self.snapshots_mock.create.assert_not_called() - self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) + self.volume_sdk_client.create_snapshot.assert_not_called() class TestVolumeSnapshotDelete(volume_fakes.TestVolume): diff --git a/openstackclient/tests/unit/volume/v3/test_volume_snapshot.py b/openstackclient/tests/unit/volume/v3/test_volume_snapshot.py index 2fecf631dc..26c9932200 100644 --- a/openstackclient/tests/unit/volume/v3/test_volume_snapshot.py +++ b/openstackclient/tests/unit/volume/v3/test_volume_snapshot.py @@ -14,6 +14,7 @@ from unittest import mock from openstack.block_storage.v3 import snapshot as _snapshot +from openstack.block_storage.v3 import volume as _volume from openstack import exceptions as sdk_exceptions from openstack.test import fakes as sdk_fakes from osc_lib.cli import format_columns @@ -40,7 +41,7 @@ class TestVolumeSnapshot(volume_fakes_v3.TestVolume): self.volume_sdk_client.unmanage_snapshot.return_value = None -class TestVolumeSnapshotCreate(TestVolumeSnapshot): +class TestVolumeSnapshotCreate(volume_fakes_v3.TestVolume): columns = ( 'created_at', 'description', @@ -55,57 +56,59 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): def setUp(self): super().setUp() - self.volume = volume_fakes.create_one_volume() - self.new_snapshot = volume_fakes.create_one_snapshot( - attrs={'volume_id': self.volume.id} + self.volume = sdk_fakes.generate_fake_resource(_volume.Volume) + self.volume_sdk_client.find_volume.return_value = self.volume + self.snapshot = sdk_fakes.generate_fake_resource( + _snapshot.Snapshot, volume_id=self.volume.id ) + self.volume_sdk_client.create_snapshot.return_value = self.snapshot + self.volume_sdk_client.manage_snapshot.return_value = self.snapshot self.data = ( - self.new_snapshot.created_at, - self.new_snapshot.description, - self.new_snapshot.id, - self.new_snapshot.name, - format_columns.DictColumn(self.new_snapshot.metadata), - self.new_snapshot.size, - self.new_snapshot.status, - self.new_snapshot.volume_id, + self.snapshot.created_at, + self.snapshot.description, + self.snapshot.id, + self.snapshot.name, + format_columns.DictColumn(self.snapshot.metadata), + self.snapshot.size, + self.snapshot.status, + self.snapshot.volume_id, ) - self.volumes_mock.get.return_value = self.volume - self.snapshots_mock.create.return_value = self.new_snapshot - self.snapshots_mock.manage.return_value = self.new_snapshot - # Get the command object to test self.cmd = volume_snapshot.CreateVolumeSnapshot(self.app, None) def test_snapshot_create(self): arglist = [ "--volume", - self.new_snapshot.volume_id, + self.snapshot.volume_id, "--description", - self.new_snapshot.description, + self.snapshot.description, "--force", '--property', 'Alpha=a', '--property', 'Beta=b', - self.new_snapshot.name, + self.snapshot.name, ] verifylist = [ - ("volume", self.new_snapshot.volume_id), - ("description", self.new_snapshot.description), + ("volume", self.snapshot.volume_id), + ("description", self.snapshot.description), ("force", True), - ('property', {'Alpha': 'a', 'Beta': 'b'}), - ("snapshot_name", self.new_snapshot.name), + ('properties', {'Alpha': 'a', 'Beta': 'b'}), + ("snapshot_name", self.snapshot.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.snapshots_mock.create.assert_called_with( - self.new_snapshot.volume_id, + self.volume_sdk_client.find_volume.assert_called_once_with( + self.snapshot.volume_id, ignore_missing=False + ) + self.volume_sdk_client.create_snapshot.assert_called_with( + volume_id=self.snapshot.volume_id, force=True, - name=self.new_snapshot.name, - description=self.new_snapshot.description, + name=self.snapshot.name, + description=self.snapshot.description, metadata={'Alpha': 'a', 'Beta': 'b'}, ) self.assertEqual(self.columns, columns) @@ -114,10 +117,10 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): def test_snapshot_create_without_name(self): arglist = [ "--volume", - self.new_snapshot.volume_id, + self.snapshot.volume_id, ] verifylist = [ - ("volume", self.new_snapshot.volume_id), + ("volume", self.snapshot.volume_id), ] self.assertRaises( test_utils.ParserException, @@ -130,25 +133,27 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): def test_snapshot_create_without_volume(self): arglist = [ "--description", - self.new_snapshot.description, + self.snapshot.description, "--force", - self.new_snapshot.name, + self.snapshot.name, ] verifylist = [ - ("description", self.new_snapshot.description), + ("description", self.snapshot.description), ("force", True), - ("snapshot_name", self.new_snapshot.name), + ("snapshot_name", self.snapshot.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.volumes_mock.get.assert_called_once_with(self.new_snapshot.name) - self.snapshots_mock.create.assert_called_once_with( - self.new_snapshot.volume_id, + self.volume_sdk_client.find_volume.assert_called_once_with( + self.snapshot.name, ignore_missing=False + ) + self.volume_sdk_client.create_snapshot.assert_called_with( + volume_id=self.snapshot.volume_id, force=True, - name=self.new_snapshot.name, - description=self.new_snapshot.description, + name=self.snapshot.name, + description=self.snapshot.description, metadata=None, ) self.assertEqual(self.columns, columns) @@ -161,8 +166,8 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): '--remote-source', 'source-id=test_source_id', '--volume', - self.new_snapshot.volume_id, - self.new_snapshot.name, + self.snapshot.volume_id, + self.snapshot.name, ] ref_dict = { 'source-name': 'test_source_name', @@ -170,23 +175,26 @@ class TestVolumeSnapshotCreate(TestVolumeSnapshot): } verifylist = [ ('remote_source', ref_dict), - ('volume', self.new_snapshot.volume_id), - ("snapshot_name", self.new_snapshot.name), + ('volume', self.snapshot.volume_id), + ("snapshot_name", self.snapshot.name), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.snapshots_mock.manage.assert_called_with( - volume_id=self.new_snapshot.volume_id, + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) + self.volume_sdk_client.find_volume.assert_called_once_with( + self.snapshot.volume_id, ignore_missing=False + ) + self.volume_sdk_client.manage_snapshot.assert_called_with( + volume_id=self.snapshot.volume_id, ref=ref_dict, - name=self.new_snapshot.name, + name=self.snapshot.name, description=None, metadata=None, ) - self.snapshots_mock.create.assert_not_called() - self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) + self.volume_sdk_client.create_snapshot.assert_not_called() class TestVolumeSnapshotDelete(volume_fakes_v3.TestVolume): diff --git a/openstackclient/volume/v2/volume_snapshot.py b/openstackclient/volume/v2/volume_snapshot.py index 1c607a2011..4efc5f0e6d 100644 --- a/openstackclient/volume/v2/volume_snapshot.py +++ b/openstackclient/volume/v2/volume_snapshot.py @@ -17,8 +17,10 @@ import copy import functools import logging +import typing as ty from cliff import columns as cliff_columns +from openstack.block_storage.v2 import snapshot as _snapshot from osc_lib.cli import format_columns from osc_lib.cli import parseractions from osc_lib.command import command @@ -60,6 +62,42 @@ class VolumeIdColumn(cliff_columns.FormattableColumn): return volume +def _format_snapshot(snapshot: _snapshot.Snapshot) -> dict[str, ty.Any]: + # Some columns returned by openstacksdk should not be shown because they're + # either irrelevant or duplicates + ignored_columns = { + # computed columns + 'location', + # create-only columns + 'consumes_quota', + 'force', + 'group_snapshot_id', + # ignored columns + 'os-extended-snapshot-attributes:progress', + 'os-extended-snapshot-attributes:project_id', + 'updated_at', + 'user_id', + # unnecessary columns + 'links', + } + + info = snapshot.to_dict(original_names=True) + data = {} + for key, value in info.items(): + if key in ignored_columns: + continue + + data[key] = value + + data.update( + { + 'properties': format_columns.DictColumn(data.pop('metadata')), + } + ) + + return data + + class CreateVolumeSnapshot(command.ShowOne): _description = _("Create new volume snapshot") @@ -94,6 +132,7 @@ class CreateVolumeSnapshot(command.ShowOne): "--property", metavar="", action=parseractions.KeyValueAction, + dest="properties", help=_( "Set a property to this snapshot " "(repeat option to set multiple properties)" @@ -113,11 +152,13 @@ class CreateVolumeSnapshot(command.ShowOne): return parser def take_action(self, parsed_args): - volume_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume + volume = parsed_args.volume if not parsed_args.volume: volume = parsed_args.snapshot_name - volume_id = utils.find_resource(volume_client.volumes, volume).id + volume_id = volume_client.find_volume(volume, ignore_missing=False).id + if parsed_args.remote_source: # Create a new snapshot from an existing remote snapshot source if parsed_args.force: @@ -127,30 +168,26 @@ class CreateVolumeSnapshot(command.ShowOne): "volume snapshot" ) LOG.warning(msg) - snapshot = volume_client.volume_snapshots.manage( + + snapshot = volume_client.manage_snapshot( volume_id=volume_id, ref=parsed_args.remote_source, name=parsed_args.snapshot_name, description=parsed_args.description, - metadata=parsed_args.property, + metadata=parsed_args.properties, ) else: # create a new snapshot from scratch - snapshot = volume_client.volume_snapshots.create( - volume_id, + snapshot = volume_client.create_snapshot( + volume_id=volume_id, force=parsed_args.force, name=parsed_args.snapshot_name, description=parsed_args.description, - metadata=parsed_args.property, + metadata=parsed_args.properties, ) - snapshot._info.update( - { - 'properties': format_columns.DictColumn( - snapshot._info.pop('metadata') - ) - } - ) - return zip(*sorted(snapshot._info.items())) + + data = _format_snapshot(snapshot) + return zip(*sorted(data.items())) class DeleteVolumeSnapshot(command.Command): diff --git a/openstackclient/volume/v3/volume_snapshot.py b/openstackclient/volume/v3/volume_snapshot.py index a25aa7e626..5b09c9a9b4 100644 --- a/openstackclient/volume/v3/volume_snapshot.py +++ b/openstackclient/volume/v3/volume_snapshot.py @@ -17,8 +17,10 @@ import copy import functools import logging +import typing as ty from cliff import columns as cliff_columns +from openstack.block_storage.v3 import snapshot as _snapshot from osc_lib.cli import format_columns from osc_lib.cli import parseractions from osc_lib.command import command @@ -59,6 +61,42 @@ class VolumeIdColumn(cliff_columns.FormattableColumn): return volume +def _format_snapshot(snapshot: _snapshot.Snapshot) -> dict[str, ty.Any]: + # Some columns returned by openstacksdk should not be shown because they're + # either irrelevant or duplicates + ignored_columns = { + # computed columns + 'location', + # create-only columns + 'consumes_quota', + 'force', + 'group_snapshot_id', + # ignored columns + 'os-extended-snapshot-attributes:progress', + 'os-extended-snapshot-attributes:project_id', + 'updated_at', + 'user_id', + # unnecessary columns + 'links', + } + + info = snapshot.to_dict(original_names=True) + data = {} + for key, value in info.items(): + if key in ignored_columns: + continue + + data[key] = value + + data.update( + { + 'properties': format_columns.DictColumn(data.pop('metadata')), + } + ) + + return data + + class CreateVolumeSnapshot(command.ShowOne): _description = _("Create new volume snapshot") @@ -92,6 +130,7 @@ class CreateVolumeSnapshot(command.ShowOne): parser.add_argument( "--property", metavar="", + dest='properties', action=parseractions.KeyValueAction, help=_( "Set a property to this snapshot " @@ -112,11 +151,13 @@ class CreateVolumeSnapshot(command.ShowOne): return parser def take_action(self, parsed_args): - volume_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume + volume = parsed_args.volume if not parsed_args.volume: volume = parsed_args.snapshot_name - volume_id = utils.find_resource(volume_client.volumes, volume).id + volume_id = volume_client.find_volume(volume, ignore_missing=False).id + if parsed_args.remote_source: # Create a new snapshot from an existing remote snapshot source if parsed_args.force: @@ -126,30 +167,26 @@ class CreateVolumeSnapshot(command.ShowOne): "volume snapshot" ) LOG.warning(msg) - snapshot = volume_client.volume_snapshots.manage( + + snapshot = volume_client.manage_snapshot( volume_id=volume_id, ref=parsed_args.remote_source, name=parsed_args.snapshot_name, description=parsed_args.description, - metadata=parsed_args.property, + metadata=parsed_args.properties, ) else: - # create a new snapshot from scratch - snapshot = volume_client.volume_snapshots.create( - volume_id, + # Create a new snapshot from scratch + snapshot = volume_client.create_snapshot( + volume_id=volume_id, force=parsed_args.force, name=parsed_args.snapshot_name, description=parsed_args.description, - metadata=parsed_args.property, + metadata=parsed_args.properties, ) - snapshot._info.update( - { - 'properties': format_columns.DictColumn( - snapshot._info.pop('metadata') - ) - } - ) - return zip(*sorted(snapshot._info.items())) + + data = _format_snapshot(snapshot) + return zip(*sorted(data.items())) class DeleteVolumeSnapshot(command.Command):