Repository navigation
[Nexthop] [fboss2-dev] Add show config running bgp - #1748
Open
vybhav-nexthop wants to merge 1 commit into
Open
vybhav-nexthop wants to merge 1 commit into
vybhav-nexthop wants to merge 1 commit into
Conversation
`show config running` only had an `agent` child, so the natural
`show config running bgp` failed with "A subcommand is required"; the
BGP running config was reachable only as `show bgp config running`.
Make `show config running <daemon>` the canonical shape for running-config
views: register `bgp` there, reusing CmdShowConfigRunningBgp.
`show bgp config running` is kept as an alias for backward compatibility;
it was the only OSS spelling before this change. Update the command's
description (which said the OSS build does not register this spelling)
and the fboss_cli.md command table.
Fix `--fmt json` for commands whose RetType is folly::dynamic
('show config running bgp|agent', 'show bgp config running',
'show bgp policy'): CmdHandler::printJson fed the per-host map to
SimpleJSONSerializer, which threw "TypeError: expected dynamic type
'array', but had type 'object'" and exited 1. printJson now renders
folly::dynamic results with folly::toJson.
Add integration tests that compare `show config running agent` and
`show config running bgp` against each daemon's getRunningConfig RPC,
and check that `show config running bgp` matches the
`show bgp config running` alias.
show config running bgpshow config running bgp
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runSummary
show config runningonly had anagentchild, sofboss2-dev show config running bgpfailed withA subcommand is required; BGP running config was only reachable asshow bgp config running.This makes
show config running <daemon>the canonical form for running-config views:oss/CmdList.cpp: registerbgpundershow config running, reusingCmdShowConfigRunningBgp.show bgp config runningis kept as an alias for backward compatibility; it was the only OSS spelling before this change.CmdShowConfigTraits.h: the description said the OSS build does not registershow config running bgp; updated to name it as the primary spelling.CmdHandler.cpp: fix--fmt jsonfor commands whoseRetTypeisfolly::dynamic(show config running bgp|agent,show bgp config running,show bgp policy).printJsonpassed the per-host map toSimpleJSONSerializer, which threwTypeError: expected dynamic type 'array', but had type 'object'and exited 1. It now rendersfolly::dynamicresults withfolly::toJson.docs/docs/debugging/fboss_cli.md: command table listsshow config running agent/bgp.Test Plan
On a hardware switch running
bgpd:show config running bgpA subcommand is required, rc=106show bgp config runningshow config running agentshow config running bgpoutput is byte-identical toshow bgp config running.show config running --helplistsagentandbgp.--fmt jsonfix (CmdHandler.cpp): before / afterOn a hardware switch,
bgpdrunning. Before = the installedfboss2-dev; after = this PR's build.--fmt json show bgp config runningTypeError: expected dynamic type 'array', but had type 'object'{"localhost": {...}}(861,551 bytes)--fmt json show config running bgpA subcommand is requiredcmp) to the row above--fmt json show bgp policy(no policy name)TypeError: ...{"localhost":{}}Start of the after output:
--fmt json show config running agentwas not re-checked in this run: the agent on that switch was not configured (switch is still initializing or is exiting and is not fully configured yet) with both binaries. It is covered byShowConfigRunningAgentTest.JsonMatchesDaemonRunningConfig.New integration tests in
fboss2_integration_test, all passing (3/3) on a hardware switch with a BGP peer established:ShowConfigRunningAgentTest.JsonMatchesDaemonRunningConfig:show config running agentmatches the agent'sgetRunningConfigRPC.ShowConfigRunningBgpTest.MatchesDaemonRunningConfig:show config running bgpmatches bgpd'sgetRunningConfigRPC.ShowConfigRunningBgpTest.MatchesShowBgpConfigRunningAlias:show config running bgpmatchesshow bgp config running.