Skip to content

[202605][bgp]: Skip test_bgp_suppress_fib module on 202605 (suppress-fib dynamic-toggle race)#26270

Merged
StormLiangMS merged 1 commit into
sonic-net:202605from
deepak-singhal0408:bgp-suppress-fib-skip-202605
Jul 24, 2026
Merged

[202605][bgp]: Skip test_bgp_suppress_fib module on 202605 (suppress-fib dynamic-toggle race)#26270
StormLiangMS merged 1 commit into
sonic-net:202605from
deepak-singhal0408:bgp-suppress-fib-skip-202605

Conversation

@deepak-singhal0408

@deepak-singhal0408 deepak-singhal0408 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Description of PR

202605 backport of sonic-mgmt #26176.

Summary:
Skip the whole bgp/test_bgp_suppress_fib.py module on the 202605 release branch. The module
toggles suppress-fib-pending dynamically at runtime, causing an orchagent/fpmsyncd/FRR race where
routes are intermittently not offloaded in FRR / not propagated to the upstream VM. The product
fix that makes suppress-fib-pending static (config-reload only) is master-only and is not
backported to 202605
, so the module is skipped for the life of the branch.
Tracked by sonic-mgmt issue #26175.

Holistic context — two master PRs touched this block, neither is on 202605:

  • #25598 — un-skipped select VS control-plane
    cases (refactored the whole-file VS skip into per-case skips).
  • #26176 — then skipped the whole module
    for the dynamic-toggle race (auto-lifts on master once the product fix lands + issue #26175 closes).

On 202605 the product fix will not land. Per review feedback, this PR sets the
bgp/test_bgp_suppress_fib.py: skip block byte-identical to master (the #26176 form)
so the file stays consistent across branches and future edits cherry-pick cleanly. We still do
not backport #25598's VS re-scope — the whole-module skip makes its per-case VS key inert on 202605.

Type of change

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

Back port request

  • 202205
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511

(This is the 202605 backport PR — base branch 202605.)

Approach

What is the motivation for this PR?

Each release branch's nightly runs its own sonic-mgmt checkout. Without this skip,
test_bgp_suppress_fib is a persistent nightly-noise source on 202605 (the dynamic-toggle race with
no product fix on this branch).

How did you do it?

A literal git cherry-pick of c859f53 (master #26176) does not apply cleanly,
because #26176 is stacked on
#25598 (which reshaped this block on master) and #25598 was never
backported to 202605. So #26176's content is applied by hand: the
bgp/test_bgp_suppress_fib.py: skip block is set identical to master:

bgp/test_bgp_suppress_fib.py:
  skip:
    reason: "(1) Not supported before release 202411.
             (2) suppress-fib dynamic-toggle offload/propagation race: the module toggles
             suppress-fib-pending dynamically at runtime, causing an orchagent/fpmsyncd/FRR race
             where routes are intermittently not offloaded in FRR / not propagated to the upstream VM.
             The product fix that makes suppress-fib static (sonic-swss#4333 + sonic-mgmt#22916) is
             merging to master only and is not backported. Therefore master auto-lifts once the fix
             lands and the tracking issue is closed, while the release branches 202505/202511/202605
             never receive the fix and stay skipped for the life of the branch.
             Tracked by https://github.com/sonic-net/sonic-mgmt/issues/26175"
    conditions_logical_operator: or
    conditions:
      - "release in ['201811', '201911', '202012', '202205', '202211', '202305', '202311', '202405']"
      - "release in ['202505', '202511', '202605']"
      - "release in ['master'] and https://github.com/sonic-net/sonic-mgmt/issues/26175"

On a 202605 checkout the release in ['202505', '202511', '202605'] condition matches, so the whole
module is skipped on all ASICs/topologies; the 'master' + #26175-gated
auto-lift line is inert here (never lifts on 202605, which is the intent). Keeping the block identical
to master addresses the review concern about cross-branch consistency / future cherry-pick conflicts.

Note: #25598's VS re-scope (a separate
bgp/test_bgp_suppress_fib.py::test_bgp_route_with_suppress: per-case key) is intentionally not
backported — it would be inert under the whole-module skip on 202605. As a result this region is not
100% byte-identical to master (master carries that extra per-case key), so an edit touching that exact
boundary could still need a manual resolution; the skip conditions themselves now match master.

How did you verify/test it?

  • tests_mark_conditions.yaml parses cleanly (yaml.safe_load) and the bgp/test_bgp_suppress_fib.py:
    block is verified byte-identical to master.
  • The skip condition release in ['202505', '202511', '202605'] evaluates True on a 202605
    checkout → whole module skipped on all ASICs/topologies.

Any platform specific information?

None — the whole-module skip is keyed on release only (all ASICs/topologies).

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

N/A — not a new test case; whole-module skip only.

Documentation

N/A

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@github-actions

Copy link
Copy Markdown

@StormLiangMS @wangxin @yxieca A user wants to merge changes to the conditional mark files into 202605. Please review.

@azure-pipelines

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

@deepak-singhal0408
deepak-singhal0408 force-pushed the bgp-suppress-fib-skip-202605 branch from c5d3ecd to 0d3244d Compare July 16, 2026 21:06
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@deepak-singhal0408 deepak-singhal0408 self-assigned this Jul 16, 2026
@deepak-singhal0408 deepak-singhal0408 changed the title [202605][bgp]: Skip test_bgp_suppress_fib for suppress-fib dynamic-toggle race (#26175) [202605][bgp]: Skip test_bgp_suppress_fib module on 202605 (suppress-fib dynamic-toggle race) Jul 16, 2026
Comment thread tests/common/plugins/conditional_mark/tests_mark_conditions.yaml Outdated
@deepak-singhal0408
deepak-singhal0408 force-pushed the bgp-suppress-fib-skip-202605 branch from 0d3244d to 0c1d1e3 Compare July 17, 2026 01:56
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@StormLiangMS

Copy link
Copy Markdown
Collaborator

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1167282:

✅Stage Test:

  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.

@StormLiangMS StormLiangMS left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@StormLiangMS

Copy link
Copy Markdown
Collaborator

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1167282:

✅Stage Test:

  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.

StormLiangMS pushed a commit that referenced this pull request Jul 22, 2026
…n unstamped images (#26383)

### Description of PR
Summary:
On images without `/etc/sonic/sonic_release` (all VS/KVM and self-built
images), the `release` fact produced by
`ansible/library/dut_basic_facts.py` is guessed from `build_version`.
The old fallback only recognized `201811`, `201911`, and `master`, so
every other release (`202012` … `202605`) collapsed to `"unknown"` —
even though `build_version` starts with the release token (e.g.
`202605.1166406-...`).

Because the `release` fact is what the `conditional_mark` plugin
evaluates, release-based skips such as `release in ['202605', ...]`
silently never matched on VS, so tests intended to be skipped there ran
anyway. This surfaced while triaging the CI failure on
#26270. This PR fixes the
fallback so `release` resolves to the real `YYYYMM` release on
unstamped/VS images.

### Type of change
- [ ] Bug fix
- [x] Testbed and Framework(new/improvement)
- [ ] New Test case
    - [ ] Skipped for non-supported platforms
- [ ] Test case improvement

### Back port request
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505
- [ ] 202511

### Approach
#### What is the motivation for this PR?
Make `release`-based `conditional_mark` conditions work correctly on
VS/KVM (and any self-built) images. Today they can never match because
`release` resolves to `"unknown"` on those images, which makes
release-gated skips a silent no-op on VS.

#### How did you do it?
Extracted the release resolution into a pure, dependency-free helper
`ansible/module_utils/sonic_release_utils.py::guess_release_from_build_version()`:
an already-stamped `release` (real HW) is returned unchanged; otherwise
a leading `YYYYMM` token is extracted via `re.match(r'(20\d{4})', ...)`
(`202605.1166406-...` -> `202605`; old date-stamped `20181130.31` ->
`201811`); `master` in `build_version` -> `master`; anything else ->
`unknown`. `ansible/library/dut_basic_facts.py` now imports and calls
the helper (mirroring the existing `parse_utils` import guard),
replacing the hard-coded 201811/201911/master ladder.

#### How did you verify/test it?
Added `ansible/module_utils/test_sonic_release_utils.py` — 62
parametrized cases covering all modern releases, old date-stamped
images, `master`, precedence of an already-stamped release, unparseable
-> `unknown`, build-number-not-confused, and a regression guard that
`202605` no longer collapses to `unknown`: `python3 -m pytest
--noconftest ansible/module_utils/test_sonic_release_utils.py` -> 62
passed. `flake8 --max-line-length=120` clean; `py_compile` OK.

#### Any platform specific information?
Only affects the `release` fact derivation for images lacking
`/etc/sonic/sonic_release` (VS/KVM, self-built). Real-HW images that
stamp `release` are unchanged. Old-release exclusion lists are
unaffected; the intended effect is that current-release skips now
correctly fire on VS.

#### Supported testbed topology if it's a new test case?
N/A (framework fix).

### Documentation
No.

Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@StormLiangMS

Copy link
Copy Markdown
Collaborator

/azpw retry

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) jobs...

@mssonicbld

Copy link
Copy Markdown
Collaborator

Retrying failed(or canceled) stages in build 1167282:

✅Stage Test:

  • Job impacted-area-kvmtest-t2 by Elastictest: retried.
  • Job impacted-area-kvmtest-t1-lag by Elastictest: retried.

mssonicbld added a commit that referenced this pull request Jul 23, 2026
…from build_version on unstamped images (#26445)

### Description of PR
Summary:
On images without `/etc/sonic/sonic_release` (all VS/KVM and self-built
images), the `release` fact produced by
`ansible/library/dut_basic_facts.py` is guessed from `build_version`.
The old fallback only recognized `201811`, `201911`, and `master`, so
every other release (`202012` … `202605`) collapsed to `"unknown"` —
even though `build_version` starts with the release token (e.g.
`202605.1166406-...`).

Because the `release` fact is what the `conditional_mark` plugin
evaluates, release-based skips such as `release in ['202605', ...]`
silently never matched on VS, so tests intended to be skipped there ran
anyway. This surfaced while triaging the CI failure on
#26270. This PR fixes the
fallback so `release` resolves to the real `YYYYMM` release on
unstamped/VS images.

### Type of change
- [ ] Bug fix
- [x] Testbed and Framework(new/improvement)
- [ ] New Test case
 - [ ] Skipped for non-supported platforms
- [ ] Test case improvement

### Back port request
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505
- [ ] 202511

### Approach
#### What is the motivation for this PR?
Make `release`-based `conditional_mark` conditions work correctly on
VS/KVM (and any self-built) images. Today they can never match because
`release` resolves to `"unknown"` on those images, which makes
release-gated skips a silent no-op on VS.

#### How did you do it?
Extracted the release resolution into a pure, dependency-free helper
`ansible/module_utils/sonic_release_utils.py::guess_release_from_build_version()`:
an already-stamped `release` (real HW) is returned unchanged; otherwise
a leading `YYYYMM` token is extracted via `re.match(r'(20\d{4})', ...)`
(`202605.1166406-...` -> `202605`; old date-stamped `20181130.31` ->
`201811`); `master` in `build_version` -> `master`; anything else ->
`unknown`. `ansible/library/dut_basic_facts.py` now imports and calls
the helper (mirroring the existing `parse_utils` import guard),
replacing the hard-coded 201811/201911/master ladder.

#### How did you verify/test it?
Added `ansible/module_utils/test_sonic_release_utils.py` — 62
parametrized cases covering all modern releases, old date-stamped
images, `master`, precedence of an already-stamped release, unparseable
-> `unknown`, build-number-not-confused, and a regression guard that
`202605` no longer collapses to `unknown`: `python3 -m pytest
--noconftest ansible/module_utils/test_sonic_release_utils.py` -> 62
passed. `flake8 --max-line-length=120` clean; `py_compile` OK.

#### Any platform specific information?
Only affects the `release` fact derivation for images lacking
`/etc/sonic/sonic_release` (VS/KVM, self-built). Real-HW images that
stamp `release` are unchanged. Old-release exclusion lists are
unaffected; the intended effect is that current-release skips now
correctly fire on VS.

#### Supported testbed topology if it's a new test case?
N/A (framework fix).

### Documentation
No.

Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com>
Signed-off-by: mssonicbld <sonicbld@microsoft.com>
Co-authored-by: Deepak Singhal <115033986+deepak-singhal0408@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…fib dynamic-toggle race)

Backport of the whole-module skip from sonic-net#26176 to the
202605 release branch.

The bgp/test_bgp_suppress_fib.py module toggles suppress-fib-pending
dynamically at runtime, causing an orchagent/fpmsyncd/FRR race where routes
are intermittently not offloaded in FRR / not propagated to the upstream VM.
The product fix that makes suppress-fib static (sonic-swss#4333 +
sonic-mgmt#22916) is master-only and is NOT backported to 202605, so the whole
module is skipped for the life of the branch.

To keep this file consistent with master (and avoid future cherry-pick
conflicts flagged in review), the skip block is set byte-identical to master's
sonic-net#26176 form rather than a branch-local condition. On a 202605 checkout the
'release in ["202505", "202511", "202605"]' condition matches, so the whole
module is skipped. sonic-net#25598's VS re-scope is intentionally not backported -- the
whole-module skip makes its per-case VS key inert on 202605.

Tracked by sonic-net#26175

Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 566f78e6-4a5f-4240-ac6e-9e38d1662b54
@deepak-singhal0408
deepak-singhal0408 force-pushed the bgp-suppress-fib-skip-202605 branch from 0c1d1e3 to 1720955 Compare July 23, 2026 04:19
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

@deepak-singhal0408

Copy link
Copy Markdown
Contributor Author

@StormLiangMS could you help merge this PR.

@StormLiangMS
StormLiangMS merged commit c854c49 into sonic-net:202605 Jul 24, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants