From 03e2fdd1622d79b0199e349be042991f927f0414 Mon Sep 17 00:00:00 2001 From: Rajat Dhasmana Date: Tue, 8 Oct 2024 09:26:51 +0000 Subject: [PATCH] Fix: Volume backup restore output Currently the volume backup restore command returns with error even though the restore is initiated. This patch corrects the response received from SDK and processes it in a human readable form. Change-Id: I7f020631fbb39ceef8740775fd82686d90a6c703 Closes-Bug: #2063335 Depends-On: https://review.opendev.org/c/openstack/openstacksdk/+/931755 --- .../unit/volume/v2/test_volume_backup.py | 40 ++++++++++++++----- .../unit/volume/v3/test_volume_backup.py | 40 ++++++++++++++----- openstackclient/volume/v2/volume_backup.py | 11 ++++- openstackclient/volume/v3/volume_backup.py | 11 ++++- .../fix-restore-resp-e664a643a723cd2e.yaml | 4 ++ 5 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/fix-restore-resp-e664a643a723cd2e.yaml diff --git a/openstackclient/tests/unit/volume/v2/test_volume_backup.py b/openstackclient/tests/unit/volume/v2/test_volume_backup.py index 483ca24de7..6f5c4ab69f 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume_backup.py +++ b/openstackclient/tests/unit/volume/v2/test_volume_backup.py @@ -364,16 +364,28 @@ class TestBackupRestore(volume_fakes.TestVolume): attrs={'volume_id': volume.id}, ) + columns = ( + "id", + "volume_id", + "volume_name", + ) + + data = ( + backup.id, + volume.id, + volume.name, + ) + def setUp(self): super().setUp() self.volume_sdk_client.find_backup.return_value = self.backup self.volume_sdk_client.find_volume.return_value = self.volume - self.volume_sdk_client.restore_backup.return_value = ( - volume_fakes.create_one_volume( - {'id': self.volume['id']}, - ) - ) + self.volume_sdk_client.restore_backup.return_value = { + 'id': self.backup['id'], + 'volume_id': self.volume['id'], + 'volume_name': self.volume['name'], + } # Get the command object to mock self.cmd = volume_backup.RestoreVolumeBackup(self.app, None) @@ -389,13 +401,15 @@ class TestBackupRestore(volume_fakes.TestVolume): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + columns, data = self.cmd.take_action(parsed_args) self.volume_sdk_client.restore_backup.assert_called_with( self.backup.id, volume_id=None, name=None, ) - self.assertIsNotNone(result) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) def test_backup_restore_with_volume(self): self.volume_sdk_client.find_volume.side_effect = ( @@ -411,13 +425,15 @@ class TestBackupRestore(volume_fakes.TestVolume): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + columns, data = self.cmd.take_action(parsed_args) self.volume_sdk_client.restore_backup.assert_called_with( self.backup.id, volume_id=None, name=self.backup.volume_id, ) - self.assertIsNotNone(result) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) def test_backup_restore_with_volume_force(self): arglist = [ @@ -432,13 +448,15 @@ class TestBackupRestore(volume_fakes.TestVolume): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + columns, data = self.cmd.take_action(parsed_args) self.volume_sdk_client.restore_backup.assert_called_with( self.backup.id, volume_id=self.volume.id, name=None, ) - self.assertIsNotNone(result) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) def test_backup_restore_with_volume_existing(self): arglist = [ diff --git a/openstackclient/tests/unit/volume/v3/test_volume_backup.py b/openstackclient/tests/unit/volume/v3/test_volume_backup.py index 864f162414..857c930460 100644 --- a/openstackclient/tests/unit/volume/v3/test_volume_backup.py +++ b/openstackclient/tests/unit/volume/v3/test_volume_backup.py @@ -462,16 +462,28 @@ class TestBackupRestore(volume_fakes.TestVolume): attrs={'volume_id': volume.id}, ) + columns = ( + "id", + "volume_id", + "volume_name", + ) + + data = ( + backup.id, + volume.id, + volume.name, + ) + def setUp(self): super().setUp() self.volume_sdk_client.find_backup.return_value = self.backup self.volume_sdk_client.find_volume.return_value = self.volume - self.volume_sdk_client.restore_backup.return_value = ( - volume_fakes.create_one_volume( - {'id': self.volume['id']}, - ) - ) + self.volume_sdk_client.restore_backup.return_value = { + 'id': self.backup['id'], + 'volume_id': self.volume['id'], + 'volume_name': self.volume['name'], + } # Get the command object to mock self.cmd = volume_backup.RestoreVolumeBackup(self.app, None) @@ -487,13 +499,15 @@ class TestBackupRestore(volume_fakes.TestVolume): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + columns, data = self.cmd.take_action(parsed_args) self.volume_sdk_client.restore_backup.assert_called_with( self.backup.id, volume_id=None, name=None, ) - self.assertIsNotNone(result) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) def test_backup_restore_with_volume(self): self.volume_sdk_client.find_volume.side_effect = ( @@ -509,13 +523,15 @@ class TestBackupRestore(volume_fakes.TestVolume): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + columns, data = self.cmd.take_action(parsed_args) self.volume_sdk_client.restore_backup.assert_called_with( self.backup.id, volume_id=None, name=self.backup.volume_id, ) - self.assertIsNotNone(result) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) def test_backup_restore_with_volume_force(self): arglist = [ @@ -530,13 +546,15 @@ class TestBackupRestore(volume_fakes.TestVolume): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - result = self.cmd.take_action(parsed_args) + columns, data = self.cmd.take_action(parsed_args) self.volume_sdk_client.restore_backup.assert_called_with( self.backup.id, volume_id=self.volume.id, name=None, ) - self.assertIsNotNone(result) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, data) def test_backup_restore_with_volume_existing(self): arglist = [ diff --git a/openstackclient/volume/v2/volume_backup.py b/openstackclient/volume/v2/volume_backup.py index fd59c0cefd..bbcd351808 100644 --- a/openstackclient/volume/v2/volume_backup.py +++ b/openstackclient/volume/v2/volume_backup.py @@ -359,6 +359,12 @@ class RestoreVolumeBackup(command.ShowOne): ignore_missing=False, ) + columns = ( + 'id', + 'volume_id', + 'volume_name', + ) + volume_name = None volume_id = None try: @@ -378,12 +384,15 @@ class RestoreVolumeBackup(command.ShowOne): ) raise exceptions.CommandError(msg % parsed_args.volume) - return volume_client.restore_backup( + restore = volume_client.restore_backup( backup.id, volume_id=volume_id, name=volume_name, ) + data = utils.get_dict_properties(restore, columns) + return (columns, data) + class SetVolumeBackup(command.Command): _description = _("Set volume backup properties") diff --git a/openstackclient/volume/v3/volume_backup.py b/openstackclient/volume/v3/volume_backup.py index 45d50b8437..4a7dabd396 100644 --- a/openstackclient/volume/v3/volume_backup.py +++ b/openstackclient/volume/v3/volume_backup.py @@ -412,6 +412,12 @@ class RestoreVolumeBackup(command.ShowOne): def take_action(self, parsed_args): volume_client = self.app.client_manager.sdk_connection.volume + columns = ( + 'id', + 'volume_id', + 'volume_name', + ) + backup = volume_client.find_backup( parsed_args.backup, ignore_missing=False, @@ -436,12 +442,15 @@ class RestoreVolumeBackup(command.ShowOne): ) raise exceptions.CommandError(msg % parsed_args.volume) - return volume_client.restore_backup( + restore = volume_client.restore_backup( backup.id, volume_id=volume_id, name=volume_name, ) + data = utils.get_dict_properties(restore, columns) + return (columns, data) + class SetVolumeBackup(command.Command): _description = _("Set volume backup properties") diff --git a/releasenotes/notes/fix-restore-resp-e664a643a723cd2e.yaml b/releasenotes/notes/fix-restore-resp-e664a643a723cd2e.yaml new file mode 100644 index 0000000000..2ee8f216d0 --- /dev/null +++ b/releasenotes/notes/fix-restore-resp-e664a643a723cd2e.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Fixed the output of ``volume backup restore`` command.