Skip to content

Generalizing GCU test suite to verify both ip types (IPV4, IPV6) for BGP Neighbors#13650

Merged
judyjoseph merged 3 commits intosonic-net:masterfrom
okaravasi:generalize_gcu_ipv6
Oct 8, 2024
Merged

Generalizing GCU test suite to verify both ip types (IPV4, IPV6) for BGP Neighbors#13650
judyjoseph merged 3 commits intosonic-net:masterfrom
okaravasi:generalize_gcu_ipv6

Conversation

@okaravasi
Copy link
Copy Markdown
Contributor

@okaravasi okaravasi commented Jul 12, 2024

Currently, under generic config update there is a test suite verifying only IPV6 BGP Neighbors' changes. This PR extends the existing test suite and adds support for verifying IPV4 BGP NEighbors also. This is achieved by generalizing the test code with adding dynamic variable for neighbor ip type and with test mark parameters. Also, renamed the test suite to a generic name.

Description of PR

Summary:
This is an improvement on existing test suite generic_config_updater/test_ipv6.py
Currently, under generic config update there is a test suite verifying only IPV6 BGP Neighbors' changes. This PR extends the existing test suite and adds support for verifying IPV4 BGP NEighbors also. This is achieved by generalizing the test code with adding dynamic variable for neighbor ip type and with test mark parameters. Also, renamed the test suite to a generic name.

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

Currently, there are no test cases for IPv4 BGP Neighbors in the verification of generic config updates functionality. This PR extends the existing test suite, which verifies IPv6 BGP Peers, to add support for IPv4 BGP Neighbors verification.

How did you do it?

Added argument for ip_version in test functions to dynamically verify the relevant peer IP type.
Added test mark parameters to run the same test case for both IP types.
Renamed the test suite to a generic name that reflects new test cases content.

How did you verify/test it?

Ran the newly created test suite and verified that the test cases pass for both IP types.

image

Any platform specific information?

N/A

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

N/A

Documentation

Currently, under generic config update there is a test suite verifying only IPV6 BGP Neighbors' changes.
This PR extends the existing test suite and adds support for verifying IPV4 BGP NEighbors also.
This is achieved by generalizing the test code with adding dynamic variable for neighbor ip type and with test mark parameters.
Also, renamed the test suite to a generic name.

Signed-off-by: karavasi <[email protected]>
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jul 12, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@mssonicbld
Copy link
Copy Markdown
Collaborator

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/generic_config_updater/test_ip_bgp.py:199:1: E302 expected 2 blank lines, found 1

flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

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

  1. 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.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@okaravasi okaravasi changed the title Motivation for this PR: Generalizing GCU test suite to verify both ip types (IPV4, IPV6) for BGP Neighbors Jul 12, 2024
@okaravasi
Copy link
Copy Markdown
Contributor Author

Hello @judyjoseph @arlakshm , Could you please help on reviewing this?
Thank you.

@rlhui rlhui requested a review from judyjoseph July 17, 2024 17:46
@judyjoseph
Copy link
Copy Markdown
Contributor

LGTM, @okaravasi could you resolve the conflicts. Also do add a snapshot of the test run for ipv4 and ipv6 under the section "How did you verify/test it?"

@okaravasi
Copy link
Copy Markdown
Contributor Author

LGTM, @okaravasi could you resolve the conflicts. Also do add a snapshot of the test run for ipv4 and ipv6 under the section "How did you verify/test it?"

Done: Updated the branch and included screenshot from kvm-t0 run in the description.

@judyjoseph
Copy link
Copy Markdown
Contributor

@okaravasi Arvind had a comment earlier -- that we need to add support for multi-asic as well. Do you plan to do it in this PR ?

@okaravasi
Copy link
Copy Markdown
Contributor Author

ad a comment earlier -- that we need to add support for multi-asic as well. Do you plan to do it in this PR ?

@judyjoseph Support to multi-asic will be handled by different PRs.

  1. Adding support to existing test scope shall be handled via : GCU adding Multi-ASIC support in existing test code base #14070
  2. Adding new test for t2/chassis GCU shall be handled via separate ticket: Adding new Tests for Chassis/Multi-ASIC GCU  #14887. I will comment soon when it is opened as non-draft.

@judyjoseph judyjoseph merged commit 5b83ba9 into sonic-net:master Oct 8, 2024
hdwhdw pushed a commit to hdwhdw/sonic-mgmt that referenced this pull request Oct 10, 2024
…BGP Neighbors (sonic-net#13650)

* Motivation for this PR:
Currently, under generic config update there is a test suite verifying only IPV6 BGP Neighbors' changes.
This PR extends the existing test suite and adds support for verifying IPV4 BGP NEighbors also.
This is achieved by generalizing the test code with adding dynamic variable for neighbor ip type and with test mark parameters.
Also, renamed the test suite to a generic name.

Signed-off-by: karavasi <[email protected]>

* adding extra line before test case definition due to flake8 check failure

Signed-off-by: karavasi <[email protected]>

---------

Signed-off-by: karavasi <[email protected]>
wangxin added a commit to wangxin/sonic-mgmt that referenced this pull request Oct 12, 2024
Script tests/generic_config_updater/test_ipv6.py was renamed to tests/generic_config_updater/test_ip_bgp.py in PR sonic-net#13650

Need to update file name in pr_test_scripts.yaml to ensure that renamed script is covered by PR testing.

Signed-off-by: Xin Wang <[email protected]>
StormLiangMS pushed a commit that referenced this pull request Oct 14, 2024
Script tests/generic_config_updater/test_ipv6.py was renamed to tests/generic_config_updater/test_ip_bgp.py in PR #13650

Need to update file name in pr_test_scripts.yaml to ensure that renamed script is covered by PR testing.

Signed-off-by: Xin Wang <[email protected]>
wangxin pushed a commit that referenced this pull request Oct 15, 2024
What is the motivation for this PR?
Script tests/generic_config_updater/test_ipv6.py was renamed to tests/generic_config_updater/test_ip_bgp.py in PR #13650
Need to update file name in pr_test_scripts.yaml to ensure that renamed script is covered by PR testing.

How did you do it?
Rename script name from generic_config_updater/test_ipv6.py to generic_config_updater/test_ip_bgp.py
vikshaw-Nokia pushed a commit to vikshaw-Nokia/sonic-mgmt that referenced this pull request Oct 23, 2024
…BGP Neighbors (sonic-net#13650)

* Motivation for this PR:
Currently, under generic config update there is a test suite verifying only IPV6 BGP Neighbors' changes.
This PR extends the existing test suite and adds support for verifying IPV4 BGP NEighbors also.
This is achieved by generalizing the test code with adding dynamic variable for neighbor ip type and with test mark parameters.
Also, renamed the test suite to a generic name.

Signed-off-by: karavasi <[email protected]>

* adding extra line before test case definition due to flake8 check failure

Signed-off-by: karavasi <[email protected]>

---------

Signed-off-by: karavasi <[email protected]>
vikshaw-Nokia pushed a commit to vikshaw-Nokia/sonic-mgmt that referenced this pull request Oct 23, 2024
Script tests/generic_config_updater/test_ipv6.py was renamed to tests/generic_config_updater/test_ip_bgp.py in PR sonic-net#13650

Need to update file name in pr_test_scripts.yaml to ensure that renamed script is covered by PR testing.

Signed-off-by: Xin Wang <[email protected]>
vikshaw-Nokia pushed a commit to vikshaw-Nokia/sonic-mgmt that referenced this pull request Oct 23, 2024
What is the motivation for this PR?
Script tests/generic_config_updater/test_ipv6.py was renamed to tests/generic_config_updater/test_ip_bgp.py in PR sonic-net#13650
Need to update file name in pr_test_scripts.yaml to ensure that renamed script is covered by PR testing.

How did you do it?
Rename script name from generic_config_updater/test_ipv6.py to generic_config_updater/test_ip_bgp.py
@bingwang-ms
Copy link
Copy Markdown
Collaborator

@wen587 Can you please check if this change is required for 202405?

@wen587
Copy link
Copy Markdown
Contributor

wen587 commented Nov 15, 2024

@wen587 Can you please check if this change is required for 202405?

Hi @bingwang-ms , this is an improvement from ipv6 only test to ipv4+ipv6 test. It is good to have in 202405 but not bug fix related.

devin-ai-integration bot added a commit to arthur-cog-sonic/sonic-mgmt that referenced this pull request Feb 16, 2026
test_ipv6.py was renamed to test_ip_bgp.py in sonic-net#13650
but the reference in kvmtest.sh was not updated, causing 'file not found'
errors during kvmtest t0 runs.

Co-Authored-By: Arthur Poon <[email protected]>
devin-ai-integration bot added a commit to arthur-cog-sonic/sonic-buildimage that referenced this pull request Feb 16, 2026
Add sed patch to fix the stale test_ipv6.py reference in kvmtest.sh
that was renamed to test_ip_bgp.py in sonic-net/sonic-mgmt#13650.
This was causing 'file or directory not found' errors during kvmtest.

Co-Authored-By: Arthur Poon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants