From b153e667907ed68164a15207ad5f91ad4c39927c Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Mon, 26 Sep 2011 16:02:26 -0700 Subject: [PATCH] python: Upgrade daemon module to argparse. This patch also updates it's callers. --- debian/ovs-monitor-ipsec | 52 +++++----------- python/ovs/daemon.py | 51 ++++++++++------ tests/daemon-py.at | 18 +++--- tests/jsonrpc-py.at | 6 +- tests/ovs-monitor-ipsec.at | 2 +- tests/ovs-xapi-sync.at | 2 +- tests/test-daemon.py | 44 ++++---------- tests/test-jsonrpc.py | 60 ++++++++----------- ...sr_share_openvswitch_scripts_ovs-xapi-sync | 56 ++++++----------- 9 files changed, 118 insertions(+), 173 deletions(-) mode change 100755 => 100644 debian/ovs-monitor-ipsec diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec old mode 100755 new mode 100644 index 5aca0f7c..0c1d6a87 --- a/debian/ovs-monitor-ipsec +++ b/debian/ovs-monitor-ipsec @@ -25,7 +25,7 @@ # adding an interface to racoon.conf. -import getopt +import argparse import glob import logging import logging.handlers @@ -405,17 +405,6 @@ def prune_schema(schema): schema.tables = new_tables -def usage(): - print "usage: %s [OPTIONS] DATABASE" % sys.argv[0] - print "where DATABASE is a socket on which ovsdb-server is listening." - ovs.daemon.usage() - print """\ -Other options: - --root-prefix=DIR Use DIR as alternate root directory (for testing). - -h, --help Display this help message.""" - sys.exit(0) - - def update_ipsec(ipsec, interfaces, new_interfaces): for name, vals in interfaces.iteritems(): if name not in new_interfaces: @@ -446,32 +435,23 @@ def get_ssl_cert(data): return None -def main(argv): - try: - options, args = getopt.gnu_getopt( - argv[1:], 'h', ['help', 'root-prefix='] + ovs.daemon.LONG_OPTIONS) - except getopt.GetoptError, geo: - sys.stderr.write("%s: %s\n" % (ovs.util.PROGRAM_NAME, geo.msg)) - sys.exit(1) - - for key, value in options: - if key in ['-h', '--help']: - usage() - elif key == "--root-prefix": - global root_prefix - root_prefix = value - elif not ovs.daemon.parse_opt(key, value): - sys.stderr.write("%s: unhandled option %s\n" - % (ovs.util.PROGRAM_NAME, key)) - sys.exit(1) +def main(): + + parser = argparse.ArgumentParser() + parser.add_argument("database", metavar="DATABASE", + help="A socket on which ovsdb-server is listening.") + parser.add_argument("--root-prefix", metavar="DIR", + help="Use DIR as alternate root directory" + " (for testing).") - if len(args) != 1: - sys.stderr.write("%s: exactly one nonoption argument is required " - "(use --help for help)\n" % ovs.util.PROGRAM_NAME) - sys.exit(1) + ovs.daemon.add_args(parser) + args = parser.parse_args() + ovs.daemon.handle_args(args) - remote = args[0] + global root_prefix + root_prefix = args.root_prefix + remote = args.database schema_file = "%s/vswitch.ovsschema" % ovs.dirs.PKGDATADIR schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schema_file)) prune_schema(schema) @@ -533,7 +513,7 @@ def main(argv): if __name__ == '__main__': try: - main(sys.argv) + main() except SystemExit: # Let system.exit() calls complete normally raise diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py index 184e7832..864a1633 100644 --- a/python/ovs/daemon.py +++ b/python/ovs/daemon.py @@ -487,26 +487,43 @@ def _check_already_running(): _fatal("%s: pidfile check failed (%s), aborting" % (_pidfile, os.strerror(pid))) -# XXX Python's getopt does not support options with optional arguments, so we -# have to separate --pidfile (with no argument) from --pidfile-name (with an -# argument). Need to write our own getopt I guess. -LONG_OPTIONS = ["detach", "no-chdir", "pidfile", "pidfile-name=", - "overwrite-pidfile", "monitor"] +def add_args(parser): + """Populates 'parser', an ArgumentParser allocated using the argparse + module, with the command line arguments required by the daemon module.""" -def parse_opt(option, arg): - if option == '--detach': + pidfile = make_pidfile_name(None) + + group = parser.add_argument_group(title="Daemon Options") + group.add_argument("--detach", action="store_true", + help="Run in background as a daemon.") + group.add_argument("--no-chdir", action="store_true", + help="Do not chdir to '/'.") + group.add_argument("--monitor", action="store_true", + help="Monitor %s process." % ovs.util.PROGRAM_NAME) + group.add_argument("--pidfile", nargs="?", default=pidfile, + help="Create pidfile (default %s)." % pidfile) + group.add_argument("--overwrite-pidfile", action="store_true", + help="With --pidfile, start even if already running.") + + +def handle_args(args): + """Handles daemon module settings in 'args'. 'args' is an object + containing values parsed by the parse_args() method of ArgumentParser. The + parent ArgumentParser should have been prepared by add_args() before + calling parse_args().""" + + if args.detach: set_detach() - elif option == '--no-chdir': + + if args.no_chdir: set_no_chdir() - elif option == '--pidfile': - set_pidfile(None) - elif option == '--pidfile-name': - set_pidfile(arg) - elif option == '--overwrite-pidfile': + + if args.pidfile: + set_pidfile(args.pidfile) + + if args.overwrite_pidfile: ignore_existing_pidfile() - elif option == '--monitor': + + if args.monitor: set_monitor() - else: - return False - return True diff --git a/tests/daemon-py.at b/tests/daemon-py.at index bc5a0511..39ab3ccd 100644 --- a/tests/daemon-py.at +++ b/tests/daemon-py.at @@ -6,7 +6,7 @@ AT_CAPTURE_FILE([pid]) AT_CAPTURE_FILE([expected]) # Start the daemon and wait for the pidfile to get created # and that its contents are the correct pid. -AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid& echo $! > expected], [0]) +AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid& echo $! > expected], [0]) OVS_WAIT_UNTIL([test -s pid], [kill `cat expected`]) AT_CHECK( [pid=`cat pid` && expected=`cat expected` && test "$pid" = "$expected"], @@ -25,7 +25,7 @@ AT_CAPTURE_FILE([parent]) AT_CAPTURE_FILE([parentpid]) AT_CAPTURE_FILE([newpid]) # Start the daemon and wait for the pidfile to get created. -AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid --monitor& echo $! > parent], [0]) +AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid --monitor& echo $! > parent], [0]) OVS_WAIT_UNTIL([test -s pid], [kill `cat parent`]) # Check that the pidfile names a running process, # and that the parent process of that process is our child process. @@ -68,7 +68,7 @@ AT_CAPTURE_FILE([parent]) AT_CAPTURE_FILE([parentpid]) AT_CAPTURE_FILE([newpid]) # Start the daemon and wait for the pidfile to get created. -AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid --monitor& echo $! > parent], [0]) +AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid --monitor& echo $! > parent], [0]) OVS_WAIT_UNTIL([test -s pid], [kill `cat parent`]) # Check that the pidfile names a running process, # and that the parent process of that process is our child process. @@ -110,7 +110,7 @@ AT_CAPTURE_FILE([pid]) # Start the daemon and make sure that the pidfile exists immediately. # We don't wait for the pidfile to get created because the daemon is # supposed to do so before the parent exits. -AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid --detach], [0]) +AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid --detach], [0]) AT_CHECK([test -s pid]) AT_CHECK([kill -0 `cat pid`]) # Kill the daemon and make sure that the pidfile gets deleted. @@ -133,7 +133,7 @@ AT_CAPTURE_FILE([init]) # Start the daemon and make sure that the pidfile exists immediately. # We don't wait for the pidfile to get created because the daemon is # supposed to do so before the parent exits. -AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/daemon --detach --monitor], [0]) +AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/daemon --detach --monitor], [0]) AT_CHECK([test -s daemon]) # Check that the pidfile names a running process, # and that the parent process of that process is a running process, @@ -172,7 +172,7 @@ AT_CLEANUP AT_SETUP([daemon --detach startup errors - Python]) AT_SKIP_IF([test $HAVE_PYTHON = no]) AT_CAPTURE_FILE([pid]) -AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid --detach --bail], [1], [], [stderr]) +AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid --detach --bail], [1], [], [stderr]) AT_CHECK([grep 'test-daemon.py: exiting after daemonize_start() as requested' stderr], [0], [ignore], []) AT_CHECK([test ! -s pid]) @@ -181,7 +181,7 @@ AT_CLEANUP AT_SETUP([daemon --detach --monitor startup errors - Python]) AT_SKIP_IF([test $HAVE_PYTHON = no]) AT_CAPTURE_FILE([pid]) -AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid --detach --monitor --bail], [1], [], [stderr]) +AT_CHECK([$PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid --detach --monitor --bail], [1], [], [stderr]) AT_CHECK([grep 'test-daemon.py: exiting after daemonize_start() as requested' stderr], [0], [ignore], []) AT_CHECK([test ! -s pid]) @@ -192,7 +192,7 @@ AT_SKIP_IF([test $HAVE_PYTHON = no]) AT_CAPTURE_FILE([pid]) AT_CAPTURE_FILE([status]) AT_CAPTURE_FILE([stderr]) -AT_CHECK([(yes 2>stderr; echo $? > status) | $PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid --detach], [0], [], []) +AT_CHECK([(yes 2>stderr; echo $? > status) | $PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid --detach], [0], [], []) AT_CHECK([kill `cat pid`]) AT_CHECK([test -s status]) if grep '[[bB]]roken pipe' stderr >/dev/null 2>&1; then @@ -212,7 +212,7 @@ AT_CAPTURE_FILE([pid]) AT_CAPTURE_FILE([status]) AT_CAPTURE_FILE([stderr]) OVSDB_INIT([db]) -AT_CHECK([(yes 2>stderr; echo $? > status) | $PYTHON $srcdir/test-daemon.py --pidfile-name=$PWD/pid --detach], [0], [], []) +AT_CHECK([(yes 2>stderr; echo $? > status) | $PYTHON $srcdir/test-daemon.py --pidfile=`pwd`/pid --detach], [0], [], []) AT_CHECK([kill `cat pid`]) AT_CHECK([test -s status]) if grep '[[bB]]roken pipe' stderr >/dev/null 2>&1; then diff --git a/tests/jsonrpc-py.at b/tests/jsonrpc-py.at index cda34ab5..3d3bd3b1 100644 --- a/tests/jsonrpc-py.at +++ b/tests/jsonrpc-py.at @@ -2,7 +2,7 @@ AT_BANNER([JSON-RPC - Python]) AT_SETUP([JSON-RPC request and successful reply - Python]) AT_SKIP_IF([test $HAVE_PYTHON = no]) -AT_CHECK([$PYTHON $srcdir/test-jsonrpc.py --detach --pidfile-name=$PWD/pid listen punix:socket]) +AT_CHECK([$PYTHON $srcdir/test-jsonrpc.py --detach --pidfile=`pwd`/pid listen punix:socket]) AT_CHECK([test -s pid]) AT_CHECK([kill -0 `cat pid`]) AT_CHECK( @@ -14,7 +14,7 @@ AT_CLEANUP AT_SETUP([JSON-RPC request and error reply - Python]) AT_SKIP_IF([test $HAVE_PYTHON = no]) -AT_CHECK([$PYTHON $srcdir/test-jsonrpc.py --detach --pidfile-name=$PWD/pid listen punix:socket]) +AT_CHECK([$PYTHON $srcdir/test-jsonrpc.py --detach --pidfile=`pwd`/pid listen punix:socket]) AT_CHECK([test -s pid]) AT_CHECK([kill -0 `cat pid`]) AT_CHECK( @@ -26,7 +26,7 @@ AT_CLEANUP AT_SETUP([JSON-RPC notification - Python]) AT_SKIP_IF([test $HAVE_PYTHON = no]) -AT_CHECK([$PYTHON $srcdir/test-jsonrpc.py --detach --pidfile-name=$PWD/pid listen punix:socket]) +AT_CHECK([$PYTHON $srcdir/test-jsonrpc.py --detach --pidfile=`pwd`/pid listen punix:socket]) AT_CHECK([test -s pid]) # When a daemon dies it deletes its pidfile, so make a copy. AT_CHECK([cp pid pid2]) diff --git a/tests/ovs-monitor-ipsec.at b/tests/ovs-monitor-ipsec.at index ad1e96e7..f2794e03 100644 --- a/tests/ovs-monitor-ipsec.at +++ b/tests/ovs-monitor-ipsec.at @@ -47,7 +47,7 @@ OVS_VSCTL_SETUP ### AT_CHECK( [$PYTHON $top_srcdir/debian/ovs-monitor-ipsec "--root-prefix=`pwd`" \ - "--pidfile-name=`pwd`/ovs-monitor-ipsec.pid" \ + "--pidfile=`pwd`/ovs-monitor-ipsec.pid" \ unix:socket 2>log 3>actions &]) AT_CAPTURE_FILE([log]) AT_CAPTURE_FILE([actions]) diff --git a/tests/ovs-xapi-sync.at b/tests/ovs-xapi-sync.at index b2bfff56..7c467bb8 100644 --- a/tests/ovs-xapi-sync.at +++ b/tests/ovs-xapi-sync.at @@ -27,7 +27,7 @@ ovs_vsctl () { OVS_VSCTL_SETUP # Start ovs-xapi-sync. -AT_CHECK([$PYTHON ./ovs-xapi-sync "--pidfile-name=`pwd`/ovs-xapi-sync.pid" \ +AT_CHECK([$PYTHON ./ovs-xapi-sync "--pidfile=`pwd`/ovs-xapi-sync.pid" \ "--root-prefix=`pwd`" unix:socket >log 2>&1 &]) AT_CAPTURE_FILE([log]) diff --git a/tests/test-daemon.py b/tests/test-daemon.py index 816304fa..a15068dd 100644 --- a/tests/test-daemon.py +++ b/tests/test-daemon.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -import getopt +import argparse import logging import signal import sys @@ -26,31 +26,22 @@ def handler(signum, _): raise Exception("Signal handler called with %d" % signum) -def main(argv): +def main(): logging.basicConfig(level=logging.DEBUG) signal.signal(signal.SIGHUP, handler) - try: - options, args = getopt.gnu_getopt( - argv[1:], 'b', ["bail", "help"] + ovs.daemon.LONG_OPTIONS) - except getopt.GetoptError, geo: - sys.stderr.write("%s: %s\n" % (ovs.util.PROGRAM_NAME, geo.msg)) - sys.exit(1) + parser = argparse.ArgumentParser( + description="Open vSwitch daemonization test program for Python.") + parser.add_argument("-b", "--bail", action="store_true", + help="Exit with an error after daemonize_start().") - bail = False - for key, value in options: - if key == '--help': - usage() - elif key in ['-b', '--bail']: - bail = True - elif not ovs.daemon.parse_opt(key, value): - sys.stderr.write("%s: unhandled option %s\n" - % (ovs.util.PROGRAM_NAME, key)) - sys.exit(1) + ovs.daemon.add_args(parser) + args = parser.parse_args() + ovs.daemon.handle_args(args) ovs.daemon.daemonize_start() - if bail: + if args.bail: sys.stderr.write("%s: exiting after daemonize_start() as requested\n" % ovs.util.PROGRAM_NAME) sys.exit(1) @@ -60,22 +51,9 @@ def main(argv): time.sleep(1) -def usage(): - sys.stdout.write("""\ -%s: Open vSwitch daemonization test program for Python -usage: %s [OPTIONS] -""" % ovs.util.PROGRAM_NAME) - ovs.daemon.usage() - sys.stdout.write(""" -Other options: - -h, --help display this help message - -b, --bail exit with an error after daemonize_start() -""") - sys.exit(0) - if __name__ == '__main__': try: - main(sys.argv) + main() except SystemExit: # Let system.exit() calls complete normally raise diff --git a/tests/test-jsonrpc.py b/tests/test-jsonrpc.py index 5ce45ad2..9a24933b 100644 --- a/tests/test-jsonrpc.py +++ b/tests/test-jsonrpc.py @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import argparse import errno -import getopt import os import sys @@ -158,28 +158,35 @@ def do_notify(name, method, params_string): def main(argv): - try: - options, args = getopt.gnu_getopt( - argv[1:], 'h', ["help"] + ovs.daemon.LONG_OPTIONS) - except getopt.GetoptError, geo: - sys.stderr.write("%s: %s\n" % (ovs.util.PROGRAM_NAME, geo.msg)) - sys.exit(1) - for key, value in options: - if key in ['-h', '--help']: - usage() - elif not ovs.daemon.parse_opt(key, value): - sys.stderr.write("%s: unhandled option %s\n" - % (ovs.util.PROGRAM_NAME, key)) - sys.exit(1) + parser = argparse.ArgumentParser( + description="JSON-RPC test utility for Python.", + formatter_class=argparse.RawDescriptionHelpFormatter) commands = {"listen": (do_listen, 1), "request": (do_request, 3), "notify": (do_notify, 3), - "help": (usage, (0,))} - - command_name = args[0] - args = args[1:] + "help": (parser.print_help, (0,))} + + group_description = """\ +listen LOCAL listen for connections on LOCAL +request REMOTE METHOD PARAMS send request, print reply +notify REMOTE METHOD PARAMS send notification and exit +""" + ovs.stream.usage("JSON-RPC") + + group = parser.add_argument_group(title="Commands", + description=group_description) + group.add_argument('command', metavar="COMMAND", nargs=1, + choices=commands, help="Command to use.") + group.add_argument('command_args', metavar="ARG", nargs='*', + help="Arguments to COMMAND.") + + ovs.daemon.add_args(parser) + args = parser.parse_args() + ovs.daemon.handle_args(args) + + command_name = args.command[0] + args = args.command_args if not command_name in commands: sys.stderr.write("%s: unknown command \"%s\" " "(use --help for help)\n" % (argv[0], command_name)) @@ -204,22 +211,5 @@ def main(argv): func(*args) -def usage(): - sys.stdout.write("""\ -%s: JSON-RPC test utility for Python -usage: %s [OPTIONS] COMMAND [ARG...] - listen LOCAL listen for connections on LOCAL - request REMOTE METHOD PARAMS send request, print reply - notify REMOTE METHOD PARAMS send notification and exit -""" % (ovs.util.PROGRAM_NAME, ovs.util.PROGRAM_NAME)) - sys.stdout.write(ovs.stream.usage("JSON-RPC") + "\n") - ovs.daemon.usage() - sys.stdout.write(""" -Other options: - -h, --help display this help message -""") - sys.exit(0) - - if __name__ == '__main__': main(sys.argv) diff --git a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync index f4586544..1a174a05 100755 --- a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync +++ b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync @@ -22,7 +22,7 @@ # - Set the "iface-id" key in the Interface table. # - Set the fail-mode on internal bridges. -import getopt +import argparse import logging import logging.handlers import os @@ -215,24 +215,13 @@ def prune_schema(schema): schema.tables = new_tables -def usage(): - print "usage: %s [OPTIONS] DATABASE" % sys.argv[0] - print "where DATABASE is a socket on which ovsdb-server is listening." - ovs.daemon.usage() - print """\ -Other options: - --root-prefix=DIR Use DIR as alternate root directory (for testing). - -h, --help display this help message""" - sys.exit(0) - - def handler(signum, _): global force_run if (signum == signal.SIGHUP): force_run = True -def main(argv): +def main(): global force_run s_log.addHandler(logging.StreamHandler()) @@ -247,30 +236,21 @@ def main(argv): s_log.warn("failed to open logfile (%s)" % e) s_log.setLevel(logging.INFO) - try: - options, args = getopt.gnu_getopt( - argv[1:], 'h', ['help', 'root-prefix='] + ovs.daemon.LONG_OPTIONS) - except getopt.GetoptError, geo: - sys.stderr.write("%s: %s\n" % (ovs.util.PROGRAM_NAME, geo.msg)) - sys.exit(1) - - for key, value in options: - if key in ['-h', '--help']: - usage() - elif key == '--root-prefix': - global root_prefix - root_prefix = value - elif not ovs.daemon.parse_opt(key, value): - sys.stderr.write("%s: unhandled option %s\n" - % (ovs.util.PROGRAM_NAME, key)) - sys.exit(1) - - if len(args) != 1: - sys.stderr.write("%s: exactly one nonoption argument is required " - "(use --help for help)\n" % ovs.util.PROGRAM_NAME) - sys.exit(1) - - remote = args[0] + parser = argparse.ArgumentParser() + parser.add_argument("database", metavar="DATABASE", + help="A socket on which ovsdb-server is listening.") + parser.add_argument("--root-prefix", metavar="DIR", + help="Use DIR as alternate root directory" + " (for testing).") + + ovs.daemon.add_args(parser) + args = parser.parse_args() + ovs.daemon.handle_args(args) + + global root_prefix + root_prefix = args.root_prefix + + remote = args.database schema_file = "%s/vswitch.ovsschema" % ovs.dirs.PKGDATADIR schema = ovs.db.schema.DbSchema.from_json(ovs.json.from_file(schema_file)) prune_schema(schema) @@ -376,7 +356,7 @@ def main(argv): if __name__ == '__main__': try: - main(sys.argv) + main() except SystemExit: # Let system.exit() calls complete normally raise -- 2.30.2