Issue 24036: Add help for 'show ip route' subcommands #4123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Add a new `VtyshCommand` class that can be used to extend click cli commands with `vtysh` help text. The class calls down into `vtysh` to get subcommand information and then generates help text following the existing format used by the top-level SONiC CLI. Add bash completions for the `vtysh` subcommands. Fixes: [24036](sonic-net/sonic-buildimage#24036)
anders-nexthop
force-pushed
the
anders.24036.add-vtysh-help
branch
from
11f956f to
3a44baf
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
@qiluo-msft @liamkearney-msft please take a look |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I did
Add a new VtyshCommand class that can be used to extend click cli commands with vtysh help text. The class calls down into vtysh to get subcommand information and then generates help text following the existing format used by the top-level SONiC CLI.
Additionally, add bash completions for the vtysh subcommands.
Closes: #24036
How I did it
Call down into vtysh to extract subcommands and descriptions, and use that information to populate help text and completions. Create a VtyshCommand wrapper class that handles everything, so that other commands can be easily extended to use the functionality. Add a completion hook so that bash completions can detect subcommands correctly.
An important consideration with this proposed fix is whether it's acceptable to call down into vtysh to retrieve this information. That adds a lot of complexity for what should really be a simple operation, and it involves overriding a lot of click internals to get the result to match the format of other SONiC cli commands. The underlying issue is that the 'show ip route' command tree is not actually implemented in SONiC itself, but in FRR. The current CLI is a thin shim between FRR and the main SONiC terminal that allows operators to skip having to use vtysh directly. The question is, how much support do we want to provide for this top-level CLI? And whats the right way to interface with the underlying vtysh functionality?
The approach presented here is a general solution to this issue, which could easily be used for any show command that passes through directly to vtysh (several of these exist beyond "show ip route"). Any changes to the vtysh CLI is supported automatically, so new vtysh commands will just work.
The alternative would be to actually implement the command tree. This is considerably more work, but would have the benefit of allowing for better help strings, and modifiable behavior at any level of the tree. However, this becomes a significant effort for every command, and is subject to breakage if the underlying vtysh CLI changes.
I thought that since the existing commands have already gone the quick and general route of just passing command strings wholesale into vtysh, it isn't a much greater sin to do the same work for generating help strings. Even as an intermediary step, this change would get command help online immediately. If or when proper subcommands get implemented, this workaround could be phased out accordingly.
Note: This PR does not attempt to retrieve description strings for user-defined arguments (stuff like IPADDRESS or vrf NAME, or range values). This could be done by defining a set of known patterns which could be stated as regexes and then compared against command lists, and returning description strings for a command if a match is found. However, these instances would have to be tracked down and added manually; they could also conflict with each other (for instance trying to match A.B.C.D and A.B.C.D/E, you have to make sure the regex are applied in the correct order if you want to get the most correct match). Given the concerns there, and given the scope of the current PR, I decided to leave that handling off for now. It can be added in a later patch.
How to verify it
Manually verified that subcommand help works, that existing help works, that tab completions work, and that the existing commands and subcommands still function properly.
Added unit tests for the new functionality:
Performance: the performance cost is negligable:
Previous command output (if the output of a command-line utility has changed)
All commands under the "show ip route" subtree ("show ip route", "show ip route summary", "show ip route json", etc) have the same help output:
New command output (if the output of a command-line utility has changed)
There is now actual help output for a whole bunch of commands/subcommands. Also, note that usage string for "show ip route" itself has changed to match the regular click usage style found with other (non-vtysh) commands. The arguments (IPADDRESS and vrf) are listed as regular subcommands.
There are many possible valid combinations of subcommands under this tree, and they will all have slightly different help text. The description is pulled from the short description tag given for the command in the command list of its parent command. Here are a few examples:
Leaf entries have the correct usage arg (they do not prompt for more commands):
Error handling matches what the click library does for regular commands:
compare to:
Finally, all "show ip route" commands now have completions for subcommands (note that for commands with no completion available, the completion falls back to trying to find file names in the current directory, which is the default click behavior):