Merge "Refactor "volume backup restore" command"
This commit is contained in:
commit
ec95b58482
@ -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):
|
||||
|
@ -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):
|
||||
|
||||
|
@ -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):
|
||||
|
@ -363,18 +363,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
10
releasenotes/notes/bug-1597189-02a8d8a402725860.yaml
Normal 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>`_]
|
Loading…
Reference in New Issue
Block a user