openstack
/
python-openstackclient
Code Issues Proposed changes

Refactor "volume backup restore" command 

Make the positional argument "volume" optional and add a "--force"
option (volume v2 only) to the "volume backup restore" command.

Closes-Bug: #1597189
Change-Id: If944e10158bd18e8331be63e96187a23e23095d7
This commit is contained in:
Huanxuan Ao 2016-09-22 17:10:32 +08:00 committed by Stephen Finucane
parent 8c975ba097
commit de4a69a29f
5 changed files with 174 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
openstackclient/tests/unit/volume/v1/test_volume_backup.py
View File

@ -319,29 +319,69 @@ class TestBackupRestore(TestBackup):
attrs={'volume_id': volume.id})
def setUp(self):
super(TestBackupRestore, self).setUp()
super().setUp()
self.backups_mock.get.return_value = self.backup
self.volumes_mock.get.return_value = self.volume
self.restores_mock.restore.return_value = None
self.restores_mock.restore.return_value = (
volume_fakes.FakeVolume.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):
arglist = [
self.backup.id,
self.backup.volume_id
]
verifylist = [
("backup", self.backup.id),
("volume", self.backup.volume_id)
("volume", None),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.restores_mock.restore.assert_called_with(self.backup.id,
self.backup.volume_id)
self.assertIsNone(result)
None)
self.assertIsNotNone(result)
def test_backup_restore_with_existing_volume(self):
arglist = [
self.backup.id,
self.backup.volume_id,
]
verifylist = [
("backup", self.backup.id),
("volume", self.backup.volume_id),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.restores_mock.restore.assert_called_with(
self.backup.id, self.backup.volume_id,
)
self.assertIsNotNone(result)
def test_backup_restore_with_invalid_volume(self):
arglist = [
self.backup.id,
"unexist_volume",
]
verifylist = [
("backup", self.backup.id),
("volume", "unexist_volume"),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
with mock.patch.object(
utils, 'find_resource',
side_effect=exceptions.CommandError(),
):
self.assertRaises(
exceptions.CommandError,
self.cmd.take_action,
parsed_args,
)
class TestBackupShow(TestBackup):

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

@ -458,35 +458,95 @@ class TestBackupRestore(TestBackup):
volume = volume_fakes.FakeVolume.create_one_volume()
backup = volume_fakes.FakeBackup.create_one_backup(
attrs={'volume_id': volume.id})
attrs={'volume_id': volume.id},
)
def setUp(self):
super(TestBackupRestore, self).setUp()
super().setUp()
self.backups_mock.get.return_value = self.backup
self.volumes_mock.get.return_value = self.volume
self.restores_mock.restore.return_value = (
volume_fakes.FakeVolume.create_one_volume(
{'id': self.volume['id']}))
{'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()
arglist = [
self.backup.id,
self.backup.volume_id
self.backup.id
]
verifylist = [
("backup", self.backup.id),
("volume", self.backup.volume_id)
("volume", None),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.restores_mock.restore.assert_called_with(self.backup.id,
self.backup.volume_id)
self.restores_mock.restore.assert_called_with(
self.backup.id, None, 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()
arglist = [
self.backup.id,
self.backup.volume_id,
]
verifylist = [
("backup", self.backup.id),
("volume", self.backup.volume_id),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.restores_mock.restore.assert_called_with(
self.backup.id, None, self.backup.volume_id,
)
self.assertIsNotNone(result)
def test_backup_restore_with_volume_force(self):
arglist = [
"--force",
self.backup.id,
self.volume.name,
]
verifylist = [
("force", True),
("backup", self.backup.id),
("volume", self.volume.name),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args)
self.restores_mock.restore.assert_called_with(
self.backup.id, self.volume.id, None,
)
self.assertIsNotNone(result)
def test_backup_restore_with_volume_existing(self):
arglist = [
self.backup.id,
self.volume.name,
]
verifylist = [
("backup", self.backup.id),
("volume", self.volume.name),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(
exceptions.CommandError,
self.cmd.take_action,
parsed_args,
)
class TestBackupSet(TestBackup):

19
openstackclient/volume/v1/volume_backup.py
View File

@ -231,18 +231,23 @@ class RestoreVolumeBackup(command.Command):
parser.add_argument(
'volume',
metavar='<volume>',
help=_('Volume to restore to (name or ID)')
nargs='?',
help=_('Volume to restore to (name or ID) (default to None)')
)
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)
destination_volume = utils.find_resource(volume_client.volumes,
parsed_args.volume)
return volume_client.restores.restore(backup.id,
destination_volume.id)
backup = utils.find_resource(
volume_client.backups, parsed_args.backup,
)
volume_id = None
if parsed_args.volume is not None:
volume_id = utils.find_resource(
volume_client.volumes,
parsed_args.volume,
).id
return volume_client.restores.restore(backup.id, volume_id)
class ShowVolumeBackup(command.ShowOne):

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

@ -355,18 +355,50 @@ class RestoreVolumeBackup(command.ShowOne):
parser.add_argument(
"volume",
metavar="<volume>",
help=_("Volume to restore to (name or ID)")
nargs="?",
help=_(
"Volume to restore to "
"(name or ID for existing volume, name only for new volume) "
"(default to None)"
)
)
parser.add_argument(
"--force",
action="store_true",
help=_(
"Restore the backup to an existing volume "
"(default to False)"
)
)
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)
destination_volume = utils.find_resource(volume_client.volumes,
parsed_args.volume)
backup = volume_client.restores.restore(backup.id,
destination_volume.id)
return zip(*sorted(backup._info.items()))
volume_name = None
volume_id = None
try:
volume_id = utils.find_resource(
volume_client.volumes,
parsed_args.volume,
).id
except Exception:
volume_name = parsed_args.volume
else:
# If we didn't fail, the volume must already exist. We only allow
# this to work if the user forced things
if not parsed_args.force:
msg = _(
"Volume '%s' already exists; if you want to restore the "
"backup to it you need to specify the '--force' option"
) % parsed_args.volume
raise exceptions.CommandError(msg)
return volume_client.restores.restore(
backup.id, volume_id, volume_name,
)
class SetVolumeBackup(command.Command):

10
releasenotes/notes/bug-1597189-02a8d8a402725860.yaml Normal file
View File

@ -0,0 +1,10 @@
---
features:
- |
The ``volume`` argument of the ``volume backup restore`` command is now
optional and can refer to the name of a new volume that should be created
rather than a name or ID of an existing volume (which would be
overwritten). If not provided, cinder will generate a new volume with a
unique name. To restore a backup to an existing volume, you must now
specify the ``--force`` option (volume v2, v3 only).
[Bug `1597189 <https://bugs.launchpad.net/bugs/1597189>`_]