search

sensu-plugins / sensu-plugins-dns

This plugin provides native DNS instrumentation for monitoring, including: record resolution
http://sensu-plugins.io
MIT License
5 stars 26 forks source link

support a comma separated list for -r (--result) #26

Closed adamdecaf closed 7 years ago

adamdecaf commented 7 years ago

Pull Request Checklist

Fixes: https://github.com/sensu-plugins/sensu-plugins-dns/issues/25

General

Known Compatibility Issues

This change is backwards compatible.

adamdecaf commented 7 years ago

I'm not sure what "binstubs" are...

majormoses commented 7 years ago

bin stubs are for non ruby scripts that wrap in a ruby script so when we install the gem we can create an executable script in the right path. Example:

majormoses commented 7 years ago

Thanks for your contribution to Sensu plugins! Without people like you submitting PRs we couldn't run the project. I will review it shortly.

majormoses commented 7 years ago

Looks like your tests are failing, can you fix those up?

adamdecaf commented 7 years ago

@majormoses Sure thing, bundle exec rake default passes for me now.

adamdecaf commented 7 years ago

This passed for me locally, but could it be due to this library actually resolving the google record?

adam@~/code/src/github.com/sensu-plugins/sensu-plugins-dns$ bundle exec rake default 
/usr/local/Cellar/ruby/2.4.2_1/bin/ruby -I/usr/local/lib/ruby/gems/2.4.0/gems/rspec-core-3.7.0/lib:/usr/local/lib/ruby/gems/2.4.0/gems/rspec-support-3.7.0/lib /usr/local/lib/ruby/gems/2.4.0/gems/rspec-core-3.7.0/exe/rspec test/check-dns_spec.rb

DNS
  returns ok with a successful lookup
  returns critical with no values to check against
  returns critical with a timeout
  returns critical with a nxdomain
  returns ok when request_count > 1
  returns ok with multiple valid resolutions
  returns critical with multiple results including a timeout
  returns OK with multiple results including a single timeout and threshold 50%

Finished in 0.84792 seconds (files took 0.45253 seconds to load)
8 examples, 0 failures

Files:           3
Modules:         2 (    2 undocumented)
Classes:         2 (    1 undocumented)
Constants:       4 (    4 undocumented)
Attributes:      0 (    0 undocumented)
Methods:         6 (    6 undocumented)
 7.14% documented
Running RuboCop...
Inspecting 9 files
.........

9 files inspected, no offenses detected
majormoses commented 7 years ago

k I will take a look, but I am thinking that we probably need a more deterministic test location than google.com or find a way to more dynamically pull the correct value in the test rather than hardcoding it. I will probably get to it this weekend.

adamdecaf commented 7 years ago

@majormoses Agreed. I was assuming the fixtures/google_success contained that, but that might not be the case.

adam@~$ hexdump -C ~/code/src/github.com/sensu-plugins/sensu-plugins-dns/test/fixtures/google_success 
00000000  37 77 81 90 00 01 00 01  00 04 00 05 06 67 6f 6f  |7w...........goo|
00000010  67 6c 65 03 63 6f 6d 00  00 01 00 01 c0 0c 00 01  |gle.com.........|
00000020  00 01 00 00 01 08 00 04  d8 3a c6 ee c0 0c 00 02  |.........:......|
00000030  00 01 00 00 8c 79 00 06  03 6e 73 31 c0 0c c0 0c  |.....y...ns1....|
00000040  00 02 00 01 00 00 8c 79  00 06 03 6e 73 34 c0 0c  |.......y...ns4..|
00000050  c0 0c 00 02 00 01 00 00  8c 79 00 06 03 6e 73 33  |.........y...ns3|
00000060  c0 0c c0 0c 00 02 00 01  00 00 8c 79 00 06 03 6e  |...........y...n|
00000070  73 32 c0 0c c0 38 00 01  00 01 00 00 a5 03 00 04  |s2...8..........|
00000080  d8 ef 20 0a c0 6e 00 01  00 01 00 00 a5 03 00 04  |.. ..n..........|
00000090  d8 ef 22 0a c0 5c 00 01  00 01 00 00 a5 03 00 04  |.."..\..........|
000000a0  d8 ef 24 0a c0 4a 00 01  00 01 00 00 a5 03 00 04  |..$..J..........|
000000b0  d8 ef 26 0a 00 00 29 10  00 00 00 80 00 00 00     |..&...)........|
000000bf
majormoses commented 7 years ago

I hope you don't mind but I decided that I just needed to roll out some more real testing and committed to your PR. These are very real integration tests and what I prefer over unit tests that don't really do all that much.

majormoses commented 7 years ago

Awaing review from @eheydrick before merging.

adamdecaf commented 7 years ago

@majormoses No worries. I'm glad this looks mergable now! Thanks for the help.

majormoses commented 7 years ago

released: https://rubygems.org/gems/sensu-plugins-dns/versions/1.3.0

adamdecaf commented 7 years ago

Thanks @majormoses !