Closed samroy92 closed 5 years ago
Ok I finally got RuboCop to pass checks. I bumped the minor revision. Also it is worth discussing whether or not we should change /bin/check-zpool.rb to /bin/check-zfs-zpool.rb. I changed it for consistency reasons but this will require all check definitions to be re-written. Maybe not worth the hassle for everyone?
Also line 29, return 0 was removed as a "redundant return" so hopefully this doesn't break anything. Test cases would include, datasets with a quota set AND datasets with no quota a.k.a. value = 0 for quota size.
It's really too long since I have written ruby.
Can't we make the file rename backwards compatible by creating a file with the old name which just delegates to the logic in the file with the new name? If we can do that, we should.
Otherwise we need to either drop the rename or bump the major version.
Ok just renamed the file back to "check-zpool.rb" for practicality's sake, now we just need a few good test cases to confirm everything is working as intended.
It's quite hard to test this as the checks are dependent on ZFS being available.
@blacksails could we create a docker container with a ZFS volume and test in that? I have been pretty happy with using test-kitchen
, kitchen-docker
, and serverspec
for plugins testing.
Can you please include a manual or automated testing artifact?
In case you wanted to rename and keep backwards compatibility you can create a binstub
for any file (whether its a rubyu, shell, python, etc) like this: https://github.com/sensu-plugins/sensu-plugins-mongodb/blob/master/bin/check-mongodb.rb I would include printing out a message to let the user know that in the next major release we will remove the bin stub since its there for migration purposes rather than ensuring that when the gem is installed it puts a binary in the path.
Any chance you can just show me it works with the input/output? I dont use zfs anywhere. I will keep this open for another month and after that if there is no response I will close this out.
closing due to inactivity, to see this work over the line either re-open this pull request or open a new one.
Pull Request Checklist
Is this in reference to an existing issue? No existing issue, added functionality. Added check-zfs-dataset.rb to find the current quota use percentage and warn+crit if above a certain number.
The most impacting change would be changing check-zpool.rb to check-zfs-zpool.rb, this was only aesthetic and for accounting purposes. It follows the naming convention more closely. This will break peoples check definitions using check-zpool.rb.
General
[ ] Update Changelog following the conventions laid out here
[ X] Update README with any necessary configuration snippets
[ ] Binstubs are created if needed
[ ] RuboCop passes
[ ] Existing tests pass
New Plugins
[X ] Tests
[X ] Add the plugin to the README
[ ] Does it have a complete header as outlined here
Purpose
Known Compatibility Issues
This fork followed the convention of using
sudo
before all system commands. Some systems will not work, some will. I retained the original functionality of usingsudo