Merge "volume: Migrate 'snapshot create' to SDK"

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):

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):

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="<key=value>",
             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):

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="<key=value>",
+            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):