Update ptf_runner to detect PTF image type and test by opcoder0 · Pull Request #13644 · sonic-net/sonic-mgmt

opcoder0 · 2024-07-12T04:34:38Z

The PR makes the following changes with backward compatibility (with older branches to allow backporting) -

Enhances ptf_runner function to detect the python environment and set ptf binary path accordingly.
Removes use of is_python3
Adds exabgp v4 config and uses six to keep the calls to Py2 and Py3 to support back-porting.

The PR makes backward compatible changes to support -

Python 2 only
Python 2 + Python 3 (mixed environments with python3 virtualenv)
Python 3 only environments

Summary:
Fixes # Not applicable

Type of change

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

Back port request

Approach

What is the motivation for this PR?

As Python 2 has reached EOL it is important to migrate all the PTF test scripts running in Python 2 to 3.

How did you do it?

Create a Python 3 only PTF image buildable with sonic-buildimage - Completed
Migrate individual PTF test scripts to Python 3 - Completed (several PRs)
Update test scripts to run in the Python 3 only environment - This PR

How did you verify/test it?

By running individual PTF tests on S6100 and Cisco 8111 devices.

Any platform specific information?

These changes are not platform specific.

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

Not applicable

- Modify http_api to work with Flask in Py3 environment - Modify exabgp configuration to be compatible with v4 - All PTF test scripts are Py3 compatible the 'py3' directory is no longer required and hence removed. - PTF path on PTF container is /usr/local/bin/ptf - Remove Python 3 virtual environment path /root/env-python3 and use Python 3 from global environment /usr/bin - Remove is_python3 from ptf_runner and update all callers

mssonicbld · 2024-08-02T06:30:41Z

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/ptf_runner.py:89:24: F821 undefined name 'pdb' tests/ptf_runner.py:102:12: F821 undefined name 'pdb' 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:

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>

…; Modify ptf_runner to check environment and invoke appropriate ptf binary

opcoder0 · 2024-08-07T02:20:43Z

These build failures have been fixed in the PR #14001

wangxin · 2024-08-13T07:02:21Z

ansible/library/exabgp.py

 http_api_py = '''\
 from flask import Flask, request
 import sys
+import six


Duplicated import six here.

This import here is part of http_api_py which is the HTTP API processor generated as /usr/share/exabgp/http_api.py on the PTF container and is not for the exabgp.py.

wangxin · 2024-08-13T07:04:58Z

ansible/library/exabgp.py

    }
 '''

+# exabgp v3 configuration file format


exabgp v4 means IPv4. So, the v3 here is misleading.

I think the file here was referring to ExaBGP v3 in the context of version 3. And v4 is for ExaBGP v4 which supported Python 3. I can probably modify the variable name to indicate release v4 instead of just v4.

Modified variable names to disambiguate.

wangxin · 2024-08-13T07:05:34Z

ansible/library/exabgp.py

 }
 '''

+# exabgp v4 for py3 uses a different configuration file


exabgp v4 is kind of misleading as well.

Same as above

Modified variable names to disambiguate.

wangxin · 2024-08-13T07:08:56Z

tests/templates/icmp_responder.conf.j2

 [program:icmp_responder]
 command=/root/env-python3/bin/python /opt/icmp_responder.py {{ icmp_responder_args }}
 process_name=icmp_responder
-environment=PATH="/root/env-python3/bin"


Why remove this line and keep /root/env-python3/bin/python in line 2? If the purpose is to use whatever default python binary, then line 2 should be updated as well. Eventually /root/env-python3 may be removed in "py3only" ptf image.

The environment is not required for the script to run. To keep backporting simpler I have done away from modifying the path to invoke Python 3. This PR enables that sonic-net/sonic-buildimage#19813. Can you please review this too.

wangxin · 2024-08-13T07:22:53Z

tests/ptf_runner.py

+    logger.info('Test file path {}, in py3: {}'.format(test_fpath, in_py3))
+    is_python3 = is_py3_compat(test_fpath)
+
+    ptf_cmd = None


Probably add some comments here to better explain the logic here.
The expectation of PTF image is either "mixed" or "py3only". "mixed" means that global python is python2. Python3 and dependent package is installed in /root/env-python3. "py3only" means that there is no python2. The global python is python3 and it has all required dependent packages.
For "mixed" ptf image, use python3 version ptf command to run python3 compatible ptf scripts. Use python2 version ptf command to run legacy python2 PTF scripts.
For "py3only" ptf image, always use the global python3 ptf command to run PTF scripts. Raise exception if ptf script is not python3 compatible.

I have added the comment.

…used with IP v4

wangxin · 2024-08-15T09:11:32Z

/azp run

azure-pipelines · 2024-08-15T09:11:43Z

Azure Pipelines successfully started running 1 pipeline(s).

The PR makes the following changes with backward compatibility (with older branches to allow backporting) - Enhances ptf_runner function to detect the python environment and set ptf binary path accordingly. Removes use of is_python3 Adds exabgp v4 config and uses six to keep the calls to Py2 and Py3 to support back-porting. The PR makes backward compatible changes to support - Python 2 only Python 2 + Python 3 (mixed environments with python3 virtualenv) Python 3 only environments What is the motivation for this PR? As Python 2 has reached EOL it is important to migrate all the PTF test scripts running in Python 2 to 3. How did you do it? Create a Python 3 only PTF image buildable with sonic-buildimage - Completed Migrate individual PTF test scripts to Python 3 - Completed (several PRs) Update test scripts to run in the Python 3 only environment - This PR How did you verify/test it? By running individual PTF tests on S6100 and Cisco 8111 devices. Any platform specific information? These changes are not platform specific.

opcoder0 requested a review from StormLiangMS as a code owner July 12, 2024 04:34

opcoder0 marked this pull request as draft July 12, 2024 04:34

opcoder0 requested review from wangxin, wsycqyz, xwjiang-ms and yutongzhang-microsoft July 12, 2024 04:35

opcoder0 added 4 commits August 1, 2024 07:31

Undo moving files out of py3 directory

0b2e3ab

Update paths after making PTF path same as Py2 image

ce055f1

Use six to support backporting

baa2798

Merge branch 'master' into sagummaraj/ptf-py3

a8172e4

opcoder0 added 5 commits August 2, 2024 06:33

Fix merge conflict

8016b01

Make invoking PTF compatible across venv, py3only and py2only envs

2bef6bf

Revert to use /root/env-python3 paths after updating docker-ptf image…

7e2a198

…; Modify ptf_runner to check environment and invoke appropriate ptf binary

restore files to correct py3 path

77daf5f

Undo wrong path fixing

f3610b0

opcoder0 marked this pull request as ready for review August 5, 2024 06:40

opcoder0 added 7 commits August 7, 2024 08:35

Make ptf runner compatible across different image type and backport safe

337681e

change logic to check ptf venv file

0937985

Merge branch 'master' into sagummaraj/ptf-py3

3285d8e

Fix LAG test in py3 env

f7369c3

Restore is_python3 calling flag

1e6c119

Fix formatting

f97550e

Undo py3 comment change

5643355

opcoder0 changed the title ~~Update all PTF scripts to run in Python 3 (py3) PTF container~~ Update ptf_runner to detect PTF image type and test Aug 8, 2024

xwjiang-ms approved these changes Aug 8, 2024

View reviewed changes

wangxin reviewed Aug 13, 2024

View reviewed changes

Change ExaBGP Version 4 conf template variable name so it is not conf…

b7fdfb8

…used with IP v4

Add comment to explain ptf_runner logic

630ba45

wangxin approved these changes Aug 15, 2024

View reviewed changes

Merge branch 'master' into sagummaraj/ptf-py3

75692ba

wangxin merged commit 18e6509 into sonic-net:master Aug 21, 2024

Conversation

opcoder0 commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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?

Uh oh!

mssonicbld commented Aug 2, 2024

Uh oh!

opcoder0 commented Aug 7, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

opcoder0 Aug 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangxin commented Aug 15, 2024

Uh oh!

azure-pipelines bot commented Aug 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

opcoder0 commented Jul 12, 2024 •

edited

Loading

opcoder0 Aug 14, 2024 •

edited

Loading