[202511] bgp: fix test_bgp_vnet setup failure on topologies without a Vnet2 dynamic peer#25172
Open
nsoma-cisco wants to merge 1 commit into
Open
[202511] bgp: fix test_bgp_vnet setup failure on topologies without a Vnet2 dynamic peer#25172nsoma-cisco wants to merge 1 commit into
nsoma-cisco wants to merge 1 commit into
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…namic peer
test_bgp_vnet's setup_vnet fixture failed at setup with:
% Specify remote-as or peer-group commands first
leading to "Vnet testing setup failed". This was reproduced on
t0-isolated-v6-d32u32s2, but the trigger is not IPv6-specific.
setup_vnet_cfg unconditionally ran "neighbor BGPSLBPassive allowas-in 1" in
the Vnet2 VRF, but the BGPSLBPassive dynamic peer-group is only created when an
IPv4 ARISTA03T1 neighbor exists (see the BGP_PEER_RANGE guard in
templates/vnet_config_db.j2). Any topology lacking an IPv4 ARISTA03T1 neighbor
never creates the peer-group, so the vtysh command fails and raises during
setup. This includes:
- IPv6-only topologies (ARISTA03T1, if present, is IPv6 only), and
- topologies whose VM naming has no ARISTA03T1 at all, e.g. the stride-8
t0-isolated-* d32u32 variants (both the v6 and the dual-stack/IPv4 ones).
Standard t0 topologies (t0-64, t0-56, t0-isolated-d128u128s2, ...) do have an
IPv4 ARISTA03T1, which is why sonic-net#24074 passed CI on t0-64 and the regression went
unnoticed.
Guard the allowas-in configuration with the same condition the template uses,
so it only runs when an IPv4 dynamic peer is actually present. This is safe
because the AS-path-loop scenario allowas-in addresses does not apply when there
are no IPv4 dynamic peers.
Fixes sonic-net#25167
Signed-off-by: Nageswara Soma <nsoma@cisco.com>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Description of PR
tests/bgp/test_bgp_vnet.pyfails at thesetup_vnetfixture with:leading to
Failed: Vnet testing setup failed. Reproduced ont0-isolated-v6-d32u32s2, but the trigger is not IPv6-specific (see below).Root cause
setup_vnet_cfgunconditionally runs (added in #24074):But the
BGPSLBPassivedynamic peer-group is only created when an IPv4ARISTA03T1neighbor exists — see theBGP_PEER_RANGEguard intests/bgp/templates/vnet_config_db.j2(data['name'] == 'ARISTA03T1' and ':' not in neigh).Any topology that lacks an IPv4
ARISTA03T1neighbor never writesBGP_PEER_RANGE, sobgpcfgdnever creates the peer-group inVnet2and theallowas-incommand fails with a non-zero return code, raising inside the fixture'stryblock and triggeringpytest.fail("Vnet testing setup failed").This affects more than IPv6-only topologies:
t0-isolated-v6-d32u32s2) —ARISTA03T1, if present, is IPv6 only.ARISTA03T1VM at all — e.g. the stride-8t0-isolated-*d32u32 variants (ARISTA01T1, ARISTA09T1, ARISTA17T1, …). This includes the dual-stack/IPv4t0-isolated-d32u32s2, which has IPv4 neighbors but still noARISTA03T1, so it hits the identical failure.Standard t0 topologies (
t0-64,t0-56,t0-isolated-d128u128s2, …) do have an IPv4ARISTA03T1, which is why #24074 passed CI ont0-64and the regression went unnoticed.Fix
Guard the
allowas-inconfiguration with the same condition the template uses, so it only runs when an IPv4 dynamic peer is actually present. The check keys off the realBGP_NEIGHBORconfig (not the IP family), so it correctly covers both the IPv6-only and the "no ARISTA03T1" cases. This is safe because the AS-path-loop scenarioallowas-inwas added to handle does not apply when there are no IPv4 dynamic peers.This is the 202511 backport of #25171 (master fix for #25167).
Type of change
Approach
What is the motivation for this PR?
Unblock
tests/bgp/test_bgp_vnet.pyon topologies that have no IPv4ARISTA03T1dynamic peer, where the entire module currently fails at fixture setup.How did you do it?
Compute
has_ipv4_dynamic_peerfromcfg_facts['BGP_NEIGHBOR']using the sameARISTA03T1+ non-IPv6 condition the config template uses, and only issue theallowas-invtysh command when it is true.How did you verify/test it?
Logic mirrors the template guard exactly. On topologies without an IPv4
ARISTA03T1the command is skipped (there is no peer-group to configure); on topologies that do have one, behavior is unchanged.Which release branch to backport (provide reason below if selected)
This PR targets 202511 directly as the backport of #25171. Companion to #24074 / #24272, which landed the
allowas-inchange on bothmasterand202511. The same setup failure occurs on202511, so this fix is needed there as well.