volume: Migrate 'backup set', 'backup unset' to SDK

Change-Id: Iced346df828faab1ff08a2645ff64f4cfea25cb1
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

openstackclient/tests/unit/volume/v2/test_volume_backup.py

@@ -13,26 +13,15 @@
 
 from unittest.mock import call
 
+from openstack.block_storage.v2 import backup as _backup
+from openstack import exceptions as sdk_exceptions
+from openstack.test import fakes as sdk_fakes
 from osc_lib import exceptions
 
 from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes
 from openstackclient.volume.v2 import volume_backup
 
 
-class TestBackupLegacy(volume_fakes.TestVolume):
-    def setUp(self):
-        super().setUp()
-
-        self.backups_mock = self.volume_client.backups
-        self.backups_mock.reset_mock()
-        self.volumes_mock = self.volume_client.volumes
-        self.volumes_mock.reset_mock()
-        self.snapshots_mock = self.volume_client.volume_snapshots
-        self.snapshots_mock.reset_mock()
-        self.restores_mock = self.volume_client.restores
-        self.restores_mock.reset_mock()
-
-
 class TestBackupCreate(volume_fakes.TestVolume):
     volume = volume_fakes.create_one_volume()
     snapshot = volume_fakes.create_one_snapshot()
@@ -476,17 +465,15 @@ class TestBackupRestore(volume_fakes.TestVolume):
         )
 
 
-class TestBackupSet(TestBackupLegacy):
+class TestBackupSet(volume_fakes.TestVolume):
-    backup = volume_fakes.create_one_backup(
-        attrs={'metadata': {'wow': 'cool'}},
-    )
-
     def setUp(self):
         super().setUp()
 
-        self.backups_mock.get.return_value = self.backup
+        self.backup = sdk_fakes.generate_fake_resource(
+            _backup.Backup, metadata={'wow': 'cool'}
+        )
+        self.volume_sdk_client.find_backup.return_value = self.backup
 
-        # Get the command object to test
         self.cmd = volume_backup.SetVolumeBackup(self.app, None)
 
     def test_backup_set_state(self):
@@ -496,26 +483,34 @@ class TestBackupSet(TestBackupLegacy):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.backups_mock.reset_state.assert_called_once_with(
-            self.backup.id, 'error'
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
+        self.volume_sdk_client.reset_backup_status.assert_called_with(
+            self.backup, status='error'
+        )
+
     def test_backup_set_state_failed(self):
-        self.backups_mock.reset_state.side_effect = exceptions.CommandError()
+        self.volume_sdk_client.reset_backup_status.side_effect = (
+            sdk_exceptions.NotFoundException('foo')
+        )
+
         arglist = ['--state', 'error', self.backup.id]
         verifylist = [('state', 'error'), ('backup', self.backup.id)]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        try:
+        exc = self.assertRaises(
-            self.cmd.take_action(parsed_args)
+            exceptions.CommandError, self.cmd.take_action, parsed_args
-            self.fail('CommandError should be raised.')
+        )
-        except exceptions.CommandError as e:
+        self.assertEqual('One or more of the set operations failed', str(exc))
-            self.assertEqual(
+
-                'One or more of the set operations failed', str(e)
+        self.volume_sdk_client.find_backup.assert_called_with(
-            )
+            self.backup.id, ignore_missing=False
-        self.backups_mock.reset_state.assert_called_with(
+        )
-            self.backup.id, 'error'
+        self.volume_sdk_client.reset_backup_status.assert_called_with(
+            self.backup, status='error'
         )
 
 

openstackclient/tests/unit/volume/v3/test_volume_backup.py

@@ -13,26 +13,15 @@
 
 from unittest.mock import call
 
+from openstack.block_storage.v3 import backup as _backup
+from openstack import exceptions as sdk_exceptions
+from openstack.test import fakes as sdk_fakes
 from osc_lib import exceptions
 
 from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes
 from openstackclient.volume.v3 import volume_backup
 
 
-class TestBackupLegacy(volume_fakes.TestVolume):
-    def setUp(self):
-        super().setUp()
-
-        self.backups_mock = self.volume_client.backups
-        self.backups_mock.reset_mock()
-        self.volumes_mock = self.volume_client.volumes
-        self.volumes_mock.reset_mock()
-        self.snapshots_mock = self.volume_client.volume_snapshots
-        self.snapshots_mock.reset_mock()
-        self.restores_mock = self.volume_client.restores
-        self.restores_mock.reset_mock()
-
-
 class TestBackupCreate(volume_fakes.TestVolume):
     volume = volume_fakes.create_one_volume()
     snapshot = volume_fakes.create_one_snapshot()
@@ -574,17 +563,15 @@ class TestBackupRestore(volume_fakes.TestVolume):
         )
 
 
-class TestBackupSet(TestBackupLegacy):
+class TestBackupSet(volume_fakes.TestVolume):
-    backup = volume_fakes.create_one_backup(
-        attrs={'metadata': {'wow': 'cool'}},
-    )
-
     def setUp(self):
         super().setUp()
 
-        self.backups_mock.get.return_value = self.backup
+        self.backup = sdk_fakes.generate_fake_resource(
+            _backup.Backup, metadata={'wow': 'cool'}
+        )
+        self.volume_sdk_client.find_backup.return_value = self.backup
 
-        # Get the command object to test
         self.cmd = volume_backup.SetVolumeBackup(self.app, None)
 
     def test_backup_set_name(self):
@@ -601,14 +588,16 @@ class TestBackupSet(TestBackupLegacy):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        # In base command class ShowOne in cliff, abstract method take_action()
-        # returns nothing
         result = self.cmd.take_action(parsed_args)
-        self.backups_mock.update.assert_called_once_with(
-            self.backup.id, **{'name': 'new_name'}
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
+        self.volume_sdk_client.update_backup.assert_called_once_with(
+            self.backup, name='new_name'
+        )
+
     def test_backup_set_name_pre_v39(self):
         self.set_volume_api_version('3.8')
 
@@ -644,14 +633,15 @@ class TestBackupSet(TestBackupLegacy):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-
-        # Set expected values
-        kwargs = {'description': 'new_description'}
-        self.backups_mock.update.assert_called_once_with(
-            self.backup.id, **kwargs
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
+        self.volume_sdk_client.update_backup.assert_called_once_with(
+            self.backup, description='new_description'
+        )
+
     def test_backup_set_description_pre_v39(self):
         self.set_volume_api_version('3.8')
 
@@ -679,26 +669,34 @@ class TestBackupSet(TestBackupLegacy):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.backups_mock.reset_state.assert_called_once_with(
-            self.backup.id, 'error'
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
+        self.volume_sdk_client.reset_backup_status.assert_called_with(
+            self.backup, status='error'
+        )
+
     def test_backup_set_state_failed(self):
-        self.backups_mock.reset_state.side_effect = exceptions.CommandError()
+        self.volume_sdk_client.reset_backup_status.side_effect = (
+            sdk_exceptions.NotFoundException('foo')
+        )
+
         arglist = ['--state', 'error', self.backup.id]
         verifylist = [('state', 'error'), ('backup', self.backup.id)]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-        try:
+        exc = self.assertRaises(
-            self.cmd.take_action(parsed_args)
+            exceptions.CommandError, self.cmd.take_action, parsed_args
-            self.fail('CommandError should be raised.')
+        )
-        except exceptions.CommandError as e:
+        self.assertEqual('One or more of the set operations failed', str(exc))
-            self.assertEqual(
+
-                'One or more of the set operations failed', str(e)
+        self.volume_sdk_client.find_backup.assert_called_with(
-            )
+            self.backup.id, ignore_missing=False
-        self.backups_mock.reset_state.assert_called_with(
+        )
-            self.backup.id, 'error'
+        self.volume_sdk_client.reset_backup_status.assert_called_with(
+            self.backup, status='error'
         )
 
     def test_backup_set_no_property(self):
@@ -715,16 +713,15 @@ class TestBackupSet(TestBackupLegacy):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-
-        # Set expected values
-        kwargs = {
-            'metadata': {},
-        }
-        self.backups_mock.update.assert_called_once_with(
-            self.backup.id, **kwargs
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
+        self.volume_sdk_client.update_backup.assert_called_once_with(
+            self.backup, metadata={}
+        )
+
     def test_backup_set_no_property_pre_v343(self):
         self.set_volume_api_version('3.42')
 
@@ -758,16 +755,15 @@ class TestBackupSet(TestBackupLegacy):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-
-        # Set expected values
-        kwargs = {
-            'metadata': {'wow': 'cool', 'foo': 'bar'},
-        }
-        self.backups_mock.update.assert_called_once_with(
-            self.backup.id, **kwargs
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
+        self.volume_sdk_client.update_backup.assert_called_once_with(
+            self.backup, metadata={'wow': 'cool', 'foo': 'bar'}
+        )
+
     def test_backup_set_property_pre_v343(self):
         self.set_volume_api_version('3.42')
 
@@ -788,17 +784,16 @@ class TestBackupSet(TestBackupLegacy):
         self.assertIn("--os-volume-api-version 3.43 or greater", str(exc))
 
 
-class TestBackupUnset(TestBackupLegacy):
+class TestBackupUnset(volume_fakes.TestVolume):
-    backup = volume_fakes.create_one_backup(
-        attrs={'metadata': {'foo': 'bar'}},
-    )
-
     def setUp(self):
         super().setUp()
 
-        self.backups_mock.get.return_value = self.backup
+        self.backup = sdk_fakes.generate_fake_resource(
+            _backup.Backup, metadata={'foo': 'bar', 'wow': 'cool'}
+        )
+        self.volume_sdk_client.find_backup.return_value = self.backup
+        self.volume_sdk_client.delete_backup_metadata.return_value = None
 
-        # Get the command object to test
         self.cmd = volume_backup.UnsetVolumeBackup(self.app, None)
 
     def test_backup_unset_property(self):
@@ -816,16 +811,15 @@ class TestBackupUnset(TestBackupLegacy):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-
-        # Set expected values
-        kwargs = {
-            'metadata': {},
-        }
-        self.backups_mock.update.assert_called_once_with(
-            self.backup.id, **kwargs
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
+        self.volume_sdk_client.delete_backup_metadata.assert_called_once_with(
+            self.backup, keys=['wow']
+        )
+
     def test_backup_unset_property_pre_v343(self):
         self.set_volume_api_version('3.42')
 
@@ -907,7 +901,9 @@ class TestBackupShow(volume_fakes.TestVolume):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         columns, data = self.cmd.take_action(parsed_args)
-        self.volume_sdk_client.find_backup.assert_called_with(self.backup.id)
+        self.volume_sdk_client.find_backup.assert_called_with(
+            self.backup.id, ignore_missing=False
+        )
 
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)

openstackclient/volume/v2/volume_backup.py

@@ -417,13 +417,19 @@ class SetVolumeBackup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
-        backup = utils.find_resource(volume_client.backups, parsed_args.backup)
+
+        backup = volume_client.find_backup(
+            parsed_args.backup,
+            ignore_missing=False,
+        )
 
         result = 0
         if parsed_args.state:
             try:
-                volume_client.backups.reset_state(backup.id, parsed_args.state)
+                volume_client.reset_backup_status(
+                    backup, status=parsed_args.state
+                )
             except Exception as e:
                 LOG.error(_("Failed to set backup state: %s"), e)
                 result += 1

openstackclient/volume/v3/volume_backup.py

@@ -18,7 +18,6 @@ import copy
 import functools
 import logging
 
-from cinderclient import api_versions
 from cliff import columns as cliff_columns
 from openstack import utils as sdk_utils
 from osc_lib.cli import parseractions
@@ -512,13 +511,19 @@ class SetVolumeBackup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
-        backup = utils.find_resource(volume_client.backups, parsed_args.backup)
+
+        backup = volume_client.find_backup(
+            parsed_args.backup,
+            ignore_missing=False,
+        )
 
         result = 0
         if parsed_args.state:
             try:
-                volume_client.backups.reset_state(backup.id, parsed_args.state)
+                volume_client.reset_backup_status(
+                    backup, status=parsed_args.state
+                )
             except Exception as e:
                 LOG.error(_("Failed to set backup state: %s"), e)
                 result += 1
@@ -526,7 +531,7 @@ class SetVolumeBackup(command.Command):
         kwargs = {}
 
         if parsed_args.name:
-            if volume_client.api_version < api_versions.APIVersion('3.9'):
+            if not sdk_utils.supports_microversion(volume_client, '3.9'):
                 msg = _(
                     '--os-volume-api-version 3.9 or greater is required to '
                     'support the --name option'
@@ -536,7 +541,7 @@ class SetVolumeBackup(command.Command):
             kwargs['name'] = parsed_args.name
 
         if parsed_args.description:
-            if volume_client.api_version < api_versions.APIVersion('3.9'):
+            if not sdk_utils.supports_microversion(volume_client, '3.9'):
                 msg = _(
                     '--os-volume-api-version 3.9 or greater is required to '
                     'support the --description option'
@@ -546,7 +551,7 @@ class SetVolumeBackup(command.Command):
             kwargs['description'] = parsed_args.description
 
         if parsed_args.no_property:
-            if volume_client.api_version < api_versions.APIVersion('3.43'):
+            if not sdk_utils.supports_microversion(volume_client, '3.43'):
                 msg = _(
                     '--os-volume-api-version 3.43 or greater is required to '
                     'support the --no-property option'
@@ -554,14 +559,14 @@ class SetVolumeBackup(command.Command):
                 raise exceptions.CommandError(msg)
 
         if parsed_args.properties:
-            if volume_client.api_version < api_versions.APIVersion('3.43'):
+            if not sdk_utils.supports_microversion(volume_client, '3.43'):
                 msg = _(
                     '--os-volume-api-version 3.43 or greater is required to '
                     'support the --property option'
                 )
                 raise exceptions.CommandError(msg)
 
-        if volume_client.api_version >= api_versions.APIVersion('3.43'):
+        if sdk_utils.supports_microversion(volume_client, '3.43'):
             metadata = copy.deepcopy(backup.metadata)
 
             if parsed_args.no_property:
@@ -572,7 +577,7 @@ class SetVolumeBackup(command.Command):
 
         if kwargs:
             try:
-                volume_client.backups.update(backup.id, **kwargs)
+                volume_client.update_backup(backup, **kwargs)
             except Exception as e:
                 LOG.error("Failed to update backup: %s", e)
                 result += 1
@@ -608,16 +613,18 @@ class UnsetVolumeBackup(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
 
-        if volume_client.api_version < api_versions.APIVersion('3.43'):
+        if not sdk_utils.supports_microversion(volume_client, '3.43'):
             msg = _(
                 '--os-volume-api-version 3.43 or greater is required to '
                 'support the --property option'
             )
             raise exceptions.CommandError(msg)
 
-        backup = utils.find_resource(volume_client.backups, parsed_args.backup)
+        backup = volume_client.find_backup(
+            parsed_args.backup, ignore_missing=False
+        )
         metadata = copy.deepcopy(backup.metadata)
 
         for key in parsed_args.properties:
@@ -632,11 +639,7 @@ class UnsetVolumeBackup(command.Command):
 
             del metadata[key]
 
-        kwargs = {
+        volume_client.delete_backup_metadata(backup, keys=list(metadata))
-            'metadata': metadata,
-        }
-
-        volume_client.backups.update(backup.id, **kwargs)
 
 
 class ShowVolumeBackup(command.ShowOne):
@@ -653,7 +656,9 @@ class ShowVolumeBackup(command.ShowOne):
 
     def take_action(self, parsed_args):
         volume_client = self.app.client_manager.sdk_connection.volume
-        backup = volume_client.find_backup(parsed_args.backup)
+        backup = volume_client.find_backup(
+            parsed_args.backup, ignore_missing=False
+        )
         columns: tuple[str, ...] = (
             "availability_zone",
             "container",