Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for Cisco FTD #654

Merged
merged 5 commits into from
Apr 29, 2020
Merged

Conversation

micahculpepper
Copy link
Contributor

Adds template for parsing ping, and recycles the existing ASA template for parsing arp. See also: ktbyers/netmiko#1681

ISSUE TYPE
  • New Template Pull Request
COMPONENT

cisco_ftd ping, and cisco_ftd show arp

SUMMARY

Adds support for cisco FTD ping and show arp commands. The show arp command shares the ASA's template.

tox output:

=================================================================== 846 passed in 19.76s ====================================================================
__________________________________________________________________________ summary __________________________________________________________________________
  py36: commands succeeded
  congratulations :)

Adds template for parsing ping, and recycles the existing ASA template for parsing arp.
@FragmentedPacket
Copy link
Contributor

I don't have an ASA to test on, but is the biggest thing between FTD and ASA just the paging and that's the reason for the new driver in Netmiko? Does the output differ? I'm wondering if we should go the route you did with the ARP command and just have the OS as either cisco_(asa|ftd) within the index file if that is the only difference.

@micahculpepper
Copy link
Contributor Author

The FTD CLI is a chimera of ASA and non-ASA commands and output. I think it makes sense to have a separate folder for FTD templates since it will surely need some of its own.

However, in the case of ping output, it does look just like ASA output, so the same template could be used for both. I was actually a bit surprised that there was no template for ASA pings to begin with. But I wanted to get FTD support off the ground without worrying about whether my new template would work for all ASAs, which have much broader support in ntc_templates already. In other words, doing it this way seemed less risky.

templates/index Outdated Show resolved Hide resolved
templates/cisco_ftd_ping.textfsm Outdated Show resolved Hide resolved
@FragmentedPacket FragmentedPacket added changes_requested Waiting on user to address feedback and removed Internal Review Requested labels Apr 28, 2020
templates/index Outdated
cisco_asa_show_nat.textfsm, .*, cisco_asa, sh[[ow]] nat
cisco_asa_dir.textfsm, .*, cisco_asa, dir

cisco_ftd_ping.textfsm, .*, cisco_(asa|ftd), ping
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about this! I should have been clearer. Do you mind moving this up and then renaming to cisco_asa_ping.textfsm? I think we'll keep the naming convention the same, but introduce the cisco_(asa|ftd) into the index. The tests will have to be updated as well.

Let me know if you have any issues with this in our public Slack.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, should be fixed now.

@FragmentedPacket FragmentedPacket merged commit 18da301 into networktocode:master Apr 29, 2020
@micahculpepper micahculpepper deleted the ftd branch April 29, 2020 20:23
@micahculpepper micahculpepper restored the ftd branch May 12, 2020 16:24
thomasblass pushed a commit to thomasblass/ntc-templates that referenced this pull request Oct 25, 2020
guillaume-mbali pushed a commit to unyc-io/ntc-templates that referenced this pull request Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes_requested Waiting on user to address feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants