diff --git a/doc/source/command-objects/port.rst b/doc/source/command-objects/port.rst index e4cf2cd212..880161f971 100644 --- a/doc/source/command-objects/port.rst +++ b/doc/source/command-objects/port.rst @@ -54,7 +54,8 @@ Create new port .. option:: --binding-profile - Custom data to be passed as binding:profile: = + Custom data to be passed as binding:profile. Data may + be passed as = or JSON. (repeat option to set multiple binding:profile data) .. option:: --host @@ -162,7 +163,8 @@ Set port properties .. option:: --binding-profile - Custom data to be passed as binding:profile: = + Custom data to be passed as binding:profile. Data may + be passed as = or JSON. (repeat option to set multiple binding:profile data) .. option:: --no-binding-profile diff --git a/openstackclient/network/v2/port.py b/openstackclient/network/v2/port.py index 1c5db706e3..a25bdf8d71 100644 --- a/openstackclient/network/v2/port.py +++ b/openstackclient/network/v2/port.py @@ -14,6 +14,7 @@ """Port action implementations""" import argparse +import json import logging from osc_lib.cli import parseractions @@ -63,6 +64,32 @@ def _get_columns(item): return tuple(sorted(columns)) +class JSONKeyValueAction(argparse.Action): + """A custom action to parse arguments as JSON or key=value pairs + + Ensures that ``dest`` is a dict + """ + + def __call__(self, parser, namespace, values, option_string=None): + + # Make sure we have an empty dict rather than None + if getattr(namespace, self.dest, None) is None: + setattr(namespace, self.dest, {}) + + # Try to load JSON first before falling back to =. + current_dest = getattr(namespace, self.dest) + try: + current_dest.update(json.loads(values)) + except ValueError as e: + if '=' in values: + current_dest.update([values.split('=', 1)]) + else: + msg = _("Expected '=' or JSON data for option " + "%(option)s, but encountered JSON parsing error: " + "%(error)s") % {"option": option_string, "error": e} + raise argparse.ArgumentTypeError(msg) + + def _get_attrs(client_manager, parsed_args): attrs = {} @@ -219,9 +246,9 @@ class CreatePort(command.ShowOne): parser.add_argument( '--binding-profile', metavar='', - action=parseractions.KeyValueAction, - help=_("Custom data to be passed as binding:profile: " - "= " + action=JSONKeyValueAction, + help=_("Custom data to be passed as binding:profile. Data may " + "be passed as = or JSON. " "(repeat option to set multiple binding:profile data)") ) admin_group = parser.add_mutually_exclusive_group() @@ -390,9 +417,9 @@ class SetPort(command.Command): binding_profile.add_argument( '--binding-profile', metavar='', - action=parseractions.KeyValueAction, - help=_("Custom data to be passed as binding:profile: " - "= " + action=JSONKeyValueAction, + help=_("Custom data to be passed as binding:profile. Data may " + "be passed as = or JSON. " "(repeat option to set multiple binding:profile data)") ) binding_profile.add_argument( diff --git a/openstackclient/tests/network/v2/test_port.py b/openstackclient/tests/network/v2/test_port.py index 779dca0519..a998585e4b 100644 --- a/openstackclient/tests/network/v2/test_port.py +++ b/openstackclient/tests/network/v2/test_port.py @@ -11,6 +11,7 @@ # under the License. # +import argparse import mock from mock import call @@ -174,6 +175,58 @@ class TestCreatePort(TestPort): self.assertEqual(ref_columns, columns) self.assertEqual(ref_data, data) + def test_create_invalid_json_binding_profile(self): + arglist = [ + '--network', self._port.network_id, + '--binding-profile', '{"parent_name":"fake_parent"', + 'test-port', + ] + self.assertRaises(argparse.ArgumentTypeError, + self.check_parser, + self.cmd, + arglist, + None) + + def test_create_invalid_key_value_binding_profile(self): + arglist = [ + '--network', self._port.network_id, + '--binding-profile', 'key', + 'test-port', + ] + self.assertRaises(argparse.ArgumentTypeError, + self.check_parser, + self.cmd, + arglist, + None) + + def test_create_json_binding_profile(self): + arglist = [ + '--network', self._port.network_id, + '--binding-profile', '{"parent_name":"fake_parent"}', + '--binding-profile', '{"tag":42}', + 'test-port', + ] + verifylist = [ + ('network', self._port.network_id,), + ('enable', True), + ('binding_profile', {'parent_name': 'fake_parent', 'tag': 42}), + ('name', 'test-port'), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = (self.cmd.take_action(parsed_args)) + + self.network.create_port.assert_called_once_with(**{ + 'admin_state_up': True, + 'network_id': self._port.network_id, + 'binding:profile': {'parent_name': 'fake_parent', 'tag': 42}, + 'name': 'test-port', + }) + + ref_columns, ref_data = self._get_common_cols_data(self._port) + self.assertEqual(ref_columns, columns) + self.assertEqual(ref_data, data) + class TestDeletePort(TestPort): @@ -442,6 +495,48 @@ class TestSetPort(TestPort): self.network.update_port.assert_called_once_with(self._port, **attrs) self.assertIsNone(result) + def test_set_invalid_json_binding_profile(self): + arglist = [ + '--binding-profile', '{"parent_name"}', + 'test-port', + ] + self.assertRaises(argparse.ArgumentTypeError, + self.check_parser, + self.cmd, + arglist, + None) + + def test_set_invalid_key_value_binding_profile(self): + arglist = [ + '--binding-profile', 'key', + 'test-port', + ] + self.assertRaises(argparse.ArgumentTypeError, + self.check_parser, + self.cmd, + arglist, + None) + + def test_set_mixed_binding_profile(self): + arglist = [ + '--binding-profile', 'foo=bar', + '--binding-profile', '{"foo2": "bar2"}', + self._port.name, + ] + verifylist = [ + ('binding_profile', {'foo': 'bar', 'foo2': 'bar2'}), + ('port', self._port.name), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + result = self.cmd.take_action(parsed_args) + + attrs = { + 'binding:profile': {'foo': 'bar', 'foo2': 'bar2'}, + } + self.network.update_port.assert_called_once_with(self._port, **attrs) + self.assertIsNone(result) + class TestShowPort(TestPort): diff --git a/releasenotes/notes/bp-neutron-client-a0552f8ca909b665.yaml b/releasenotes/notes/bp-neutron-client-a0552f8ca909b665.yaml index f8de4ee6ae..8402e2c9c3 100644 --- a/releasenotes/notes/bp-neutron-client-a0552f8ca909b665.yaml +++ b/releasenotes/notes/bp-neutron-client-a0552f8ca909b665.yaml @@ -1,5 +1,9 @@ --- features: + - Update ``--binding-profile`` option on the ``port create`` and + ``port set`` commands to support JSON input for more advanced + binding profile data. + [Blueprint :oscbp:`neutron-client`] - Add ``geneve`` choice to the ``network create`` command ``--provider-network-type`` option. [Blueprint :oscbp:`neutron-client`]