Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TS/TSA/B/C]: Update traffic-shift scripts with bgp neighbor check enhancement #8990

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Stephenxf
Copy link
Contributor

Why I did it

The test case test_TSA_B_C_with_no_neighbors from bgp/test_traffic_shift.py may fail because of the way ebgp neighbors are counted in sonic-buildimage/dockers/docker-fpm-frr/TSA in source code or /usr/bin/TSA in bgp container (the same applies to TSB and TSC as well). Our (LinkedIn) configuration profile is a bit different, but the issue should be common.

If there is no eBGP neighbor, TSA/TSB/TSC are supposed to return "System Mode: No external neighbors". However, this "no eBGP" check is done by looking at the number of route-maps from running-config.

In buildimage/dockers/docker-fpm-frr/TSA:

find_num_routemap
routemap_count=$?
check_not_installed
not_installed=$?

if [[ $routemap_count -eq 0 ]];
then
  echo "System Mode: No external neighbors"

In buildimage/dockers/docker-fpm-frr/TS:

function find_num_routemap()
{
  c=0
  config=$(vtysh -c "show run")
  for route_map_name in $(echo "$config" | sed -ne 's/  neighbor \S* route-map \(\S*\) out/\1/p' | egrep 'V4|V6' | uniq);
  do
    is_internal_route_map $route_map_name && continue
    c=$((c+1))
  done
  return $c
}

The idea is to find out the number of ebgp neighbors by checking if any ebgp-specific outbound route-maps are being used by any neighbors. However, this can be problematic and lead to incorrect result, in twofold:

  • The numebr of "neighbor ... route-map ... out" doesn't always equal the number of neighbors. For example, if peer-groups are configured and a set of bgp neighbors are inheriting outbound route-maps via peer-groups, the find_num_routemap won't return the correct number of ebgp neighbors. Similar problem would happen with dynamic neighbors.
  • As demonstrated by the test case test_TSA_B_C_with_no_neighbors, when all the neighbors are gone (e.g., deleted via sonic-cfggen), the peer-groups can still be there in running-config. As a result, TSA/TSB/TSC won't show the expected result, i.e., "System Mode: No external neighbors".

How I did it

We can get the eBGP neighbor count by checking "show bgp summary". This proposal introduces a function find_num_ebgp_neighbors to parse "show bgp summary json" output and return the number of eBGP neighbors (remote-ASN different from local-ASN).

How to verify it

  1. Run Pytest case test_TSA_B_C_with_no_neighbors from bgp/test_traffic_shift.py.
  2. Unit test of TSA/TSB/TSC with:
  • no BGP neighbor
  • no eBGP neighbor but iBGP neighbors
  • both eBGP and iBGP neighbors

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

Description for the changelog

Update traffic-shift scripts with bgp neighbor check enhancement.

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

…hancement

  Enhance TS scripts with a more appropriate way to find the number of ebgp neighbors.
  It also fixes the pytest failure of test_traffic_shift.py::test_TSA_B_C_with_no_neighbors.

  Signed-off-by: Stephen Xu <stexu@linkedin.com, xufeng8333@gmail.com>
@ghost
Copy link

ghost commented Oct 16, 2021

CLA assistant check
All CLA requirements met.

@lguohan
Copy link
Collaborator

lguohan commented Oct 24, 2021

@abdosi , can you check this pr?

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.

None yet

2 participants