Skip to content

Commit 372a647

Browse files
hillol-nexthopclaude
hillol-nexthop
and
claude
committed
NOS-6616..NOS-6642: fboss2 config/delete protocol bgp peer-group commands
Collapses the 18 per-attribute `config protocol bgp peer-group` command classes (built on the deprecated `folly::dynamic` BgpConfigSession) into a single typed dispatcher, mirroring CmdConfigProtocolBgpNeighbor. Also adds `delete protocol bgp peer-group <name>`. The group name is the first positional token; the next one or two tokens name the attribute, matched longest-prefix-first so `timers hold-time` wins over any `timers` prefix; the rest are its value(s). Handlers mutate the typed bgp::thrift::PeerGroup through ConfigSession::getBgpConfig() / saveBgpConfig(), so adding a tunable is a one-entry change in the dispatch table rather than a new command class. 36 command files are deleted. - Covers the 29 dispatch keys that map to a PeerGroup thrift field: remote-asn / local-asn (4-byte-bounded), description, peer-tag, ingress-policy / egress-policy, rr-client, confed-peer, redistribute-peer, enhanced-route-refresh, connect-mode, add-path send|receive, afi disable-ipv4-afi|disable-ipv6-afi|ipv4-over-ipv6-nh, graceful-restart restart-time|stateful-ha, max-route pre-filter|post-filter (plus the warning-threshold / warning-only knobs), timers hold-time|keepalive|out-delay|withdraw-unprog-delay, and next-hop-self. - Rejected rather than persisted as dead config (precedent: cluster-id in the global command, connect-mode BOTH in the neighbor command): connect-mode BOTH, since thrift only models is_passive. add-path send|receive merges into the AddPath enum bitmask, and clearing the last direction unsets the field. - Attributes with no per-peer-group thrift field (afi ipv4-labeled-unicast, afi ipv6-labeled-unicast, peer-port) are absent from the dispatch table, so they are refused at parse time as unknown attributes; a rejected value never lands on disk. - positionals_at_end() stops parent-chain subcommand fallthrough from reclassifying an attribute token that matches a sibling command name once the group name has been consumed (same fix as the neighbor command). - unit tests (16): CmdConfigBgpPeerGroupTest (12) covering arg validation, longest-prefix match, the add-path merge matrix, connect-mode, the bool/string/route-limit attributes, value validation and unknown-attribute rejection, plus CmdDeleteBgpPeerGroupTest (4). - integration tests (3): ConfigBgpPeerGroupTest asserts the committed peer_groups through bgpd's getRunningConfig RPC, proving the daemon parsed and adopted the promoted config rather than only checking the file the CLI wrote. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent e876f28 commit 372a647

50 files changed

Lines changed: 1168 additions & 2227 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cmake/CliFboss2.cmake‎

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -880,42 +880,6 @@ add_library(fboss2_config_lib
880880
fboss/cli/fboss2/commands/config/protocol/bgp/neighbor/CmdConfigProtocolBgpNeighbor.h
881881
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp
882882
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h
883-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.cpp
884-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h
885-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.cpp
886-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.h
887-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDisableIpv4Afi.cpp
888-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDisableIpv4Afi.h
889-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupEgressPolicy.cpp
890-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupEgressPolicy.h
891-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupIngressPolicy.cpp
892-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupIngressPolicy.h
893-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupMaxRoutes.cpp
894-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupMaxRoutes.h
895-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupNextHopSelf.cpp
896-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupNextHopSelf.h
897-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupPeerTag.cpp
898-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupPeerTag.h
899-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRemoteAsn.cpp
900-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRemoteAsn.h
901-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRrClient.cpp
902-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRrClient.h
903-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimers.cpp
904-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimers.h
905-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersHoldTime.cpp
906-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersHoldTime.h
907-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersKeepalive.cpp
908-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersKeepalive.h
909-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersOutDelay.cpp
910-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersOutDelay.h
911-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersWithdrawUnprogDelay.cpp
912-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersWithdrawUnprogDelay.h
913-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupV4OverV6Nh.cpp
914-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupV4OverV6Nh.h
915-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupWarningLimit.cpp
916-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupWarningLimit.h
917-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupWarningOnly.cpp
918-
fboss/cli/fboss2/commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupWarningOnly.h
919883
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.cpp
920884
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h
921885
fboss/cli/fboss2/commands/config/ptp/transparent_clock/CmdConfigPtpTransparentClock.cpp
@@ -1012,6 +976,8 @@ add_library(fboss2_config_lib
1012976
fboss/cli/fboss2/commands/delete/protocol/bgp/CmdDeleteProtocolBgp.h
1013977
fboss/cli/fboss2/commands/delete/protocol/bgp/neighbor/CmdDeleteProtocolBgpNeighbor.cpp
1014978
fboss/cli/fboss2/commands/delete/protocol/bgp/neighbor/CmdDeleteProtocolBgpNeighbor.h
979+
fboss/cli/fboss2/commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.cpp
980+
fboss/cli/fboss2/commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.h
1015981
fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.h
1016982
fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp
1017983
fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h

‎cmake/CliFboss2TestConfig.cmake‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ add_executable(fboss2_cmd_config_test
1010
fboss/cli/fboss2/test/config/CmdConfigArpTest.cpp
1111
fboss/cli/fboss2/test/config/CmdConfigIcmpV4UnavailableSrcAddrTest.cpp
1212
fboss/cli/fboss2/test/config/CmdConfigBgpNeighborTest.cpp
13+
fboss/cli/fboss2/test/config/CmdConfigBgpPeerGroupTest.cpp
1314
fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp
1415
fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp
1516
fboss/cli/fboss2/test/config/CmdConfigHostnameTest.cpp
@@ -40,6 +41,7 @@ add_executable(fboss2_cmd_config_test
4041
fboss/cli/fboss2/test/config/CmdDeleteAclRuleTest.cpp
4142
fboss/cli/fboss2/test/config/CmdDeleteArpTest.cpp
4243
fboss/cli/fboss2/test/config/CmdDeleteBgpNeighborTest.cpp
44+
fboss/cli/fboss2/test/config/CmdDeleteBgpPeerGroupTest.cpp
4345
fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp
4446
fboss/cli/fboss2/test/config/CmdDeleteDhcpTest.cpp
4547
fboss/cli/fboss2/test/config/CmdDeleteInterfaceIpv6NdpTest.cpp

‎cmake/CliFboss2TestIntegrationTest.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ add_executable(fboss2_integration_test
1313
fboss/cli/fboss2/test/integration_test/ConfigArpTest.cpp
1414
fboss/cli/fboss2/test/integration_test/ConfigBgpGlobalTest.cpp
1515
fboss/cli/fboss2/test/integration_test/ConfigBgpNeighborTest.cpp
16+
fboss/cli/fboss2/test/integration_test/ConfigBgpPeerGroupTest.cpp
1617
fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
1718
fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
1819
fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp

‎fboss/cli/fboss2/BUCK‎

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,24 +1100,6 @@ cpp_library(
11001100
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.cpp",
11011101
"commands/config/protocol/bgp/neighbor/CmdConfigProtocolBgpNeighbor.cpp",
11021102
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.cpp",
1103-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.cpp",
1104-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.cpp",
1105-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDisableIpv4Afi.cpp",
1106-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupEgressPolicy.cpp",
1107-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupIngressPolicy.cpp",
1108-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupMaxRoutes.cpp",
1109-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupNextHopSelf.cpp",
1110-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupPeerTag.cpp",
1111-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRemoteAsn.cpp",
1112-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRrClient.cpp",
1113-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimers.cpp",
1114-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersHoldTime.cpp",
1115-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersKeepalive.cpp",
1116-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersOutDelay.cpp",
1117-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersWithdrawUnprogDelay.cpp",
1118-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupV4OverV6Nh.cpp",
1119-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupWarningLimit.cpp",
1120-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupWarningOnly.cpp",
11211103
"commands/config/protocol/static/CmdConfigProtocolStatic.cpp",
11221104
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp",
11231105
"commands/config/ptp/CmdConfigPtp.cpp",
@@ -1166,6 +1148,7 @@ cpp_library(
11661148
"commands/delete/protocol/CmdDeleteProtocol.cpp",
11671149
"commands/delete/protocol/bgp/CmdDeleteProtocolBgp.cpp",
11681150
"commands/delete/protocol/bgp/neighbor/CmdDeleteProtocolBgpNeighbor.cpp",
1151+
"commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.cpp",
11691152
"commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp",
11701153
"commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp",
11711154
"commands/delete/qos/CmdDeleteQos.cpp",
@@ -1224,20 +1207,6 @@ cpp_library(
12241207
"commands/config/protocol/bgp/global/CmdConfigProtocolBgpGlobal.h",
12251208
"commands/config/protocol/bgp/neighbor/CmdConfigProtocolBgpNeighbor.h",
12261209
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroup.h",
1227-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupConfedPeer.h",
1228-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDescription.h",
1229-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupDisableIpv4Afi.h",
1230-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupEgressPolicy.h",
1231-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupIngressPolicy.h",
1232-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupMaxRoutes.h",
1233-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupNextHopSelf.h",
1234-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRemoteAsn.h",
1235-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupRrClient.h",
1236-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimers.h",
1237-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersHoldTime.h",
1238-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersKeepalive.h",
1239-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupTimersOutDelay.h",
1240-
"commands/config/protocol/bgp/peer-group/CmdConfigProtocolBgpPeerGroupV4OverV6Nh.h",
12411210
"commands/config/protocol/static/CmdConfigProtocolStatic.h",
12421211
"commands/config/protocol/static/route/StaticRouteUtils.h",
12431212
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h",
@@ -1287,6 +1256,7 @@ cpp_library(
12871256
"commands/delete/protocol/CmdDeleteProtocol.h",
12881257
"commands/delete/protocol/bgp/CmdDeleteProtocolBgp.h",
12891258
"commands/delete/protocol/bgp/neighbor/CmdDeleteProtocolBgpNeighbor.h",
1259+
"commands/delete/protocol/bgp/peer-group/CmdDeleteProtocolBgpPeerGroup.h",
12901260
"commands/delete/protocol/static/CmdDeleteProtocolStatic.h",
12911261
"commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h",
12921262
"commands/delete/qos/CmdDeleteQos.h",

0 commit comments

Comments
 (0)