[Nexthop][fboss2-dev] Harden fboss2 CLI integration tests #1368
Open
vybhav-nexthop wants to merge 2 commits into
Open
[Nexthop][fboss2-dev] Harden fboss2 CLI integration tests #1368vybhav-nexthop wants to merge 2 commits into
vybhav-nexthop wants to merge 2 commits into
Conversation
vybhav-nexthop
changed the title
…rmboot timing Make the fboss2-dev CLI integration tests robust across hardware variants instead of assuming a fixed L2 VLAN topology and an always-ready agent: - Fboss2IntegrationTest: findFirstEthInterfaceWithMtu() only returns interfaces that are UP, so tests don't pick an interface the agent is still bringing up. - ConfigInterfaceMtuTest: wait for the agent to be ready after committing an MTU change; skip the kernel-MTU verification when the interface has no VLAN (routed-only platforms) instead of asserting. - ConfigVlanCreateTest: discover an existing VLAN and port member dynamically in SetUpTestSuite (cached in static members) instead of hard-coding VLAN 2001; skip on pure routed-mode DUTs where no VLAN with a port member exists. - ConfigVlanDefaultTest: auto-create the target VLAN if absent before the happy-path test so the test works on switches without a pre-provisioned VLAN. - ConfigPfcTest: snapshot the full running config in SetUp and restore the entire SwitchConfig in TearDown via ConfigSession, instead of a per-field PFC disable. The buffer-pool / priority-group-policy / port-PFC objects have no `delete` CLI, so a per-field revert is impossible; leftover PFC state made a later test's hitless config apply crash the agent (EventBase re-entrancy on gracefulExit). The restore is committed with a forced AGENT_WARMBOOT rather than the default hitless apply, which is the crashing path.
The interface selector for the MTU test required intf.status == "up" in addition to a configured MTU. This over-constrains selection: the MTU set path operates on the L3 config (show interface reports MTU from SwitchState, not the datapath), so an interface that is administratively or link down but has a configured MTU is still fully testable. Requiring UP could therefore skip the test entirely on a switch whose configured L3 interfaces happen to have down ports, silently losing coverage. Agent-readiness is already guaranteed by the waitForAgentReady() call in SetUp() (the agent rejects show interface until fully configured), so the UP check is not needed to avoid the warmboot-timing races it was originally paired with. Keep the mtu > 0 requirement (ensures a cfg::Interface exists to update) and update the skip message accordingly.
rabbit-nexthop
force-pushed
the
fboss2-integration-test-robustness
branch
from
5c0a25f to
44009db
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
1 participant
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
Harden the fboss2-dev CLI integration tests to pass across hardware variants, instead of assuming a fixed L2 VLAN (VLAN 2001) topology and an always-ready agent. On routed-mode devices and right after an agent warmboot the old tests crashed the agent or failed spuriously.
What
vlanssection; restore usesconfig vlan default(notstatic-mac add, which would permanently attach acfg::Interfaceto the default VLAN and break the suite's refusal test).SwitchConfiginSetUpand restore it inTearDownviaConfigSession, replacing the per-field PFC disable (buffer-pool / priority-group / port-PFC objects have nodeleteCLI). Leftover PFC state made a later hitless apply crash the agent (EventBase re-entrancy ongracefulExit); the restore forcesAGENT_WARMBOOTto avoid that path.Known limitations
ConfigPfcTest::TearDownforcesAGENT_WARMBOOTunconditionally, so every test in that fixture pays an agent restart even when nothing was mutated.Test Plan