Skip to content

Conversation

@benoit-nexthop
Copy link
Contributor

@benoit-nexthop benoit-nexthop commented Jan 23, 2026

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:

  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

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

1. Some of the configuration commands like QoS buffer pool modification
requires an agent restart and some are hitless. Inorder to take
appropriate action during the commit, track the highest impact action by
storing the same in `$HOME/.fboss2/action_level` at the end of each
configuration change. The proposed change only prints a warning if the
action needs to be taken. Later this can be modified to perform the
action automatically

2. Add QoS buffer pool configuration commands
```
- fboss2 config qos buffer-pool <name> shared-bytes <value>
- fboss2 config qos buffer-pool <name> headroom-bytes <value>
- fboss2 config qos buffer-pool <name> reserved-bytes <value>
```

1. Updated existing UT for action information
2. Added new tests for QoS buffer bool configurations.

```
[admin@fboss101 ~]$ ~/benoit/fboss2-dev config qos buffer-pool testpool headroom-bytes 1024
Successfully set headroom-bytes for buffer-pool 'testpool' to 1024
[admin@fboss101 ~]$ ~/benoit/fboss2-dev config session diff
--- current live config
+++ session config
@@ -121,6 +121,12 @@
     "arpAgerInterval": 5,
     "arpRefreshSeconds": 20,
     "arpTimeoutSeconds": 60,
+    "bufferPoolConfigs": {
+      "testpool": {
+        "headroomBytes": 1024,
+        "sharedBytes": 0
+      }
+    },
     "clientIdToAdminDistance": {
       "0": 20,
       "1": 1,
[admin@fboss101 ~]$ ll ~/.fboss2
total 216
-rw-r--r-- 1 admin admin 213283 Jan 13 05:15 agent.conf
-rw-r--r-- 1 admin admin     42 Jan 13 05:15 conf_metadata.json
[admin@fboss101 ~]$ cat ~/.fboss2/conf_metadata.json
{
  "action": {
    "WEDGE_AGENT": "AGENT_RESTART"
  }
}
```
@benoit-nexthop benoit-nexthop force-pushed the fboss2-cli-prototype_part16 branch from 8aad3e0 to 80c535a Compare January 28, 2026 17:23
…mand.

This doesn't yet automatically create the VLAN if it doesn't exist.
A recent merge introduced a duplicate command by mistake (bad merge on
my part) and this escaped because of lack of test coverage.

Also make sure we keep `cmake/CliFboss2Test.cmake` sorted.
Now every config command is saved in the CLI session metadata so we can
easily tell what commands were used in a given session. The metadata is
now also saved along the config when we commit the session. A future
commit will make rollback also rely on this metadata to decide whether
or not to restart the agent.
Replace the basic file-based configuration versioning mechanism with
Git-based versioning for the CLI config session.

Key changes:

- Add new Git class (Git.h/cpp) providing a simple interface for Git
  operations: init, commit, log, show, resolveRef, getHead, hasCommits
- 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

Test updates:
- Update all CLI config tests to use Git-based setup
- Initialize Git repo and create initial commit in test fixtures
…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
@benoit-nexthop benoit-nexthop force-pushed the fboss2-cli-prototype_part16 branch from 80c535a to e91d0a0 Compare January 28, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants