Skip to content

Commit

Permalink
frr-mgmt-framework: admin_status should be up/down but true/false inc…
Browse files Browse the repository at this point in the history
…orrectly wanted (PR sonic-net#21055)

As seen in
src/sonic-frr-mgmt-framework/templates/bgpd/bgpd.conf.db.nbr_af.j2
and
src/sonic-frr-mgmt-framework/templates/bgpd/bgpd.conf.db.nbr_or_peer.j2
and
src/sonic-frr-mgmt-framework/frrcfgd/frrcfgd.py

The script checks for true/false not up/down.

However the yang models use stypes:admin_status and not boolean.

There are test cases spread around that all depend on up/down for
these values and this appears to be simply an issue with the newer
frr-mgmt-framework code introduce in 2021.

Signed-off-by: Brad House (@bradh352)
  • Loading branch information
bradh352 committed Dec 31, 2024
1 parent 4df3fc0 commit fed73ab
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 6 deletions.
8 changes: 4 additions & 4 deletions src/sonic-frr-mgmt-framework/frrcfgd/frrcfgd.py
Original file line number Diff line number Diff line change
Expand Up @@ -1843,7 +1843,7 @@ class BGPConfigDaemon:
cmn_key_map = [('asn&peer_type', '{no:no-prefix}neighbor {} remote-as {}'),
(['local_asn', '+local_as_no_prepend',
'+local_as_replace_as'], '{no:no-prefix}neighbor {} local-as {} {:no-prepend} {:replace-as}'),
(['admin_status', '+shutdown_message'], '{no:no-prefix}neighbor {} shutdown {:shutdown-msg}', ['false', 'true']),
(['admin_status', '+shutdown_message'], '{no:no-prefix}neighbor {} shutdown {:shutdown-msg}', ['down', 'up']),
('local_addr', '{no:no-prefix}neighbor {} update-source {}'),
('name', '{no:no-prefix}neighbor {} description {}'),
(['ebgp_multihop', '+ebgp_multihop_ttl'],'{no:no-prefix}neighbor {} ebgp-multihop {}', ['true', 'false']),
Expand All @@ -1870,9 +1870,9 @@ class BGPConfigDaemon:
nbr_key_map = [('peer_group_name', '{no:no-prefix}neighbor {} peer-group {}')]

nbr_af_key_map = [(['allow_as_in', '+allow_as_count&allow_as_origin'], '{no:no-prefix}neighbor {} allowas-in {:allow-as-in}', ['true', 'false']),
('admin_status|ipv4', '{no:no-prefix}neighbor {} activate', ['true', 'false', False]),
('admin_status|ipv6', '{no:no-prefix}neighbor {} activate', ['true', 'false', False]),
('admin_status|l2vpn', '{no:no-prefix}neighbor {} activate', ['true', 'false', False]),
('admin_status|ipv4', '{no:no-prefix}neighbor {} activate', ['up', 'down', False]),
('admin_status|ipv6', '{no:no-prefix}neighbor {} activate', ['up', 'down', False]),
('admin_status|l2vpn', '{no:no-prefix}neighbor {} activate', ['up', 'down', False]),
(['send_default_route', '+default_rmap'], '{no:no-prefix}neighbor {} default-originate {:default-rmap}', ['true', 'false']),
('default_rmap', '{no:no-prefix}neighbor {} default-originate route-map {}'),
(['max_prefix_limit', '++max_prefix_warning_threshold',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{# this is called with the "vrf" and "address-family matched #}
{# ------------------------------------------------------------ #}
{% if 'admin_status' in n_af_val %}
{% if n_af_val['admin_status'] == 'true' %}
{% if n_af_val['admin_status'] == 'up' or n_af_val['admin_status'] == 'true' %}
neighbor {{nbr_name}} activate
{% else %}
no neighbor {{nbr_name}} activate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
{% if 'name' in nbr_or_peer %}
neighbor {{name_or_ip}} description {{nbr_or_peer['name']}}
{% endif %}
{% if 'admin_status' in nbr_or_peer and nbr_or_peer['admin_status'] == 'false' %}
{% if 'admin_status' in nbr_or_peer and (nbr_or_peer['admin_status'] == 'down' or nbr_or_peer['admin_status'] == 'false') %}
{% if 'shutdown_message' in nbr_or_peer %}
neighbor {{name_or_ip}} shutdown message {{nbr_or_peer['shutdown_message']}}
{% else %}
Expand Down

0 comments on commit fed73ab

Please sign in to comment.