sonic-net / sonic-buildimage

Scripts which perform an installable binary image build for SONiC
Other
724 stars 1.38k forks source link

Two issues when using config node as "unified" #20019

Open eddieruan-alibaba opened 3 weeks ago

eddieruan-alibaba commented 3 weeks ago

Description

When using config mode as "unified", two issues observed during testing.

  1. frr.conf can't be generated via https://github.com/sonic-net/sonic-buildimage/blob/master/dockers/docker-fpm-frr/docker_init.sh#L91

The following error is seen File "/usr/local/bin/sonic-cfggen", line 502, in main() File "/usr/local/bin/sonic-cfggen", line 466, in main template_data = template.render(data) File "/usr/local/lib/python3.9/dist-packages/jinja2/environment.py", line 1304, in render self.environment.handle_exception() File "/usr/local/lib/python3.9/dist-packages/jinja2/environment.py", line 939, in handle_exception raise rewrite_traceback_stack(source=source) File "/usr/share/sonic/templates/gen_frr.conf.j2", line 2, in top-level template code {% include "/usr/local/sonic/frrcfgd/frr.conf.j2" %} File "/usr/local/sonic/frrcfgd/frr.conf.j2", line 23, in top-level template code {% include "staticd.db.default_route.conf.j2" %} File "/usr/local/lib/python3.9/dist-packages/jinja2/loaders.py", line 207, in get_source raise TemplateNotFound(template) jinja2.exceptions.TemplateNotFound: staticd.db.default_route.conf.j2

When using -t /usr/share/sonic/templates/gen_frr.conf.j2 sonic-cfggen would only add /usr/share/sonic/templates/ in j2 path.

https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-config-engine/sonic-cfggen#L468

But when using unified mode, we pick frr.conf.j2 from {% include "/usr/local/sonic/frrcfgd/frr.conf.j2" %}

which causes this issue.

The fix is to use -T to provide additional Template directory.

diff --git a/dockers/docker-fpm-frr/docker_init.sh b/dockers/docker-fpm-frr/docker_init.sh index 0ed274ec7..662a78b5a 100755 --- a/dockers/docker-fpm-frr/docker_init.sh +++ b/dockers/docker-fpm-frr/docker_init.sh @@ -88,6 +88,7 @@ elif [ "$CONFIG_TYPE" == "unified" ]; then CFGGEN_PARAMS=" \ -d \ -y /etc/sonic/constants.yml \

  1. The expected generated configuration should be

    neighbor 10.10.246.254 route-map pass_all in neighbor 10.10.246.254 route-map pass_all out

But it generated as

neighbor 10.10.246.254 route-map p in neighbor 10.10.246.254 route-map a in neighbor 10.10.246.254 route-map s in neighbor 10.10.246.254 route-map s in neighbor 10.10.246.254 route-map in neighbor 10.10.246.254 route-map a in neighbor 10.10.246.254 route-map l in neighbor 10.10.246.254 route-map l in neighbor 10.10.246.254 route-map p out neighbor 10.10.246.254 route-map a out neighbor 10.10.246.254 route-map s out neighbor 10.10.246.254 route-map s out neighbor 10.10.246.254 route-map out neighbor 10.10.246.254 route-map a out neighbor 10.10.246.254 route-map l out neighbor 10.10.246.254 route-map l out

It is due to the following for loop is not correct. {% for map in n_af_val['route_map_in'] %} neighbor {{nbr_name}} route-map {{map}} in {% endfor %}

https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-frr-mgmt-framework/templates/bgpd/bgpd.conf.db.nbr_af.j2#L115

The fix should remove this for loop.

diff --git a/src/sonic-frr-mgmt-framework/templates/bgpd/bgpd.conf.db.nbr_af.j2 b/src/sonic-frr-mgmt-framework/templates/bgpd/bgpd.conf.db.nbr_af.j2 index 8afb64e99..a3dd20723 100644 --- a/src/sonic-frr-mgmt-framework/templates/bgpd/bgpd.conf.db.nbr_af.j2 +++ b/src/sonic-frr-mgmt-framework/templates/bgpd/bgpd.conf.db.nbr_af.j2 @@ -112,16 +112,12 @@ {% endif %} {# ------- route-map in --------------------------- #} {% if 'route_map_in' in n_af_val %} -{% for map in n_af_val['route_map_in'] %}

Steps to reproduce the issue:

1. 4. 5.

Describe the results you received:

Describe the results you expected:

Output of show version:

(paste your output here)

Output of show techsupport:

(paste your output here or download and attach the file here )

Additional information you deem important (e.g. issue happens only occasionally):

eddieruan-alibaba commented 3 weeks ago

Discussed with @venkatmahalingam For issue 2, Yang defines route-map as a listhttps://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-bgp-common.yang#L385 . We need to keep j2's approach and change frrcfgd to handle it as a route-map as a list.

Venket's comments from https://github.com/sonic-net/sonic-buildimage/pull/20020 " frrcfgd should handle the value in a list."

The following config's handling need to be changed to a list handling. https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-frr-mgmt-framework/frrcfgd/frrcfgd.py#L1827

gechiang commented 3 weeks ago

@eddieruan-alibaba if the PR addressed this issue, please help close this one. Thanks!