Check if BGP has Loopback0 IP as its router ID by kellyyeh · Pull Request #8576 · sonic-net/sonic-mgmt

kellyyeh · 2023-06-13T05:27:02Z

Description of PR

Summary:
Fixes # (issue)

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

An issue was found that T0 did not announce Loopback to T1, this check is to bridge the test gap to ensure bgp router identifier matches Loopback0 IP.

How did you do it?

Added in sanity_check to match bgp router id and loopback0 ip

How did you verify/test it?

Ran a advanced_reboot manually and checked expected log: BGP router identifier:
https://elastictest.org/scheduler/testplan/649c780c75768730ce0de9f3

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

This reverts commit 7e8dd58.

mssonicbld · 2023-06-26T20:11:27Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:225:13: E117 over-indented check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

vaibhavhd · 2023-06-28T19:19:33Z

tests/common/fixtures/advanced_reboot.py

+        check_bgo_router_id_cmd = r"show ip bgp sum"
+        bgp_sum_result = self.duthost.shell(check_bgo_router_id_cmd, module_ignore_errors=True)
+        if bgp_sum_result["rc"] == 0:
+            match = re.search(r'BGP router identifier (\d+\.\d+\.\d+\.\d+)', bgp_sum_result["stdout"])


Is there no way this data can be collected from DB or from a JSON output? The concern w/ CLI parsing is that if the format changes in future then the regex will fail and lead to false positives.

If there is no other way then this is acceptable.

Found a way to get this in JSON! You could execute below vtysh command and get the json output. From it. routerId can be fetched.

$ vtysh -c "show ip bgp summary json" { "ipv4Unicast":{ "routerId":"10.1.0.32",

vaibhavhd · 2023-06-28T19:21:23Z

tests/common/fixtures/advanced_reboot.py

+
+        loopback0 = str(self.mgFacts['minigraph_lo_interfaces'][0]['addr'])
+
+        return bgp_router_id == loopback0


bgp_router_id is defined only when the match is successful. The check here can fail w/ bgp_router_id not defined if the match is not successful and fail ungracefully.

vaibhavhd · 2023-06-28T19:25:01Z

@StormLiangMS just FYI: this is a test gap that is being addressed. Today no test matches the router-id to be same as Loopback0's IP. With this change the verification will be done as part of sanity tests.

…r-id

mssonicbld · 2023-06-28T22:16:52Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:574:121: E501 line too long (125 > 120 characters) check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-06-28T22:19:18Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:575:121: E501 line too long (125 > 120 characters) check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-06-28T23:01:30Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:240:95: E502 the backslash is redundant between brackets tests/common/fixtures/advanced_reboot.py:241:34: E127 continuation line over-indented for visual indent check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-06-28T23:04:56Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:240:95: E502 the backslash is redundant between brackets check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-06-29T01:09:32Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:239:29: E128 continuation line under-indented for visual indent check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

vaibhavhd · 2023-06-29T20:05:10Z

tests/common/fixtures/advanced_reboot.py

+        """
+        check_bgp_router_id_cmd = r'vtysh -c "show ip bgp summary json"'
+        bgp_summary = self.duthost.shell(check_bgp_router_id_cmd, module_ignore_errors=True)
+        bgp_summary_json = json.loads(bgp_summary['stdout'])


Possible exception here if stdout is not in json format for some reason.

vaibhavhd · 2023-06-29T20:07:54Z

tests/common/plugins/sanity_check/checks.py

            return not check_result['failed']

+        def _check_bgp_router_id(tbinfo):
+            duthost = kwargs['node']


Looks like there is duplication of whole logic here. So can we put this into commons and import at both the places?

…r-id

mssonicbld · 2023-07-18T07:44:39Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:558:121: E501 line too long (125 > 120 characters) tests/common/fixtures/advanced_reboot.py:559:29: E128 continuation line under-indented for visual indent check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2023-07-18T07:48:39Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/fixtures/advanced_reboot.py:559:25: E128 continuation line under-indented for visual indent check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

arlakshm · 2023-07-21T23:44:25Z

Hi @kellyyeh @vaibhavhd, this check will not work for chassis linecard which have single asic. Please check.

kellyyeh · 2023-07-22T01:02:37Z

Hi @kellyyeh @vaibhavhd, this check will not work for chassis linecard which have single asic. Please check.

Created PR #9090

mssonicbld · 2023-07-27T02:28:10Z

Cherry-pick PR to 202205: #9134

mssonicbld · 2023-07-27T02:34:37Z

@kellyyeh PR conflicts with 202012 branch

This is a cherry-pick PR for #8576 and #9090 * Check if BGP has Loopback0 IP as its router ID (#8576) * Skip check_bgp_router_id on T2 devices (#9090) What is the motivation for this PR? An issue was found that T0 did not announce Loopback to T1, this check is to bridge the test gap to ensure bgp router identifier matches Loopback0 IP. How did you do it? Added in sanity_check to match bgp router id and loopback0 ip How did you verify/test it? Ran a advanced_reboot manually and checked expected log: BGP router identifier: https://elastictest.org/scheduler/testplan/649c780c75768730ce0de9f3

kellyyeh added 3 commits June 13, 2023 05:26

Add bgp router id check to sanity_check

7e8dd58

Revert "Add bgp router id check to sanity_check"

66c8807

This reverts commit 7e8dd58.

Add __checkBgpRouterId in advanced_reboot test

fc456fa

Update advanced_reboot.py

aa0132b

kellyyeh marked this pull request as ready for review June 27, 2023 16:22

kellyyeh requested a review from vaibhavhd June 27, 2023 16:22

kellyyeh and others added 2 commits June 27, 2023 20:07

Update log

02e6b18

Merge branch 'sonic-net:master' into router-id

ca97b7f

vaibhavhd reviewed Jun 28, 2023

View reviewed changes

kellyyeh added 2 commits June 28, 2023 22:14

Retrieve bgp id router through json

0e4d7c2

Merge branch 'router-id' of github.com:kellyyeh/sonic-mgmt into route…

4fb3ee9

…r-id

Handle return

0e811f3

Fix pre-commit

fc6613e

Update

7187b81

Address comment

80929f0

kellyyeh and others added 2 commits June 29, 2023 01:14

precommit

e544d29

Merge branch 'sonic-net:master' into router-id

f572a85

vaibhavhd reviewed Jun 29, 2023

View reviewed changes

kellyyeh added 2 commits July 18, 2023 07:43

Make check_bgp_router_id common

bcba008

Merge branch 'router-id' of github.com:kellyyeh/sonic-mgmt into route…

d238add

…r-id

Fix pre-commit

a1702fb

Fix pre-commit

d3a0cf4

kellyyeh requested a review from vaibhavhd July 18, 2023 07:53

kellyyeh added 2 commits July 18, 2023 19:16

Add missing parameter

d175a66

Skip on multi-asic

2f2a6b4

vaibhavhd approved these changes Jul 21, 2023

View reviewed changes

kellyyeh merged commit 3598bd8 into sonic-net:master Jul 21, 2023

kellyyeh deleted the router-id branch July 21, 2023 17:01

kellyyeh added Request for 202012 branch Request for 202205 branch labels Jul 21, 2023

wangxin added the Approved for 202205 branch label Jul 27, 2023

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jul 27, 2023

Check if BGP has Loopback0 IP as its router ID (sonic-net#8576)

a26b30c

mssonicbld added the Created PR to 202205 branch label Jul 27, 2023

mssonicbld mentioned this pull request Jul 27, 2023

[action] [PR:8576] Check if BGP has Loopback0 IP as its router ID #9134

Merged

6 tasks

wangxin added the Approved for 202012 Branch label Jul 27, 2023

mssonicbld added the Cherry Pick Conflict_202012 label Jul 27, 2023

mssonicbld pushed a commit that referenced this pull request Jul 27, 2023

Check if BGP has Loopback0 IP as its router ID (#8576)

1f582ec

mssonicbld added Included in 202205 branch and removed Created PR to 202205 branch labels Jul 27, 2023

kellyyeh added a commit to kellyyeh/sonic-mgmt that referenced this pull request Jul 31, 2023

Check if BGP has Loopback0 IP as its router ID (sonic-net#8576)

85694e7

kellyyeh mentioned this pull request Jul 31, 2023

[202012] Check if BGP has Loopback0 IP as its router ID (#8576) #9195

Merged

6 tasks

wangxin added the Included in 202012 branch label Aug 1, 2023

AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024

Check if BGP has Loopback0 IP as its router ID (sonic-net#8576)

efd5104


		loopback0 = str(self.mgFacts['minigraph_lo_interfaces'][0]['addr'])

		return bgp_router_id == loopback0

Conversation

kellyyeh commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

mssonicbld commented Jun 26, 2023

Uh oh!

vaibhavhd Jun 28, 2023

Choose a reason for hiding this comment

Uh oh!

vaibhavhd Jun 28, 2023

Choose a reason for hiding this comment

Uh oh!

vaibhavhd Jun 28, 2023

Choose a reason for hiding this comment

Uh oh!

vaibhavhd commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 28, 2023

Uh oh!

mssonicbld commented Jun 29, 2023

Uh oh!

vaibhavhd Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

vaibhavhd Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jul 18, 2023

Uh oh!

mssonicbld commented Jul 18, 2023

Uh oh!

arlakshm commented Jul 21, 2023

Uh oh!

kellyyeh commented Jul 22, 2023

Uh oh!

mssonicbld commented Jul 27, 2023

Uh oh!

mssonicbld commented Jul 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kellyyeh commented Jun 13, 2023 •

edited

Loading