From 2b3100bd4856e3c425b4b2f3bbe1a8ed0ec2684e Mon Sep 17 00:00:00 2001 From: Saikumar Pulluri Date: Tue, 20 Jan 2026 06:06:15 -0500 Subject: [PATCH] Metadata for Share Replica Extend these into OSC capabilities where appropriate. Added metadata for create/show/set commands and implemented unset operation for unified CLI. Bumps max microversion to 2.95. Partially-implements: bp metadata-for-share-resources Depends-On: https://review.opendev.org/c/openstack/manila/+/973777 Change-Id: I29bd79340698c407be3113d76b4d05b31643c194 Signed-off-by: Saikumar Pulluri --- manilaclient/api_versions.py | 2 +- manilaclient/osc/v2/share_replicas.py | 168 ++++++++++++++++- manilaclient/tests/unit/osc/v2/fakes.py | 1 + .../tests/unit/osc/v2/test_share_replicas.py | 174 +++++++++++++++++- .../tests/unit/v2/test_share_replicas.py | 23 +++ manilaclient/v2/share_replicas.py | 46 ++++- pyproject.toml | 1 + ...are-replica-metadata-e6c1c2e00295d2dd.yaml | 5 + 8 files changed, 405 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/add-share-replica-metadata-e6c1c2e00295d2dd.yaml diff --git a/manilaclient/api_versions.py b/manilaclient/api_versions.py index 621487917..f4b4bbdcb 100644 --- a/manilaclient/api_versions.py +++ b/manilaclient/api_versions.py @@ -27,7 +27,7 @@ from manilaclient import utils LOG = logging.getLogger(__name__) -MAX_VERSION = '2.94' +MAX_VERSION = '2.95' MIN_VERSION = '2.0' DEPRECATED_VERSION = '1.0' _VERSIONED_METHOD_MAP = {} diff --git a/manilaclient/osc/v2/share_replicas.py b/manilaclient/osc/v2/share_replicas.py index 1bbbbd610..4bab9e3bc 100644 --- a/manilaclient/osc/v2/share_replicas.py +++ b/manilaclient/osc/v2/share_replicas.py @@ -11,6 +11,7 @@ # under the License. import logging +from osc_lib.cli import format_columns from osc_lib.cli import parseractions from osc_lib.command import command from osc_lib import exceptions @@ -70,6 +71,17 @@ class CreateShareReplica(command.ShowOne): 'microversion >= 2.72' ), ) + parser.add_argument( + "--property", + metavar="", + default={}, + action=parseractions.KeyValueAction, + help=_( + "Set a property to this replica " + "(repeat option to set multiple properties). " + "Available only for microversion >= 2.95" + ), + ) return parser def take_action(self, parsed_args): @@ -98,6 +110,21 @@ class CreateShareReplica(command.ShowOne): 'share': share, 'availability_zone': parsed_args.availability_zone, } + + metadata = {} + if parsed_args.property: + if share_client.api_version < api_versions.APIVersion("2.95"): + raise exceptions.CommandError( + _( + "arg '--property' is available only starting " + "with API microversion '2.95'." + ) + ) + metadata = parsed_args.property + + if metadata: + body['metadata'] = metadata + if scheduler_hints: body['scheduler_hints'] = scheduler_hints @@ -213,6 +240,16 @@ class ListShareReplica(command.Lister): default=None, help=_("Name or ID of the share to list replicas for."), ) + parser.add_argument( + '--property', + metavar='', + action=parseractions.KeyValueAction, + help=_( + 'Filter replicas having a given metadata key=value ' + 'property. (repeat option to filter by multiple ' + 'properties)' + ), + ) return parser def take_action(self, parsed_args): @@ -224,7 +261,32 @@ class ListShareReplica(command.Lister): share_client.shares, parsed_args.share ) - replicas = share_client.share_replicas.list(share=share) + properties = parsed_args.property or {} + if properties: + if share_client.api_version < api_versions.APIVersion("2.95"): + raise exceptions.CommandError( + "Property based filtering is only available " + "with manila API version >= 2.95" + ) + + search_opts = {} + if properties: + meta_str = ",".join(f"{k}:{v}" for k, v in properties.items()) + search_opts['metadata'] = meta_str + + replicas = share_client.share_replicas.list( + share=share, search_opts=search_opts + ) + + if properties: + replicas = [ + r + for r in replicas + if all( + r._info.get('metadata', {}).get(k) == v + for k, v in properties.items() + ) + ] columns = [ 'id', @@ -235,7 +297,6 @@ class ListShareReplica(command.Lister): 'availability_zone', 'updated_at', ] - column_headers = utils.format_column_headers(columns) data = ( osc_utils.get_dict_properties(replica._info, columns) @@ -284,6 +345,15 @@ class ShowShareReplica(command.ShowOne): replica._info['export_locations'] ) ) + # Special mapping for columns to make the output easier to read: + # 'metadata' --> 'properties' + replica._info.update( + { + 'properties': format_columns.DictColumn( + replica._info.pop('metadata', {}) + ), + }, + ) replica._info.pop('links', None) @@ -294,7 +364,8 @@ class SetShareReplica(command.Command): """Set share replica""" _description = _( - "Explicitly set share replica status and/or replica-state" + "Explicitly set share replica status and/or replica-state " + "and/or property" ) def get_parser(self, prog_name): @@ -329,6 +400,17 @@ class SetShareReplica(command.Command): "error_deleting." ), ) + parser.add_argument( + "--property", + metavar="", + default={}, + action=parseractions.KeyValueAction, + help=_( + "Set a property to this replica " + "(repeat option to set multiple properties). " + "Available only for microversion >= 2.95" + ), + ) return parser def take_action(self, parsed_args): @@ -369,11 +451,35 @@ class SetShareReplica(command.Command): {'status': parsed_args.status, 'exception': e}, ) - if not parsed_args.replica_state and not parsed_args.status: + if parsed_args.property: + if share_client.api_version < api_versions.APIVersion("2.95"): + raise exceptions.CommandError( + _( + "Setting properties in share replicas is available " + "only starting with API microversion '2.95'." + ) + ) + try: + replica.set_metadata(parsed_args.property) + except Exception as e: + LOG.error( + _( + "Failed to set share replica properties " + "'%(properties)s': %(exception)s" + ), + {'properties': parsed_args.property, 'exception': e}, + ) + result += 1 + + if ( + not parsed_args.replica_state + and not parsed_args.status + and not parsed_args.property + ): raise exceptions.CommandError( _( "Nothing to set. Please define " - "either '--replica_state' or '--status'." + "either '--replica_state' or '--status' or '--property'." ) ) if result > 0: @@ -382,6 +488,58 @@ class SetShareReplica(command.Command): ) +class UnsetShareReplica(command.Command): + """Unset a share replica property.""" + + _description = _("Unset a share replica property") + + def get_parser(self, prog_name): + parser = super().get_parser(prog_name) + parser.add_argument( + "replica", + metavar="", + help=_("Unset a property for"), + ) + parser.add_argument( + '--property', + metavar='', + action='append', + help=_( + 'Remove a property from replica ' + '(repeat option to remove multiple properties)' + ), + ) + return parser + + def take_action(self, parsed_args): + share_client = self.app.client_manager.share + + replica = osc_utils.find_resource( + share_client.share_replicas, parsed_args.replica + ) + + if parsed_args.property: + if share_client.api_version < api_versions.APIVersion("2.95"): + raise exceptions.CommandError( + _( + "arg '--property' is available only starting " + "with API microversion '2.95'." + ) + ) + for key in parsed_args.property: + try: + replica.delete_metadata([key]) + except Exception as e: + msg = _( + "Failed to unset replica property '%(key)s': %(e)s" + ) + raise exceptions.CommandError(msg % {'key': key, 'e': e}) + else: + raise exceptions.CommandError( + "Please specify '--property ' to unset a property." + ) + + class PromoteShareReplica(command.Command): """Promote share replica""" diff --git a/manilaclient/tests/unit/osc/v2/fakes.py b/manilaclient/tests/unit/osc/v2/fakes.py index 5e5cdd830..d8e0ad57a 100644 --- a/manilaclient/tests/unit/osc/v2/fakes.py +++ b/manilaclient/tests/unit/osc/v2/fakes.py @@ -821,6 +821,7 @@ class FakeShareReplica: 'share_server_id': None, 'status': None, 'updated_at': None, + "properties": format_columns.DictColumn({}), } share_replica.update(attrs) diff --git a/manilaclient/tests/unit/osc/v2/test_share_replicas.py b/manilaclient/tests/unit/osc/v2/test_share_replicas.py index d1592abd0..06372baa3 100644 --- a/manilaclient/tests/unit/osc/v2/test_share_replicas.py +++ b/manilaclient/tests/unit/osc/v2/test_share_replicas.py @@ -216,6 +216,61 @@ class TestShareReplicaCreate(TestShareReplica): self.assertCountEqual(self.columns, columns) self.assertCountEqual(self.data, data) + def test_share_replica_create_with_metadata(self): + self.app.client_manager.share.api_version = api_versions.APIVersion( + "2.95" + ) + + arglist = [ + self.share.id, + '--availability-zone', + self.share.availability_zone, + '--property', + 'fake_key1=fake_value1', + '--property', + 'fake_key2=fake_value2', + ] + verifylist = [ + ('share', self.share.id), + ('availability_zone', self.share.availability_zone), + ( + 'property', + {'fake_key1': 'fake_value1', 'fake_key2': 'fake_value2'}, + ), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.replicas_mock.create.assert_called_with( + share=self.share, + availability_zone=self.share.availability_zone, + metadata={'fake_key1': 'fake_value1', 'fake_key2': 'fake_value2'}, + ) + + self.assertCountEqual(self.columns, columns) + self.assertCountEqual(self.data, data) + + def test_share_replica_create_metadata_api_version_exception(self): + self.app.client_manager.share.api_version = api_versions.APIVersion( + '2.91' + ) + arglist = [ + self.share.id, + '--property', + 'fake_key=fake_value', + ] + verifylist = [ + ('share', self.share.id), + ('property', {'fake_key': 'fake_value'}), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + exceptions.CommandError, self.cmd.take_action, parsed_args + ) + def test_share_replica_create_wait(self): arglist = [self.share.id, '--wait'] verifylist = [('share', self.share.id), ('wait', True)] @@ -396,7 +451,7 @@ class TestShareReplicaList(TestShareReplica): columns, data = self.cmd.take_action(parsed_args) - self.replicas_mock.list.assert_called_with(share=None) + self.replicas_mock.list.assert_called_with(share=None, search_opts={}) self.assertEqual(self.column_headers, columns) self.assertEqual(list(self.values), list(data)) @@ -409,7 +464,9 @@ class TestShareReplicaList(TestShareReplica): columns, data = self.cmd.take_action(parsed_args) - self.replicas_mock.list.assert_called_with(share=self.share) + self.replicas_mock.list.assert_called_with( + share=self.share, search_opts={} + ) self.assertEqual(self.column_headers, columns) self.assertEqual(list(self.values), list(data)) @@ -469,7 +526,9 @@ class TestShareReplicaSet(TestShareReplica): def setUp(self): super().setUp() - self.share_replica = manila_fakes.FakeShareReplica.create_one_replica() + self.share_replica = manila_fakes.FakeShareReplica.create_one_replica( + methods={"set_metadata": None} + ) self.replicas_mock.get.return_value = self.share_replica self.cmd = osc_share_replicas.SetShareReplica(self.app, None) @@ -549,6 +608,115 @@ class TestShareReplicaSet(TestShareReplica): exceptions.CommandError, self.cmd.take_action, parsed_args ) + def test_share_replica_set_property(self): + self.app.client_manager.share.api_version = api_versions.APIVersion( + '2.95' + ) + arglist = [ + self.share_replica.id, + '--property', + 'fake_key=fake_value', + ] + verifylist = [ + ('replica', self.share_replica.id), + ('property', {'fake_key': 'fake_value'}), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + self.share_replica.set_metadata.assert_called_once_with( + {'fake_key': 'fake_value'}, + ) + + def test_share_replica_set_property_exception(self): + self.app.client_manager.share.api_version = api_versions.APIVersion( + '2.95' + ) + arglist = [ + self.share_replica.id, + '--property', + 'fake_key=fake_value', + ] + verifylist = [ + ('replica', self.share_replica.id), + ('property', {'fake_key': 'fake_value'}), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + self.share_replica.set_metadata.assert_called_once_with( + {'fake_key': 'fake_value'}, + ) + + self.share_replica.set_metadata.side_effect = exceptions.BadRequest + self.assertRaises( + exceptions.CommandError, self.cmd.take_action, parsed_args + ) + + +class TestShareReplicaUnset(TestShareReplica): + def setUp(self): + super().setUp() + + self.share_replica = manila_fakes.FakeShareReplica.create_one_replica( + methods={"delete_metadata": None} + ) + self.replicas_mock.get.return_value = self.share_replica + + self.cmd = osc_share_replicas.UnsetShareReplica(self.app, None) + + def test_share_replica_unset_property(self): + self.app.client_manager.share.api_version = api_versions.APIVersion( + '2.95' + ) + arglist = [ + '--property', + 'test_key', + self.share_replica.id, + ] + verifylist = [ + ('property', ['test_key']), + ('replica', self.share_replica.id), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + self.share_replica.delete_metadata.assert_called_once_with( + parsed_args.property + ) + + def test_share_replica_unset_property_exception(self): + self.app.client_manager.share.api_version = api_versions.APIVersion( + '2.95' + ) + arglist = [ + self.share_replica.id, + '--property', + 'test_key', + ] + verifylist = [ + ('replica', self.share_replica.id), + ('property', ['test_key']), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + + self.share_replica.delete_metadata.assert_called_once_with( + parsed_args.property + ) + + # 404 Not Found would be raised, if property 'test_key' doesn't exist. + self.share_replica.delete_metadata.side_effect = exceptions.NotFound + self.assertRaises( + exceptions.CommandError, self.cmd.take_action, parsed_args + ) + class TestShareReplicaPromote(TestShareReplica): def setUp(self): diff --git a/manilaclient/tests/unit/v2/test_share_replicas.py b/manilaclient/tests/unit/v2/test_share_replicas.py index 52ce16c59..f7174f36e 100644 --- a/manilaclient/tests/unit/v2/test_share_replicas.py +++ b/manilaclient/tests/unit/v2/test_share_replicas.py @@ -84,6 +84,29 @@ class ShareReplicasTest(utils.TestCase): self.assertEqual(share_replicas.RESOURCE_NAME, result['resp_key']) self.assertEqual(body_expected, result['body']) + @ddt.data("2.95") + def test_create_with_metadata(self, microversion): + api_version = api_versions.APIVersion(microversion) + values = { + 'availability_zone': 'az1', + 'share': 's1', + 'share_network': 'sn1', + 'metadata': {"fake_key": "fake_value"}, + } + + manager = share_replicas.ShareReplicaManager( + fakes.FakeClient(api_version=api_version) + ) + with mock.patch.object(manager, '_create', fakes.fake_create): + result = manager.create(**values) + + values['share_id'] = values.pop('share') + values['share_network_id'] = values.pop('share_network') + body_expected = {share_replicas.RESOURCE_NAME: values} + self.assertEqual(share_replicas.RESOURCES_PATH, result['url']) + self.assertEqual(share_replicas.RESOURCE_NAME, result['resp_key']) + self.assertEqual(body_expected, result['body']) + def test_delete_str(self): with mock.patch.object(self.manager, '_delete', mock.Mock()): self.manager.delete(FAKE_REPLICA) diff --git a/manilaclient/v2/share_replicas.py b/manilaclient/v2/share_replicas.py index 158c5fe2c..430d49218 100644 --- a/manilaclient/v2/share_replicas.py +++ b/manilaclient/v2/share_replicas.py @@ -24,7 +24,7 @@ RESOURCES_NAME = 'share_replicas' RESOURCE_NAME = 'share_replica' -class ShareReplica(base.Resource): +class ShareReplica(base.MetadataCapableResource): """A replica is 'mirror' instance of a share at some point in time.""" def __repr__(self): @@ -47,10 +47,11 @@ class ShareReplica(base.Resource): self.manager.reset_replica_state(self, replica_state) -class ShareReplicaManager(base.ManagerWithFind): +class ShareReplicaManager(base.MetadataCapableManager): """Manage :class:`ShareReplica` resources.""" resource_class = ShareReplica + resource_path = '/share-replicas' @api_versions.wraps("2.11", constants.REPLICA_PRE_GRADUATION_VERSION) @api_versions.experimental_api @@ -83,16 +84,26 @@ class ShareReplicaManager(base.ManagerWithFind): """List all share replicas or list replicas belonging to a share. :param share: either share object or its UUID. - :param search_opts: default None + :param search_opts: dict of search options (e.g., metadata filters) :rtype: list of :class:`ShareReplica` """ + search_opts = search_opts or {} + + # This will turn {'metadata': {'foo': 'bar', 'baz': 'qux'}} + # into ?metadata=foo:bar,baz:qux + query_string = self._build_query_string(search_opts) if share: share_id = '?share_id=' + base.getid(share) url = RESOURCES_PATH + '/detail' + share_id + if query_string: + url += '&' + query_string.lstrip('?') return self._list(url, RESOURCES_NAME) else: - return self._list(RESOURCES_PATH + '/detail', RESOURCES_NAME) + url = RESOURCES_PATH + '/detail' + if query_string: + url += query_string + return self._list(url, RESOURCES_NAME) @api_versions.wraps("2.11", constants.REPLICA_PRE_GRADUATION_VERSION) @api_versions.experimental_api @@ -144,7 +155,7 @@ class ShareReplicaManager(base.ManagerWithFind): scheduler_hints=scheduler_hints, ) - @api_versions.wraps("2.72") # noqa + @api_versions.wraps("2.72", "2.94") # noqa def create( # noqa self, share, # pylint: disable=function-redefined # noqa F811 @@ -159,12 +170,30 @@ class ShareReplicaManager(base.ManagerWithFind): share_network=share_network, ) + @api_versions.wraps("2.95") # noqa + def create( # noqa + self, + share, # pylint: disable=function-redefined # noqa F811 + availability_zone=None, + scheduler_hints=None, + share_network=None, + metadata=None, + ): + return self._create_share_replica( + share, + availability_zone=availability_zone, + scheduler_hints=scheduler_hints, + share_network=share_network, + metadata=metadata, + ) + def _create_share_replica( self, share, availability_zone=None, scheduler_hints=None, share_network=None, + metadata=None, ): """Create a replica for a share. @@ -174,11 +203,13 @@ class ShareReplicaManager(base.ManagerWithFind): :param scheduler_hints: The scheduler_hints as key=value pair. Only supported key is 'only_host'. :param share_network: either share network object or its UUID. + :param metadata: dict - optional metadata to set on share replica + creation """ share_id = base.getid(share) - body = {'share_id': share_id} + body = {'share_id': share_id} if availability_zone: body['availability_zone'] = base.getid(availability_zone) @@ -188,6 +219,9 @@ class ShareReplicaManager(base.ManagerWithFind): if share_network: body['share_network_id'] = base.getid(share_network) + if metadata: + body['metadata'] = metadata + return self._create( RESOURCES_PATH, {RESOURCE_NAME: body}, RESOURCE_NAME ) diff --git a/pyproject.toml b/pyproject.toml index a8f81b040..99dd0cb37 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,6 +108,7 @@ share_replica_delete = "manilaclient.osc.v2.share_replicas:DeleteShareReplica" share_replica_list = "manilaclient.osc.v2.share_replicas:ListShareReplica" share_replica_show = "manilaclient.osc.v2.share_replicas:ShowShareReplica" share_replica_set = "manilaclient.osc.v2.share_replicas:SetShareReplica" +share_replica_unset = "manilaclient.osc.v2.share_replicas:UnsetShareReplica" share_replica_promote = "manilaclient.osc.v2.share_replicas:PromoteShareReplica" share_replica_resync = "manilaclient.osc.v2.share_replicas:ResyncShareReplica" share_replica_export_location_list = "manilaclient.osc.v2.share_replica_export_locations:ShareReplicaListExportLocation" diff --git a/releasenotes/notes/add-share-replica-metadata-e6c1c2e00295d2dd.yaml b/releasenotes/notes/add-share-replica-metadata-e6c1c2e00295d2dd.yaml new file mode 100644 index 000000000..e06e41df2 --- /dev/null +++ b/releasenotes/notes/add-share-replica-metadata-e6c1c2e00295d2dd.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds support to create/update/delete replica metadata (properties) + with set and unset command.