tools: fix vtysh failure error handling

Based on the current code, I think the intent was to gracefully handle
vtysh failures and print a useful error message. Barriers in the way of
that:

- Despite reading the results of subprocess.communicate(), there won't
  be anything there, because we aren't passing subprocess.PIPE as stdin
  and stderr when calling subprocess.Popen()
- Despite catching subprocess.TimeoutExpired, if we were to actually hit
  this case frr-reload.py would just crash because it's calling
  .communicate() on an unbound process variable, probably a copy-paste
  error
- Aside from that, building a kwargs dict to pass to a function that
  contains something if something else is not None and nothing if it is,
  is pointless when we could just pass the thing itself

Net result is that if vtysh fails to read an frr.conf due to syntax
errors, instead of crashing with a traceback, we actually handle the
error condition, log the problem and vtysh's output, and exit. Actually
we were printing the failed line just by chance because stderr wasn't
captured from the subprocess and I guess showed up as part of systemd's
error capturing or something, but the traceback did a good job of
obscuring that with useless noise.

Old:

frrinit.sh[32183]:  * Started watchfrr
frrinit.sh[32183]: line 20: % Unknown command: eee
frrinit.sh[32183]: Traceback (most recent call last):
frrinit.sh[32183]:   File "/usr/lib/frr/frr-reload.py", line 1316, in <module>
frrinit.sh[32183]:     newconf.load_from_file(args.filename)
frrinit.sh[32183]:   File "/usr/lib/frr/frr-reload.py", line 231, in load_from_file
frrinit.sh[32183]:     file_output = self.vtysh.mark_file(filename)
frrinit.sh[32183]:   File "/usr/lib/frr/frr-reload.py", line 146, in mark_file
frrinit.sh[32183]:     % (child.returncode, stderr))
frrinit.sh[32183]: __main__.VtyshException: vtysh (mark file) exited with status 2:
frrinit.sh[32183]: None

New:

frrinit.sh[30090]:  * Started watchfrr
frrinit.sh[30090]: vtysh failed to process new configuration: vtysh (mark file) exited with status 2:
frrinit.sh[30090]: line 20: % Unknown command: eee

Signed-off-by: Quentin Young <qlyoung@nvidia.com>

diff --git a/tools/frr-reload.py b/tools/frr-reload.py
index 8ffc313c04b2b4d0258c9832da7fb57eb3a90e17..88873da904599cfee227582b9145ccaca1c47f04 100755 (executable)
--- a/tools/frr-reload.py
+++ b/tools/frr-reload.py
@@ -128,17 +128,13 @@ class Vtysh(object):
                     % (child.returncode))
 
     def mark_file(self, filename, stdin=None):
-        kwargs = {}
-        if stdin is not None:
-            kwargs['stdin'] = stdin
-
         child = self._call(['-m', '-f', filename],
-                stdout=subprocess.PIPE, **kwargs)
+                stdout=subprocess.PIPE, stdin=subprocess.PIPE, stderr=subprocess.PIPE)
         try:
             stdout, stderr = child.communicate()
         except subprocess.TimeoutExpired:
             child.kill()
-            stdout, stderr = proc.communicate()
+            stdout, stderr = child.communicate()
             raise VtyshException('vtysh call timed out!')
 
         if child.wait() != 0:
@@ -1313,8 +1309,12 @@ if __name__ == '__main__':
 
     # Create a Config object from the config generated by newconf
     newconf = Config(vtysh)
-    newconf.load_from_file(args.filename)
-    reload_ok = True
+    try:
+        newconf.load_from_file(args.filename)
+        reload_ok = True
+    except VtyshException as ve:
+        log.error("vtysh failed to process new configuration: {}".format(ve))
+        reload_ok = False
 
     if args.test: