[Nexthop][fboss2-dev] Add `config session rebase` command for concurrent session conflict resolution by benoit-nexthop · Pull Request #832 · facebook/fboss

benoit-nexthop · 2026-01-23T10:02:40Z

Pre-submission checklist

I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
pre-commit run

Summary

When multiple users have concurrent config sessions, the first user to commit succeeds, but subsequent users get an error because their session is based on a stale commit. Previously, whoever committed last would overwrite changes of other users that committed before them.

This commit adds a config session rebase command that:

Takes the diff between the config as of the base commit and the session
Re-applies this diff on top of the current HEAD (similar to git rebase)
Uses a recursive 3-way merge algorithm for JSON objects
Detects and reports conflicts at specific JSON paths
Updates the session's base to the current HEAD on success

The 3-way merge algorithm handles:

Objects: recursively merges each key
Arrays: element-by-element merge if sizes match
Scalars: conflict if both head and session changed differently from base

Note: this change is part of a series, the previous one is #825, the next one is #837.

Test Plan

Add unit tests covering:

Successful rebase with non-conflicting changes
Rebase failure when changes conflict
Rebase not needed when session is already up-to-date

Sample usage

TBD

Summary: **Pre-submission checklist** - [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install` - [x] `pre-commit run` Replace the basic file-based configuration versioning mechanism with Git-based versioning for the CLI config session. Key changes: - Add new `Git` class providing a simple interface for Git operations: init, commit, log, show, etc - Use `folly::Subprocess` with full path `/usr/bin/git` for all Git commands - Replace revision files (`agent-rN.conf` + symlink) with atomic writes to agent.conf tracked in a local Git repository - Use Git commit SHAs as revision identifiers instead of rN format - Update RevisionList validation to accept Git SHAs (7+ hex chars) Repository initialization: - Automatically initialize Git repo if it doesn't exist - Automatically create initial commit if repo has no commits but config file exists - Use `--shared=group` flag and umask 0002 to ensure `.git` directory is group-writable when `/etc/coop` is group-writable Commands updated: - config history: Shows Git commit log with SHA, author, timestamp, message - config session diff: Uses git show to compare commits - config session commit: Creates Git commits with username as author - config rollback: Reads config from Git history and creates new commit Note: this change is part of a series, the previous one is #809, the next one is #832. Pull Request resolved: #825 Test Plan: Test updates: - Update all CLI config tests to use Git-based setup - Initialize Git repo and create initial commit in test fixtures - Added new unit tests for the Git layer ## Sample usage Simple change and session commit: ``` [admin@fboss101 benoit]$ ./fboss2-dev show interface eth1/1/1 +-----------+--------+-------+------+------+------------------------------+-------------+ | Interface | Status | Speed | VLAN | MTU | Addresses | Description | ----------------------------------------------------------------------------------------- | eth1/1/1 | down | 800G | 2001 | 9000 | 10.0.0.0/24 | demo | | | | | | | 2400::/64 | | | | | | | | fe80::b4db:91ff:fe95:ff07/64 | | ----------------------------------------------------------------------------------------- [admin@fboss101 benoit]$ ./fboss2-dev config interface eth1/1/1 description hello Successfully set description for interface(s) eth1/1/1 [admin@fboss101 benoit]$ ./fboss2-dev config session diff --- current live config +++ session config @@ -2200,7 +2200,7 @@ "ports": [ { "conditionalEntropyRehash": false, - "description": "demo", + "description": "hello", "drainState": 0, "expectedLLDPValues": { "2": "eth1/5/1" [admin@fboss101 benoit]$ ./fboss2-dev config session commit Config session committed successfully as b8245b4 and config reloaded. [admin@fboss101 benoit]$ ./fboss2-dev config history | head Commit Author Commit Time Message ------------------------------------------------------------------------ b8245b40 admin 2026-01-21 11:13:57 Config commit by admin 7241abad admin 2026-01-21 11:12:51 Config commit by admin 02fe511c admin 2026-01-21 10:51:36 Config commit by admin e5650a98 admin 2026-01-21 10:51:35 Config commit by admin ad07138f admin 2026-01-21 10:51:35 Config commit by admin 1845e710 admin 2026-01-21 10:51:35 Config commit by admin ede5a527 admin 2026-01-20 14:28:10 Config commit by admin e900ef88 admin 2026-01-20 14:23:25 Config commit by admin [admin@fboss101 benoit]$ cd /etc/coop [admin@fboss101 coop]$ git -c safe.directory=/etc/coop show commit b8245b40b2b3c8797cbeed2fb91161e5440b7fd9 (HEAD -> main) Author: admin <fboss-cli@localhost> Date: Wed Jan 21 11:13:57 2026 -0800 Config commit by admin diff --git a/cli/agent.conf b/cli/agent.conf index 406b341..f61516f 100644 --- a/cli/agent.conf +++ b/cli/agent.conf @@ -2200,7 +2200,7 @@ "ports": [ { "conditionalEntropyRehash": false, - "description": "demo", + "description": "hello", "drainState": 0, "expectedLLDPValues": { "2": "eth1/5/1" diff --git a/cli/cli_metadata.json b/cli/cli_metadata.json index 68f340a..6c6a925 100644 --- a/cli/cli_metadata.json +++ b/cli/cli_metadata.json @@ -1,6 +1,6 @@ { "action": {}, "commands": [ - "config interface eth1/1/1 description demo" + "config interface eth1/1/1 description hello" ] } \ No newline at end of file ``` Rollback flow: ``` [admin@fboss101 benoit]$ ./fboss2-dev config interface eth1/1/1 description hello2 Successfully set description for interface(s) eth1/1/1 [admin@fboss101 benoit]$ ./fboss2-dev config session commit Config session committed successfully as 476f9ee and config reloaded. [admin@fboss101 benoit]$ ./fboss2-dev config history | head Commit Author Commit Time Message ------------------------------------------------------------------------ 476f9ee4 admin 2026-01-21 11:15:23 Config commit by admin b8245b40 admin 2026-01-21 11:13:57 Config commit by admin 7241abad admin 2026-01-21 11:12:51 Config commit by admin <-- we're going to rollback to here 02fe511c admin 2026-01-21 10:51:36 Config commit by admin e5650a98 admin 2026-01-21 10:51:35 Config commit by admin ad07138f admin 2026-01-21 10:51:35 Config commit by admin 1845e710 admin 2026-01-21 10:51:35 Config commit by admin ede5a527 admin 2026-01-20 14:28:10 Config commit by admin [admin@fboss101 benoit]$ ./fboss2-dev config rollback 7241abad Successfully rolled back. New commit: 8be163e7. Config reloaded. [admin@fboss101 benoit]$ ./fboss2-dev show interface eth1/1/1 +-----------+--------+-------+------+------+------------------------------+-------------+ | Interface | Status | Speed | VLAN | MTU | Addresses | Description | ----------------------------------------------------------------------------------------- | eth1/1/1 | down | 800G | 2001 | 9000 | 10.0.0.0/24 | demo | | | | | | | 2400::/64 | | | | | | | | fe80::b4db:91ff:fe95:ff07/64 | | ----------------------------------------------------------------------------------------- [admin@fboss101 benoit]$ ./fboss2-dev config history | head Commit Author Commit Time Message --------------------------------------------------------------------------- 8be163e7 admin 2026-01-21 11:17:26 Rollback to 7241abad by admin 476f9ee4 admin 2026-01-21 11:15:23 Config commit by admin b8245b40 admin 2026-01-21 11:13:57 Config commit by admin 7241abad admin 2026-01-21 11:12:51 Config commit by admin 02fe511c admin 2026-01-21 10:51:36 Config commit by admin e5650a98 admin 2026-01-21 10:51:35 Config commit by admin ad07138f admin 2026-01-21 10:51:35 Config commit by admin 1845e710 admin 2026-01-21 10:51:35 Config commit by admin [admin@fboss101 benoit]$ cd /etc/coop [admin@fboss101 coop]$ git -c safe.directory=/etc/coop show commit 8be163e7cd60574278dc7201b157e4238fa55380 (HEAD -> main) Author: admin <fboss-cli@localhost> Date: Wed Jan 21 11:17:26 2026 -0800 Rollback to 7241abad by admin diff --git a/cli/agent.conf b/cli/agent.conf index d918bae..406b341 100644 --- a/cli/agent.conf +++ b/cli/agent.conf @@ -2200,7 +2200,7 @@ "ports": [ { "conditionalEntropyRehash": false, - "description": "hello2", + "description": "demo", "drainState": 0, "expectedLLDPValues": { "2": "eth1/5/1" diff --git a/cli/cli_metadata.json b/cli/cli_metadata.json index 5435c54..68f340a 100644 --- a/cli/cli_metadata.json +++ b/cli/cli_metadata.json @@ -1,6 +1,6 @@ { "action": {}, "commands": [ - "config interface eth1/1/1 description hello2" + "config interface eth1/1/1 description demo" ] } \ No newline at end of file ``` Reviewed By: srikrishnagopu Differential Revision: D94975358 Pulled By: joseph5wu fbshipit-source-id: 197bc68feed6f5fe9443ad91d24999546ebbb0e7

…esolution When multiple users have concurrent config sessions, the first user to commit succeeds, but subsequent users get an error because their session is based on a stale commit. Previously, whoever committed last would overwrite changes of other users that committed before them. This commit adds a `config session rebase` command that: 1. Takes the diff between the config as of the base commit and the session 2. Re-applies this diff on top of the current HEAD (similar to git rebase) 3. Uses a recursive 3-way merge algorithm for JSON objects 4. Detects and reports conflicts at specific JSON paths 5. Updates the session's base to the current HEAD on success The 3-way merge algorithm handles: - Objects: recursively merges each key - Arrays: element-by-element merge if sizes match - Scalars: conflict if both head and session changed differently from base Add unit tests covering: - Successful rebase with non-conflicting changes - Rebase failure when changes conflict - Rebase not needed when session is already up-to-date NOS-3962 #done

facebook-github-bot · 2026-03-03T21:26:30Z

@benoit-nexthop has updated the pull request. You must reimport the pull request before landing.

joseph5wu

I have some general questions about the rebasing problem we want to solve here.

Like in your scenario here, User1 make changes on agent.conf based on agent_0.conf and User2 also make changes based on agent_0.conf.
Now because you switched to use Git control in the previous PR, so git doesn't allow you to commit as the second user.
So my question is if you want to have config session rebase, wouldn't it be more intuitive that you change your current session's base to head, and re-apply all the clis with the new base config?
Basically you're doing the same thing. But your approach will stop as long as two sessions are changing the same field. Now you basically leave the second user to "discard" this change, and they will just re-type the commands again. So if they do that, this just means it's the "rebase" to head approach I meant above. Hence I'm not sure why can't we automatically rebase to head and re-apply the commands.

joseph5wu · 2026-03-03T19:28:50Z

fboss/cli/fboss2/session/ConfigSession.cpp

+  // 1. Base config (what the session was originally based on)
+  // 2. Current HEAD config (what someone else committed)
+  // 3. Session config (user's changes)
+  std::string cliConfigRelPath = "cli/agent.conf";


Please make a plan to support multiple configs(services) in the ConfigSession.
BGP/DNE teams recently landed https://github.com/facebook/fboss/blob/main/fboss/cli/fboss2/commands/config/protocol/bgp/BgpConfigSession.h
I'm not sure whether this is a good design that have each service has its own version of XXXConfigSession.
We should address/polish this in the future PRs

Yes we can do that easily in a subsequent PR.

joseph5wu · 2026-03-03T20:02:58Z

fboss/cli/fboss2/test/config/CmdConfigSessionTest.cpp

+  // User2 tries to commit but fails due to stale base
+  EXPECT_THROW(session2.commit(localhost()), std::runtime_error);


Can you also verify the error with the proper conflicts info?

joseph5wu · 2026-03-03T20:05:44Z

fboss/cli/fboss2/test/config/CmdConfigSessionTest.cpp

+        try {
+          session2.rebase();
+        } catch (const std::runtime_error& e) {
+          EXPECT_THAT(e.what(), ::testing::HasSubstr("conflict"));


Can we add more check other than just looking for "conflict" string in the error?
Maybe something about port[0]? So we can confirm the error message could be detailed enough.

fboss/oss/cli_tests/test_config_concurrent_sessions.py

joseph5wu · 2026-03-03T20:32:24Z

fboss/cli/fboss2/session/ConfigSession.cpp

+    const folly::dynamic& session,
+    const std::string& path,
+    std::vector<std::string>& conflicts) {
+  // If we've already hit max conflicts, stop recursing


Why do we want to stop recursing after we reach the max conflicts?
I think it might be better that if the second user tries to call commit and then we get the error about how many conflicts for this commit?
And then this user can decide whether they want to rebase or discard(due to there're too many conflicts).
IIUC, as long as there's one conflict, you won't allow a merge, aka the session will never rebase on head anyway. Hence I don't quite get the kMaxConflicts is for.

I think we need to decide what's the expectation of this merge function.
If we're already using git to control the commits merging, I'm not so sure why do we need implement this function here.

It's a bit like if you have a lot compilation errors, gcc/clang will just tell you about the first 20, I felt like there was no point in flooding the user with a ton of conflicts if there are too many.

meta-codesync · 2026-03-04T00:32:38Z

@joseph5wu has imported this pull request. If you are a Meta employee, you can view this in D95099578.

benoit-nexthop · 2026-03-04T01:57:48Z

I have some general questions about the rebasing problem we want to solve here.
Like in your scenario here, User1 make changes on agent.conf based on agent_0.conf and User2 also make changes based on agent_0.conf.
Now because you switched to use Git control in the previous PR, so git doesn't allow you to commit as the second user.

Actually git will allow you to commit as the second user. The problem is that the second user will overwrite the changes of the first user, so the first user's changes will be lost. That's why we now detect if there was another commit while you were working on your config session (by saving the base sha1 of your session and comparing it when you commit). Rebase is just an easy solution to "port" your changes on top of the changes that were committed in the meantime.

So my question is if you want to have config session rebase, wouldn't it be more intuitive that you change your current session's base to head, and re-apply all the clis with the new base config?

We could re-apply the CLIs but it seemed faster / simpler to not re-run all the code of all the CLI commands you ran and just "patch" the JSON changes you made on top of the new JSON tree.

Basically you're doing the same thing. But your approach will stop as long as two sessions are changing the same field.

Yes if you changed the same attribute of the same entity as another user but you set it to a different value then we call that a "conflict" and don't let you rebase otherwise you'd overwrite the other user's change. You need to make a conscious decision at this point, whether or not you want to keep your change or keep the other user's change.

Now you basically leave the second user to "discard" this change, and they will just re-type the commands again. So if they do that, this just means it's the "rebase" to head approach I meant above. Hence I'm not sure why can't we automatically rebase to head and re-apply the commands.

Are you saying it's a hassle to resolve conflicts you have to start your session over from scratch and re-apply the commands? If yes, that's true, today you'd have to do that (well, that's one way of resolving the conflicts, although you would need to decide for the conflicting areas whether you want to preserve your change or the other change). We could add a subcommand as a shortcut that resolves all the conflicts using either your changes or the other users changes (basically the equivalent of git checkout --ours and git checkout --theirs). Again, doing this by merging JSON trees is much simpler than by re-running CLI commands.

Summary: When reviewing D95099578 (#832), I noticed our internal on-diff signals complained about the python code formatting issue: ``` This code is not well-formatted. See https://fburl.com/fbsource-linters#black Lint code: BLACK Lint name: format To apply all changes to your local repo, run this command:jf apply --suggested --version 347129055 --all -from cli_test_lib import ( - find_first_eth_interface, - get_fboss_cli, - get_interface_info, -) +from cli_test_lib import find_first_eth_interface, get_fboss_cli, get_interface_info ``` Not so sure why existing `black-pre-commit-mirror` didn't work, I decided to use `ruff-check` for any python files in fboss/ now fboss-distro already uses ruff.toml but only checks files in `^fboss-image/distro_cli/` Now I make a universal ruff.toml using the Meta standard (suggested by Metamate) Reviewed By: KevinYakar Differential Revision: D95140581 fbshipit-source-id: 77f01ca4e45eec7d636ec2059d0f8f940803a59f

meta-codesync · 2026-03-05T16:46:28Z

@joseph5wu merged this pull request in 762c8eb.

Summary: Add new fboss2-dev CLI commands for managing static MAC address entries on VLANs: - `config vlan <vlan-id> static-mac add <mac-address> <port-id>` - `config vlan <vlan-id> static-mac delete <mac-address>` These commands allow operators to configure static MAC address entries in the FBOSS switch configuration, which is useful for scenarios where MAC addresses need to be pinned to specific ports. Features: * VLAN ID validation (1-4094 range) * MAC address format validation * Port existence validation with automatic logical ID resolution * Duplicate entry detection for add operations * Integration with ConfigSession for proper config management Unit tests: ``` /var/FBOSS/tmp_bld_dir/build/fboss/fboss2_cmd_config_test --gtest_filter='CmdConfigVlanStaticMacTestFixture.*' Output: [==========] Running 31 tests from 1 test suite. ... [ PASSED ] 31 tests. ``` Note: this change is part of a series, the previous one is #832, the next one is #838. ## Sample usage ``` $ fboss2-dev config vlan 2001 static-mac add 02:00:00:E2:E2:01 eth1/1/1 Successfully added static MAC entry: MAC 02:00:00:E2:E2:01 -> VLAN 2001, Port eth1/1/1 (ID 9) $ fboss2-dev config session commit Config session committed successfully as r77 and config reloaded. $ fboss2-dev config vlan 2001 static-mac delete 02:00:00:E2:E2:01 Successfully deleted static MAC entry: MAC 02:00:00:E2:E2:01 from VLAN 2001 ``` Original change by manoharan-nexthop Pull Request resolved: #837 Reviewed By: srikrishnagopu Differential Revision: D95454566 Pulled By: joseph5wu fbshipit-source-id: fb224affbe826d0d13f0e8011a28b408983c9d04

meta-cla bot added the CLA Signed label Jan 23, 2026

This was referenced Jan 26, 2026

[Nexthop][fboss2-dev] Replace file-based config versioning with Git #825

Closed

[Nexthop][fboss2-dev] Add CLI command for managing static MAC entries on VLANs #837

Closed

benoit-nexthop force-pushed the fboss2-cli-prototype_part16 branch 2 times, most recently from 80c535a to e91d0a0 Compare January 28, 2026 18:48

srikrishnagopu added the nexthop label Feb 3, 2026

benoit-nexthop force-pushed the fboss2-cli-prototype_part16 branch from e91d0a0 to 5924d74 Compare March 3, 2026 21:26

benoit-nexthop requested review from a team as code owners March 3, 2026 21:26

joseph5wu reviewed Mar 4, 2026

View reviewed changes

meta-codesync bot closed this in 762c8eb Mar 5, 2026

facebook-github-bot added the Merged label Mar 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Nexthop][fboss2-dev] Add `config session rebase` command for concurrent session conflict resolution#832

[Nexthop][fboss2-dev] Add `config session rebase` command for concurrent session conflict resolution#832
benoit-nexthop wants to merge 1 commit intofacebook:mainfrom
nexthop-ai:fboss2-cli-prototype_part16

benoit-nexthop commented Jan 23, 2026 •

edited

Loading

Uh oh!

facebook-github-bot commented Mar 3, 2026

Uh oh!

joseph5wu left a comment

Uh oh!

joseph5wu Mar 3, 2026

Uh oh!

benoit-nexthop Mar 4, 2026

Uh oh!

joseph5wu Mar 3, 2026

Uh oh!

joseph5wu Mar 3, 2026

Uh oh!

Uh oh!

joseph5wu Mar 3, 2026

Uh oh!

benoit-nexthop Mar 4, 2026

Uh oh!

meta-codesync bot commented Mar 4, 2026

Uh oh!

benoit-nexthop commented Mar 4, 2026

Uh oh!

meta-codesync bot commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		// User2 tries to commit but fails due to stale base
		EXPECT_THROW(session2.commit(localhost()), std::runtime_error);

Conversation

benoit-nexthop commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Sample usage

Uh oh!

facebook-github-bot commented Mar 3, 2026

Uh oh!

joseph5wu left a comment

Choose a reason for hiding this comment

Uh oh!

joseph5wu Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

benoit-nexthop Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

joseph5wu Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

joseph5wu Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joseph5wu Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

benoit-nexthop Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

meta-codesync bot commented Mar 4, 2026

Uh oh!

benoit-nexthop commented Mar 4, 2026

Uh oh!

meta-codesync bot commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benoit-nexthop commented Jan 23, 2026 •

edited

Loading