diff --git a/openstackclient/tests/unit/volume/v2/fakes.py b/openstackclient/tests/unit/volume/v2/fakes.py index 81e3a8a24b..d3d0aecb52 100644 --- a/openstackclient/tests/unit/volume/v2/fakes.py +++ b/openstackclient/tests/unit/volume/v2/fakes.py @@ -18,6 +18,7 @@ from unittest import mock import uuid from cinderclient import api_versions +from openstack.block_storage.v3 import backup as _backup from openstack.block_storage.v3 import volume from osc_lib.cli import format_columns @@ -543,7 +544,17 @@ def create_one_backup(attrs=None): # Set default attributes. backup_info = { + "created_at": 'time-' + uuid.uuid4().hex, + "data_timestamp": 'time-' + uuid.uuid4().hex, "id": 'backup-id-' + uuid.uuid4().hex, + "encryption_key_id": None, + "fail_reason": "Service not found for creating backup.", + "has_dependent_backups": False, + "is_incremental": False, + "metadata": {}, + "project_id": uuid.uuid4().hex, + "updated_at": 'time-' + uuid.uuid4().hex, + "user_id": uuid.uuid4().hex, "name": 'backup-name-' + uuid.uuid4().hex, "volume_id": 'volume-id-' + uuid.uuid4().hex, "snapshot_id": 'snapshot-id' + uuid.uuid4().hex, @@ -558,7 +569,7 @@ def create_one_backup(attrs=None): # Overwrite default attributes. backup_info.update(attrs) - backup = fakes.FakeResource(info=copy.deepcopy(backup_info), loaded=True) + backup = _backup.Backup(**backup_info) return backup diff --git a/openstackclient/tests/unit/volume/v2/test_volume_backup.py b/openstackclient/tests/unit/volume/v2/test_volume_backup.py index 82adea1fc7..206fcbe721 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume_backup.py +++ b/openstackclient/tests/unit/volume/v2/test_volume_backup.py @@ -16,14 +16,14 @@ from unittest import mock from unittest.mock import call from cinderclient import api_versions +from openstack import utils as sdk_utils from osc_lib import exceptions -from osc_lib import utils from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes from openstackclient.volume.v2 import volume_backup -class TestBackup(volume_fakes.TestVolume): +class TestBackupLegacy(volume_fakes.TestVolume): def setUp(self): super().setUp() @@ -37,6 +37,31 @@ class TestBackup(volume_fakes.TestVolume): self.restores_mock.reset_mock() +class TestBackup(volume_fakes.TestVolume): + def setUp(self): + super().setUp() + + self.app.client_manager.sdk_connection = mock.Mock() + self.app.client_manager.sdk_connection.volume = mock.Mock() + self.sdk_client = self.app.client_manager.sdk_connection.volume + patcher = mock.patch.object( + sdk_utils, 'supports_microversion', return_value=True + ) + self.addCleanup(patcher.stop) + self.supports_microversion_mock = patcher.start() + self._set_mock_microversion( + self.app.client_manager.volume.api_version.get_string() + ) + + def _set_mock_microversion(self, mock_v): + """Set a specific microversion for the mock supports_microversion().""" + self.supports_microversion_mock.reset_mock(return_value=True) + self.supports_microversion_mock.side_effect = ( + lambda _, v: api_versions.APIVersion(v) + <= api_versions.APIVersion(mock_v) + ) + + class TestBackupCreate(TestBackup): volume = volume_fakes.create_one_volume() snapshot = volume_fakes.create_one_snapshot() @@ -45,36 +70,22 @@ class TestBackupCreate(TestBackup): ) columns = ( - 'availability_zone', - 'container', - 'description', 'id', 'name', - 'object_count', - 'size', - 'snapshot_id', - 'status', 'volume_id', ) data = ( - new_backup.availability_zone, - new_backup.container, - new_backup.description, new_backup.id, new_backup.name, - new_backup.object_count, - new_backup.size, - new_backup.snapshot_id, - new_backup.status, new_backup.volume_id, ) def setUp(self): super().setUp() - self.volumes_mock.get.return_value = self.volume - self.snapshots_mock.get.return_value = self.snapshot - self.backups_mock.create.return_value = self.new_backup + self.sdk_client.find_volume.return_value = self.volume + self.sdk_client.find_snapshot.return_value = self.snapshot + self.sdk_client.create_backup.return_value = self.new_backup # Get the command object to test self.cmd = volume_backup.CreateVolumeBackup(self.app, None) @@ -106,8 +117,8 @@ class TestBackupCreate(TestBackup): columns, data = self.cmd.take_action(parsed_args) - self.backups_mock.create.assert_called_with( - self.new_backup.volume_id, + self.sdk_client.create_backup.assert_called_with( + volume_id=self.new_backup.volume_id, container=self.new_backup.container, name=self.new_backup.name, description=self.new_backup.description, @@ -119,9 +130,7 @@ class TestBackupCreate(TestBackup): self.assertEqual(self.data, data) def test_backup_create_with_properties(self): - self.app.client_manager.volume.api_version = api_versions.APIVersion( - '3.43' - ) + self._set_mock_microversion('3.43') arglist = [ "--property", @@ -138,8 +147,8 @@ class TestBackupCreate(TestBackup): columns, data = self.cmd.take_action(parsed_args) - self.backups_mock.create.assert_called_with( - self.new_backup.volume_id, + self.sdk_client.create_backup.assert_called_with( + volume_id=self.new_backup.volume_id, container=None, name=None, description=None, @@ -151,9 +160,7 @@ class TestBackupCreate(TestBackup): self.assertEqual(self.data, data) def test_backup_create_with_properties_pre_v343(self): - self.app.client_manager.volume.api_version = api_versions.APIVersion( - '3.42' - ) + self._set_mock_microversion('3.42') arglist = [ "--property", @@ -174,9 +181,7 @@ class TestBackupCreate(TestBackup): self.assertIn("--os-volume-api-version 3.43 or greater", str(exc)) def test_backup_create_with_availability_zone(self): - self.app.client_manager.volume.api_version = api_versions.APIVersion( - '3.51' - ) + self._set_mock_microversion('3.51') arglist = [ "--availability-zone", @@ -191,8 +196,8 @@ class TestBackupCreate(TestBackup): columns, data = self.cmd.take_action(parsed_args) - self.backups_mock.create.assert_called_with( - self.new_backup.volume_id, + self.sdk_client.create_backup.assert_called_with( + volume_id=self.new_backup.volume_id, container=None, name=None, description=None, @@ -204,9 +209,7 @@ class TestBackupCreate(TestBackup): self.assertEqual(self.data, data) def test_backup_create_with_availability_zone_pre_v351(self): - self.app.client_manager.volume.api_version = api_versions.APIVersion( - '3.50' - ) + self._set_mock_microversion('3.50') arglist = [ "--availability-zone", @@ -241,8 +244,8 @@ class TestBackupCreate(TestBackup): columns, data = self.cmd.take_action(parsed_args) - self.backups_mock.create.assert_called_with( - self.new_backup.volume_id, + self.sdk_client.create_backup.assert_called_with( + volume_id=self.new_backup.volume_id, container=self.new_backup.container, name=None, description=self.new_backup.description, @@ -259,8 +262,8 @@ class TestBackupDelete(TestBackup): def setUp(self): super().setUp() - self.backups_mock.get = volume_fakes.get_backups(self.backups) - self.backups_mock.delete.return_value = None + self.sdk_client.find_backup = volume_fakes.get_backups(self.backups) + self.sdk_client.delete_backup.return_value = None # Get the command object to mock self.cmd = volume_backup.DeleteVolumeBackup(self.app, None) @@ -272,7 +275,9 @@ class TestBackupDelete(TestBackup): result = self.cmd.take_action(parsed_args) - self.backups_mock.delete.assert_called_with(self.backups[0].id, False) + self.sdk_client.delete_backup.assert_called_with( + self.backups[0].id, ignore_missing=False, force=False + ) self.assertIsNone(result) def test_backup_delete_with_force(self): @@ -285,7 +290,9 @@ class TestBackupDelete(TestBackup): result = self.cmd.take_action(parsed_args) - self.backups_mock.delete.assert_called_with(self.backups[0].id, True) + self.sdk_client.delete_backup.assert_called_with( + self.backups[0].id, ignore_missing=False, force=True + ) self.assertIsNone(result) def test_delete_multiple_backups(self): @@ -301,8 +308,8 @@ class TestBackupDelete(TestBackup): calls = [] for b in self.backups: - calls.append(call(b.id, False)) - self.backups_mock.delete.assert_has_calls(calls) + calls.append(call(b.id, ignore_missing=False, force=False)) + self.sdk_client.delete_backup.assert_has_calls(calls) self.assertIsNone(result) def test_delete_multiple_backups_with_exception(self): @@ -318,7 +325,7 @@ class TestBackupDelete(TestBackup): find_mock_result = [self.backups[0], exceptions.CommandError] with mock.patch.object( - utils, 'find_resource', side_effect=find_mock_result + self.sdk_client, 'find_backup', side_effect=find_mock_result ) as find_mock: try: self.cmd.take_action(parsed_args) @@ -326,12 +333,14 @@ class TestBackupDelete(TestBackup): except exceptions.CommandError as e: self.assertEqual('1 of 2 backups failed to delete.', str(e)) - find_mock.assert_any_call(self.backups_mock, self.backups[0].id) - find_mock.assert_any_call(self.backups_mock, 'unexist_backup') + find_mock.assert_any_call(self.backups[0].id, ignore_missing=False) + find_mock.assert_any_call('unexist_backup', ignore_missing=False) self.assertEqual(2, find_mock.call_count) - self.backups_mock.delete.assert_called_once_with( - self.backups[0].id, False + self.sdk_client.delete_backup.assert_called_once_with( + self.backups[0].id, + ignore_missing=False, + force=False, ) @@ -383,10 +392,11 @@ class TestBackupList(TestBackup): def setUp(self): super().setUp() - self.volumes_mock.list.return_value = [self.volume] - self.backups_mock.list.return_value = self.backups - self.volumes_mock.get.return_value = self.volume - self.backups_mock.get.return_value = self.backups[0] + self.sdk_client.volumes.return_value = [self.volume] + self.sdk_client.backups.return_value = self.backups + self.sdk_client.find_volume.return_value = self.volume + self.sdk_client.find_backup.return_value = self.backups[0] + # Get the command to test self.cmd = volume_backup.ListVolumeBackup(self.app, None) @@ -405,16 +415,13 @@ class TestBackupList(TestBackup): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - search_opts = { - "name": None, - "status": None, - "volume_id": None, - 'all_tenants': False, - } - self.volumes_mock.get.assert_not_called() - self.backups_mock.get.assert_not_called() - self.backups_mock.list.assert_called_with( - search_opts=search_opts, + self.sdk_client.find_volume.assert_not_called() + self.sdk_client.find_backup.assert_not_called() + self.sdk_client.backups.assert_called_with( + name=None, + status=None, + volume_id=None, + all_tenants=False, marker=None, limit=None, ) @@ -449,16 +456,17 @@ class TestBackupList(TestBackup): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - search_opts = { - "name": self.backups[0].name, - "status": "error", - "volume_id": self.volume.id, - 'all_tenants': True, - } - self.volumes_mock.get.assert_called_once_with(self.volume.id) - self.backups_mock.get.assert_called_once_with(self.backups[0].id) - self.backups_mock.list.assert_called_with( - search_opts=search_opts, + self.sdk_client.find_volume.assert_called_once_with( + self.volume.id, ignore_missing=False + ) + self.sdk_client.find_backup.assert_called_once_with( + self.backups[0].id, ignore_missing=False + ) + self.sdk_client.backups.assert_called_with( + name=self.backups[0].name, + status="error", + volume_id=self.volume.id, + all_tenants=True, marker=self.backups[0].id, limit=3, ) @@ -475,19 +483,20 @@ class TestBackupRestore(TestBackup): def setUp(self): super().setUp() - self.backups_mock.get.return_value = self.backup - self.volumes_mock.get.return_value = self.volume - self.restores_mock.restore.return_value = ( + self.sdk_client.find_backup.return_value = self.backup + self.sdk_client.find_volume.return_value = self.volume + self.sdk_client.restore_backup.return_value = ( volume_fakes.create_one_volume( {'id': self.volume['id']}, ) ) + # Get the command object to mock self.cmd = volume_backup.RestoreVolumeBackup(self.app, None) def test_backup_restore(self): - self.volumes_mock.get.side_effect = exceptions.CommandError() - self.volumes_mock.find.side_effect = exceptions.CommandError() + self.sdk_client.find_volume.side_effect = exceptions.CommandError() + # self.sdk_client.side_effect = exceptions.CommandError() arglist = [self.backup.id] verifylist = [ ("backup", self.backup.id), @@ -496,16 +505,16 @@ class TestBackupRestore(TestBackup): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.restores_mock.restore.assert_called_with( + self.sdk_client.restore_backup.assert_called_with( self.backup.id, - None, - None, + volume_id=None, + name=None, ) self.assertIsNotNone(result) def test_backup_restore_with_volume(self): - self.volumes_mock.get.side_effect = exceptions.CommandError() - self.volumes_mock.find.side_effect = exceptions.CommandError() + self.sdk_client.find_volume.side_effect = exceptions.CommandError() + # self.volumes_mock.find.side_effect = exceptions.CommandError() arglist = [ self.backup.id, self.backup.volume_id, @@ -517,10 +526,10 @@ class TestBackupRestore(TestBackup): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.restores_mock.restore.assert_called_with( + self.sdk_client.restore_backup.assert_called_with( self.backup.id, - None, - self.backup.volume_id, + volume_id=None, + name=self.backup.volume_id, ) self.assertIsNotNone(result) @@ -538,10 +547,10 @@ class TestBackupRestore(TestBackup): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.restores_mock.restore.assert_called_with( + self.sdk_client.restore_backup.assert_called_with( self.backup.id, - self.volume.id, - None, + volume_id=self.volume.id, + name=None, ) self.assertIsNotNone(result) @@ -563,7 +572,7 @@ class TestBackupRestore(TestBackup): ) -class TestBackupSet(TestBackup): +class TestBackupSet(TestBackupLegacy): backup = volume_fakes.create_one_backup( attrs={'metadata': {'wow': 'cool'}}, ) @@ -793,7 +802,7 @@ class TestBackupSet(TestBackup): self.assertIn("--os-volume-api-version 3.43 or greater", str(exc)) -class TestBackupUnset(TestBackup): +class TestBackupUnset(TestBackupLegacy): backup = volume_fakes.create_one_backup( attrs={'metadata': {'foo': 'bar'}}, ) @@ -859,34 +868,54 @@ class TestBackupShow(TestBackup): backup = volume_fakes.create_one_backup() columns = ( - 'availability_zone', - 'container', - 'description', - 'id', - 'name', - 'object_count', - 'size', - 'snapshot_id', - 'status', - 'volume_id', + "availability_zone", + "container", + "created_at", + "data_timestamp", + "description", + "encryption_key_id", + "fail_reason", + "has_dependent_backups", + "id", + "is_incremental", + "metadata", + "name", + "object_count", + "project_id", + "size", + "snapshot_id", + "status", + "updated_at", + "user_id", + "volume_id", ) data = ( backup.availability_zone, backup.container, + backup.created_at, + backup.data_timestamp, backup.description, + backup.encryption_key_id, + backup.fail_reason, + backup.has_dependent_backups, backup.id, + backup.is_incremental, + backup.metadata, backup.name, backup.object_count, + backup.project_id, backup.size, backup.snapshot_id, backup.status, + backup.updated_at, + backup.user_id, backup.volume_id, ) def setUp(self): super().setUp() - self.backups_mock.get.return_value = self.backup + self.sdk_client.get_backup.return_value = self.backup # Get the command object to test self.cmd = volume_backup.ShowVolumeBackup(self.app, None) @@ -896,7 +925,7 @@ class TestBackupShow(TestBackup): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.backups_mock.get.assert_called_with(self.backup.id) + self.sdk_client.get_backup.assert_called_with(self.backup.id) self.assertEqual(self.columns, columns) self.assertEqual(self.data, data) diff --git a/openstackclient/volume/v2/volume_backup.py b/openstackclient/volume/v2/volume_backup.py index 7040216a39..b3f5c250ec 100644 --- a/openstackclient/volume/v2/volume_backup.py +++ b/openstackclient/volume/v2/volume_backup.py @@ -20,6 +20,7 @@ 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 from osc_lib.command import command from osc_lib import exceptions @@ -126,23 +127,23 @@ class CreateVolumeBackup(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_id = utils.find_resource( - volume_client.volumes, + volume_id = volume_client.find_volume( parsed_args.volume, + ignore_missing=False, ).id kwargs = {} if parsed_args.snapshot: - kwargs['snapshot_id'] = utils.find_resource( - volume_client.volume_snapshots, + kwargs['snapshot_id'] = volume_client.find_snapshot( parsed_args.snapshot, + ignore_missing=False, ).id 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' @@ -152,7 +153,7 @@ class CreateVolumeBackup(command.ShowOne): kwargs['metadata'] = parsed_args.properties if parsed_args.availability_zone: - if volume_client.api_version < api_versions.APIVersion('3.51'): + if not sdk_utils.supports_microversion(volume_client, '3.51'): msg = _( '--os-volume-api-version 3.51 or greater is required to ' 'support the --availability-zone option' @@ -161,8 +162,13 @@ class CreateVolumeBackup(command.ShowOne): kwargs['availability_zone'] = parsed_args.availability_zone - backup = volume_client.backups.create( - volume_id, + columns = ( + "id", + "name", + "volume_id", + ) + backup = volume_client.create_backup( + volume_id=volume_id, container=parsed_args.container, name=parsed_args.name, description=parsed_args.description, @@ -170,8 +176,8 @@ class CreateVolumeBackup(command.ShowOne): incremental=parsed_args.incremental, **kwargs, ) - backup._info.pop("links", None) - return zip(*sorted(backup._info.items())) + data = utils.get_dict_properties(backup, columns) + return (columns, data) class DeleteVolumeBackup(command.Command): @@ -194,16 +200,19 @@ class DeleteVolumeBackup(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 result = 0 for i in parsed_args.backups: try: - backup_id = utils.find_resource( - volume_client.backups, - i, + backup_id = volume_client.find_backup( + i, ignore_missing=False ).id - volume_client.backups.delete(backup_id, parsed_args.force) + volume_client.delete_backup( + backup_id, + ignore_missing=False, + force=parsed_args.force, + ) except Exception as e: result += 1 LOG.error( @@ -298,7 +307,7 @@ class ListVolumeBackup(command.Lister): return parser def take_action(self, parsed_args): - volume_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume columns = ('id', 'name', 'description', 'status', 'size') column_headers = ('ID', 'Name', 'Description', 'Status', 'Size') @@ -309,7 +318,7 @@ class ListVolumeBackup(command.Lister): # Cache the volume list volume_cache = {} try: - for s in volume_client.volumes.list(): + for s in volume_client.volumes(): volume_cache[s.id] = s except Exception: # Just forget it if there's any trouble @@ -322,9 +331,9 @@ class ListVolumeBackup(command.Lister): filter_volume_id = None if parsed_args.volume: try: - filter_volume_id = utils.find_resource( - volume_client.volumes, + filter_volume_id = volume_client.find_volume( parsed_args.volume, + ignore_missing=False, ).id except exceptions.CommandError: # Volume with that ID does not exist, but search for backups @@ -338,19 +347,16 @@ class ListVolumeBackup(command.Lister): marker_backup_id = None if parsed_args.marker: - marker_backup_id = utils.find_resource( - volume_client.backups, + marker_backup_id = volume_client.find_backup( parsed_args.marker, + ignore_missing=False, ).id - search_opts = { - 'name': parsed_args.name, - 'status': parsed_args.status, - 'volume_id': filter_volume_id, - 'all_tenants': parsed_args.all_projects, - } - data = volume_client.backups.list( - search_opts=search_opts, + data = volume_client.backups( + name=parsed_args.name, + status=parsed_args.status, + volume_id=filter_volume_id, + all_tenants=parsed_args.all_projects, marker=marker_backup_id, limit=parsed_args.limit, ) @@ -399,16 +405,19 @@ class RestoreVolumeBackup(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 - backup = utils.find_resource(volume_client.backups, parsed_args.backup) + backup = volume_client.find_backup( + parsed_args.backup, + ignore_missing=False, + ) volume_name = None volume_id = None try: - volume_id = utils.find_resource( - volume_client.volumes, + volume_id = volume_client.find_volume( parsed_args.volume, + ignore_missing=False, ).id except Exception: volume_name = parsed_args.volume @@ -422,10 +431,10 @@ class RestoreVolumeBackup(command.ShowOne): ) raise exceptions.CommandError(msg % parsed_args.volume) - return volume_client.restores.restore( + return volume_client.restore_backup( backup.id, - volume_id, - volume_name, + volume_id=volume_id, + name=volume_name, ) @@ -630,7 +639,29 @@ class ShowVolumeBackup(command.ShowOne): return parser def take_action(self, parsed_args): - volume_client = self.app.client_manager.volume - backup = utils.find_resource(volume_client.backups, parsed_args.backup) - backup._info.pop("links", None) - return zip(*sorted(backup._info.items())) + volume_client = self.app.client_manager.sdk_connection.volume + backup = volume_client.get_backup(parsed_args.backup) + columns = ( + "availability_zone", + "container", + "created_at", + "data_timestamp", + "description", + "encryption_key_id", + "fail_reason", + "has_dependent_backups", + "id", + "is_incremental", + "metadata", + "name", + "object_count", + "project_id", + "size", + "snapshot_id", + "status", + "updated_at", + "user_id", + "volume_id", + ) + data = utils.get_dict_properties(backup, columns) + return (columns, data) diff --git a/releasenotes/notes/migrate-backup-commands-0becc8f18cf9737b.yaml b/releasenotes/notes/migrate-backup-commands-0becc8f18cf9737b.yaml new file mode 100644 index 0000000000..290a8d7869 --- /dev/null +++ b/releasenotes/notes/migrate-backup-commands-0becc8f18cf9737b.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Migrated the following backup commands to SDK: + + * Create Backup + * Show Backup + * List Backup + * Restore Backup + * Delete Backup