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

Check gnxi path and get correct path value before test #5634

Merged
merged 2 commits into from
May 11, 2022

Conversation

ZhaohuiS
Copy link
Contributor

@ZhaohuiS ZhaohuiS commented May 11, 2022

Description of PR

Summary:
Fixes # (issue)
Signed-off-by: Zhaohui Sun zhaohuisun@microsoft.com

Type of change

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

Back port request

  • 201911
  • 202012

Approach

What is the motivation for this PR?

gnxi's location is updated from /gnxi to /root/gnxi in RP sonic-net/sonic-buildimage#10599.
But if test case run on old docker-ptf image which doesn't have this update, it will fail. Because it should still call /gnxi files.

How did you do it?

For avoiding this conflict, check gnxi path before test and set GNXI_PATH to correct value.
Add it in verify_telemetry_dockerimage autouse module fixture to make sure to set GNXI_PATH before test.

How did you verify/test it?

run telemetry/test_telemetry.py

Any platform specific information?

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

Documentation

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
"""If telemetry docker is available in image then return true
"""
global GNXI_PATH
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make it a fixture instead of global variable?

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

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
Copy link
Collaborator

@wangxin wangxin left a comment

Choose a reason for hiding this comment

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

LGTM

@wangxin wangxin merged commit 1d51936 into sonic-net:master May 11, 2022
wangxin pushed a commit that referenced this pull request May 11, 2022
What is the motivation for this PR?
gnxi's location is updated from /gnxi to /root/gnxi in RP sonic-net/sonic-buildimage#10599.
But if test case run on old docker-ptf image which doesn't have this update, it will fail. Because it should still call /gnxi files.

How did you do it?
For avoiding this conflict, check gnxi path before test and set GNXI_PATH to correct value.
Add it in verify_telemetry_dockerimage autouse module fixture to make sure to set GNXI_PATH before test.

How did you verify/test it?
run telemetry/test_telemetry.py

Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>
@wenyiz2021
Copy link
Contributor

wenyiz2021 commented Sep 19, 2022

Hi @wangxin @ZhaohuiS
Noticed test_telemetry_output is still failing at this cmd on 202012 test branch:

2022-09-17T07:17:09.6730990Z "cmd": "python /gnxi/gnmi_cli_py/py_gnmicli.py -g -t 10.3.147.150 -p 50051 -m get -x COUNTERS/Ethernet0 -xt COUNTERS_DB -o "ndastreamingservertest"",

2022-09-17T07:17:09.6787480Z "stderr": "Traceback (most recent call last):\n File "/gnxi/gnmi_cli_py/py_gnmicli.py", line 510, in \n main()\n File "/gnxi/gnmi_cli_py/py_gnmicli.py", line 477, in main\n response = _get(stub, paths, user, password, prefix)\n File "/gnxi/gnmi_cli_py/py_gnmicli.py", line 297, in _get\n **kwargs)\n File "/usr/local/lib/python2.7/dist-packages/grpc/_channel.py", line 550, in call\n return _end_unary_response_blocking(state, call, False, None)\n File "/usr/local/lib/python2.7/dist-packages/grpc/_channel.py", line 467, in _end_unary_response_blocking\n raise _Rendezvous(state, None, None, deadline)\ngrpc._channel._Rendezvous: <_Rendezvous of RPC that terminated with:\n\tstatus = StatusCode.NOT_FOUND\n\tdetails = "redis: nil"\n\tdebug_error_string = "{"created":"@1663399029.573632194","description":"Error received from peer","file":"src/core/lib/surface/call.cc","file_line":1036,"grpc_message":"redis: nil","grpc_status":5}"\n>",

any idea why it failed? thanks

@ZhaohuiS
Copy link
Contributor Author

202012

@wenyiz2021
I searched test results, telemetry.test_telemetry.test_telemetry_ouput failed on 3164 with 201911 image.
After reading test cases, I think this case should be skipped for 201911 or older image.

    skip_201911_and_older(duthost)
    cmd = generate_client_cli(duthost=duthost, gnxi_path=gnxi_path, method=METHOD_GET, target="OTHERS", xpath="osversion/build")

def skip_201911_and_older(duthost):
    """ Skip the current test if the DUT version is 201911 or older.
    """
    if parse_version(duthost.kernel_version) <= parse_version('4.9.0'):
        pytest.skip("Test not supported for 201911 images. Skipping the test")

It's better to check os version instead of kernel version and move it to tests_mark_conditions.yml.

@wenyiz2021
Copy link
Contributor

202012

@wenyiz2021 I searched test results, telemetry.test_telemetry.test_telemetry_ouput failed on 3164 with 201911 image. After reading test cases, I think this case should be skipped for 201911 or older image.

    skip_201911_and_older(duthost)
    cmd = generate_client_cli(duthost=duthost, gnxi_path=gnxi_path, method=METHOD_GET, target="OTHERS", xpath="osversion/build")
def skip_201911_and_older(duthost):
    """ Skip the current test if the DUT version is 201911 or older.
    """
    if parse_version(duthost.kernel_version) <= parse_version('4.9.0'):
        pytest.skip("Test not supported for 201911 images. Skipping the test")

It's better to check os version instead of kernel version and move it to tests_mark_conditions.yml.

thanks @ZhaohuiS I'll raise a PR to skip test_telemetry_output in test_conditional_mark.yml

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.

3 participants