-
Notifications
You must be signed in to change notification settings - Fork 597
ITL: Enhanced SMART attributes monitoring plugin check configuration to more parameters #10564
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
oxzi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, thanks a lot for your contribution!
Please sign the CLA as the bot indicated and comment here that you did so. Afterwards, we can proceed with your PR.
I have added a few single comments for improvements. Please address them as you see fit.
Since I am not familiar with the check_smart_attributes check, I could only perform a "vibe check" and compare your changes against the Perl code upstream. It seems you have added all options from the current master, thanks.
One task left would be to list the new options in the documentation. Please update the table in the the ./doc/10-icinga-template-library.md file.
icinga2/doc/10-icinga-template-library.md
Lines 3028 to 3038 in bc7debe
| #### smart-attributes <a id="plugin-contrib-command-smart-attributes"></a> | |
| The [check_smart_attributes](https://github.com/thomas-krenn/check_smart_attributes) plugin | |
| uses the `smartctl` binary to monitor SMART values of SSDs and HDDs. | |
| Custom variables passed as [command parameters](03-monitoring-basics.md#command-passing-parameters): | |
| Name | Description | |
| --------------------------------|----------------------------------------------------------------------- | |
| smart_attributes_config_path | **Required.** Path to the smart attributes config file (e.g. check_smartdb.json). | |
| smart_attributes_device | **Required.** Device name (e.g. /dev/sda) to monitor. |
|
Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA). Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA. After that, please reply here with a comment and we'll verify. Contributors that have not signed yet: @freaknils
|
|
CLA is now verified |
|
@cla-bot check |
oxzi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again. In general, LGTM.
Please also add yourself to the AUTHORS file:
--- a/AUTHORS
+++ b/AUTHORS
@@ -203,6 +203,7 @@ Nicolas Limage <[email protected]>
Nicolas Rodriguez <[email protected]>
Nicole Lang <[email protected]>
Niflou <[email protected]>
+Nils Czernia <[email protected]>
Noah Hilverling <[email protected]>
Obihörnchen <[email protected]>
Oleg Artenii <[email protected]>…to more parameters
oxzi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again.
Added more parameters for check_smart_attributes.