arguments that start with `--no-*` will negated which leads to an `undefined method to_sym` on a `FalseClass`

[bdeluca]

_ works - does not work

Is this in reference to an existing issue? https://github.com/sensu-plugins/sensu-plugins-disk-checks/issues/94

a tiny change the the cmdline parser so it works

[majormoses]

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]

Hmm while your solution is valid I think it would be best fixed upstream either here to be @options[name.to_s.to_sym] = args I will open an issue there to see about adding support there, if we don't get something pretty quickly we can look at making this change but I'd prefer if we can make the change once and have it fix any issues with all plugins (after pulling in a new version).

[majormoses]

Hm so I looked into this more and I think there is something weird implementation wise going on here on our end. I was able to add another option with hypens in the long option and it worked just fine:

$ git diff
diff --git a/bin/check-smart.rb b/bin/check-smart.rb
index c70ab87..7fa813a 100755
--- a/bin/check-smart.rb
+++ b/bin/check-smart.rb
@@ -111,6 +111,13 @@ class CheckSMART < Sensu::Plugin::Check::CLI
          default: :unknown,
          in: %i[unknown ok warn critical]

+  option :foo_bar,
+         long: '--foo-bar-baz FOOBAR',
+         description: 'FOOBAR',
+         proc: proc(&:to_sym),
+         default: :unknown,
+         in: %i[unknown ok warn critical]
+
   option :no_smart_capable_disks,
          long: '--no-smart-capable-disks EXIT_CODE',
          description: 'Exit code when there are no SMART capable disks',
$ bundle exec ./bin/check-smart.rb --foo-bar-baz ok
sudo: smartctl: command not found
CheckSMART UNKNOWN: No SMART capable devices found

Yet I can indeed replicate the issue while passing the existing argument:

$ bundle exec ./bin/check-smart.rb --foo-bar-baz ok --no-smart-capable-disks ok
Check failed to run: undefined method `to_sym' for false:FalseClass
Did you mean?  to_s, ["/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/mixlib-cli-1.7.0/lib/mixlib/cli.rb:290:in `block (3 levels) in opt_parser'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1552:in `block in parse_in_order'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1538:in `catch'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1538:in `parse_in_order'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1532:in `order!'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1626:in `permute!'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1648:in `parse!'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/mixlib-cli-1.7.0/lib/mixlib/cli.rb:230:in `parse_options'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sensu-plugin-1.4.5/lib/sensu-plugin/cli.rb:13:in `initialize'", "./bin/check-smart.rb:145:in `initialize'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sensu-plugin-1.4.5/lib/sensu-plugin/cli.rb:57:in `new'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sensu-plugin-1.4.5/lib/sensu-plugin/cli.rb:57:in `block in <class:CLI>'"]

Ah hah! I have been able to narrow down where this happens:

# legit
$ git diff
diff --git a/bin/check-smart.rb b/bin/check-smart.rb
index c70ab87..c61499b 100755
--- a/bin/check-smart.rb
+++ b/bin/check-smart.rb
@@ -111,6 +111,13 @@ class CheckSMART < Sensu::Plugin::Check::CLI
          default: :unknown,
          in: %i[unknown ok warn critical]

+  option :r2d2,
+         long: '--beep-bee-bee-boop-bee-doo-weep FOOBAR',
+         description: 'FOOBAR',
+         proc: proc(&:to_sym),
+         default: :unknown,
+         in: %i[unknown ok warn critical]
+
   option :no_smart_capable_disks,
          long: '--no-smart-capable-disks EXIT_CODE',
          description: 'Exit code when there are no SMART capable disks',
@@ -171,7 +178,7 @@ class CheckSMART < Sensu::Plugin::Check::CLI
   def run
     unless @devices.length > 0
       exit_with(
-        config[:no_smart_capable_disks],
+        config[:r2d2],
         'No SMART capable devices found'
       )
     end
$ bundle exec ./bin/check-smart.rb --beep-bee-bee-boop-bee-doo-weep ok
sudo: smartctl: command not found
CheckSMART OK: No SMART capable devices found
# when an option with --no-* it tries to negate it
$ bundle exec ./bin/check-smart.rb --no-beep-bee-bee-boop-bee-doo-weep ok
Check failed to run: undefined method `to_sym' for false:FalseClass
Did you mean?  to_s, ["/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/mixlib-cli-1.7.0/lib/mixlib/cli.rb:290:in `block (3 levels) in opt_parser'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1552:in `block in parse_in_order'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1538:in `catch'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1538:in `parse_in_order'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1532:in `order!'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1626:in `permute!'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/2.4.0/optparse.rb:1648:in `parse!'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/mixlib-cli-1.7.0/lib/mixlib/cli.rb:230:in `parse_options'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sensu-plugin-1.4.5/lib/sensu-plugin/cli.rb:13:in `initialize'", "./bin/check-smart.rb:145:in `initialize'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sensu-plugin-1.4.5/lib/sensu-plugin/cli.rb:57:in `new'", "/home/babrams/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sensu-plugin-1.4.5/lib/sensu-plugin/cli.rb:57:in `block in <class:CLI>'"]

[majormoses]

To fix it I think the best thing would be to remove the no prefix and replace it with something like zero:

$ git diff
diff --git a/bin/check-smart.rb b/bin/check-smart.rb
index c70ab87..3015f0e 100755
--- a/bin/check-smart.rb
+++ b/bin/check-smart.rb
@@ -111,8 +111,15 @@ class CheckSMART < Sensu::Plugin::Check::CLI
          default: :unknown,
          in: %i[unknown ok warn critical]

+  option :r2d2,
+         long: '--no-beep-bee-bee-boop-bee-doo-weep FOOBAR',
+         description: 'FOOBAR',
+         proc: proc(&:to_sym),
+         default: :unknown,
+         in: %i[unknown ok warn critical]
+
   option :no_smart_capable_disks,
-         long: '--no-smart-capable-disks EXIT_CODE',
+         long: '--zero-smart-capable-disks EXIT_CODE',
          description: 'Exit code when there are no SMART capable disks',
          proc: proc(&:to_sym),
          default: :unknown,
$ bundle exec ./bin/check-smart.rb --zero-smart-capable-disks ok
sudo: smartctl: command not found
CheckSMART OK: No SMART capable devices found

As a CLI flag I think that it does make sense that when starting with --no-* it would be a boolean option so I think we should change it regardless of any upstream changes.

[majormoses]

Have not heard back from you, would you like to keep working on this or should I close this out and make the proposed changes?

[adfel70]

Hi, we also need this fix. Is there a reason why it is not being merged yet?

[majormoses]

@adfel70 because I have not heard back from the author as to the changes requested, let me see if I can checkout and commit to this PR otherwise I can open a new PR.

[majormoses]

The CI failure is a known issue with older EOL ruby versions, I will merge, release, and manually publish new gem.

[majormoses]

released: https://rubygems.org/gems/sensu-plugins-disk-checks/versions/4.0.0