[Nexthop][fboss2-dev] BGP-aware fboss2 config session (ConfigSession infra)#1344
Open
hillol-nexthop wants to merge 1 commit into
Open
[Nexthop][fboss2-dev] BGP-aware fboss2 config session (ConfigSession infra)#1344hillol-nexthop wants to merge 1 commit into
hillol-nexthop wants to merge 1 commit into
Conversation
91eccdd to
699ef91
Compare
Make the fboss2 ConfigSession and `config session` lifecycle BGP-aware. This is
the base for the `config protocol bgp` commands, which stack on top.
- ConfigSession owns the BGP config as a typed bgp::thrift::BgpConfig, exactly
the way it owns cfg::AgentConfig for the agent. getBgpConfig()/saveBgpConfig()
expose the WHOLE typed config and stage it to ~/.fboss2/bgp_config.json.
ConfigSession is BGP-aware but agnostic about the config's internal structure:
it has no notion of "global" vs "peer" vs "peer-group" -- a command mutates
whichever typed fields it needs, mirroring how interface commands mutate
getAgentConfig().sw()->ports(). Because the whole config round-trips through
the typed struct, no surgical merge / preserve-list is needed.
- commit() promotes bgp_config.json to /etc/coop/bgpcpp/bgpcpp.conf, restarts
bgp_pp, and versions it in the same /etc/coop git repo as agent.conf.
- config session clear/diff/rebase/rollback are BGP-aware: clear removes a
staged BGP session, diff shows staged BGP changes, rebase 3-way-merges
bgpcpp.conf, rollback restores bgpcpp.conf + restarts bgp_pp (using metadata
history so BGP-only commits are reachable).
- FbossServiceUtil + cli_metadata gain the bgp_pp (BGP_PP) service/action; the
config lib gains the bgp_config cpp2 dependency.
- Unit tests in CmdConfigSession{,Diff}Test cover the typed round-trip, commit,
rebase, and rollback behavior.
Test Plan:
- bazel test //fboss/cli/fboss2/test/config:cmd_config_test //fboss/cli/fboss2/test:cmd_test
- fboss2_integration_test on a DUT (ConfigConcurrentSessionsTest.ConflictAndRebase,
ConfigSessionClearTest, ConfigInterfaceMtuTest, ConfigInterfaceDescriptionTest): 5/5 pass
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
699ef91 to
843c28f
Compare
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.
Summary
Make the fboss2
ConfigSessionandconfig sessionlifecycle BGP-aware. Thisis the base for the
config protocol bgpcommands, which build on top (see thestacked PR).
ConfigSessionowns the BGP config as a typedbgp::thrift::BgpConfig,exactly the way it owns
cfg::AgentConfigfor the agent.getBgpConfig()/saveBgpConfig()expose the whole typed config and stage itto
~/.fboss2/bgp_config.json.ConfigSessionis BGP-aware but agnosticabout the config's internal structure: a command mutates whichever typed
fields it needs, mirroring how interface commands mutate
getAgentConfig().sw()->ports(). The whole config round-trips through thetyped struct, so no surgical merge / preserve-list is needed.
commit()promotesbgp_config.jsonto/etc/coop/bgpcpp/bgpcpp.conf,restarts
bgp_pp, and versions it in the same/etc/coopgit repo asagent.conf.config session clear/diff/rebase/rollbackare BGP-aware: clear removes astaged BGP session, diff shows staged BGP changes, rebase 3-way-merges
bgpcpp.conf, rollback restoresbgpcpp.conf+ restartsbgp_pp(usingmetadata history so BGP-only commits are reachable).
FbossServiceUtil+cli_metadatagain thebgp_pp(BGP_PP)service/action; the config lib gains the
bgp_configcpp2 dependency.Test Plan
fboss/cli/fboss2/test/config:cmd_config_test— 455 tests pass(the BGP-session unit tests for the fixes below land in this PR).
//fboss/cli/...builds on top ofupstream/main.bgp_ppruns asConfigBgpSessionTest, which is added in the stacked PR (the integrationharness ships with the
config protocol bgpcommands) — 6/6 pass there.