Merge branch 'frr/pull/558'

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

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)

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);