[Nexthop][fboss2-dev] Add config admin-distance CLI command#1302
Open
vybhav-nexthop wants to merge 1 commit into
Open
[Nexthop][fboss2-dev] Add config admin-distance CLI command#1302vybhav-nexthop wants to merge 1 commit into
vybhav-nexthop wants to merge 1 commit into
Conversation
Adds a new write command:
fboss2-dev config admin-distance <client-id> <distance>
Sets the administrative distance for a routing client in the agent
running config via a HITLESS config session.
### What
- New command CmdConfigAdminDistance with AdminDistanceArg validating
two positional args: client-id (non-negative integer) and distance
(0-255)
- Read-modify-write on clientIdToAdminDistance map — all other client
entries survive untouched
- No-op detection — if the value is already set, returns early without
saving
- Registered in kConfigCommandTree() alongside existing config
subcommands
- Wired into CMake (fboss2_config_lib, fboss2_cmd_config_test,
fboss2_integration_test) and BUCK
### Why HITLESS (not in ApplyThriftConfig.cpp)
`clientIdToAdminDistance` is absent from `ApplyThriftConfig.cpp` — it
does not go through the SwitchState delta pipeline and is never
programmed to hardware. Instead, `SwSwitch::clientIdToAdminDistance()`
reads it directly from the live agent config at route-update time (via
`getAdminDistanceForClientId(getConfig(), clientId)` in `Utils.cpp`).
Saving the config is sufficient for the change to take effect on the
next route update, so no agent restart is needed — HITLESS is correct.
# Test Plan
### Unit tests — //fboss/cli/fboss2/test/config:cmd_config_test
[----------] 4 tests from CmdConfigAdminDistanceTestFixture
[ RUN ] CmdConfigAdminDistanceTestFixture.argValidation
[ OK ] CmdConfigAdminDistanceTestFixture.argValidation (85 ms)
[ RUN ] CmdConfigAdminDistanceTestFixture.updateExistingClient
[ OK ] CmdConfigAdminDistanceTestFixture.updateExistingClient (95 ms)
[ RUN ] CmdConfigAdminDistanceTestFixture.addNewClient
[ OK ] CmdConfigAdminDistanceTestFixture.addNewClient (76 ms)
[ RUN ] CmdConfigAdminDistanceTestFixture.alreadySet
[ OK ] CmdConfigAdminDistanceTestFixture.alreadySet (60 ms)
[----------] 4 tests from CmdConfigAdminDistanceTestFixture (318 ms total)
[ PASSED ] 260 tests.
//fboss/cli/fboss2/test/config:cmd_config_test PASSED in 16.4s
### Integration test — ConfigAdminDistanceTest.SetAndRestoreAdminDistance on NH-4010-F
[==========] Running 1 test from 1 test suite.
[ RUN ] ConfigAdminDistanceTest.SetAndRestoreAdminDistance
[ OK ] ConfigAdminDistanceTest.SetAndRestoreAdminDistance (154 ms)
[ PASSED ] 1 test.
I0601 10:01:04] [Step 1] Reading current admin distances...
I0601 10:01:04] client-id 0 -> 20
I0601 10:01:04] [Step 2] Setting client-id 0 to 42...
I0601 10:01:04] Running CLI: config admin-distance 0 42
I0601 10:01:04] Running CLI: config session commit
I0601 10:01:04] [Step 3] Restoring client-id 0 to 20...
I0601 10:01:04] Running CLI: config admin-distance 0 20
I0601 10:01:04] Running CLI: config session commit
I0601 10:01:04] TEST PASSED
e597f21 to
30f92ad
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.
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runSummary
Adds a new write command:
Sets the administrative distance for a routing client in the agent running config via a HITLESS config session.
What
Why HITLESS (not in ApplyThriftConfig.cpp)
clientIdToAdminDistanceis absent fromApplyThriftConfig.cpp— it does not go through the SwitchState delta pipeline and is never programmed to hardware. Instead,SwSwitch::clientIdToAdminDistance()reads it directly from the live agent config at route-update time (viagetAdminDistanceForClientId(getConfig(), clientId)inUtils.cpp). Saving the config is sufficient for the change to take effect on the next route update, so no agent restart is needed — HITLESS is correct.Test Plan
Unit tests — //fboss/cli/fboss2/test/config:cmd_config_test
Integration test — ConfigAdminDistanceTest.SetAndRestoreAdminDistance on NH-4010-F