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

[python3 migration] Added Python3 support for new test cases #4867

Conversation

OleksandrKozodoi
Copy link
Contributor

Signed-off-by: Oleksandr Kozodoi [email protected]

Description of PR

This PR includes necessary changes from #9277 for the setup of the Python3 virtual environment in the sonic-mgmt docker container.

Summary:

  • Added Python3 support for lldp test case.
  • Added Python3 support for dhcp_relay test case.
  • Added Python3 support for decap test case.
  • Added Python3 support for fib test case.
  • Added Python3 support for bgp_bounce test case.

How to use Python3 environment for running test cases?

  1. Connect to the sonic-mgmt container
$ docker exec -ti sonic-mgmt bash
  1. Activate the virtual environment
$ source /var/user/env-python3/bin/activate
  1. Deploy t0/t1 topology
  2. Run test case
py.test --testbed=testbed-t0 --inventory=../ansible/lab --testbed_file=../ansible/testbed.csv --host-pattern=testbed-t0 -- module-path=../ansible/library lldp
py.test --testbed=testbed-t0 --inventory=../ansible/lab --testbed_file=../ansible/testbed.csv --host-pattern=testbed-t0 -- module-path=../ansible/library dhcp_relay
py.test --testbed=testbed-t0 --inventory=../ansible/lab --testbed_file=../ansible/testbed.csv --host-pattern=testbed-t0 -- module-path=../ansible/library decap
py.test --testbed=testbed-t0 --inventory=../ansible/lab --testbed_file=../ansible/testbed.csv --host-pattern=testbed-t0 -- module-path=../ansible/library fib
py.test --testbed=testbed-t0 --inventory=../ansible/lab --testbed_file=../ansible/testbed.csv --host-pattern=testbed-t0 -- module-path=../ansible/library bgp/test_bgp_bounce.py

Type of change

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

Back port request

  • 201911

Approach

What is the motivation for this PR?

Migration of sonic-mgmt codebase from Python 2 to Python 3

How did you do it?

Updated codebase of sonic-mgmt for supporting Python3

How did you verify/test it?

Run test cases. Tests passed.

Any platform specific information?

SONiC Software Version: SONiC.master.60600-dirty-20211221.191419
Distribution: Debian 11.2
Kernel: 5.10.0-8-2-amd64
Build commit: 67e40b553
Build date: Tue Dec 21 19:22:34 UTC 2021
ASIC: barefoot

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

t0, t1

Documentation

@OleksandrKozodoi OleksandrKozodoi requested a review from a team as a code owner December 22, 2021 09:20
@@ -258,7 +258,7 @@ def main():

lldp_data = dict()

for intf in lldp_rem_sys.viewkeys():
for intf in lldp_rem_sys.keys():

Choose a reason for hiding this comment

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

for loop will iter over keys by default. keys method call is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks

nexthop_ip = ipaddress.ip_address(unicode(m.group(1)))
nexthop_ip = ipaddress.ip_address(m.group(1).encode().decode("utf-8"))
Copy link

@globaltrouble globaltrouble Dec 22, 2021

Choose a reason for hiding this comment

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

Here and later you can use six for porting code from py2 to py3 and wise verse. For example six.text_type to always work with unicode in both py versions and six.binary_type can be used to always work with bytes in both py versions.

Are you sure you need to do sequential call of opposite methods encode and decode? what is the type of m.group(1) and how does it change after conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@OleksandrKozodoi
Copy link
Contributor Author

@yxieca @wangxin Can you take a look? Thank you in advance.

@@ -26,8 +26,7 @@ def __init__(self, msg, results=None):
self.results = results

def _to_string(self):
return unicode(u"{}, Ansible Results =>\n{}".format(self.message, dump_ansible_results(self.results)))\
.encode('ascii', 'backslashreplace')
return "{}, Ansible Results =>\n{}".format(self.message, dump_ansible_results(self.results)).encode().decode("utf-8")
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for calling encode() followed by decode("utf-8") here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This approach is used for backward compatibility of code between different versions of Python. Ansible returns different types of dump depending on Python versions.

@wangxin
Copy link
Collaborator

wangxin commented Feb 7, 2022

@OleksandrKozodoi Thanks for the efforts of making the scripts working for python3!
Have you tested the changes using python2? I believe there are more compatibilities issues in the repository. Before all the issues are resolved, we need to stick with python2 for some time. So, all the changes need to be both python2&3 compatible.

@OleksandrKozodoi
Copy link
Contributor Author

@wangxin All scripts were tested using python2 and python3 versions. All necessary dependencies for testing python2&3 compatible will be available in python venv of the sonic-mgmt docker container after merging 9277. These changes allow running tests on different versions in a single sonic-mgmt container. Thanks!

wangxin
wangxin previously approved these changes Mar 1, 2022
@wangxin
Copy link
Collaborator

wangxin commented Mar 1, 2022

LGTM, @OleksandrKozodoi Can you help resolve the conflicts?

@OleksandrKozodoi OleksandrKozodoi force-pushed the update_new_test_cases_for_supporting_python3 branch from f7f8676 to 48b39b8 Compare March 3, 2022 17:07
@OleksandrKozodoi
Copy link
Contributor Author

@wangxin I've resolved conflicts and added some minor changes for supporting the original codebase. But should be merged 9277 PR of sonic-buildimage repo for checking compatibility of new changes with python3. Thanks.

@OleksandrKozodoi OleksandrKozodoi force-pushed the update_new_test_cases_for_supporting_python3 branch from 48b39b8 to 9252d27 Compare March 31, 2022 11:38
@ZhaohuiS ZhaohuiS merged commit 771f7d1 into sonic-net:master Apr 25, 2022
@ppikh
Copy link
Contributor

ppikh commented May 3, 2022

Hi @OleksandrKozodoi

Looks like this commit cause failures: TypeError: () takes exactly 2 arguments (1 given)

Issue happen when run tests from folder: qos/test_qos_sai.py

It's related to file: tests/common/fixtures/ptfhost_utils.py

Example of python traceback:

duthosts = [<MultiAsicSonicHost arc-switch1038>], rand_one_dut_hostname = 'arc-switch1038', ptfhost = <tests.common.devices.ptf.PTFHost object at 0x7f60d0924590>

    @pytest.fixture(scope='class')
    def ptf_portmap_file(duthosts, rand_one_dut_hostname, ptfhost):
        """
            Prepare and copys port map file to PTF host
    
            Args:
                request (Fixture): pytest request object
                duthost (AnsibleHost): Device Under Test (DUT)
                ptfhost (AnsibleHost): Packet Test Framework (PTF)
    
            Returns:
                filename (str): returns the filename copied to PTF host
        """
        duthost = duthosts[rand_one_dut_hostname]
        intfInfo = duthost.show_interface(command = "status")['ansible_facts']['int_status']
        portList = natsorted([port for port in intfInfo if port.startswith('Ethernet')])
        portMapFile = "/tmp/default_interface_to_front_map.ini"
        with open(portMapFile, 'w') as file:
            file.write("# ptf host interface @ switch front port name\n")
            file.writelines(
                map(
                        lambda index, port: "{0}@{1}\n".format(index, port),
>                       enumerate(portList)
                    )
                )
E           TypeError: <lambda>() takes exactly 2 arguments (1 given)

duthost    = <MultiAsicSonicHost arc-switch1038>
duthosts   = [<MultiAsicSonicHost arc-switch1038>]
file       = <closed file '/tmp/default_interface_to_front_map.ini', mode 'w' at 0x7f60d028cd20>
intfInfo   = {'Ethernet0': {'admin_state': u'up', 'alias': u'etp1', 'fec': u'rs', 'name': u'Ethernet0', ...}, 'Ethernet100': {'admi...et104', ...}, 'Ethernet108': {'admin_state': u'up', 'alias': u'etp28', 'fec': u'rs', 'name': u'Ethernet108', ...}, ...}
port       = 'Ethernet68'
portList   = ['Ethernet0', 'Ethernet4', 'Ethernet8', 'Ethernet12', 'Ethernet16', 'Ethernet20', ...]
portMapFile = '/tmp/default_interface_to_front_map.ini'
ptfhost    = <tests.common.devices.ptf.PTFHost object at 0x7f60d0924590>
rand_one_dut_hostname = 'arc-switch1038'

common/fixtures/ptfhost_utils.py:186: TypeError
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> entering PDB >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB post_mortem (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /root/mars/workspace/sonic-mgmt/tests/common/fixtures/ptfhost_utils.py(186)ptf_portmap_file()
-> enumerate(portList)
(Pdb) portList
[u'Ethernet0', u'Ethernet4', u'Ethernet8', u'Ethernet12', u'Ethernet16', u'Ethernet20', u'Ethernet24', u'Ethernet28', u'Ethernet32', u'Ethernet36', u'Ethernet40', u'Ethernet44', u'Ethernet48', u'Ethernet52', u'Ethernet56', u'Ethernet60', u'Ethernet64', u'Ethernet65', u'Ethernet66', u'Ethernet67', u'Ethernet68', u'Ethernet72', u'Ethernet76', u'Ethernet80', u'Ethernet82', u'Ethernet84', u'Ethernet88', u'Ethernet92', u'Ethernet96', u'Ethernet100', u'Ethernet104', u'Ethernet108', u'Ethernet112', u'Ethernet116', u'Ethernet120', u'Ethernet124']
(Pdb) index
*** NameError: name 'index' is not defined
(Pdb) port
u'Ethernet68'

@OleksandrKozodoi
Copy link
Contributor Author

Hi @OleksandrKozodoi

Looks like this commit cause failures: TypeError: () takes exactly 2 arguments (1 given)

Issue happen when run tests from folder: qos/test_qos_sai.py

It's related to file: tests/common/fixtures/ptfhost_utils.py

Example of python traceback:

duthosts = [<MultiAsicSonicHost arc-switch1038>], rand_one_dut_hostname = 'arc-switch1038', ptfhost = <tests.common.devices.ptf.PTFHost object at 0x7f60d0924590>

    @pytest.fixture(scope='class')
    def ptf_portmap_file(duthosts, rand_one_dut_hostname, ptfhost):
        """
            Prepare and copys port map file to PTF host
    
            Args:
                request (Fixture): pytest request object
                duthost (AnsibleHost): Device Under Test (DUT)
                ptfhost (AnsibleHost): Packet Test Framework (PTF)
    
            Returns:
                filename (str): returns the filename copied to PTF host
        """
        duthost = duthosts[rand_one_dut_hostname]
        intfInfo = duthost.show_interface(command = "status")['ansible_facts']['int_status']
        portList = natsorted([port for port in intfInfo if port.startswith('Ethernet')])
        portMapFile = "/tmp/default_interface_to_front_map.ini"
        with open(portMapFile, 'w') as file:
            file.write("# ptf host interface @ switch front port name\n")
            file.writelines(
                map(
                        lambda index, port: "{0}@{1}\n".format(index, port),
>                       enumerate(portList)
                    )
                )
E           TypeError: <lambda>() takes exactly 2 arguments (1 given)

duthost    = <MultiAsicSonicHost arc-switch1038>
duthosts   = [<MultiAsicSonicHost arc-switch1038>]
file       = <closed file '/tmp/default_interface_to_front_map.ini', mode 'w' at 0x7f60d028cd20>
intfInfo   = {'Ethernet0': {'admin_state': u'up', 'alias': u'etp1', 'fec': u'rs', 'name': u'Ethernet0', ...}, 'Ethernet100': {'admi...et104', ...}, 'Ethernet108': {'admin_state': u'up', 'alias': u'etp28', 'fec': u'rs', 'name': u'Ethernet108', ...}, ...}
port       = 'Ethernet68'
portList   = ['Ethernet0', 'Ethernet4', 'Ethernet8', 'Ethernet12', 'Ethernet16', 'Ethernet20', ...]
portMapFile = '/tmp/default_interface_to_front_map.ini'
ptfhost    = <tests.common.devices.ptf.PTFHost object at 0x7f60d0924590>
rand_one_dut_hostname = 'arc-switch1038'

common/fixtures/ptfhost_utils.py:186: TypeError
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> entering PDB >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB post_mortem (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /root/mars/workspace/sonic-mgmt/tests/common/fixtures/ptfhost_utils.py(186)ptf_portmap_file()
-> enumerate(portList)
(Pdb) portList
[u'Ethernet0', u'Ethernet4', u'Ethernet8', u'Ethernet12', u'Ethernet16', u'Ethernet20', u'Ethernet24', u'Ethernet28', u'Ethernet32', u'Ethernet36', u'Ethernet40', u'Ethernet44', u'Ethernet48', u'Ethernet52', u'Ethernet56', u'Ethernet60', u'Ethernet64', u'Ethernet65', u'Ethernet66', u'Ethernet67', u'Ethernet68', u'Ethernet72', u'Ethernet76', u'Ethernet80', u'Ethernet82', u'Ethernet84', u'Ethernet88', u'Ethernet92', u'Ethernet96', u'Ethernet100', u'Ethernet104', u'Ethernet108', u'Ethernet112', u'Ethernet116', u'Ethernet120', u'Ethernet124']
(Pdb) index
*** NameError: name 'index' is not defined
(Pdb) port
u'Ethernet68'

Hi @ppikh. #5605 should fix this issue.

ZhaohuiS added a commit that referenced this pull request Jun 22, 2022
What is the motivation for this PR?
Fix the issue reported in #4867, failures: TypeError: () takes exactly 2 arguments (1 given)
The fix in #5605 still uses lambda which is not easy to read and may have compatibility issue.

How did you do it?
Use for loop and concentrate string together and append them to a list.

How did you verify/test it?
run tests/test_qos_sai.py

Signed-off-by: Zhaohui Sun <[email protected]>
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.

6 participants