Skip to content

Fix loganalyzer no common config issue and re-balance PR testing#5435

Merged
wangxin merged 6 commits intosonic-net:masterfrom
wangxin:fix-log-analyzer
Apr 11, 2022
Merged

Fix loganalyzer no common config issue and re-balance PR testing#5435
wangxin merged 6 commits intosonic-net:masterfrom
wangxin:fix-log-analyzer

Conversation

@wangxin
Copy link
Collaborator

@wangxin wangxin commented Mar 30, 2022

Description of PR

Summary:
Fixes # (issue)

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?

Loganalyzer was broken in PR #3235. The issue is that common config was loaded
in subprocess for adding marks to syslog. After the subprocess exited, the
common config is lost.

PR #5193 tried to fix this issue. However, because of many new error logs sneaked
in when log analyzer was not working, PR testing started to fail by these error
logs after PR #5193 was merged.

PR #5391 and #5411 tried to work around the PR testing failure to unblock PR
testing. PR #5391 is to address the GCU related error logs and was merged.
PR #5411 tried to add other error logs to the common ignore list. But the effort
took too long because the ignore list seemed endless.

To unblock PR testing as soon as possible, the original fix #5193 was reverted
in #5433.

This PR tries to complete the work left over from #5411 and #5433.

How did you do it?

Changes:

  1. Fix the log analyzer common config not loaded issue.
  2. Temporarily add error logs to the common ignore list.
  3. Improve the logging of log analyzer and parallel_run
  4. PR testing t0_part2 takes much more time than t0_part1 after the GCU test
    scripts are added. This change re-balanced t0 part1&part2 testing by moving
    some of the tests from part2 to part1.
  5. Sorted the PR testing scripts in alphabetic order.

How did you verify/test it?

Tested run a few test scripts with log analyzer enabled on KVM testbed.

Any platform specific information?

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

Documentation

@wangxin wangxin requested a review from a team as a code owner March 30, 2022 06:46
@wangxin wangxin requested a review from StormLiangMS March 30, 2022 06:50
StormLiangMS
StormLiangMS previously approved these changes Mar 30, 2022
Copy link
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

@lgtm-com
Copy link

lgtm-com bot commented Mar 30, 2022

This pull request fixes 4 alerts when merging b110d48c9decc60f364e2ef1693d2f108963de32 into 753e956 - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable
  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Mar 30, 2022

This pull request fixes 4 alerts when merging 3775b00c36e682abfd709d3013c11533b968bf1e into 753e956 - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable
  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Mar 31, 2022

This pull request fixes 4 alerts when merging 9ae6d326d261b8f97b4841698dcb20ad00565ac4 into a6d0104 - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable
  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Apr 1, 2022

This pull request fixes 4 alerts when merging aa6f3ab3d8311857fb0fd0e221924b3a1c3ea2d4 into 10e6928 - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable
  • 2 for Unused import

Loganalyzer was broken in PR sonic-net#3235. The issue is that common config was loaded
in subprocess for adding marks to syslog. After the subprocess exited, the
common config is lost.

PR sonic-net#5193 tried to fix this issue. However, because of many new error logs sneaked
in when log analyzer was not working, PR testing started to fail by these error
logs after PR sonic-net#5193 was merged.

PR sonic-net#5191 and sonic-net#5411 tried to workaround the PR testing failure to unblock PR
testing. PR sonic-net#5191 is to address the GCU related error logs and was merged.
PR sonic-net#5411 tried to add other error logs to the common ignore list. But the effort
took too long because the ignore list seemed endless.

To unblock PR testing as soon as possible, the original fix sonic-net#5193 was reverted
in sonic-net#5433.

This PR tries to complete the work left over from sonic-net#5411 and sonic-net#5433.

Changes:
1. Fix the log analyzer common config not loaded issue.
2. Temporarily add error logs to the common ignore list.
3. Improve the logging of log analyzer and parallel_run
4. PR testing t0_part2 takes much more time than t0_part1 after the GCU test
   scripts are added. This change re-balanced t0 part1&part2 testing by moving
   some of the tests from part2 to part1.
5. Sorted the PR testing scripts in alphabetic order.

Signed-off-by: Xin Wang <[email protected]>
@wangxin wangxin force-pushed the fix-log-analyzer branch from aa6f3ab to 25d7808 Compare April 2, 2022 06:33
@lgtm-com
Copy link

lgtm-com bot commented Apr 2, 2022

This pull request fixes 4 alerts when merging 25d7808 into 1c6a0bc - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable
  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2022

This pull request fixes 4 alerts when merging 7bbcf0d into 6e2bb2e - view on LGTM.com

fixed alerts:

  • 2 for Unused local variable
  • 2 for Unused import

Copy link
Collaborator

@StormLiangMS StormLiangMS 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 a5619d8 into sonic-net:master Apr 11, 2022
wangxin added a commit that referenced this pull request Apr 11, 2022
What is the motivation for this PR?
Loganalyzer was broken in PR #3235. The issue is that common config was loaded
in subprocess for adding marks to syslog. After the subprocess exited, the
common config is lost.

PR #5193 tried to fix this issue. However, because of many new error logs sneaked
in when log analyzer was not working, PR testing started to fail by these error
logs after PR #5193 was merged.

PR #5391 and #5411 tried to work around the PR testing failure to unblock PR
testing. PR #5391 is to address the GCU related error logs and was merged.
PR #5411 tried to add other error logs to the common ignore list. But the effort
took too long because the ignore list seemed endless.

To unblock PR testing as soon as possible, the original fix #5193 was reverted
in #5433.

This PR tries to complete the work left over from #5411 and #5433.

How did you do it?
Changes:
* Fix the log analyzer common config not loaded issue.
* Temporarily add error logs to the common ignore list.
* Improve the logging of log analyzer and parallel_run
* PR testing t0_part2 takes much more time than t0_part1 after the GCU test
  scripts are added. This change re-balanced t0 part1&part2 testing by moving
  some of the tests from part2 to part1.
* Sorted the PR testing scripts in alphabetic order.

How did you verify/test it?
Tested run a few test scripts with log analyzer enabled on KVM testbed.

Signed-off-by: Xin Wang <[email protected]>
@wangxin wangxin deleted the fix-log-analyzer branch May 4, 2022 00:05
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