From 6eee4767d06e06dadf19d5163a2a26a9a5d60d77 Mon Sep 17 00:00:00 2001 From: Duncan Eastoe Date: Wed, 15 Jul 2020 12:35:42 +0100 Subject: [PATCH] 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(-) diff --git a/tools/frr-reload.py b/tools/frr-reload.py index 772a9bc88d..25922e3bf7 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))