openstack/python-openstackclient
Code Issues Proposed changes

Migrate backup commands to SDK

Browse Source 
This patch migrates the following backup commands to SDK:

* Create Backup
* Show Backup
* List Backup
* Restore Backup
* Delete Backup

Change-Id: Icfadd65b92f4cd343bc327d33e29767bc9b969f9
This commit is contained in:
whoami-rajat 2023-07-27 00:45:17 +05:30 committed by Rajat Dhasmana
parent 7ea78b6ef6
commit bfd6170b03
4 changed files with 227 additions and 146 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
openstackclient/tests/unit/volume/v2/fakes.py
View File

@ -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

237
openstackclient/tests/unit/volume/v2/test_volume_backup.py
View File

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

113
openstackclient/volume/v2/volume_backup.py
View File

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

10
releasenotes/notes/migrate-backup-commands-0becc8f18cf9737b.yaml Normal file
View File

@ -0,0 +1,10 @@
---
features:
- |
Migrated the following backup commands to SDK:
* Create Backup
* Show Backup
* List Backup
* Restore Backup
* Delete Backup