[Nexthop][fboss2-dev] Add a very basic prototype of a config session. #756
Conversation
benoit-nexthop
force-pushed
the
fboss2-cli-prototype_part04
branch
2 times, most recently
from
4e3a7a0 to
f8a4f7b
Compare
benoit-nexthop
force-pushed
the
fboss2-cli-prototype_part04
branch
5 times, most recently
from
6337d9c to
76c74c2
Compare
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` This is a small helper class used to lookup ports and interfaces in a consistent fashion in CLI commands. Note: this change is part of a series, the previous one is #754, the next one is #756. Pull Request resolved: #755 Test Plan: Unit tests. Reviewed By: KevinYakar Differential Revision: D89903145 Pulled By: joseph5wu fbshipit-source-id: e36f8198ed19bb4141285b56da93632f8d660a06
benoit-nexthop
force-pushed
the
fboss2-cli-prototype_part04
branch
from
76c74c2 to
7765776
Compare
|
@joseph5wu has imported this pull request. If you are a Meta employee, you can view this in D90088260. |
joseph5wu
left a comment
joseph5wu
left a comment
There was a problem hiding this comment.
Please address the linter issues, especially the std::regex one as it's a hard stop for us to land any diff using the std::regex instead of RE2.
I still have some concerns about the session revision and username.
IIRC, in your original design, you want to maintain a version metadata, which at least include some useful info like timestamp, or the exact commands before the commit and etc.
Without such metadata, it will be difficult for anyone to locate which revision to rollback. Sure, I can use git diff or diff to compare agent-r4 and agent-r5. But the point to introduce the config cli is to use more user-friendly commands instead of "understanding" the details of our FBOSS services' config.
Besides, the current 100K revision limitation and the unique username might not work well in long term.
I think it's OKAY for this PR as basic prototype, but let's discuss a long term solution
fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.cpp
Outdated
Show resolved
Hide resolved
| // Try up to 100000 revision numbers to handle concurrent commits | ||
| // In practice, we should find one quickly | ||
| for (int revision = 1; revision <= 100000; ++revision) { |
There was a problem hiding this comment.
This looks kinda hacky with the magic number 100K here.
If we are using the cli in any automation tests, 100K might be quickly exhausted.
We need to consider a long term solution to deal with the limit of revision.
There was a problem hiding this comment.
This will change soon, when we switch over to storing the commits in git.
| "This likely indicates a problem with the filesystem or too many revisions."); | ||
| } | ||
|
|
||
| std::string getUsername() { |
There was a problem hiding this comment.
In Meta, we limit most of users/developers to use netops with specific(limited) permission on production FBOSS devices. While in our lab, we allow any user to log in the device as root.
In either case, the limitation of per session per user might not be very useful/valid since anyone can log in as either netops or root.
Even for vendors to provision a FBOSS lab/test device, I would say creating specific users might be the last priority.
We probably need to consider managing our session without the assumption of the unique username
There was a problem hiding this comment.
Will rework this in a follow up change.
This can handle only one session per user, the current live config is copied into `~/.fboss2/agent.conf` and when committing an atomic symlink update is performed on /etc/coop/agent.conf, which is the only config file supported by this basic prototype.
benoit-nexthop
force-pushed
the
fboss2-cli-prototype_part04
branch
from
7765776 to
7c1c6c0
Compare
|
@benoit-nexthop has updated the pull request. You must reimport the pull request before landing. |
|
@joseph5wu merged this pull request in a6be413. |
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` This adds just the command `fboss2-dev config session diff`. Note: this change is part of a series, the previous one is #756, the next one is #758. Pull Request resolved: #757 Test Plan: Unit tests. ## Sample usage ``` [admin@fboss101 benoit]$ ./fboss2-dev show interface | head +-----------+--------+-------+------+------+------------------------------+----------------------------+ | Interface | Status | Speed | VLAN | MTU | Addresses | Description | -------------------------------------------------------------------------------------------------------- | eth1/1/1 | down | 800G | 2001 | 1500 | 10.0.0.0/24 | Test port for diff command | | | | | | | 2400::/64 | | | | | | | | fe80::b4db:91ff:fe95:ff07/64 | | +-----------+--------+-------+------+------+------------------------------+----------------------------+ | eth1/2/1 | down | 200G | 2003 | 9216 | 11.0.0.0/24 | Another test description | | | | | | | 2401::/64 | | | | | | | | fe80::b4db:91ff:fe95:ff07/64 | | [admin@fboss101 benoit]$ ./fboss2-dev config interface eth1/1/1 mtu 9000 Successfully set MTU for interface(s) eth1/1/1 to 9000 [admin@fboss101 benoit]$ ./fboss2-dev config interface eth1/2/1 description 'This is a test.' Successfully set description for interface(s) eth1/2/1 [admin@fboss101 benoit]$ ./fboss2-dev config session diff --- /etc/coop/agent.conf 2025-11-05 12:49:14.497415902 -0800 +++ /home/admin/.fboss2/agent.conf 2025-11-05 12:57:27.123758309 -0800 @@ -369,7 +369,7 @@ ], "isStateSyncDisabled": true, "isVirtual": false, - "mtu": 1500, + "mtu": 9000, "routerID": 0, "scope": 0, "type": 1, @@ -2230,7 +2230,7 @@ }, { "conditionalEntropyRehash": false, - "description": "Another test description", + "description": "This is a test.", "drainState": 0, "expectedLLDPValues": { "2": "eth1/6/1" ``` ``` [admin@fboss101 benoit]$ ./fboss2-dev config history Revision Owner Commit Time ------------------------------------------ r1 root 2025-11-05 13:26:58 r2 root 2025-11-05 13:27:35 r3 root 2025-11-05 13:28:13 r4 admin 2025-11-05 14:37:51 r5 admin 2025-11-05 14:38:31 ``` You can also do `fboss2-dev config diff r2 r4` or `fboss2-dev config diff r1 current` to compare committed revisions. Reviewed By: shiva-menta Differential Revision: D90277033 Pulled By: joseph5wu fbshipit-source-id: 5fcdac6c2e01c55169d68a8f5d227da241d8b65e
3 participants
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runSummary
This can handle only one session per user, the current live config is
copied into
~/.fboss2/agent.confand when committing an atomic symlinkupdate is performed on /etc/coop/agent.conf, which is the only config
file supported by this basic prototype.
Note: this change is part of a series, the previous one is #755, the next one is #757.
Test Plan
Unit tests.