]> git.puffer.fish Git - mirror/frr.git/commit
tools: fix vtysh failure error handling 7125/head
authorQuentin Young <qlyoung@nvidia.com>
Thu, 17 Sep 2020 19:46:55 +0000 (15:46 -0400)
committerQuentin Young <qlyoung@nvidia.com>
Thu, 17 Sep 2020 19:53:42 +0000 (15:53 -0400)
commitfdaee098f31691148c39f595c81b0a5393b0bded
tree1aebc66367d9b322929e2bba8560382722472842
parent70d400cefa05bdd6fa64ed6fdb42d082a6a5bc25
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>
tools/frr-reload.py