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

Removing null characters while decoding from syseeprom #338

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

mihirpat1
Copy link
Contributor

@mihirpat1 mihirpat1 commented Dec 19, 2022

Description

This PR will track the resolution for failure seen on Mellanox devices with SONiC management test failure for test_pmon_syseepromd_kill_and_start_status test case. The testcase is failing with the below error:

========================================================================= ERRORS =========================================================================
______________________________________________ ERROR at setup of test_pmon_syseepromd_term_and_start_status ______________________________________________

duthosts = [<MultiAsicSonicHost str2-sn3800-02>], rand_one_dut_hostname = 'str2-sn3800-02'

    @pytest.fixture(scope='module')
    def data_before_restart(duthosts, rand_one_dut_hostname):
        duthost = duthosts[rand_one_dut_hostname]
    
>       data = collect_data(duthost)

duthost    = <MultiAsicSonicHost str2-sn3800-02>
duthosts   = [<MultiAsicSonicHost str2-sn3800-02>]
rand_one_dut_hostname = 'str2-sn3800-02'

platform_tests/daemon/test_syseepromd.py:99: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
platform_tests/daemon/test_syseepromd.py:80: in collect_data
    data = compose_dict_from_cli(data)
common/utilities.py:515: in compose_dict_from_cli
    return literal_eval(str_output)
/usr/lib/python2.7/ast.py:49: in literal_eval
    node_or_string = parse(node_or_string, mode='eval')
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

source = u"{'Name': 'Part Number', 'Len': '20', 'Value': 'MSN3800-CS2FO\x00\x00\x00\x00\x00\x00\x00'}", filename = '<unknown>', mode = 'eval'

    def parse(source, filename='<unknown>', mode='exec'):
        """
        Parse the source into an AST node.
        Equivalent to compile(source, filename, mode, PyCF_ONLY_AST).
        """
>       return compile(source, filename, mode, PyCF_ONLY_AST)
E       TypeError: compile() expected string without null bytes

filename   = '<unknown>'
mode       = 'eval'
source     = u"{'Name': 'Part Number', 'Len': '20', 'Value': 'MSN3800-CS2FO\x00\x00\x00\x00\x00\x00\x00'}"

/usr/lib/python2.7/ast.py:37: TypeError
-------------------------------------------- generated xml file: /var/src/test_lpm/tests/logs/tmp.log/tr.xml ---------------------------------------------
================================================================ short test summary info =================================================================
ERROR platform_tests/daemon/test_syseepromd.py::test_pmon_syseepromd_term_and_start_status - TypeError: compile() expected string without null bytes
=============================================================== 1 error in 113.67 seconds ================================================================
INFO:root:Can not get Allure report URL. Please check logs

Motivation and Context

Overall, the testcase failed on Mellanox since the syseeprom has some fields wherein the string is appended by null characters. When the above testcase is executed, the null characters in the string are causing the error seen.
We observed that other platforms have string not terminated with null characters for various fields and the length mentioned in the EEPROM is same that of the string on EEPROM.
The below o/p was taken from Mellanox 3800 and it can be seen that the length of string is 20 which includes multiple null characters (^@ is 1 null character). The actual length of the string is 13 and there are 7 additional null characters.
root@str2-sn3800-02:/home/admin# sonic-db-cli STATE_DB HGETALL "EEPROM_INFO|0x22" | cat --show-nonprinting
{'Name': 'Part Number', 'Len': '20', 'Value': 'MSN3800-CS2FO^@^@^@^@^@^@^@'}
root@str2-sn3800-02:/home/admin#

How Has This Been Tested?

Please refer to the below for the details related to the tests performed
test_pmon_syseepromd_kill_and_start_status_test.txt

Additional Information (Optional)

Signed-off-by: Mihir Patel patelmi@microsoft.com

Signed-off-by: Mihir Patel <patelmi@microsoft.com>
@mihirpat1 mihirpat1 marked this pull request as ready for review December 19, 2022 07:19
@prgeor
Copy link
Collaborator

prgeor commented Dec 19, 2022

@mihirpat1 Thanks Mihir. Can you test this on Celestica, Dell and arista platforms?

@mihirpat1
Copy link
Contributor Author

mihirpat1 commented Dec 19, 2022

@mihirpat1 Thanks Mihir. Can you test this on Celestica, Dell and arista platforms?

Hi @prgeor ,
I have currently tested on Celestica (dx010) and Dell (s6100) and have attached the details to test_pmon_syseepromd_kill_and_start_status_test.txt
Also, I have tested the change on Arista 7060dx4 but haven't attached the details since Arista has its own implementation of the APIs.

@mihirpat1 mihirpat1 merged commit 676b329 into sonic-net:master Dec 19, 2022
keboliu pushed a commit to keboliu/sonic-platform-common that referenced this pull request Jan 12, 2023
Signed-off-by: Mihir Patel <patelmi@microsoft.com>
StormLiangMS pushed a commit that referenced this pull request Jan 23, 2023
Signed-off-by: Mihir Patel <patelmi@microsoft.com>
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-platform-common that referenced this pull request Oct 25, 2024
…dor API CLI return key-values pairs (sonic-net#338)

The main motivation of this PR is improve coverage of ycabled.
While fixing unit test cases found an improvement to the name key-values returned to CLI, changed those names for keys as well

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

Description
Motivation and Context
How Has This Been Tested?
Unit-Tests and running the changes on testbed.

Additional Information (Optional)
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.

5 participants