Skip to content

Loganalyze fix revert#659

Merged
maggiemsft merged 3 commits intosonic-net:masterfrom
stepanblyschak:loganalyze_fix_revert
Jul 24, 2018
Merged

Loganalyze fix revert#659
maggiemsft merged 3 commits intosonic-net:masterfrom
stepanblyschak:loganalyze_fix_revert

Conversation

@stepanblyschak
Copy link
Copy Markdown
Contributor

Description of PR

Fixes logrotate issue during loganalyze phase in some tests (CRM, PFC WD)

Type of change

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

Approach

  • How did you do it?
    Revert back Sihui's fix for loganalyzer(Revert "[LogAnalyzer]: Accumulate syslog files rather than force logrotate" #641), added a task before "Extract log" to make sure logrotate won't run during log accumulation. Improved performance of "extract_log" ansible module to look for "start_string" starting from the most recent log file.
  • How did you verify/test it?
    Run CRM, PFC WD tests several times.

Documentation

Copy link
Copy Markdown
Contributor

@pavel-shirshov pavel-shirshov left a comment

Choose a reason for hiding this comment

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

as comments

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please explain, what is the sort order here
From older to newer or otherwise?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The sort order depends whether reversed param is True or False in sort function. By default, since older log file is considered greater then newer, the sort order will be from newer to older:
>>> f = ['syslog', 'syslog.1', 'syslog.3.gz', 'syslog.2.gz']
>>> sorted(f, cmp=filename_comparator)
['syslog', 'syslog.1', 'syslog.2.gz', 'syslog.3.gz']
This order was chosen for convenience, since we need to start looking from the most recent log file (from the begging of the list)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggest to rename extract_line to extract_lines.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

actually it's not a copy operation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggest to put this in
ansible always block

@stepanblyschak stepanblyschak force-pushed the loganalyze_fix_revert branch from 3af6d27 to fc4332e Compare July 23, 2018 09:41
stepanb added 2 commits July 23, 2018 13:27
Signed-off-by: stepanb <stepanb@mellanox.com>
In most scenarios of using this module it is reasonable to search
'start_string' in the most recent log file instead of searching
all appearence of 'start_string' in all logs in /var/log/syslog*.
This improves performance of tests which use loganalyze frequently
(e.g crm, pfc wd)

Signed-off-by: stepanb <stepanb@mellanox.com>
@stepanblyschak stepanblyschak force-pushed the loganalyze_fix_revert branch from fc4332e to da33a2b Compare July 23, 2018 10:28
@maggiemsft maggiemsft merged commit 5d30bad into sonic-net:master Jul 24, 2018
wangxin pushed a commit to wangxin/sonic-mgmt that referenced this pull request Oct 27, 2025
…o validation for values in output (sonic-net#659)

This PR duplicates sonic-net#8457
<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/sonic-net/SONiC/blob/gh-pages/CONTRIBUTING.md

Please provide following information to help code review process a bit easier:
-->
### Description of PR
<!--
- Please include a summary of the change and which issue is fixed.
- Please also include relevant motivation and context. Where should reviewer start? background context?
- List any dependencies that are required for this change.
-->
Enhanced test case "show platform ssdhealth" to do validation for values in the output

Originally test did just check for keys: "Device Mode", "Health", "Temperature" - now it also checks the values detected(not "N/A"), does check that SSD health is bigger than 50%, and checks that temperature is less than 100C

Added support to get the path to SSD based on platform type.
Added support to define sensors that have "N/A" value per platform.

Summary: Enhanced test case "show platform ssdhealth" to do validation for values in the output
Fixes # (issue)

### Type of change

<!--
- Fill x for your type of change.
- e.g.
- [x] Bug fix
-->

- [ ] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
 - [ ] Skipped for non-supported platforms
- [x] Test case improvement

### Back port request
- [ ] 202205
- [ ] 202305
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [x] 202505

### Approach
#### What is the motivation for this PR?
Enhanced test case "show platform ssdhealth" to do validation for values in the output

#### How did you do it?
Added validation for values in output

#### How did you verify/test it?
Executed test on different platforms

#### Any platform specific information?

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

### Documentation
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->
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.

3 participants