From 8a2f3a9db13c8db9d60d6a193f4bd937eb9d6ce6 Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Wed, 11 Sep 2024 22:19:37 -0700 Subject: [PATCH] Fix export location metadata updates by drivers Drivers can set arbitrary key=value metadata to export paths. The share manager now checks updates to previously set metadata when drivers update this metadata with the "ensure_shares" routine. Change-Id: Ia42de608ba056d71a1deae2c59bfb43a11672ab7 Partial-Bug: #2053100 (cherry picked from commit 3e9d535be1799a2f2d590537125e3b7630dc4aee) (cherry picked from commit ffa4233b9c31cdc2a4ad90d162c6bc15d7dabde4) --- manila/db/sqlalchemy/api.py | 12 +++- manila/tests/db/sqlalchemy/test_api.py | 59 +++++++++++++++++++ ...eferred-attr-updates-32db001aacfc8563.yaml | 6 ++ 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/bug-2053100-fix-export-path-preferred-attr-updates-32db001aacfc8563.yaml diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 480d96ebed..091d101d52 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -4534,9 +4534,17 @@ def _export_locations_update( 'deleted': 0, }) el.save(session=context.session) - if el['el_metadata']: + + new_export_metadata = next( + exl.get('metadata', {}) + for exl in export_locations + if exl['path'] == el['path'] + ) + new_export_metadata = new_export_metadata or el['el_metadata'] + + if new_export_metadata: _export_location_metadata_update( - context, el['uuid'], el['el_metadata'], + context, el['uuid'], new_export_metadata, ) # Now add new export locations diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index fa990faa30..a1d722ead9 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -2277,6 +2277,65 @@ class ShareExportLocationsDatabaseAPITestCase(test.TestCase): # actual result should contain locations in exact same order self.assertEqual(actual_result, update_locations) + def test_update_export_locations_with_metadata(self): + share = db_utils.create_share() + original_export_locations = [ + { + 'path': 'fake1/1/', + 'is_admin_only': True, + 'metadata': { + 'foo': 'bar', + 'preferred': '1' + }, + }, + { + 'path': 'fake2/1/', + 'is_admin_only': True, + 'metadata': { + 'clem': 'son', + 'preferred': '0' + }, + }, + ] + + # add initial locations + db_api.export_locations_update( + self.ctxt, share.instance['id'], original_export_locations, False) + + updated_export_locations = [ + { + 'path': 'fake1/1/', + 'is_admin_only': True, + 'metadata': { + 'foo': 'quz', + 'preferred': '0' + }, + }, + { + 'path': 'fake2/1/', + 'is_admin_only': True, + 'metadata': { + 'clem': 'son', + 'preferred': '1', + }, + }, + ] + + # update locations + db_api.export_locations_update( + self.ctxt, share.instance['id'], updated_export_locations, True) + actual_result = db_api.export_location_get_all_by_share_id( + self.ctxt, share['id']) + + actual_export_locations = [ + { + 'path': el['path'], + 'is_admin_only': el['is_admin_only'], + 'metadata': el['el_metadata'], + } for el in actual_result + ] + self.assertEqual(updated_export_locations, actual_export_locations) + def test_update_string(self): share = db_utils.create_share() initial_location = 'fake1/1/' diff --git a/releasenotes/notes/bug-2053100-fix-export-path-preferred-attr-updates-32db001aacfc8563.yaml b/releasenotes/notes/bug-2053100-fix-export-path-preferred-attr-updates-32db001aacfc8563.yaml new file mode 100644 index 0000000000..990cc557f7 --- /dev/null +++ b/releasenotes/notes/bug-2053100-fix-export-path-preferred-attr-updates-32db001aacfc8563.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Share manager drivers can now update export location metadata + (such as the `preferred` attribute) during the `ensure_shares` + routine. (`Launchpad bug: 2053100 `_)