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

Support for static and dynamic attrs as part of stormond implementation #439

Merged
merged 24 commits into from
May 30, 2024

Conversation

assrinivasan
Copy link
Contributor

@assrinivasan assrinivasan commented Feb 9, 2024

Description

This PR enhances the ssdutil class to now be able to fetch the following information:

  1. Disk reads
  2. Disk writes
  3. Number of reserved blocks
  4. FSIO Reads
  5. FSIO Writes

It also adds support for Micron and Intel storage devices.

Motivation and Context

This is a sister PR to the stormond implementation.
SONiC Storage Monitoring Daemon HLD

How Has This Been Tested?

Has been manually tested on following platforms:

Arista-7050CX3
Mellanox-SN2700
Dell-S6100

Additional Information (Optional)

Verified that build was successful and ssdutil output was unaffected:

7050cx3.txt
S6100.txt
SN2700.txt

@assrinivasan assrinivasan changed the title Adds support for several pri.0 and pri.1 attrs as part of storagemnd implementation Support for several pri.0 and pri.1 attrs as part of storagemnd implementation Feb 9, 2024
@assrinivasan assrinivasan changed the title Support for several pri.0 and pri.1 attrs as part of storagemnd implementation Support for several stattc and dynamic attrs as part of storagemond implementation Feb 17, 2024
@assrinivasan assrinivasan changed the title Support for several stattc and dynamic attrs as part of storagemond implementation Support for static and dynamic attrs as part of storagemond implementation Feb 17, 2024
@prgeor
Copy link
Collaborator

prgeor commented May 14, 2024

@assrinivasan PR description says unit test pending

@prgeor
Copy link
Collaborator

prgeor commented May 14, 2024

@assrinivasan please fix the build failure

@assrinivasan assrinivasan changed the title Support for static and dynamic attrs as part of storagemond implementation Support for static and dynamic attrs as part of stormond implementation May 15, 2024
sonic_platform_base/sonic_storage/storage_base.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/storage_base.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/storage_common.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/ssd.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/storage_devices.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/storage_devices.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/storage_devices.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/storage_devices.py Outdated Show resolved Hide resolved
sonic_platform_base/sonic_storage/storage_devices.py Outdated Show resolved Hide resolved
@prgeor
Copy link
Collaborator

prgeor commented May 24, 2024

@assrinivasan "How Has This Been Tested?"

Add the output one for each utility you modified?

Copy link
Collaborator

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@assrinivasan please make sure we are able to build sonic image for all platforms.

storage_devices.py:
	Moved imports to the beginning of file
	renamed function to _storage_device_object_factory
	Above function now returns an object

storage_common.py:
	Changed logic to only get reads and writes using psutil library.
	Changed parse_id_number logic to prevent phantom regex matches
	Fixed a bug in parse_id_number that was causing the code to disregard vendor_ssd_info
	Fixed issues with Innodisk and Virtium parsers
        Fixed a ValueError bug in Virtium parser logic

Added UTs to test the various changes
@assrinivasan
Copy link
Contributor Author

@assrinivasan "How Has This Been Tested?"

Add the output one for each utility you modified?

Added as attachments to the PR

@assrinivasan
Copy link
Contributor Author

@assrinivasan please make sure we are able to build sonic image for all platforms.

https://dev.azure.com/mssonic/build/_build/results?buildId=557473&view=results -- Verified that we are able to successfully build for all platforms.

Copy link
Collaborator

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@assrinivasan please use SysLogger as direct usage of logger pollutes the identifier name for others daemons

@assrinivasan
Copy link
Contributor Author

@assrinivasan please use SysLogger as direct usage of logger pollutes the identifier name for others daemons

Done in latest.

@prgeor prgeor merged commit cd6a5a4 into sonic-net:master May 30, 2024
5 checks passed
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