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

NAS-130745 / 25.04 / Update smartctl usage to use JSON #14479

Merged
merged 32 commits into from
Sep 18, 2024
Merged

Conversation

aiden3c
Copy link
Contributor

@aiden3c aiden3c commented Sep 10, 2024

I've updated our smartctl usage throughout middleware to use the JSON flag to get serialized data. Unfortunately I haven't been able to run this through a full gauntlet of SMART test types for different drives. Going through the smartmontools source code and doing what I can with different drive types I had available, I've gotten the JSON parsing as accurate as I could.

This is something that might end up breaking, but getting tickets will let us fix any assumed/implicit keys we're accessing.

Passing tests, but the majority are our CI build tests, which are passing.

@bugclerk
Copy link
Contributor

@bugclerk bugclerk changed the title Nas 130745 NAS-130745 / 25.04 / Nas 130745 Sep 10, 2024
@aiden3c aiden3c force-pushed the NAS-130745 branch 2 times, most recently from ab0d655 to fa3011d Compare September 11, 2024 18:23
@aiden3c aiden3c changed the title NAS-130745 / 25.04 / Nas 130745 NAS-130745 / 25.04 / Update smartctl usage to use JSON Sep 11, 2024
@aiden3c aiden3c requested a review from a team September 11, 2024 18:30
Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

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

Good job! There are some details I want to pay attention too.

src/middlewared/middlewared/plugins/disk.py Outdated Show resolved Hide resolved
src/middlewared/middlewared/plugins/smart.py Show resolved Hide resolved
src/middlewared/middlewared/plugins/smart.py Outdated Show resolved Hide resolved
src/middlewared/middlewared/utils/disks.py Show resolved Hide resolved
@aiden3c
Copy link
Contributor Author

aiden3c commented Sep 17, 2024

New passing tests!

@aiden3c aiden3c requested review from themylogin and a team September 17, 2024 19:14
Copy link
Contributor

@themylogin themylogin left a comment

Choose a reason for hiding this comment

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

Good, only minor issues are left

@aiden3c aiden3c merged commit cc1be39 into master Sep 18, 2024
3 checks passed
@aiden3c aiden3c deleted the NAS-130745 branch September 18, 2024 14:38
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Sep 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants