Skip to content

BFD+VXLAN Automation#6212

Merged
siqbal1986 merged 24 commits intosonic-net:masterfrom
gollarharsha:master
Nov 9, 2022
Merged

BFD+VXLAN Automation#6212
siqbal1986 merged 24 commits intosonic-net:masterfrom
gollarharsha:master

Conversation

@gollarharsha
Copy link
Contributor

BFD automation with VXLAN was done for the first 3 cases:

Here are additional changes that were added:

As discussed in last session, I have enabled BFD in the same file as VXLAN. By passing bfd argument, we should be able to run existing VXLAN test cases.
Bfd_sniffer script will extract BFD packets and automatically populate the config file for bfd_responder.
Cleanup for BFD was added in existing vxlan script itself.

I have ran first 3 test cases for all 4 encap types. All 12 cases passed as well.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 23, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@lgtm-com
Copy link

lgtm-com bot commented Aug 23, 2022

This pull request introduces 18 alerts and fixes 5 when merging 80124202714a42af6186655c23658509584c681c into 5a7b5a0 - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 6 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None
  • 1 for Variable defined multiple times

fixed alerts:

  • 2 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None

@gollarharsha
Copy link
Contributor Author

gollarharsha commented Aug 24, 2022

@siqbal1986 - I cleared those CLA errors. Now, reviewers can be added.
Cc: @prsunny

@lgtm-com
Copy link

lgtm-com bot commented Aug 26, 2022

This pull request introduces 19 alerts and fixes 5 when merging cff8962 into 9b5a439 - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 6 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Variable defined multiple times

fixed alerts:

  • 2 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None

@lgtm-com
Copy link

lgtm-com bot commented Aug 30, 2022

This pull request introduces 19 alerts and fixes 5 when merging 7381622 into 176a2fe - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 6 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Variable defined multiple times

fixed alerts:

  • 2 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None

@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2022

This pull request introduces 19 alerts and fixes 5 when merging ab3ca28 into 00b9f3e - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 6 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Variable defined multiple times

fixed alerts:

  • 2 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None

@siqbal1986
Copy link
Contributor

Can you please specify which Vxlan tests you ran with the BFD exactly?

@lgtm-com
Copy link

lgtm-com bot commented Sep 1, 2022

This pull request introduces 19 alerts and fixes 5 when merging d28e7d1 into 9ef9448 - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 6 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Variable defined multiple times

fixed alerts:

  • 2 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None

@gollarharsha
Copy link
Contributor Author

gollarharsha commented Sep 1, 2022

Hi @siqbal1986 ,

Here are the Test cases that will work with BFD:
Test_VxLAN_route_tests
test_vxlan_single_endpoint
test_vxlan_modify_route_different_endpoint
test_vxlan_remove_all_route

Test_VxLAN_ecmp_create
test_vxlan_configure_route1_ecmp_group_a
test_vxlan_configure_route1_ecmp_group_b
test_vxlan_configure_route2_ecmp_group_b

Test_VxLAN_NHG_Modify
test_vxlan_route2_shared_nh
test_vxlan_remove_ecmp_route2

@lgtm-com
Copy link

lgtm-com bot commented Sep 2, 2022

This pull request introduces 19 alerts and fixes 5 when merging 0bb7626 into 9ef9448 - view on LGTM.com

new alerts:

  • 8 for Unused import
  • 6 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None
  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Variable defined multiple times

fixed alerts:

  • 2 for Unused local variable
  • 2 for Result of integer division may be truncated
  • 1 for Testing equality to None

@yejianquan
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yejianquan
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@gollarharsha gollarharsha changed the title BFD+VXLAN First 3 Test cases BFD+VXLAN Automation Sep 6, 2022
Copy link

@ihorchekh ihorchekh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review these changes

@azure-pipelines
Copy link

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/vxlan/test_vxlan_ecmp.py
Fixing tests/vxlan/vxlan_ecmp_utils.py
Fixing tests/vxlan/bfd_sniffer.py
Fixing tests/vxlan/conftest.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/helpers/bfd_responder.py
Fixing tests/templates/bfd_responder.conf.j2
Fixing tests/vxlan/test_vxlan_ecmp.py
Fixing ansible/roles/test/files/ptftests/vxlan_traffic.py
Fixing tests/vxlan/bfd_sniffer.py
...
[truncated extra lines, please run pre-commit locally to view full check results]

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>

@lgtm-com
Copy link

lgtm-com bot commented Oct 18, 2022

This pull request introduces 12 alerts and fixes 3 when merging 60cba0f into 5ead051 - view on LGTM.com

new alerts:

  • 8 for Unused local variable
  • 1 for Testing equality to None
  • 1 for Except block handles 'BaseException'
  • 1 for Unused import
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 2 for Unused local variable
  • 1 for Testing equality to None

@azure-pipelines
Copy link

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/vxlan/test_vxlan_ecmp.py
Fixing tests/vxlan/vxlan_ecmp_utils.py
Fixing tests/vxlan/bfd_sniffer.py
Fixing tests/vxlan/conftest.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/helpers/bfd_responder.py
Fixing tests/templates/bfd_responder.conf.j2
Fixing tests/vxlan/test_vxlan_ecmp.py
Fixing ansible/roles/test/files/ptftests/vxlan_traffic.py
Fixing tests/vxlan/bfd_sniffer.py
...
[truncated extra lines, please run pre-commit locally to view full check results]

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>

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 12 alerts and fixes 3 when merging 108efe3 into da0aae7 - view on LGTM.com

new alerts:

  • 8 for Unused local variable
  • 1 for Testing equality to None
  • 1 for Except block handles 'BaseException'
  • 1 for Unused import
  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 2 for Unused local variable
  • 1 for Testing equality to None

@kevinskwang
Copy link
Contributor

@gollarharsha please check the pre-commit-check failures and fix it.

@azure-pipelines
Copy link

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/vxlan/conftest.py
Fixing tests/vxlan/bfd_sniffer.py
Fixing tests/vxlan/vxlan_ecmp_utils.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/helpers/bfd_responder.py
Fixing tests/templates/bfd_responder.conf.j2
Fixing tests/vxlan/test_vxlan_ecmp.py
Fixing tests/vxlan/bfd_sniffer.py

check yaml...........................................(no files to check)Skipped
...
[truncated extra lines, please run pre-commit locally to view full check results]

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>

@rraghav-cisco
Copy link
Contributor

Adding logs from the last changes.
script_logs.tar.gz
@siqbal1986 , pls let us know how it goes.

@lgtm-com
Copy link

lgtm-com bot commented Oct 27, 2022

This pull request introduces 14 alerts and fixes 3 when merging b82073d into e0ede4d - view on LGTM.com

new alerts:

  • 7 for Unused local variable
  • 3 for Except block handles 'BaseException'
  • 2 for Unused import
  • 1 for Testing equality to None
  • 1 for Unused argument in a formatting call

fixed alerts:

  • 2 for Unused local variable
  • 1 for Testing equality to None

1. Updated the traffic script to pump all the packets of an iteration in one
   go, and then wait in a timed-loop for reply packets. This speeds up the
   traffic script greatly. The full runtime is now 3.5 hours.
2. Added --include_long_tests argument, so that user can run basic tests
   by default(45 minutes runtime for all 4 encap types). If the long tests
   are included, the total runtime is now 3.5 hours.
3. Updated the scripts for improved pylint score. The new scores:
   a) test_vxlan_ecmp.py: 8.89/10
   b) vxlan_traffic.py  : 7.18/10
   c) vxlan_ecmp_utils.py : 9.31/10
@azure-pipelines
Copy link

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/vxlan/conftest.py
Fixing tests/vxlan/bfd_sniffer.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/helpers/bfd_responder.py
Fixing tests/templates/bfd_responder.conf.j2
Fixing tests/vxlan/bfd_sniffer.py

check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Failed
...
[truncated extra lines, please run pre-commit locally to view full check results]

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>

@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2022

This pull request introduces 6 alerts and fixes 6 when merging 90362dd into 8580f64 - view on LGTM.com

new alerts:

  • 3 for Except block handles 'BaseException'
  • 1 for Unused local variable
  • 1 for Unused import
  • 1 for Unused argument in a formatting call

fixed alerts:

  • 3 for Unused local variable
  • 2 for Unused import
  • 1 for Testing equality to None

… flake8 checks

are clean.
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 ansible/roles/test/files/ptftests/vxlan_traffic.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 ansible/roles/test/files/ptftests/bfd_responder.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 tests/vxlan/test_vxlan_ecmp.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 tests/vxlan/vxlan_ecmp_utils.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 tests/vxlan/conftest.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root#

Run and verified:
iINFO:root:Can not get Allure report URL. Please check logs
=========================== short test summary info ============================
SKIPPED [4] vxlan/test_vxlan_ecmp.py:1962: This test will be run only if '--include_crm=True' is provided.
SKIPPED [4] vxlan/test_vxlan_ecmp.py:1908: This test will be run only if '--include_crm=True' is provided.
SKIPPED [4] vxlan/test_vxlan_ecmp.py:1948: This test will be run only if '--include_crm=True' is provided.
ERROR vxlan/test_vxlan_ecmp.py::Test_VxLAN_Crm::test_crm_128_group_members[v6_in_v6]
============= 104 passed, 12 skipped, 1 error in 10992.56 seconds ==============
@azure-pipelines
Copy link

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/vxlan/bfd_sniffer.py

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/helpers/bfd_responder.py
Fixing tests/templates/bfd_responder.conf.j2
Fixing tests/vxlan/bfd_sniffer.py

check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Failed
- hook id: check-ast
...
[truncated extra lines, please run pre-commit locally to view full check results]

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>

@lgtm-com
Copy link

lgtm-com bot commented Nov 6, 2022

This pull request introduces 4 alerts and fixes 12 when merging d62bc03 into 8580f64 - view on LGTM.com

new alerts:

  • 3 for Except block handles 'BaseException'
  • 1 for Unused local variable

fixed alerts:

  • 5 for Unused local variable
  • 4 for Unused import
  • 1 for Constant in conditional expression or statement
  • 1 for Testing equality to None
  • 1 for Comparison of constants

@rraghav-cisco
Copy link
Contributor

@siqbal1986

  1. This new update includes fixing the pre-commit errors.
  2. Fixing the time to run: By default the script will run the quick tests, which take a total of 45 minutes for 4 encap types.
  3. If "--include_long_tests=True" is provided, the total run time is 3:15 hours.
  4. CRM is still excluded.
  5. Updated the ptf traffic script to send all packets quickly and then wait a fixed time for all the return packets. This greatly reduced the time to run for 1000 packets from more than 10 minutes to less than 2 minutes.

@rraghav-cisco
Copy link
Contributor

@siqbal1986, pls merge it.

@siqbal1986 siqbal1986 merged commit 77a4b01 into sonic-net:master Nov 9, 2022
@prsunny
Copy link
Contributor

prsunny commented Nov 10, 2022

@siqbal1986 , is this planned to be cherry-picked to 202012 or raising a separate PR?

@siqbal1986
Copy link
Contributor

@siqbal1986 , is this planned to be cherry-picked to 202012 or raising a separate PR?

I don't think automatic cherry pick on this would work. @gollarharsha @rraghav-cisco I think a separate PR would be required. Can you please raise one

@rraghav-cisco
Copy link
Contributor

@siqbal1986 and @prsunny, I will take it up.

@prsunny
Copy link
Contributor

prsunny commented Nov 10, 2022

@ihorchekh , can you please check if all comments were addressed and signoff?

@rraghav-cisco
Copy link
Contributor

@prsunny , @siqbal1986 : I have raised #6807

wangxin pushed a commit that referenced this pull request May 9, 2023
1. Updated the traffic script to pump all the packets of an iteration in one
   go, and then wait in a timed-loop for reply packets. This speeds up the
   traffic script greatly. The full runtime is now 3.5 hours.
2. Added --include_long_tests argument, so that user can run basic tests
   by default(45 minutes runtime for all 4 encap types). If the long tests
   are included, the total runtime is now 3.5 hours.
3. Updated the scripts for improved pylint score. The new scores:
   a) test_vxlan_ecmp.py: 8.89/10
   b) vxlan_traffic.py  : 7.18/10
   c) vxlan_ecmp_utils.py : 9.31/10

* Re-formatted and fixed the scripts to pass the pre-commit checks. All flake8 checks
are clean.
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 ansible/roles/test/files/ptftests/vxlan_traffic.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 ansible/roles/test/files/ptftests/bfd_responder.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 tests/vxlan/test_vxlan_ecmp.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 tests/vxlan/vxlan_ecmp_utils.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root# flake8 tests/vxlan/conftest.py
(venv3) RRAGHAV-M-C3KR:sonic-mgmt-hgolla root#

Run and verified:
iINFO:root:Can not get Allure report URL. Please check logs
=========================== short test summary info ============================
SKIPPED [4] vxlan/test_vxlan_ecmp.py:1962: This test will be run only if '--include_crm=True' is provided.
SKIPPED [4] vxlan/test_vxlan_ecmp.py:1908: This test will be run only if '--include_crm=True' is provided.
SKIPPED [4] vxlan/test_vxlan_ecmp.py:1948: This test will be run only if '--include_crm=True' is provided.
ERROR vxlan/test_vxlan_ecmp.py::Test_VxLAN_Crm::test_crm_128_group_members[v6_in_v6]
============= 104 passed, 12 skipped, 1 error in 10992.56 seconds ==============

Co-authored-by: rraghav-cisco <rraghav@cisco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants