bgp: enable lma/uma topologies for bgp tests

[wenyiz2021]

• Add 'lma'/'uma' topology markers to Tier 2 BGP tests so they run on the single-node mgmt-aggregator testbeds.
• test_bgp_peer_shutdown: add Lower/UpperMgmtAggregator branch that adds the test neighbor to FRR via vtysh when running under BGP confederation.

Description of PR

Summary:
Fixes # (issue)

Type of change

• Bug fix
• Testbed and Framework(new/improvement)
• New Test case
  • Skipped for non-supported platforms
• Test case improvement

Back port request

• 202311
• 202405
• 202411
• 202505
• 202511
• 202512
• 202605

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO):
Failure type:

Tested branch

• master
• 202311
• 202405
• 202411
• 202505
• 202511
• 202512
• 202605
• N/A

Test result

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

run on both lma and uma topology

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Enable running existing BGP/FRR test coverage on single-node management-aggregator topologies by expanding pytest topology markers and adjusting BGP neighbor setup logic for mgmt-aggregator DUT roles.

Changes:

• Add lma/uma to pytest topology markers across multiple BGP/FRR test modules.
• Extend test_bgp_peer_shutdown setup logic to handle LowerMgmtAggregator / UpperMgmtAggregator DUT types (including vtysh-based neighbor configuration under confederation).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/bgp/test_frr_config_check.py	Allow FRR config checks/stress to run on lma/uma by expanding topology markers.
tests/bgp/test_bgp_update_replication.py	Expand supported topologies to include lma/uma.
tests/bgp/test_bgp_stress_link_flap.py	Expand supported topologies to include lma/uma.
tests/bgp/test_bgp_session.py	Expand supported topologies to include lma/uma.
tests/bgp/test_bgp_session_flap.py	Expand supported topologies to include lma/uma.
tests/bgp/test_bgp_peer_shutdown.py	Expand supported topologies to include lma/uma and add mgmt-aggregator DUT-type handling for neighbor setup.
tests/bgp/test_bgp_command.py	Expand supported topologies to include lma/uma.

[yejianquan]

Reviewed together with #26639, #26723, #26724 and #26742 as one dependency chain. The marker additions themselves look right, but there are two things worth resolving before this merges.

1. test_bgp_stress_link_flap will silently degrade on lma/uma — constants.py still has no entries

tests/common/helpers/constants.py defines UPSTREAM_NEIGHBOR_MAP / DOWNSTREAM_NEIGHBOR_MAP (and the *_ALL_* variants) for every topo type — t0, t1, t2, m0, m1, mx, lt2, ft2, c0, lrh, urh — but not lma / uma. This PR adds the lma/uma marker to test_bgp_stress_link_flap.py, which calls:

upstream_nbr_type = get_upstream_neigh_type(tbinfo, is_upper=True)   # tests/bgp/test_bgp_stress_link_flap.py:102

get_upstream_neigh_type() returns None when the topo isn't in the map. get_dut_listen_range() then decides whether a neighbor is upstream via two checks, and on these topologies both are false:

if 'spine' in v.get('properties', []):            # line 116 — topo uses 'core', not 'spine'
elif upstream_nbr_type and upstream_nbr_type in k.upper():   # line 120 — short-circuits on None

So ipv4_subnet / ipv6_subnet stay None and spine_bp_addr stays empty, and those Nones are handed to setup_bgp_sentinel(). Nothing raises — the test just runs with no listen range, so the result is meaningless rather than red.

Suggest adding the entries in this PR:

UPSTREAM_NEIGHBOR_MAP:        "uma": "rwa",        "lma": "uma"
UPSTREAM_ALL_NEIGHBOR_MAP:    "uma": ["rwa"],      "lma": ["uma"]
DOWNSTREAM_NEIGHBOR_MAP:      "uma": "lma",        "lma": "m2, m3"
DOWNSTREAM_ALL_NEIGHBOR_MAP:  "uma": ["lma", "m1"], "lma": ["m2", "m3"]

Related nearby landmine that the same change would defuse: tests/conftest.py:2703 does get_downstream_neigh_type(...).split(',') with no None guard (and the if downstream_nbr_type is None check on the next line is dead, since the value is already a list by then). enum_downstream_dut_hostname isn't used by any of the seven tests in this PR, so it doesn't blow up today — but it will AttributeError as soon as lma/uma coverage widens.

2. The UpperMgmtAggregator mapping contradicts #26724

Same dut_type → neigh_type chain, same author, two different answers:

dut_type	#26724 (test_bgp_update_timer.py)	this PR (test_bgp_peer_shutdown.py)
LowerMgmtAggregator	MgmtSpineRouter	MgmtSpineRouter
UpperMgmtAggregator	LowerMgmtAggregator	MgmtSpineRouter

Which one is right depends on #26742: as that PR stands today topo_uma.yml sets no device_type, so its LMA neighbors actually render as MgmtSpineRouter (the 'MA' in dev branch in minigraph_png.j2 — details in my comment there), which would make this PR accidentally correct and #26724 wrong. If topo_uma.yml is fixed to declare device_type: LowerMgmtAggregator, it flips and #26724 becomes the correct one.

Either way the two PRs shouldn't land with different mappings. Worth settling #26742 first, then aligning both.

While doing that: this dut_type → neigh_type chain now exists in three copies (test_bgp_update_timer.py, test_bgp_peer_shutdown.py, and the setup in tests/bgp/conftest.py). Pulling it into a single helper in bgp_helpers.py would stop this from diverging again.

3. Comment is inaccurate

# Mgmt aggregators run BGP confederation. The test neighbor is an
# external eBGP peer, so it must be added directly to FRR via vtysh

The M2 neighbors in topo_lma.yml are peer_in_bgp_confed: true with an ASN equal to the DUT's dut_confed_peers — they're confederation peers, not external eBGP. The vtysh part is right; suggest rewording to say the neighbor must be configured through vtysh on confederation topologies (which is the same reason the existing FabricSpineRouter / LowerRegionalHub / UpperRegionalHub branches do it).

Verified — no action needed

I checked all seven tests getting the new marker for uses of NEIGHBOR_MAP, get_eth_port, get_vm_offset, get_upstream_ptf_intfs and bgp_allow_list_setup; only test_bgp_stress_link_flap is affected (via get_upstream_neigh_type, item 1 above). The rest are clean.

PR description

Template is unfilled — no "Type of change" checked and no test result. Since this widens coverage to a new topology, the results from an actual lma and uma run would be the useful part here.

[mssonicbld]

This PR has backport request label(s) for branch(es): msft-202603, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202603: <test result>) in the Test result section as well in your PR description.

_{---Powered by SONiC BuildBot}

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[yejianquan]

LGTM

[mssonicbld]

This PR is approved for branch msft-202603. No cherry pick PR has been created because of code conflict. @wenyiz2021, please manually create the cherry pick PR.
If this change is already in msft-202603, please comment "already in msft-202603". Thanks!

_{---Powered by SONiC BuildBot}

[yejianquan]

Azure/sonic-mgmt.msft#1350