fix(sonic): keep AF admin_status true/false

[ideaship]

Problem

The vendored community YANG types BGP_NEIGHBOR_AF.admin_status as
stypes:admin_status — the up/down enumeration shared with
PORT — so the ConfigDB validator flags every address-family row the config
generator writes.

Reading that as a defect in the generator points at the wrong fix. Emitting
the model's spelling produces a config the switches do not act on.

Why true/false is the correct value

BGP_NEIGHBOR_AF belongs to the unified FRR management interface, and that
feature's own ConfigDB schema types the leaf as a boolean — in the same
document where PORT and the interface tables are up/down:

admin_status = "true" / "false" ; Neighbor admin status

— SONiC_Design_Doc_Unified_FRR_Mgmt_Interface.md §3.2.1.7

The consumer agrees. In frrcfgd, the address-family admin_status entries
of nbr_af_key_map are bound to a bool token pair, ['true', 'false', False]
on 202411 and older:

('admin_status|ipv4', '{no:no-prefix}neighbor {} activate', ['true', 'false', False]),

— frrcfgd.py

A value outside that pair does not fall back to anything: get_command_cmn
logs and returns None, so no vtysh command is produced at all.

Upstream resolved the contradiction in the consumer rather than in the model.
frrcfgd was taught up/down in addition to true/false, explicitly so
that existing deployments keep working (sonic-buildimage#21697, merged
2025-03-12). That change is in community master and 202505; it is not
in 202411, 202405 or 202311.

Measured

Driving BGPConfigDaemon directly on both revisions — swsscommon mocked,
g_run_command captured, a BGP_GLOBALS row followed by a BGP_NEIGHBOR_AF
row:

frrcfgd	admin_status: "true"	admin_status: "up"
master	neighbor 10.0.0.1 activate	neighbor 10.0.0.1 activate
202411	neighbor 10.0.0.1 activate	no vtysh call at all

The 202411 up case logs, and does nothing else:

Input token up is neither true or false for cmd: {no:no-prefix}neighbor {} activate
failed to get upd cmd from value: ('up',)
Add admin_status data up to cache

That last line caches the value as if it had been applied, so rewriting the
unchanged row never retries; only a frrcfgd restart re-reads CONFIG_DB from
scratch. The visible result on a switch is two syslog lines and a BGP peering
that carries no routes.

An independent cross-check: osism/network-design
sonic-l3-core-vrf/*/config_db.json, taken off Accton hardware with
frr_mgmt_framework_config: true, carries PORT.admin_status: "up" and
BGP_NEIGHBOR_AF.admin_status: "true" in the same file. Those are not
generator output — the default-VRF BGP_NEIGHBOR rows lack the v6only this
generator always writes, BGP_GLOBALS_AF lacks ibgp_equal_cluster_length and
the l2vpn advertise-* pair, and always_compare_med is false where the
generator hardcodes true.

What this changes

The generator already emits true. This makes the validator agree, without
giving up the table.

PLATFORM_DIVERGENT_TABLES is table-granular: using it here would drop the
schema for the whole table, including the neighbor leafref and every other
field. So this adds a field-granular sibling. PLATFORM_DIVERGENT_FIELDS maps
(table, leaf) to the annotation the platform accepts plus a reason, which is
emitted as a comment beside the generated field:

class BgpNeighborAfListRow(BaseModel):
    ...
    # Platform divergence: the frr-mgmt-framework CONFIG_DB schema types this
    # leaf true/false, and frrcfgd only activates the address family for those
    # two tokens; see docs/sonic-config-validation.md
    admin_status: Optional[Literal["true", "false"]] = None

The rest of the table stays validated, and so does the leaf — against the
platform rather than against the model. Emitting the model's spelling is now a
test failure rather than a silent regression.

docs/sonic-config-validation.md records the rationale and, more usefully,
what would let the entry be dropped again: switches running builds whose
frrcfgd carries the upstream change, established on a device rather than
from a version number, since no published mapping ties an Enterprise build to
its community branch. Both sides have to move together when that happens.

Tests

Three assertions in the validator test module: the platform value accepted,
the model's spelling rejected, and a bad send_community in the same table
still flagged — that last one is what distinguishes this from opting the table
out. 3244 passed, 3 pre-existing xfails in tests/unit, flake8 and black
clean. Regenerating the schemas reproduces the committed output exactly.

Note for reviewers

• This supersedes the opposite change, now closed: fix(sonic): emit up/down for BGP_NEIGHBOR_AF #2618
• Its former stacked follow-up, since rebased onto main: feat(sonic): validate BGP_NEIGHBOR_AF key references #2619
• Goldens on the draft E2E series hold up for this table. They were folded
  into that series' history while it was expected to land after fix(sonic): emit up/down for BGP_NEIGHBOR_AF #2618, so they
  need regenerating rather than editing — and that series is 68 commits behind
  main, predating the merged LAG peer resolution, so it needs rebasing first.
  Until then its goldens would also fail the artifact test added in fix(sonic): make the ConfigDB validator usable as a gate #2626.

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the platform actually expects up/down, this validation change could allow generated true/false values that leave BGP address families inactive, disrupting route exchange and potentially causing a production network outage. Reverting the validator would not undo any already-applied configuration, so affected devices would require a separate configuration correction.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}