[openstackclient] Fix volume backup restore
The python-openstackclient package was upversioned from v6.2.0 to v6.6.0 [1]. The latest version of python-openstackclient replaced the python-cinderclient by the python-openstacksdk to handle backup operations [2]. Currently, the command "openstack volume backup restore" is failing in stx-openstack with the error "Proxy object has no attribute restores". This is happening because the patch [3] targeted to handle the cinderclient behavior described at [4] is no longer supported. This changes removes the previous/unsuported patch from the starlingx/python-openstackclient package and adds two new patches by cherry-picking the changes [5] and [6] already available upstream in the openstackclient and openstacksdk repos. [1]https://review.opendev.org/c/starlingx/openstack-armada-app/+/941975 [2]https://review.opendev.org/c/openstack/python-openstackclient/+/889748 [3]878e1ebd73/upstream/openstack/python-openstackclient/debian/patches/0003-Fix-output-for-openstack-volume-backup-restore.patch
[4]a64cf9a045
[5]https://review.opendev.org/c/openstack/python-openstackclient/+/931756 [6]https://review.opendev.org/c/openstack/openstacksdk/+/931755 Test Plan: [PASS] build package python-openstackclient [PASS] build package python-openstacksdk [PASS] build image stx-openstackclients [PASS] upload the image to a running system [PASS] create a volume [PASS] create a backup [PASS] restore the volume using the backup [PASS] launch a VM from the restored volume Closes-Bug: #2103457 Change-Id: I323cd6a69b6e35d2411fcd0f51f005bb255026af Signed-off-by: Alex Figueiredo <alex.fernandesfigueiredo@windriver.com>
This commit is contained in:
parent
3356ea6a6f
commit
08b13a7a0e
@ -1,53 +0,0 @@
|
||||
From 0fdefb9c0ff7ffae1b618a1e3fb5e06f4aab9cdd Mon Sep 17 00:00:00 2001
|
||||
From: Lucas de Ataides <lucas.deataidesbarreto@windriver.com>
|
||||
Date: Wed, 29 Nov 2023 17:12:09 -0300
|
||||
Subject: [PATCH] Fix output for "openstack volume backup restore"
|
||||
|
||||
Previously, on Openstack Pike, an issue [1] was reported that the
|
||||
`openstack volume backup restore` command was not able to parse the
|
||||
output correctly, even though the restore operation succeeds. This was
|
||||
fixed by [2], for the Stein release (and cherry-picked to both Rocky and
|
||||
Queens). The issue was that cliff expects a list with two tuples to
|
||||
display the results, whereas the restore function was returning a
|
||||
VolumeBackupsRestore object. The solution was to use the `_info` field
|
||||
from the VolumeBackupsRetore object, instead of the whole object.
|
||||
|
||||
This was done not only for the VolumeBackupsRetore object, but also for
|
||||
the VolumeBackup one, as can be seen on [3] and [4]. However, the commit
|
||||
[5] removed this essential parsing, and caused the previously fixed
|
||||
issue to reappear.
|
||||
|
||||
[1] https://bugs.launchpad.net/python-openstackclient/+bug/1733315
|
||||
[2] https://review.opendev.org/c/openstack/python-openstackclient/+/624860
|
||||
[3] https://opendev.org/openstack/python-openstackclient/src/branch/stable/2023.1/openstackclient/volume/v2/volume_backup.py#L174
|
||||
[4] https://opendev.org/openstack/python-openstackclient/src/branch/stable/2023.1/openstackclient/volume/v2/volume_backup.py#L619
|
||||
[5] https://review.opendev.org/c/openstack/python-openstackclient/+/353931
|
||||
|
||||
Signed-off-by: Lucas de Ataides <lucas.deataidesbarreto@windriver.com>
|
||||
[ Ported this patch to python-openstackclient v6.6.0-5 @ Caracal ]
|
||||
Signed-off-by: Jose Claudio <joseclaudio.paespires@windriver.com>
|
||||
---
|
||||
openstackclient/volume/v2/volume_backup.py | 3 ++-
|
||||
1 file changed, 2 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/openstackclient/volume/v2/volume_backup.py b/openstackclient/volume/v2/volume_backup.py
|
||||
index 92770f55..57909e0d 100644
|
||||
--- a/openstackclient/volume/v2/volume_backup.py
|
||||
+++ b/openstackclient/volume/v2/volume_backup.py
|
||||
@@ -440,11 +440,12 @@ class RestoreVolumeBackup(command.ShowOne):
|
||||
)
|
||||
raise exceptions.CommandError(msg % parsed_args.volume)
|
||||
|
||||
- return volume_client.restore_backup(
|
||||
+ backup_restore = volume_client.restores.restore(
|
||||
backup.id,
|
||||
volume_id=volume_id,
|
||||
name=volume_name,
|
||||
)
|
||||
+ return zip(*sorted(backup_restore._info.items()))
|
||||
|
||||
|
||||
class SetVolumeBackup(command.Command):
|
||||
--
|
||||
2.34.1
|
||||
|
@ -0,0 +1,170 @@
|
||||
From b3eeddc8d4614eba449a26957f73b238af9d0f75 Mon Sep 17 00:00:00 2001
|
||||
From: Rajat Dhasmana <rajatdhasmana@gmail.com>
|
||||
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
|
||||
|
||||
[ Cherry-picked to stx-openstack caracal ]
|
||||
Test Plan:
|
||||
[PASS] build package python-openstackclient
|
||||
[PASS] build image stx-openstackclients
|
||||
[PASS] upload the image to a running system
|
||||
[PASS] create a volume
|
||||
[PASS] create a backup
|
||||
[PASS] restore the volume
|
||||
Closes-Bug: #2103457
|
||||
Signed-off-by: Alex Figueiredo <alex.fernandesfigueiredo@windriver.com>
|
||||
---
|
||||
.../unit/volume/v2/test_volume_backup.py | 40 ++++++++++++++-----
|
||||
openstackclient/volume/v2/volume_backup.py | 11 ++++-
|
||||
.../fix-restore-resp-e664a643a723cd2e.yaml | 4 ++
|
||||
3 files changed, 43 insertions(+), 12 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 8b0c7688..3110885d 100644
|
||||
--- a/openstackclient/tests/unit/volume/v2/test_volume_backup.py
|
||||
+++ b/openstackclient/tests/unit/volume/v2/test_volume_backup.py
|
||||
@@ -492,16 +492,28 @@ class TestBackupRestore(TestBackup):
|
||||
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)
|
||||
@@ -517,13 +529,15 @@ class TestBackupRestore(TestBackup):
|
||||
]
|
||||
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 = (
|
||||
@@ -539,13 +553,15 @@ class TestBackupRestore(TestBackup):
|
||||
]
|
||||
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 = [
|
||||
@@ -560,13 +576,15 @@ class TestBackupRestore(TestBackup):
|
||||
]
|
||||
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 6d8917d5..decb5beb 100644
|
||||
--- a/openstackclient/volume/v2/volume_backup.py
|
||||
+++ b/openstackclient/volume/v2/volume_backup.py
|
||||
@@ -430,6 +430,12 @@ class RestoreVolumeBackup(command.ShowOne):
|
||||
ignore_missing=False,
|
||||
)
|
||||
|
||||
+ columns = (
|
||||
+ 'id',
|
||||
+ 'volume_id',
|
||||
+ 'volume_name',
|
||||
+ )
|
||||
+
|
||||
volume_name = None
|
||||
volume_id = None
|
||||
try:
|
||||
@@ -449,12 +455,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 00000000..2ee8f216
|
||||
--- /dev/null
|
||||
+++ b/releasenotes/notes/fix-restore-resp-e664a643a723cd2e.yaml
|
||||
@@ -0,0 +1,4 @@
|
||||
+---
|
||||
+fixes:
|
||||
+ - |
|
||||
+ Fixed the output of ``volume backup restore`` command.
|
||||
--
|
||||
2.34.1
|
||||
|
@ -1,4 +1,4 @@
|
||||
0001-Add-plugin-entry-point-sorting-mechanism.patch
|
||||
0002-Add-location-parameter-for-volume-backup-creation.patch
|
||||
0003-Fix-output-for-openstack-volume-backup-restore.patch
|
||||
0004-Support-location-metadata-for-volume-backups.patch
|
||||
0003-Support-location-metadata-for-volume-backups.patch
|
||||
0004-Fix-Volume-backup-restore-output.patch
|
||||
|
@ -0,0 +1,218 @@
|
||||
From 19927dbc2d434c655280c41a2a79b59598cb198f Mon Sep 17 00:00:00 2001
|
||||
From: Rajat Dhasmana <rajatdhasmana@gmail.com>
|
||||
Date: Tue, 8 Oct 2024 09:26:21 +0000
|
||||
Subject: [PATCH] Fix volume backup restore response
|
||||
|
||||
Previously the backup restore response only included ``id``
|
||||
whereas the restore API returns ``backup_id``, ``volume_id``
|
||||
and ``volume_name`` fields.
|
||||
Turns out the resource_response_key was missing in the translate
|
||||
response method and the has_body parameter was set to False indicating
|
||||
that the response doesn't return a body which is not true.
|
||||
This patch fixes the above stated issues.
|
||||
|
||||
Story: 2011235
|
||||
Task: 51137
|
||||
|
||||
Change-Id: Id5c7fd2f0fcb55474b44b688bfdebaca4c670bd2
|
||||
|
||||
[ Cherry-picked to stx-openstack caracal ]
|
||||
Test Plan:
|
||||
[PASS] build package python-openstackclient
|
||||
[PASS] build image stx-openstackclients
|
||||
[PASS] upload the image to a running system
|
||||
[PASS] create a volume
|
||||
[PASS] create a backup
|
||||
[PASS] restore the volume
|
||||
Closes-Bug: #2103457
|
||||
Signed-off-by: Alex Figueiredo <alex.fernandesfigueiredo@windriver.com>
|
||||
---
|
||||
openstack/block_storage/v2/backup.py | 4 ++-
|
||||
openstack/block_storage/v3/backup.py | 4 ++-
|
||||
.../unit/block_storage/v2/test_backup.py | 36 +++++++++++++++++++
|
||||
.../unit/block_storage/v3/test_backup.py | 36 +++++++++++++++++++
|
||||
.../fix-restore-resp-4e0bf3a246f3dc59.yaml | 6 ++++
|
||||
5 files changed, 84 insertions(+), 2 deletions(-)
|
||||
create mode 100644 releasenotes/notes/fix-restore-resp-4e0bf3a246f3dc59.yaml
|
||||
|
||||
diff --git a/openstack/block_storage/v2/backup.py b/openstack/block_storage/v2/backup.py
|
||||
index 12c78f1cb..afde91ea6 100644
|
||||
--- a/openstack/block_storage/v2/backup.py
|
||||
+++ b/openstack/block_storage/v2/backup.py
|
||||
@@ -86,6 +86,8 @@ class Backup(resource.Resource):
|
||||
updated_at = resource.Body("updated_at")
|
||||
#: The UUID of the volume.
|
||||
volume_id = resource.Body("volume_id")
|
||||
+ #: The name of the volume.
|
||||
+ volume_name = resource.Body("volume_name")
|
||||
|
||||
def create(self, session, prepend_key=True, base_path=None, **params):
|
||||
"""Create a remote resource based on this instance.
|
||||
@@ -186,7 +188,7 @@ class Backup(resource.Resource):
|
||||
'Either of `name` or `volume_id` must be specified.'
|
||||
)
|
||||
response = session.post(url, json=body)
|
||||
- self._translate_response(response, has_body=False)
|
||||
+ self._translate_response(response, resource_response_key='restore')
|
||||
return self
|
||||
|
||||
def force_delete(self, session):
|
||||
diff --git a/openstack/block_storage/v3/backup.py b/openstack/block_storage/v3/backup.py
|
||||
index b942fe98d..f37fdb411 100644
|
||||
--- a/openstack/block_storage/v3/backup.py
|
||||
+++ b/openstack/block_storage/v3/backup.py
|
||||
@@ -100,6 +100,8 @@ class Backup(resource.Resource):
|
||||
user_id = resource.Body('user_id')
|
||||
#: The UUID of the volume.
|
||||
volume_id = resource.Body("volume_id")
|
||||
+ #: The name of the volume.
|
||||
+ volume_name = resource.Body("volume_name")
|
||||
|
||||
_max_microversion = "3.64"
|
||||
|
||||
@@ -202,7 +204,7 @@ class Backup(resource.Resource):
|
||||
'Either of `name` or `volume_id` must be specified.'
|
||||
)
|
||||
response = session.post(url, json=body)
|
||||
- self._translate_response(response, has_body=False)
|
||||
+ self._translate_response(response, resource_response_key='restore')
|
||||
return self
|
||||
|
||||
def force_delete(self, session):
|
||||
diff --git a/openstack/tests/unit/block_storage/v2/test_backup.py b/openstack/tests/unit/block_storage/v2/test_backup.py
|
||||
index 7de7902a5..e161f6838 100644
|
||||
--- a/openstack/tests/unit/block_storage/v2/test_backup.py
|
||||
+++ b/openstack/tests/unit/block_storage/v2/test_backup.py
|
||||
@@ -137,6 +137,18 @@ class TestBackup(base.TestCase):
|
||||
def test_restore(self):
|
||||
sot = backup.Backup(**BACKUP)
|
||||
|
||||
+ restore_response = mock.Mock()
|
||||
+ restore_response.status_code = 202
|
||||
+ restore_response.json.return_value = {
|
||||
+ "restore": {
|
||||
+ "backup_id": "back",
|
||||
+ "volume_id": "vol",
|
||||
+ "volume_name": "name",
|
||||
+ }
|
||||
+ }
|
||||
+ restore_response.headers = {}
|
||||
+ self.sess.post.return_value = restore_response
|
||||
+
|
||||
self.assertEqual(sot, sot.restore(self.sess, 'vol', 'name'))
|
||||
|
||||
url = 'backups/%s/restore' % FAKE_ID
|
||||
@@ -146,6 +158,18 @@ class TestBackup(base.TestCase):
|
||||
def test_restore_name(self):
|
||||
sot = backup.Backup(**BACKUP)
|
||||
|
||||
+ restore_response = mock.Mock()
|
||||
+ restore_response.status_code = 202
|
||||
+ restore_response.json.return_value = {
|
||||
+ "restore": {
|
||||
+ "backup_id": "back",
|
||||
+ "volume_id": "vol",
|
||||
+ "volume_name": "name",
|
||||
+ }
|
||||
+ }
|
||||
+ restore_response.headers = {}
|
||||
+ self.sess.post.return_value = restore_response
|
||||
+
|
||||
self.assertEqual(sot, sot.restore(self.sess, name='name'))
|
||||
|
||||
url = 'backups/%s/restore' % FAKE_ID
|
||||
@@ -155,6 +179,18 @@ class TestBackup(base.TestCase):
|
||||
def test_restore_vol_id(self):
|
||||
sot = backup.Backup(**BACKUP)
|
||||
|
||||
+ restore_response = mock.Mock()
|
||||
+ restore_response.status_code = 202
|
||||
+ restore_response.json.return_value = {
|
||||
+ "restore": {
|
||||
+ "backup_id": "back",
|
||||
+ "volume_id": "vol",
|
||||
+ "volume_name": "name",
|
||||
+ }
|
||||
+ }
|
||||
+ restore_response.headers = {}
|
||||
+ self.sess.post.return_value = restore_response
|
||||
+
|
||||
self.assertEqual(sot, sot.restore(self.sess, volume_id='vol'))
|
||||
|
||||
url = 'backups/%s/restore' % FAKE_ID
|
||||
diff --git a/openstack/tests/unit/block_storage/v3/test_backup.py b/openstack/tests/unit/block_storage/v3/test_backup.py
|
||||
index 5e73d61c9..45c28bd21 100644
|
||||
--- a/openstack/tests/unit/block_storage/v3/test_backup.py
|
||||
+++ b/openstack/tests/unit/block_storage/v3/test_backup.py
|
||||
@@ -150,6 +150,18 @@ class TestBackup(base.TestCase):
|
||||
def test_restore(self):
|
||||
sot = backup.Backup(**BACKUP)
|
||||
|
||||
+ restore_response = mock.Mock()
|
||||
+ restore_response.status_code = 202
|
||||
+ restore_response.json.return_value = {
|
||||
+ "restore": {
|
||||
+ "backup_id": "back",
|
||||
+ "volume_id": "vol",
|
||||
+ "volume_name": "name",
|
||||
+ }
|
||||
+ }
|
||||
+ restore_response.headers = {}
|
||||
+ self.sess.post.return_value = restore_response
|
||||
+
|
||||
self.assertEqual(sot, sot.restore(self.sess, 'vol', 'name'))
|
||||
|
||||
url = 'backups/%s/restore' % FAKE_ID
|
||||
@@ -159,6 +171,18 @@ class TestBackup(base.TestCase):
|
||||
def test_restore_name(self):
|
||||
sot = backup.Backup(**BACKUP)
|
||||
|
||||
+ restore_response = mock.Mock()
|
||||
+ restore_response.status_code = 202
|
||||
+ restore_response.json.return_value = {
|
||||
+ "restore": {
|
||||
+ "backup_id": "back",
|
||||
+ "volume_id": "vol",
|
||||
+ "volume_name": "name",
|
||||
+ }
|
||||
+ }
|
||||
+ restore_response.headers = {}
|
||||
+ self.sess.post.return_value = restore_response
|
||||
+
|
||||
self.assertEqual(sot, sot.restore(self.sess, name='name'))
|
||||
|
||||
url = 'backups/%s/restore' % FAKE_ID
|
||||
@@ -168,6 +192,18 @@ class TestBackup(base.TestCase):
|
||||
def test_restore_vol_id(self):
|
||||
sot = backup.Backup(**BACKUP)
|
||||
|
||||
+ restore_response = mock.Mock()
|
||||
+ restore_response.status_code = 202
|
||||
+ restore_response.json.return_value = {
|
||||
+ "restore": {
|
||||
+ "backup_id": "back",
|
||||
+ "volume_id": "vol",
|
||||
+ "volume_name": "name",
|
||||
+ }
|
||||
+ }
|
||||
+ restore_response.headers = {}
|
||||
+ self.sess.post.return_value = restore_response
|
||||
+
|
||||
self.assertEqual(sot, sot.restore(self.sess, volume_id='vol'))
|
||||
|
||||
url = 'backups/%s/restore' % FAKE_ID
|
||||
diff --git a/releasenotes/notes/fix-restore-resp-4e0bf3a246f3dc59.yaml b/releasenotes/notes/fix-restore-resp-4e0bf3a246f3dc59.yaml
|
||||
new file mode 100644
|
||||
index 000000000..884b93c4c
|
||||
--- /dev/null
|
||||
+++ b/releasenotes/notes/fix-restore-resp-4e0bf3a246f3dc59.yaml
|
||||
@@ -0,0 +1,6 @@
|
||||
+---
|
||||
+fixes:
|
||||
+ - |
|
||||
+ Previously the volume backup restore response only
|
||||
+ returned ``id`` and now it also returns ``volume_id``
|
||||
+ and ``volume_name`` fields.
|
||||
--
|
||||
2.34.1
|
||||
|
@ -0,0 +1 @@
|
||||
0001-Fix-volume-backup-restore-response.patch
|
Loading…
x
Reference in New Issue
Block a user