From: Jakob Meng <c...@jakobmeng.de> This patch introduces support for different output formats to the Python code, as did the previous commit for ovs-xxx tools like 'ovs-appctl --format json dpif/show'. In particular, tests/appctl.py gains a global option '-f,--format' which allows users to request JSON instead of plain-text for humans.
This patch does not yet pass the choosen output format to commands. Doing so requires changes to all command_register() calls and all command callbacks. To improve readibility those changes have been split out into a follow up patch. Respectively, whenever an output format other than 'text' is choosen for tests/appctl.py, the script will fail. Reported-at: https://bugzilla.redhat.com/1824861 Signed-off-by: Jakob Meng <c...@jakobmeng.de> --- NEWS | 2 + python/ovs/unixctl/__init__.py | 39 +++++++++++++++--- python/ovs/unixctl/client.py | 20 +++++++-- python/ovs/unixctl/server.py | 74 +++++++++++++++++++++++----------- python/ovs/util.py | 9 +++++ tests/appctl.py | 7 +++- tests/unixctl-py.at | 7 ++++ 7 files changed, 126 insertions(+), 32 deletions(-) diff --git a/NEWS b/NEWS index 12a895629..fc77a1613 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,8 @@ Post-v3.2.0 Reported names adjusted accordingly. * Added new option [-f|--format] to choose the output format, e.g. 'json' or 'text' (by default). + - Python: + * Added support for choosing the output format, e.g. 'json' or 'text'. v3.2.0 - 17 Aug 2023 diff --git a/python/ovs/unixctl/__init__.py b/python/ovs/unixctl/__init__.py index 8ee312943..6d9bd5715 100644 --- a/python/ovs/unixctl/__init__.py +++ b/python/ovs/unixctl/__init__.py @@ -20,10 +20,14 @@ commands = {} class _UnixctlCommand(object): - def __init__(self, usage, min_args, max_args, callback, aux): + # FIXME: Output format will be passed as 'output_fmts' to the command in + # later patch. + def __init__(self, usage, min_args, max_args, # FIXME: output_fmts, + callback, aux): self.usage = usage self.min_args = min_args self.max_args = max_args + # FIXME: self.output_fmts = output_fmts self.callback = callback self.aux = aux @@ -42,10 +46,13 @@ def _unixctl_help(conn, unused_argv, unused_aux): conn.reply(reply) -def command_register(name, usage, min_args, max_args, callback, aux): +def command_register_fmt(name, usage, min_args, max_args, output_fmts, + callback, aux): """ Registers a command with the given 'name' to be exposed by the UnixctlServer. 'usage' describes the arguments to the command; it is used - only for presentation to the user in "help" output. + only for presentation to the user in "help" output. 'output_fmts' is a + bitmap that defines what output formats a command supports, e.g. + OVS_OUTPUT_FMT_TEXT | OVS_OUTPUT_FMT_JSON. 'callback' is called when the command is received. It is passed a UnixctlConnection object, the list of arguments as unicode strings, and @@ -63,8 +70,30 @@ def command_register(name, usage, min_args, max_args, callback, aux): assert callable(callback) if name not in commands: - commands[name] = _UnixctlCommand(usage, min_args, max_args, callback, - aux) + commands[name] = _UnixctlCommand(usage, min_args, max_args, + # FIXME: output_fmts, + callback, aux) + + +# FIXME: command_register() will be replaced with command_register_fmt() in a +# later patch of this series. It is is kept temporarily to reduce the +# amount of changes in this patch. +def command_register(name, usage, min_args, max_args, callback, aux): + """ Registers a command with the given 'name' to be exposed by the + UnixctlServer. 'usage' describes the arguments to the command; it is used + only for presentation to the user in "help" output. + + 'callback' is called when the command is received. It is passed a + UnixctlConnection object, the list of arguments as unicode strings, and + 'aux'. Normally 'callback' should reply by calling + UnixctlConnection.reply() or UnixctlConnection.reply_error() before it + returns, but if the command cannot be handled immediately, then it can + defer the reply until later. A given connection can only process a single + request at a time, so a reply must be made eventually to avoid blocking + that connection.""" + + command_register_fmt(name, usage, min_args, max_args, + ovs.util.OutputFormat.TEXT, callback, aux) def socket_name_from_target(target): diff --git a/python/ovs/unixctl/client.py b/python/ovs/unixctl/client.py index 8283f99bb..29a5f3df9 100644 --- a/python/ovs/unixctl/client.py +++ b/python/ovs/unixctl/client.py @@ -26,13 +26,20 @@ class UnixctlClient(object): assert isinstance(conn, ovs.jsonrpc.Connection) self._conn = conn - def transact(self, command, argv): + def transact(self, command, argv, fmt): assert isinstance(command, str) assert isinstance(argv, list) for arg in argv: assert isinstance(arg, str) + assert isinstance(fmt, ovs.util.OutputFormat) + plain_rpc = (fmt == ovs.util.OutputFormat.TEXT) + + if plain_rpc: + request = ovs.jsonrpc.Message.create_request(command, argv) + else: + request = ovs.jsonrpc.Message.create_request( + "execute/v1", [command, fmt.name.lower()] + argv) - request = ovs.jsonrpc.Message.create_request(command, argv) error, reply = self._conn.transact_block(request) if error: @@ -41,7 +48,14 @@ class UnixctlClient(object): return error, None, None if reply.error is not None: - return 0, str(reply.error), None + plain_rpc_error = ('"%s" is not a valid command' % + ovs.util.RPC_MARKER) + if str(reply.error).startswith(plain_rpc_error): + return 0, "JSON RPC reply indicates incompatible "\ + "server. Please upgrade server side to "\ + "newer version.", None + else: + return 0, str(reply.error), None else: assert reply.result is not None return 0, None, str(reply.result) diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py index b9cb52fad..5e92bc570 100644 --- a/python/ovs/unixctl/server.py +++ b/python/ovs/unixctl/server.py @@ -104,30 +104,57 @@ class UnixctlConnection(object): self._request_id = request.id - error = None - params = request.params - method = request.method - command = ovs.unixctl.commands.get(method) - if command is None: - error = '"%s" is not a valid command' % method - elif len(params) < command.min_args: - error = '"%s" command requires at least %d arguments' \ - % (method, command.min_args) - elif len(params) > command.max_args: - error = '"%s" command takes at most %d arguments' \ - % (method, command.max_args) - else: - for param in params: - if not isinstance(param, str): - error = '"%s" command has non-string argument' % method - break + try: + plain_rpc = request.method != ovs.util.RPC_MARKER + args_offset = 0 if plain_rpc else 2 - if error is None: - unicode_params = [str(p) for p in params] - command.callback(self, unicode_params, command.aux) + if not plain_rpc and len(request.params) < 2: + raise ValueError( + "JSON-RPC API mismatch: Unexpected # of params: %d" + % len(request.params)) - if error: - self.reply_error(error) + for param in request.params: + if not isinstance(param, str): + raise ValueError( + "command has non-string argument: %s" % param) + + method = request.method if plain_rpc else request.params[0] + if plain_rpc: + fmt = ovs.util.OutputFormat.TEXT + else: + fmt = ovs.util.OutputFormat[request.params[1].upper()] + params = request.params[args_offset:] + + command = ovs.unixctl.commands.get(method) + if command is None: + raise ValueError('"%s" is not a valid command' % method) + elif len(params) < command.min_args: + raise ValueError( + '"%s" command requires at least %d arguments' + % (method, command.min_args)) + elif len(params) > command.max_args: + raise ValueError( + '"%s" command takes at most %d arguments' + % (method, command.max_args)) + # FIXME: Uncomment when command.output_fmts is available + # elif fmt not in command.output_fmts: + # raise ValueError('"%s" command does not support output format' + # ' "%s" %d %d' % (method, fmt.name.lower(), + # command.output_fmts, fmt)) + + # FIXME: Remove this check once output format will be passed to the + # command handler below. + if fmt != ovs.util.OutputFormat.TEXT: + raise ValueError( + 'output format "%s" has not been implemented yet' + % fmt.name.lower()) + + unicode_params = [str(p) for p in params] + command.callback(self, unicode_params, # FIXME: fmt, + command.aux) + + except Exception as e: + self.reply_error(str(e)) def _unixctl_version(conn, unused_argv, version): @@ -207,7 +234,8 @@ class UnixctlServer(object): % path) return error, None - ovs.unixctl.command_register("version", "", 0, 0, _unixctl_version, + ovs.unixctl.command_register("version", "", 0, 0, + _unixctl_version, version) return 0, UnixctlServer(listener) diff --git a/python/ovs/util.py b/python/ovs/util.py index 3dba022f8..75ba4ee12 100644 --- a/python/ovs/util.py +++ b/python/ovs/util.py @@ -15,9 +15,18 @@ import os import os.path import sys +import enum PROGRAM_NAME = os.path.basename(sys.argv[0]) EOF = -1 +RPC_MARKER = "execute/v1" + + +@enum.unique +# FIXME: Use @enum.verify(enum.NAMED_FLAGS) from Python 3.11 when available. +class OutputFormat(enum.IntFlag): + TEXT = 1 << 0 + JSON = 1 << 1 def abs_file_name(dir_, file_name): diff --git a/tests/appctl.py b/tests/appctl.py index b85b364fa..42f96b2a3 100644 --- a/tests/appctl.py +++ b/tests/appctl.py @@ -49,6 +49,10 @@ def main(): help="Arguments to the command.") parser.add_argument("-T", "--timeout", metavar="SECS", help="wait at most SECS seconds for a response") + parser.add_argument("-f", "--format", metavar="FMT", + help="Output format.", default="text", + choices=[fmt.name.lower() + for fmt in ovs.util.OutputFormat]) args = parser.parse_args() signal_alarm(int(args.timeout) if args.timeout else None) @@ -56,7 +60,8 @@ def main(): ovs.vlog.Vlog.init() target = args.target client = connect_to_target(target) - err_no, error, result = client.transact(args.command, args.argv) + err_no, error, result = client.transact( + args.command, args.argv, ovs.util.OutputFormat[args.format.upper()]) client.close() if err_no: diff --git a/tests/unixctl-py.at b/tests/unixctl-py.at index 724006118..c149268af 100644 --- a/tests/unixctl-py.at +++ b/tests/unixctl-py.at @@ -65,6 +65,13 @@ AT_CHECK([head -1 stderr], [0], [dnl sed 's/ovs-appctl/appctl.py/' stderr > experr AT_CHECK([PYAPPCTL_PY bond/hash mac vlan basis extra], [2], [], [experr]) +AT_CHECK([APPCTL --format json dpif-netdev/pmd-rxq-show], [2], [], [stderr]) +AT_CHECK([head -1 stderr], [0], [dnl +"dpif-netdev/pmd-rxq-show" command does not support output format "json" 1 2 +]) +sed 's/ovs-appctl/appctl.py/' stderr > experr +AT_CHECK([PYAPPCTL_PY --format json dpif-netdev/pmd-rxq-show], [2], [], [experr]) + OVS_VSWITCHD_STOP AT_CLEANUP -- 2.39.2 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev