Inform user that reload is required after suppress-fib-pending change#4361
Open
mike-dubrovsky wants to merge 1 commit into
Open
Inform user that reload is required after suppress-fib-pending change#4361mike-dubrovsky wants to merge 1 commit into
mike-dubrovsky wants to merge 1 commit into
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced Mar 16, 2026
deepak-singhal0408
self-requested a review
April 6, 2026 06:32
mike-dubrovsky
force-pushed
the
fib-suppression
branch
from
May 14, 2026 07:14
6101a45 to
a570196
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mike-dubrovsky
changed the title
mike-dubrovsky
force-pushed
the
fib-suppression
branch
from
July 14, 2026 02:36
a570196 to
ddebeee
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mike-dubrovsky
force-pushed
the
fib-suppression
branch
from
July 14, 2026 07:59
ddebeee to
894de87
Compare
This was referenced Jul 14, 2026
| config_db = db.cfgdb_clients[ns] | ||
| config_db.mod_entry('DEVICE_METADATA', 'localhost', {"suppress-fib-pending": state}) | ||
|
|
||
| click.echo("""Wrote %s suppress-fib-pending into CONFIG_DB, reload required to apply the configuration: |
Contributor
There was a problem hiding this comment.
Wording: "reload required to apply the configuration" can be misread as a device reboot. Suggest saying config reload explicitly, which also makes the "doesn't take effect until reload" contract clear at the point of use. Proposed:
suppress-fib-pending set to '<state>' in CONFIG_DB. This does NOT take effect until a config reload.
Persist and apply with:
config save -y
config reload -y
| config_db = db.cfgdb_clients[ns] | ||
| config_db.mod_entry('DEVICE_METADATA', 'localhost', {"suppress-fib-pending": state}) | ||
|
|
||
| click.echo("""Wrote %s suppress-fib-pending into CONFIG_DB, reload required to apply the configuration: |
Contributor
There was a problem hiding this comment.
This notice prints on every invocation, including when state already matches the current CONFIG_DB value — which can misleadingly prompt the operator to reload when nothing changed. Consider reading the current suppress-fib-pending value first and, when it already equals the requested state, skip the write and instead print e.g. suppress-fib-pending is already <state>, no change (reserving the reload notice for an actual change).
mike-dubrovsky
force-pushed
the
fib-suppression
branch
from
July 16, 2026 06:32
894de87 to
c89c37e
Compare
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
StormLiangMS
pushed a commit
to sonic-net/sonic-mgmt
that referenced
this pull request
Jul 16, 2026
#26175) (#26176) ### Description of PR Summary: Fixes #26175 Temporarily skip the whole `bgp/test_bgp_suppress_fib.py` module for the suppress-fib-pending **dynamic-toggle race**. The module toggles `suppress-fib-pending` dynamically at runtime (via the shared `config_bgp_suppress_fib()` helper, no `config reload`). That triggers a race between **orchagent, fpmsyncd and FRR** where routes are intermittently **not marked offloaded in FRR** / **not propagated to the upstream VM** within the wait window, failing essentially every case in the module. The root-cause fix makes `suppress-fib-pending` **static** (takes effect only after a `config reload`), removing the race — [sonic-swss#4333](sonic-net/sonic-swss#4333), [sonic-buildimage#26151](sonic-net/sonic-buildimage#26151), [sonic-utilities#4361](sonic-net/sonic-utilities#4361), [sonic-mgmt#22916](#22916), [SONiC#2335](sonic-net/SONiC#2335). > **The product fix is master-only and is not backported.** Because of that, the skip uses **two different lift semantics**, expressed as separate conditions: > > - **`master`** — gated on the tracking issue: `release in ['master'] and https://github.com/sonic-net/sonic-mgmt/issues/26175`. When the fix lands on master and #26175 is closed, the skip **auto-lifts** on master and coverage returns — no manual edit needed. > - **`202505` / `202511` / `202605`** — a plain `release in [...]` condition with **no issue gate**. These branches never receive the product fix, so the module is skipped for the life of the branch (same nature as the existing pre-202411 "not supported" condition). > > **Per-branch effect still requires cherry-picking this skip.** Each release branch's nightly runs its own `sonic-mgmt` checkout, so this YAML change must be cherry-picked to 202505/202511/202605 for the skip to take effect there. The identical diff matches on each branch via its own `release` token. > > If a release branch ever does receive the product fix, drop that branch's token from the `release in [...]` list — a deliberate per-branch edit. #26175 tracks the lifecycle. ### 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 - [x] 202505 - [x] 202511 (also 202605 — not listed in the template checkboxes; cherry-pick required there as well.) ### Approach #### What is the motivation for this PR? The `test_bgp_suppress_fib` module is a persistent nightly-noise source across master and the active release branches. The dynamic runtime toggle of `suppress-fib-pending` races orchagent/fpmsyncd/FRR, so routes are intermittently left un-offloaded / un-propagated and nearly every case in the module fails intermittently. Skipping the module de-noises the nightly signal while the product-layer fix rolls out on master. #### How did you do it? Extended the existing `bgp/test_bgp_suppress_fib.py` skip block with `conditions_logical_operator: or` and split the affected releases by lift semantics: ```yaml conditions: - "release in ['201811', '201911', '202012', '202205', '202211', '202305', '202311', '202405']" - "release in ['202505', '202511', '202605']" - "release in ['master'] and #26175" ``` The `conditional_mark` plugin replaces the issue URL with `True`/`False` based on the issue's open/closed state and `eval()`s the condition, so the master line auto-lifts when #26175 closes while the release-branch line stays permanent. (The same file already uses the `<expr> and <issue-url>` form for the `asic_type in ['vs'] and #14449` conditions.) #### How did you verify/test it? - `tests_mark_conditions.yaml` parses cleanly and passes the `check_conditional_mark_sort.py` pre-commit hook. - Simulated the `conditional_mark` evaluation for each `release` value: - issue **open**: master + 202505/202511/202605 all skip; - issue **closed**: master unskips, 202505/202511/202605 remain skipped — matching the intended lift semantics. #### Any platform specific information? None — the skip is keyed on `release` only (all ASICs/topologies), matching the all-vendor nature of the race. #### Supported testbed topology if it's a new test case? N/A — not a new test case; module skip only. ### Documentation N/A Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
selldinesh
pushed a commit
to selldinesh/sonic-mgmt
that referenced
this pull request
Jul 16, 2026
sonic-net#26175) (sonic-net#26176) ### Description of PR Summary: Fixes sonic-net#26175 Temporarily skip the whole `bgp/test_bgp_suppress_fib.py` module for the suppress-fib-pending **dynamic-toggle race**. The module toggles `suppress-fib-pending` dynamically at runtime (via the shared `config_bgp_suppress_fib()` helper, no `config reload`). That triggers a race between **orchagent, fpmsyncd and FRR** where routes are intermittently **not marked offloaded in FRR** / **not propagated to the upstream VM** within the wait window, failing essentially every case in the module. The root-cause fix makes `suppress-fib-pending` **static** (takes effect only after a `config reload`), removing the race — [sonic-swss#4333](sonic-net/sonic-swss#4333), [sonic-buildimage#26151](sonic-net/sonic-buildimage#26151), [sonic-utilities#4361](sonic-net/sonic-utilities#4361), [sonic-mgmt#22916](sonic-net#22916), [SONiC#2335](sonic-net/SONiC#2335). > **The product fix is master-only and is not backported.** Because of that, the skip uses **two different lift semantics**, expressed as separate conditions: > > - **`master`** — gated on the tracking issue: `release in ['master'] and https://github.com/sonic-net/sonic-mgmt/issues/26175`. When the fix lands on master and sonic-net#26175 is closed, the skip **auto-lifts** on master and coverage returns — no manual edit needed. > - **`202505` / `202511` / `202605`** — a plain `release in [...]` condition with **no issue gate**. These branches never receive the product fix, so the module is skipped for the life of the branch (same nature as the existing pre-202411 "not supported" condition). > > **Per-branch effect still requires cherry-picking this skip.** Each release branch's nightly runs its own `sonic-mgmt` checkout, so this YAML change must be cherry-picked to 202505/202511/202605 for the skip to take effect there. The identical diff matches on each branch via its own `release` token. > > If a release branch ever does receive the product fix, drop that branch's token from the `release in [...]` list — a deliberate per-branch edit. sonic-net#26175 tracks the lifecycle. ### 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 - [x] 202505 - [x] 202511 (also 202605 — not listed in the template checkboxes; cherry-pick required there as well.) ### Approach #### What is the motivation for this PR? The `test_bgp_suppress_fib` module is a persistent nightly-noise source across master and the active release branches. The dynamic runtime toggle of `suppress-fib-pending` races orchagent/fpmsyncd/FRR, so routes are intermittently left un-offloaded / un-propagated and nearly every case in the module fails intermittently. Skipping the module de-noises the nightly signal while the product-layer fix rolls out on master. #### How did you do it? Extended the existing `bgp/test_bgp_suppress_fib.py` skip block with `conditions_logical_operator: or` and split the affected releases by lift semantics: ```yaml conditions: - "release in ['201811', '201911', '202012', '202205', '202211', '202305', '202311', '202405']" - "release in ['202505', '202511', '202605']" - "release in ['master'] and sonic-net#26175" ``` The `conditional_mark` plugin replaces the issue URL with `True`/`False` based on the issue's open/closed state and `eval()`s the condition, so the master line auto-lifts when sonic-net#26175 closes while the release-branch line stays permanent. (The same file already uses the `<expr> and <issue-url>` form for the `asic_type in ['vs'] and sonic-net#14449` conditions.) #### How did you verify/test it? - `tests_mark_conditions.yaml` parses cleanly and passes the `check_conditional_mark_sort.py` pre-commit hook. - Simulated the `conditional_mark` evaluation for each `release` value: - issue **open**: master + 202505/202511/202605 all skip; - issue **closed**: master unskips, 202505/202511/202605 remain skipped — matching the intended lift semantics. #### Any platform specific information? None — the skip is keyed on `release` only (all ASICs/topologies), matching the all-vendor nature of the race. #### Supported testbed topology if it's a new test case? N/A — not a new test case; module skip only. ### Documentation N/A Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
selldinesh
pushed a commit
to selldinesh/sonic-mgmt
that referenced
this pull request
Jul 16, 2026
sonic-net#26175) (sonic-net#26176) ### Description of PR Summary: Fixes sonic-net#26175 Temporarily skip the whole `bgp/test_bgp_suppress_fib.py` module for the suppress-fib-pending **dynamic-toggle race**. The module toggles `suppress-fib-pending` dynamically at runtime (via the shared `config_bgp_suppress_fib()` helper, no `config reload`). That triggers a race between **orchagent, fpmsyncd and FRR** where routes are intermittently **not marked offloaded in FRR** / **not propagated to the upstream VM** within the wait window, failing essentially every case in the module. The root-cause fix makes `suppress-fib-pending` **static** (takes effect only after a `config reload`), removing the race — [sonic-swss#4333](sonic-net/sonic-swss#4333), [sonic-buildimage#26151](sonic-net/sonic-buildimage#26151), [sonic-utilities#4361](sonic-net/sonic-utilities#4361), [sonic-mgmt#22916](sonic-net#22916), [SONiC#2335](sonic-net/SONiC#2335). > **The product fix is master-only and is not backported.** Because of that, the skip uses **two different lift semantics**, expressed as separate conditions: > > - **`master`** — gated on the tracking issue: `release in ['master'] and https://github.com/sonic-net/sonic-mgmt/issues/26175`. When the fix lands on master and sonic-net#26175 is closed, the skip **auto-lifts** on master and coverage returns — no manual edit needed. > - **`202505` / `202511` / `202605`** — a plain `release in [...]` condition with **no issue gate**. These branches never receive the product fix, so the module is skipped for the life of the branch (same nature as the existing pre-202411 "not supported" condition). > > **Per-branch effect still requires cherry-picking this skip.** Each release branch's nightly runs its own `sonic-mgmt` checkout, so this YAML change must be cherry-picked to 202505/202511/202605 for the skip to take effect there. The identical diff matches on each branch via its own `release` token. > > If a release branch ever does receive the product fix, drop that branch's token from the `release in [...]` list — a deliberate per-branch edit. sonic-net#26175 tracks the lifecycle. ### 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 - [x] 202505 - [x] 202511 (also 202605 — not listed in the template checkboxes; cherry-pick required there as well.) ### Approach #### What is the motivation for this PR? The `test_bgp_suppress_fib` module is a persistent nightly-noise source across master and the active release branches. The dynamic runtime toggle of `suppress-fib-pending` races orchagent/fpmsyncd/FRR, so routes are intermittently left un-offloaded / un-propagated and nearly every case in the module fails intermittently. Skipping the module de-noises the nightly signal while the product-layer fix rolls out on master. #### How did you do it? Extended the existing `bgp/test_bgp_suppress_fib.py` skip block with `conditions_logical_operator: or` and split the affected releases by lift semantics: ```yaml conditions: - "release in ['201811', '201911', '202012', '202205', '202211', '202305', '202311', '202405']" - "release in ['202505', '202511', '202605']" - "release in ['master'] and sonic-net#26175" ``` The `conditional_mark` plugin replaces the issue URL with `True`/`False` based on the issue's open/closed state and `eval()`s the condition, so the master line auto-lifts when sonic-net#26175 closes while the release-branch line stays permanent. (The same file already uses the `<expr> and <issue-url>` form for the `asic_type in ['vs'] and sonic-net#14449` conditions.) #### How did you verify/test it? - `tests_mark_conditions.yaml` parses cleanly and passes the `check_conditional_mark_sort.py` pre-commit hook. - Simulated the `conditional_mark` evaluation for each `release` value: - issue **open**: master + 202505/202511/202605 all skip; - issue **closed**: master unskips, 202505/202511/202605 remain skipped — matching the intended lift semantics. #### Any platform specific information? None — the skip is keyed on `release` only (all ASICs/topologies), matching the all-vendor nature of the race. #### Supported testbed topology if it's a new test case? N/A — not a new test case; module skip only. ### Documentation N/A Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
mike-dubrovsky
force-pushed
the
fib-suppression
branch
from
July 18, 2026 19:21
c89c37e to
bfc503e
Compare
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
ssithaia-ebay
pushed a commit
to ssithaia-ebay/sflow-yang-sonic-mgmt
that referenced
this pull request
Jul 21, 2026
sonic-net#26175) (sonic-net#26176) ### Description of PR Summary: Fixes sonic-net#26175 Temporarily skip the whole `bgp/test_bgp_suppress_fib.py` module for the suppress-fib-pending **dynamic-toggle race**. The module toggles `suppress-fib-pending` dynamically at runtime (via the shared `config_bgp_suppress_fib()` helper, no `config reload`). That triggers a race between **orchagent, fpmsyncd and FRR** where routes are intermittently **not marked offloaded in FRR** / **not propagated to the upstream VM** within the wait window, failing essentially every case in the module. The root-cause fix makes `suppress-fib-pending` **static** (takes effect only after a `config reload`), removing the race — [sonic-swss#4333](sonic-net/sonic-swss#4333), [sonic-buildimage#26151](sonic-net/sonic-buildimage#26151), [sonic-utilities#4361](sonic-net/sonic-utilities#4361), [sonic-mgmt#22916](sonic-net#22916), [SONiC#2335](sonic-net/SONiC#2335). > **The product fix is master-only and is not backported.** Because of that, the skip uses **two different lift semantics**, expressed as separate conditions: > > - **`master`** — gated on the tracking issue: `release in ['master'] and https://github.com/sonic-net/sonic-mgmt/issues/26175`. When the fix lands on master and sonic-net#26175 is closed, the skip **auto-lifts** on master and coverage returns — no manual edit needed. > - **`202505` / `202511` / `202605`** — a plain `release in [...]` condition with **no issue gate**. These branches never receive the product fix, so the module is skipped for the life of the branch (same nature as the existing pre-202411 "not supported" condition). > > **Per-branch effect still requires cherry-picking this skip.** Each release branch's nightly runs its own `sonic-mgmt` checkout, so this YAML change must be cherry-picked to 202505/202511/202605 for the skip to take effect there. The identical diff matches on each branch via its own `release` token. > > If a release branch ever does receive the product fix, drop that branch's token from the `release in [...]` list — a deliberate per-branch edit. sonic-net#26175 tracks the lifecycle. ### 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 - [x] 202505 - [x] 202511 (also 202605 — not listed in the template checkboxes; cherry-pick required there as well.) ### Approach #### What is the motivation for this PR? The `test_bgp_suppress_fib` module is a persistent nightly-noise source across master and the active release branches. The dynamic runtime toggle of `suppress-fib-pending` races orchagent/fpmsyncd/FRR, so routes are intermittently left un-offloaded / un-propagated and nearly every case in the module fails intermittently. Skipping the module de-noises the nightly signal while the product-layer fix rolls out on master. #### How did you do it? Extended the existing `bgp/test_bgp_suppress_fib.py` skip block with `conditions_logical_operator: or` and split the affected releases by lift semantics: ```yaml conditions: - "release in ['201811', '201911', '202012', '202205', '202211', '202305', '202311', '202405']" - "release in ['202505', '202511', '202605']" - "release in ['master'] and sonic-net#26175" ``` The `conditional_mark` plugin replaces the issue URL with `True`/`False` based on the issue's open/closed state and `eval()`s the condition, so the master line auto-lifts when sonic-net#26175 closes while the release-branch line stays permanent. (The same file already uses the `<expr> and <issue-url>` form for the `asic_type in ['vs'] and sonic-net#14449` conditions.) #### How did you verify/test it? - `tests_mark_conditions.yaml` parses cleanly and passes the `check_conditional_mark_sort.py` pre-commit hook. - Simulated the `conditional_mark` evaluation for each `release` value: - issue **open**: master + 202505/202511/202605 all skip; - issue **closed**: master unskips, 202505/202511/202605 remain skipped — matching the intended lift semantics. #### Any platform specific information? None — the skip is keyed on `release` only (all ASICs/topologies), matching the all-vendor nature of the race. #### Supported testbed topology if it's a new test case? N/A — not a new test case; module skip only. ### Documentation N/A Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: ssithaia-ebay <ssithaian@ebay.com>
Contributor
|
@mike-dubrovsky , please fix the Ci failure. |
suppress-fib-pending config changes require a reload to take effect. Changes: - config/main.py: inform user that reload is required after changing suppress-fib-pending Signed-off-by: mike-dubrovsky <mdubrovs@cisco.com>
mike-dubrovsky
force-pushed
the
fib-suppression
branch
from
July 23, 2026 08:18
bfc503e to
ff4a4ad
Compare
Collaborator
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Contributor
|
/azp run Azure.sonic-utilities |
|
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
3 participants
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.
Summary
suppress-fib-pending config changes require a reload to take effect.
Changes:
changing suppress-fib-pending
What I did
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)
This change needs to go together with
sonic-net/sonic-swss#4333
sonic-net/sonic-buildimage#26151
sonic-net/sonic-mgmt#22916
sonic-net/SONiC#2335