Skip to content

Add loganalyzer ignore regex for GCU#5391

Merged
wen587 merged 3 commits intosonic-net:masterfrom
wen587:disable_gcu_la
Mar 29, 2022
Merged

Add loganalyzer ignore regex for GCU#5391
wen587 merged 3 commits intosonic-net:masterfrom
wen587:disable_gcu_la

Conversation

@wen587
Copy link
Contributor

@wen587 wen587 commented Mar 24, 2022

Description of PR

Summary: Add LogAnalyzer ignored regex for GCU as it will report failure on incorrect ordering which is discarded by GCU
Fixes # (issue)

admin@vlab-01:~/loganalyzer$ sudo config apply-patch rm0.json
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}]
...

**(below sorting error will be treated as error by LogAnalyzer.)**     <===============

libyang[0]: Leafref "../../LOOPBACK_INTERFACE_LIST/name" of value "Loopback0" points to a non-existing leaf. (path: /sonic-loopback-interface:sonic-loopback-interface/LOOPBACK_INTERFACE/LOOPBACK_INTERFACE_IPPREFIX_LIST[name='Loopback0'][ip-prefix='FC00:1::32/128']/name)
sonic_yang(3):Data Loading Failed:Leafref "../../LOOPBACK_INTERFACE_LIST/name" of value "Loopback0" points to a non-existing leaf.
...
Patch Applier: The patch was sorted into 3 changes:
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132"}]
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128"}]
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}]
Patch Applier: Applying 3 changes in order:
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE/Loopback0|10.1.0.32~132"}]
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE/Loopback0|FC00:1::32~1128"}]
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.

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?

Recent LogAnalyzer fix report failure on discarded ordering in GCU jsonpatch. However, GCU doesn't fail actually as it will explore a correct ordering and apply.

How did you do it?

Disable LogAnalyzer for GCU tests.

How did you verify/test it?

Run GCU test without flag --disable_loganalyzer and see if it works.

Any platform specific information?

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

Documentation

@wen587 wen587 marked this pull request as ready for review March 24, 2022 11:07
@wen587 wen587 requested a review from a team as a code owner March 24, 2022 11:07
ghooo
ghooo previously approved these changes Mar 25, 2022
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.

It is not a good practice to totally disable loganalyzer. Real issue could be hide using this method. It is better to add the message pattern to ignore list.

@wen587
Copy link
Contributor Author

wen587 commented Mar 28, 2022

It is not a good practice to totally disable loganalyzer. Real issue could be hide using this method. It is better to add the message pattern to ignore list.

Changed to ignore regex. Will fix the failure in furture PR. Currently add the ignored_regex for PR check pass.

@wen587 wen587 requested a review from wangxin March 28, 2022 08:01
@wen587 wen587 changed the title disable loganalyzer for GCU Add loganalyzer ignore regex for GCU Mar 28, 2022
".*ERR kernel.*Reset adapter.*", # test_portchannel_interface replace mtu
".*ERR swss[0-9]*#orchagent: :- getPortOperSpeed.*", # test_portchannel_interface replace mtu
".*ERR.*Failed to apply Json change.*", # validator need updater submodule
".*ERR GenericConfigUpdater: Change Applier: service invoked.*", # validator need updater submodule
Copy link
Collaborator

@wangxin wangxin Mar 28, 2022

Choose a reason for hiding this comment

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

Are all these errors only be triggered during GCU testing? If these errors can be observed in other tests, they need to be added to the common ignore list. Otherwise, this change only "fix" the GCU testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These error are only for GCU test.

@wen587 wen587 requested a review from wangxin March 29, 2022 02:15
@StormLiangMS StormLiangMS self-requested a review March 29, 2022 03:04
@wen587 wen587 merged commit 4079731 into sonic-net:master Mar 29, 2022
wangxin added a commit that referenced this pull request Mar 30, 2022
… files (#5193)" (#5433)

This reverts commit 03cccf7.

Reverts #5193

After this fix was merged, PR test keeps failing because of errors in syslog.

We spent some effort trying to temporarily ignore the errors. However, the list seems endless.
Please refer to:

[loganalyzer]add log patterns to the common ignore #5411
Add loganalyzer ignore regex for GCU #5391
We need a way to temporarily unblock PR testing. Let's revert this fix for now. Then I'll submit another PR to fix the
loganalyzer issue together with a complete ignore list.
wangxin added a commit that referenced this pull request Mar 30, 2022
… files (#5193)" (#5433)

This reverts commit 03cccf7.

Reverts #5193

After this fix was merged, PR test keeps failing because of errors in syslog.

We spent some effort trying to temporarily ignore the errors. However, the list seems endless.
Please refer to:

[loganalyzer]add log patterns to the common ignore #5411
Add loganalyzer ignore regex for GCU #5391
We need a way to temporarily unblock PR testing. Let's revert this fix for now. Then I'll submit another PR to fix the
loganalyzer issue together with a complete ignore list.
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]>
@gord1306
Copy link
Contributor

@wen587 @wangxin, is it better to put all these error log to the expected error messages instead of ignore messages ??

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]>
xwjiang-ms pushed a commit to xwjiang-ms/sonic-mgmt that referenced this pull request Apr 13, 2022
Summary: 
Add LogAnalyzer ignored regex for GCU as it will report failure on incorrect ordering which is discarded by GCU.

What is the motivation for this PR?
Recent LogAnalyzer fix report failure on discarded ordering in GCU jsonpatch. However, GCU doesn't fail actually as it will explore a correct ordering and apply.
There are some real issue in ignored regex. Will fix in future PR. Currently add it for PR check pass.

How did you do it?
Add LogAnalyzer ignored regex for GCU tests.

How did you verify/test it?
Run GCU test without flag --disable_loganalyzer and see if it works.
xwjiang-ms pushed a commit to xwjiang-ms/sonic-mgmt that referenced this pull request Apr 13, 2022
… files (sonic-net#5193)" (sonic-net#5433)

This reverts commit 03cccf7.

Reverts sonic-net#5193

After this fix was merged, PR test keeps failing because of errors in syslog.

We spent some effort trying to temporarily ignore the errors. However, the list seems endless.
Please refer to:

[loganalyzer]add log patterns to the common ignore sonic-net#5411
Add loganalyzer ignore regex for GCU sonic-net#5391
We need a way to temporarily unblock PR testing. Let's revert this fix for now. Then I'll submit another PR to fix the
loganalyzer issue together with a complete ignore list.
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.

4 participants