diff --git a/tools/frr-reload.py b/tools/frr-reload.py index 80d2d6a2a3..a9151e3e7d 100755 --- a/tools/frr-reload.py +++ b/tools/frr-reload.py @@ -109,9 +109,12 @@ class Config(object): log.info('Loading Config object from file %s', filename) try: - file_output = subprocess.check_output(['/usr/bin/vtysh', '-m', '-f', filename]) + file_output = subprocess.check_output(['/usr/bin/vtysh', '-m', '-f', filename], + stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: - raise VtyshMarkException(str(e)) + ve = VtyshMarkException(e) + ve.output = e.output + raise ve for line in file_output.split('\n'): line = line.strip() @@ -134,9 +137,11 @@ class Config(object): try: config_text = subprocess.check_output( "/usr/bin/vtysh -c 'show run' | /usr/bin/tail -n +4 | /usr/bin/vtysh -m -f -", - shell=True) + shell=True, stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: - raise VtyshMarkException(str(e)) + ve = VtyshMarkException(e) + ve.output = e.output + raise ve for line in config_text.split('\n'): line = line.strip() @@ -342,14 +347,17 @@ end # the keywords that we know are single line contexts. bgp in this case # is not the main router bgp block, but enabling multi-instance oneline_ctx_keywords = ("access-list ", + "agentx", "bgp ", "debug ", "dump ", "enable ", + "frr ", "hostname ", "ip ", "ipv6 ", "log ", + "no ", "password ", "ptm-enable", "router-id ", @@ -813,6 +821,14 @@ def compare_context_objects(newconf, running): elif "router bgp" in running_ctx_keys[0] and len(running_ctx_keys) > 1 and delete_bgpd: continue + elif ("router bgp" in running_ctx_keys[0] and + len(running_ctx_keys) > 1 and + running_ctx_keys[1].startswith('address-family')): + # There's no 'no address-family' support and so we have to + # delete each line individually again + for line in running_ctx.lines: + lines_to_del.append((running_ctx_keys, line)) + # Non-global context elif running_ctx_keys and not any("address-family" in key for key in running_ctx_keys): lines_to_del.append((running_ctx_keys, None)) @@ -888,11 +904,15 @@ if __name__ == '__main__': # Verify the new config file is valid if not os.path.isfile(args.filename): - print "Filename %s does not exist" % args.filename + msg = "Filename %s does not exist" % args.filename + print msg + log.error(msg) sys.exit(1) if not os.path.getsize(args.filename): - print "Filename %s is an empty file" % args.filename + msg = "Filename %s is an empty file" % args.filename + print msg + log.error(msg) sys.exit(1) # Verify that 'service integrated-vtysh-config' is configured @@ -909,7 +929,9 @@ if __name__ == '__main__': break if not service_integrated_vtysh_config: - print "'service integrated-vtysh-config' is not configured, this is required for 'service frr reload'" + msg = "'service integrated-vtysh-config' is not configured, this is required for 'service frr reload'" + print msg + log.error(msg) sys.exit(1) if args.debug: @@ -920,6 +942,7 @@ if __name__ == '__main__': # Create a Config object from the config generated by newconf newconf = Config() newconf.load_from_file(args.filename) + reload_ok = True if args.test: @@ -993,7 +1016,20 @@ if __name__ == '__main__': (lines_to_add, lines_to_del) = compare_context_objects(newconf, running) - if lines_to_del: + # Only do deletes on the first pass. The reason being if we + # configure a bgp neighbor via "neighbor swp1 interface" frr + # will automatically add: + # + # interface swp1 + # ipv6 nd ra-interval 10 + # no ipv6 nd suppress-ra + # ! + # + # but those lines aren't in the config we are reloading against so + # on the 2nd pass they will show up in lines_to_del. This could + # apply to other scenarios as well where configuring FOO adds BAR + # to the config. + if lines_to_del and x == 0: for (ctx_keys, line) in lines_to_del: if line == '!': @@ -1029,7 +1065,7 @@ if __name__ == '__main__': # 'no ip ospf authentication message-digest 1.1.1.1' in # our example above # - Split that last entry by whitespace and drop the last word - log.warning('Failed to execute %s', ' '.join(cmd)) + log.info('Failed to execute %s', ' '.join(cmd)) last_arg = cmd[-1].split(' ') if len(last_arg) <= 2: @@ -1064,9 +1100,25 @@ if __name__ == '__main__': with open(filename, 'w') as fh: for line in lines_to_configure: fh.write(line + '\n') - subprocess.call(['/usr/bin/vtysh', '-f', filename]) + + output = subprocess.check_output(['/usr/bin/vtysh', '-f', filename]) + + # exit non-zero if we see these errors + for x in ('BGP instance name and AS number mismatch', + 'BGP instance is already running', + '% not a local address'): + for line in output.splitlines(): + if x in line: + msg = "ERROR: %s" % x + log.error(msg) + print msg + reload_ok = False + os.unlink(filename) # Make these changes persistent if args.overwrite or args.filename != '/etc/frr/frr.conf': subprocess.call(['/usr/bin/vtysh', '-c', 'write']) + + if not reload_ok: + sys.exit(1) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 51f71a46bb..2994c3507a 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -635,7 +635,7 @@ vtysh_mark_file (const char *filename) } } /* This is the end */ - fprintf(stdout, "end\n"); + fprintf(stdout, "\nend\n"); vty_close(vty); XFREE(MTYPE_VTYSH_CMD, vty_buf_copy);