Skip to content

[bgp] Add bgpd.aggregate.conf.j2 bootstrap template and sonic-cfggen tests for BGP_AGGREGATE_ADDRESS rendering - #28635

Closed
guangyao6 with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-code-for-review-comment
Closed

[bgp] Add bgpd.aggregate.conf.j2 bootstrap template and sonic-cfggen tests for BGP_AGGREGATE_ADDRESS rendering#28635
guangyao6 with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-code-for-review-comment

Conversation

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown

Adds the missing bgpd.aggregate.conf.j2 bootstrap template that renders BGP_AGGREGATE_ADDRESS into FRR config before any BGP sessions are established, closing the transient route-leak window on cold boot / restart bgp. Also adds the sonic-cfggen unit tests and test vectors that were flagged as missing in the PR review.

Why I did it

On a T1 aggregation router, AggregateAddressMgr installs suppression routes only at runtime after bgpd forms sessions. The stock bootstrap bgpd.conf carries no aggregate-address, so contributing more-specifics escape to T2 peers for the duration of the manager reconciliation window.

Additionally, PR review requested focused cfggen tests covering the BGP_AGGREGATE_ADDRESS rendering logic (strict-prefix validation, BBR gating, prefix-list generation) to prevent regressions.

Work item tracking
  • Microsoft ADO (number only):

How I did it

New template dockers/docker-fpm-frr/frr/bgpd/bgpd.aggregate.conf.j2:

  • Renders BGP_AGGREGATE_ADDRESS rows from CONFIG_DB into bootstrap bgpd.conf
  • Strict IPv4/IPv6 network validation (host bits rejected, parity with validate_prefix(strict=True))
  • BBR safety gate: bbr-required=true entries rendered only when BGP_BBR|all status == "enabled" is explicitly present — deliberately does not consult constants.yml default_state since peer-group existence cannot be verified at bootstrap
  • Command strings mirror managers_aggregate_address.py (generate_aggregate_address_commands() / generate_prefix_list_commands()) byte-for-byte so manager reconcile is a FRR no-op
  • prefix-list seq >= 10 to avoid collision with policy.conf.j2 anchor at seq 5

Updated dockers/docker-fpm-frr/frr/bgpd/bgpd.conf.j2: includes the new template before bgpd.main.conf.j2, guarded by {% if BGP_AGGREGATE_ADDRESS is defined %} — no output change when key is absent.

New sonic-cfggen test vectors src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.aggregate.conf.j2/:

Test JSON input Validates
ipv4_basic IPv4 aggregate, summary-only Basic rendering
ipv4_with_prefix_lists IPv4 + agg/contributing PL names Prefix-list stanza generation
bbr_required_enabled bbr-required=true, BBR enabled BBR gate pass
bbr_required_disabled bbr-required=true, BBR disabled BBR gate suppression (no output)

New test functions in src/sonic-bgpcfgd/tests/test_sonic-cfggen.py:

  • test_aggregate_conf_ipv4_basic — canonical file comparison
  • test_aggregate_conf_ipv4_with_prefix_lists — canonical file comparison
  • test_aggregate_conf_bbr_required_enabled — canonical file comparison
  • test_aggregate_conf_bbr_required_disabled — canonical file comparison (empty structural content)
  • test_aggregate_conf_no_bbr_required_renders_regardless — assertion: non-BBR aggregate always rendered
  • test_aggregate_conf_bbr_gate_blocks_bbr_required_when_disabled — assertion: BBR gate suppression

How to verify it

Run the sonic-cfggen test suite from src/sonic-bgpcfgd/:

pytest tests/test_sonic-cfggen.py -v -k "aggregate"

Expected: all 6 new test_aggregate_conf_* tests pass.

To verify bootstrap behavior on a device with BGP_AGGREGATE_ADDRESS in CONFIG_DB:

  1. systemctl restart bgp
  2. Inspect /etc/frr/bgpd.confaggregate-address and prefix-list entries must be present before any session reaches Established
  3. After bgpcfgd reconciles, confirm AggregateAddressMgr reconcile is a FRR no-op (no config churn in vtysh)

Which release branch to backport (provide reason below if selected)

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

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

Tested branch

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

Test result

Description for the changelog

Render BGP_AGGREGATE_ADDRESS into bootstrap bgpd.conf via new bgpd.aggregate.conf.j2 template to eliminate transient route-leak window on BGP restart; add sonic-cfggen unit tests covering IPv4/IPv6 rendering, prefix-list generation, and BBR gate logic.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Copilot AI review requested due to automatic review settings July 27, 2026 06:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@azure-pipelines

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

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.

@mssonicbld

Copy link
Copy Markdown
Collaborator

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

---Powered by SONiC BuildBot

Copilot AI review requested due to automatic review settings July 27, 2026 06:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

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

Copilot AI changed the title [WIP] Fix code for review comment and add unit tests [bgp] Add bgpd.aggregate.conf.j2 bootstrap template and sonic-cfggen tests for BGP_AGGREGATE_ADDRESS rendering Jul 27, 2026
Copilot AI requested a review from guangyao6 July 27, 2026 06:26
@guangyao6
guangyao6 marked this pull request as ready for review July 27, 2026 10:43
@azure-pipelines

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

@guangyao6

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@guangyao6

Copy link
Copy Markdown
Contributor

/azp run Azure.sonic-buildimage

@azure-pipelines

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

@guangyao6 guangyao6 closed this Jul 27, 2026
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.

4 participants