From 9c782ad2a4e5106d25946a07adf97805302faf38 Mon Sep 17 00:00:00 2001 From: Duncan Eastoe Date: Tue, 14 Jul 2020 18:03:56 +0100 Subject: tools: frr-reload: more detailed log level control Add a "--log-level" option to frr-reload to set the maximum message level to be logged. When the option is not used, the level is set to info as before. The existing --debug option is synonymous with --log-level=debug and these options are therefore mutually exclusive. Signed-off-by: Duncan Eastoe --- tools/frr-reload.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'tools') diff --git a/tools/frr-reload.py b/tools/frr-reload.py index 9e86cf215..772a9bc88 100755 --- a/tools/frr-reload.py +++ b/tools/frr-reload.py @@ -1169,7 +1169,11 @@ if __name__ == '__main__': group = parser.add_mutually_exclusive_group(required=True) group.add_argument('--reload', action='store_true', help='Apply the deltas', default=False) group.add_argument('--test', action='store_true', help='Show the deltas', default=False) - parser.add_argument('--debug', action='store_true', help='Enable debugs', default=False) + level_group = parser.add_mutually_exclusive_group() + level_group.add_argument('--debug', action='store_true', + help='Enable debugs (synonym for --log-level=debug)', default=False) + level_group.add_argument('--log-level', help='Log level', default="info", + choices=("critical", "error", "warning", "info", "debug")) parser.add_argument('--stdout', action='store_true', help='Log to STDOUT', default=False) parser.add_argument('filename', help='Location of new frr config file') parser.add_argument('--overwrite', action='store_true', help='Overwrite frr.conf with running config output', default=False) @@ -1185,8 +1189,7 @@ if __name__ == '__main__': # For --test log to stdout # For --reload log to /var/log/frr/frr-reload.log if args.test or args.stdout: - logging.basicConfig(level=logging.INFO, - format='%(asctime)s %(levelname)5s: %(message)s') + logging.basicConfig(format='%(asctime)s %(levelname)5s: %(message)s') # Color the errors and warnings in red logging.addLevelName(logging.ERROR, "\033[91m %s\033[0m" % logging.getLevelName(logging.ERROR)) @@ -1197,7 +1200,6 @@ if __name__ == '__main__': os.makedirs('/var/log/frr/') logging.basicConfig(filename='/var/log/frr/frr-reload.log', - level=logging.INFO, format='%(asctime)s %(levelname)5s: %(message)s') # argparse should prevent this from happening but just to be safe... @@ -1205,6 +1207,11 @@ if __name__ == '__main__': raise Exception('Must specify --reload or --test') log = logging.getLogger(__name__) + if args.debug: + log.setLevel(logging.DEBUG) + else: + log.setLevel(args.log_level.upper()) + # Verify the new config file is valid if not os.path.isfile(args.filename): msg = "Filename %s does not exist" % args.filename @@ -1267,9 +1274,6 @@ if __name__ == '__main__': log.error(msg) sys.exit(1) - if args.debug: - log.setLevel(logging.DEBUG) - log.info('Called via "%s"', str(args)) # Create a Config object from the config generated by newconf -- cgit v1.2.3 From 6eee4767d06e06dadf19d5163a2a26a9a5d60d77 Mon Sep 17 00:00:00 2001 From: Duncan Eastoe Date: Wed, 15 Jul 2020 12:35:42 +0100 Subject: tools: frr-reload: log exclusively through logger In several instances a call to log.error() is preceded by a print() for the same message. To prevent duplicate messages these print() calls are removed. To maintain (very) similar behaviour we add a StreamHandler to the logger, when doing logging to a file (ie. --reload without --stdout), which additionally sends error and above logs to STDOUT without any metadata (exactly as they did before, with print()). There is one subtle change - the log from Vtysh.is_config_available() is now preceded with the "vtysh 'configure' returned" text, whereas previously only the output from vtysh was sent to STDOUT. Furthermore any error logs which weren't previously explicitly logged to STDOUT will now be. Signed-off-by: Duncan Eastoe --- tools/frr-reload.py | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) (limited to 'tools') diff --git a/tools/frr-reload.py b/tools/frr-reload.py index 772a9bc88..25922e3bf 100755 --- a/tools/frr-reload.py +++ b/tools/frr-reload.py @@ -113,7 +113,6 @@ class Vtysh(object): output = self('configure') if 'VTY configuration is locked by other VTY' in output: - print(output) log.error("vtysh 'configure' returned\n%s\n" % (output)) return False @@ -1212,45 +1211,44 @@ if __name__ == '__main__': else: log.setLevel(args.log_level.upper()) + if args.reload and not args.stdout: + # Additionally send errors and above to STDOUT, with no metadata, + # when we are logging to a file. This specifically does not follow + # args.log_level, and is analagous to behaviour in earlier versions + # which additionally logged most errors using print(). + + stdout_hdlr = logging.StreamHandler(sys.stdout) + stdout_hdlr.setLevel(logging.ERROR) + stdout_hdlr.setFormatter(logging.Formatter()) + log.addHandler(stdout_hdlr) + # Verify the new config file is valid if not os.path.isfile(args.filename): - msg = "Filename %s does not exist" % args.filename - print(msg) - log.error(msg) + log.error("Filename %s does not exist" % args.filename) sys.exit(1) if not os.path.getsize(args.filename): - msg = "Filename %s is an empty file" % args.filename - print(msg) - log.error(msg) + log.error("Filename %s is an empty file" % args.filename) sys.exit(1) # Verify that confdir is correct if not os.path.isdir(args.confdir): - msg = "Confdir %s is not a valid path" % args.confdir - print(msg) - log.error(msg) + log.error("Confdir %s is not a valid path" % args.confdir) sys.exit(1) # Verify that bindir is correct if not os.path.isdir(args.bindir) or not os.path.isfile(args.bindir + '/vtysh'): - msg = "Bindir %s is not a valid path to vtysh" % args.bindir - print(msg) - log.error(msg) + log.error("Bindir %s is not a valid path to vtysh" % args.bindir) sys.exit(1) # verify that the vty_socket, if specified, is valid if args.vty_socket and not os.path.isdir(args.vty_socket): - msg = 'vty_socket %s is not a valid path' % args.vty_socket - print(msg) - log.error(msg) + log.error('vty_socket %s is not a valid path' % args.vty_socket) sys.exit(1) # verify that the daemon, if specified, is valid if args.daemon and args.daemon not in ['zebra', 'bgpd', 'fabricd', 'isisd', 'ospf6d', 'ospfd', 'pbrd', 'pimd', 'ripd', 'ripngd', 'sharpd', 'staticd', 'vrrpd', 'ldpd']: - msg = "Daemon %s is not a valid option for 'show running-config'" % args.daemon - print(msg) - log.error(msg) + log.error("Daemon %s is not a valid option for 'show running-config'" % args.daemon) sys.exit(1) vtysh = Vtysh(args.bindir, args.confdir, args.vty_socket) @@ -1269,9 +1267,7 @@ if __name__ == '__main__': break if not service_integrated_vtysh_config and not args.daemon: - msg = "'service integrated-vtysh-config' is not configured, this is required for 'service frr reload'" - print(msg) - log.error(msg) + log.error("'service integrated-vtysh-config' is not configured, this is required for 'service frr reload'") sys.exit(1) log.info('Called via "%s"', str(args)) -- cgit v1.2.3