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

Skip error messages for errors during SAI discovery #10396

Merged
merged 5 commits into from
Dec 1, 2023

Conversation

roy-sror
Copy link
Contributor

@roy-sror roy-sror commented Oct 19, 2023

Skip error messages related to the following issue:
sonic-net/sonic-buildimage#7895

Summary:
Fixes # #10272

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205
  • 202305

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?

Documentation

@roy-sror
Copy link
Contributor Author

@ZhaohuiS , @lipxu - can you pls review

@ZhaohuiS
Copy link
Contributor

@roy-sror which case is failed due to these error log? I suggest don't put the real error log into ansible/roles/test/files/tools/loganalyzer/loganalyzer_common_ignore.txt, which we used to put non-harm or not real error logs needs to be ignored usually.
For your case, it's a real issue, could you please skip or xfail the impacted cases in tests/common/plugins/conditional_mark/tests_mark_conditions.yaml. Please also add the conditions like platform,hwsku, or os version to limit the scope of skip or xfail.

@roy-sror
Copy link
Contributor Author

@roy-sror which case is failed due to these error log? I suggest don't put the real error log into ansible/roles/test/files/tools/loganalyzer/loganalyzer_common_ignore.txt, which we used to put non-harm or not real error logs needs to be ignored usually. For your case, it's a real issue, could you please skip or xfail the impacted cases in tests/common/plugins/conditional_mark/tests_mark_conditions.yaml. Please also add the conditions like platform,hwsku, or os version to limit the scope of skip or xfail.

@ZhaohuiS - the error messages are discussed in sonic-net/sonic-buildimage#7895 and could happen on any boot. This is why tests/common/plugins/conditional_mark/tests_mark_conditions.yaml is less suitable in this case.

@bingwang-ms
Copy link
Collaborator

@roy-sror which case is failed due to these error log? I suggest don't put the real error log into ansible/roles/test/files/tools/loganalyzer/loganalyzer_common_ignore.txt, which we used to put non-harm or not real error logs needs to be ignored usually. For your case, it's a real issue, could you please skip or xfail the impacted cases in tests/common/plugins/conditional_mark/tests_mark_conditions.yaml. Please also add the conditions like platform,hwsku, or os version to limit the scope of skip or xfail.

@ZhaohuiS - the error messages are discussed in sonic-net/sonic-buildimage#7895 and could happen on any boot. This is why tests/common/plugins/conditional_mark/tests_mark_conditions.yaml is less suitable in this case.

Hi @roy-sror, issue sonic-net/sonic-buildimage#7895 is a quite old issue. Why didn't we see the same error log in other branch, like 202205?

@ZhaohuiS
Copy link
Contributor

ZhaohuiS commented Nov 2, 2023

@roy-sror I don't know which case has these kinds of error log. bgp fib suppress? If there are some specific cases hit these error log during log analyzer, we can ignore these error log in script level or case level, please take the following one for reference.

https://github.com/sonic-net/sonic-mgmt/pull/10211/files

@bingwang-ms
Copy link
Collaborator

@roy-sror Can you please check and address the comments?

@roy-sror
Copy link
Contributor Author

roy-sror commented Nov 5, 2023

@bingwang-ms , @ZhaohuiS - Based on sonic-net/sonic-buildimage#7895, this might happen during any test that involves a reboot or configuration reload. To prevent having to add a skip statement for every test that involves a reload, I suggest keeping this globally skipped.

@mssonicbld
Copy link
Collaborator

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.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/tools/loganalyzer/loganalyzer_common_ignore.txt

check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.....................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
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:

  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>

1 similar comment
@mssonicbld
Copy link
Collaborator

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.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/tools/loganalyzer/loganalyzer_common_ignore.txt

check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.....................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
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:

  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>

bingwang-ms
bingwang-ms previously approved these changes Nov 9, 2023
@bingwang-ms
Copy link
Collaborator

@roy-sror Can you please address the pre_test static analysis failure?

@mssonicbld
Copy link
Collaborator

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.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing ansible/roles/test/files/tools/loganalyzer/loganalyzer_common_ignore.txt

check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.....................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
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:

  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>

bingwang-ms
bingwang-ms previously approved these changes Nov 14, 2023
@nhe-NV
Copy link
Contributor

nhe-NV commented Nov 16, 2023

/azpw run

1 similar comment
@roy-sror
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@roy-sror
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@roy-sror
Copy link
Contributor Author

@wangxin , @bingwang-ms - the checker failures are unrelated to the change and it is already failing for the 3rd time on a different issue. Is there a known stability issue of the tests/checkers? Can we merge this change?

@bingwang-ms
Copy link
Collaborator

I'm not aware of any known blocker in sonic-mgmt PR checker. Just retriggered the testing.

@bingwang-ms bingwang-ms merged commit e95994f into sonic-net:master Dec 1, 2023
13 checks passed
@bingwang-ms
Copy link
Collaborator

MFST ADO: 25581299
@StormLiangMS Can you please help cherry-pick to 202305 branch? Thanks!

@wangxin wangxin changed the title Skip error messages for https://github.com/sonic-net/sonic-buildimage… Skip error messages for "get_dispatch_attribs_handler" Dec 18, 2023
@wangxin wangxin changed the title Skip error messages for "get_dispatch_attribs_handler" Skip error messages for errors during SAI discovery Dec 18, 2023
@mssonicbld
Copy link
Collaborator

@roy-sror PR conflicts with 202305 branch

@bingwang-ms
Copy link
Collaborator

@roy-sror Can you please create another PR for 202305? Thanks!

@nhe-NV
Copy link
Contributor

nhe-NV commented Jan 19, 2024

@bingwang-ms new PR added for 202305 branch: #11334

wangxin pushed a commit that referenced this pull request Feb 1, 2024
Summary: Skip err msg for the known issue :
sonic-net/sonic-buildimage#7895
sonic-net/sonic-sairedis#582
This is a PR to fix the chery-pick conflict for the : #10396
@wangxin
Copy link
Collaborator

wangxin commented Feb 1, 2024

This PR was merged before 202311 branch was created from master branch. No need to cherry-pick for 202311 branch.

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.

6 participants