VerifyLLDPNeighbors test adding an extra carriage return, causing an extra line in Markdown report
ecrosswhite opened this issue · 9 comments
Found that the LLDP Neighbor check is adding an extra line break moving the interface to its own line in the Markdown report.
| 6 | dc2-bsl-01 | Connectivity | VerifyLLDPNeighbors | Verifies that the provided LLDP neighbors are connected properly. | Local: Ethernet1/1 - Remote: dc1-bsl-01 Ethernet1/1 | FAIL | No LLDP neighbor(s) on port(s):
Ethernet1/1 |
If I modify the file anta/tests/connectivity.py line 167 from failure_messages.append(f"{failure_type}:\n {ports_str}") to failure_messages.append(f"{failure_type}: {ports_str}") my output Markdown file is formatted correctly;
| 6 | dc2-bsl-01 | Connectivity | VerifyLLDPNeighbors | Verifies that the provided LLDP neighbors are connected properly. | Local: Ethernet1/1 - Remote: dc1-bsl-01 Ethernet1/1 | FAIL | No LLDP neighbor(s) on port(s): Ethernet1/1 |
Hi @ecrosswhite - thanks for opening this. the new line is probably present in case multiple ports are in the string to have a nicer display in some other format mode.
Can you please share how you are using the markdown report - is it coming from AVD maybe? Do you see this breaking the table formatting then? (I guess it does)
Hello @gmuloc I was thinking the same that the new line was for the case of multiple interfaces. My understanding is the ports_str variable already includes one new line ports_str = "\n ".join(ports) so by having two new lines together in the failure_message it is causing the interface to bump down into the next line in the report.
Yes, this report is coming through via AVD and it doesn't break the table exactly, just causes the interface to be on its own line rather than in the relevant cell.
Having this same issue, I resolved it locally by patching the md_report.py file in the AVD project to replace newlines with <br /> in messages as it generates rows.
I feel like AVD is the appropriate place to resolve this. As it seems reasonable for ANTA to include newlines in its output, and it's the responsibility of any formatter (like AVDs md generator) to properly escape anything that might break the format it's generating. As the newline in this case is a problem for md but wouldn't necessarily be a problem for other consumers of the ANTA report.
Wanted to run this by here before submitting a PR to AVD though. I noticed that the message field is a list. So it could be considered appropriate to return the new lines as multiple items in the list as part of the test. Regardless AVD should probably by sanitizing anything it's generating in a way that won't break the md formatting, but ANTA might want to make a design decision on whether newlines can be expected in output or not.
@nathanmusser That is a good point, I hadn't considered patching it on the AVD side but it seems the best place to fix the issue. I agree that whatever tool is generating the report, AVD in this case, should be the one to fix issues from how it generates the report.
Just noticed that #740 is adding a native markdown generator to ANTA and already has a method for sanitizing the output. Hopefully AVD migrates to this generator once it's released.
Just noticed that #740 is adding a native markdown generator to ANTA and already has a method for sanitizing the output. Hopefully AVD migrates to this generator once it's released.
this is the plan - cc @carl-baillargeon
@nathanmusser That is correct. We are transferring the report (MD/CSV) generators to ANTA directly and will eventually use them when ANTA will be integrated to PyAVD --> aristanetworks/avd#4196
Since the integration will take a little bit of time (AVD 5.0) maybe? Please feel free to open a PR in AVD for eos_validate_state to fix the current report. Thanks
@carl-baillargeon thanks for the heads up, I "borrowed" the safe markdown method from ANTA and opened a PR using it against AVD.
Issue is on the AVD side and was fixed by aristanetworks/avd#4212