Fix issue with using force method as pvcreate argument

[walkamongus]

passing a blank argument (when force => false) to the pvcreate method causes errors:

~]# puppet resource --debug --trace physical_volume /dev/sdc ensure=present
Debug: Executing: '/sbin/pvs /dev/sdc'
Debug: Executing: '/sbin/pvcreate  /dev/sdc'
Error: Execution of '/sbin/pvcreate  /dev/sdc' returned 5: Device  not found (or ignored by filtering).
  Physical volume "/dev/sdc" successfully created

Which appear to be related to the insertion of the extra "nil" argument. Note the extra space in /sbin/pvcreate /dev/sdc. This shouldn't matter if the command is executed 'as-is' on the commandline but there appears to possibly be some sort of quoting or grouping going on such that the command is literally /sbin/pvcreate ' /dev/sdc', which causes an error. I'm not sure why the PV is still created successfully despite the error, however.

This PR should fix the issue.

[iankronquist]

I can reproduce this issue, but unfortunately the proposed fix doesn't seem to work for me.

[vagrant@pop project]$ git branch
...
* pr/135
...
[vagrant@pop project]$ puppet --version
3.6.2
[vagrant@pop project]$ cat /etc/redhat-release 
CentOS Linux release 7.0.1406 (Core) 
[vagrant@pop project]$ sudo puppet resource --debug --trace physical_volume /dev/sdc ensure=present
Debug: Puppet::Type::Physical_volume::ProviderIax: file mkdev does not exist
Debug: Loaded state in 0.00 seconds
Debug: Executing '/sbin/pvs /dev/sdc'
Debug: Executing '/sbin/pvcreate /dev/sdc'
Error: Execution of '/sbin/pvcreate /dev/sdc' returned 5: Physical volume /dev/sdc not found
  Device /dev/sdc not found (or ignored by filtering).
/usr/share/ruby/vendor_ruby/puppet/util/execution.rb:197:in `execute'
/usr/share/ruby/vendor_ruby/puppet/provider/command.rb:23:in `execute'
/usr/share/ruby/vendor_ruby/puppet/provider.rb:237:in `block in has_command'
/usr/share/ruby/vendor_ruby/puppet/provider.rb:463:in `block in create_class_and_instance_method'
/home/vagrant/project/lib/puppet/provider/physical_volume/lvm.rb:82:in `create_physical_volume'
/home/vagrant/project/lib/puppet/provider/physical_volume/lvm.rb:14:in `create'
/usr/share/ruby/vendor_ruby/puppet/property/ensure.rb:16:in `block in defaultvalues'
/usr/share/ruby/vendor_ruby/puppet/property.rb:197:in `call_valuemethod'
/usr/share/ruby/vendor_ruby/puppet/property.rb:498:in `set'
/usr/share/ruby/vendor_ruby/puppet/property.rb:581:in `sync'
/usr/share/ruby/vendor_ruby/puppet/transaction/resource_harness.rb:191:in `sync'
/usr/share/ruby/vendor_ruby/puppet/transaction/resource_harness.rb:128:in `sync_if_needed'
/usr/share/ruby/vendor_ruby/puppet/transaction/resource_harness.rb:81:in `perform_changes'
/usr/share/ruby/vendor_ruby/puppet/transaction/resource_harness.rb:20:in `evaluate'
/usr/share/ruby/vendor_ruby/puppet/transaction.rb:174:in `apply'
/usr/share/ruby/vendor_ruby/puppet/transaction.rb:187:in `eval_resource'
/usr/share/ruby/vendor_ruby/puppet/transaction.rb:117:in `call'
/usr/share/ruby/vendor_ruby/puppet/transaction.rb:117:in `block (2 levels) in evaluate'
/usr/share/ruby/vendor_ruby/puppet/util.rb:327:in `block in thinmark'
/usr/share/ruby/benchmark.rb:296:in `realtime'
/usr/share/ruby/vendor_ruby/puppet/util.rb:326:in `thinmark'
/usr/share/ruby/vendor_ruby/puppet/transaction.rb:117:in `block in evaluate'
/usr/share/ruby/vendor_ruby/puppet/graph/relationship_graph.rb:118:in `traverse'
/usr/share/ruby/vendor_ruby/puppet/transaction.rb:108:in `evaluate'
/usr/share/ruby/vendor_ruby/puppet/resource/catalog.rb:167:in `block in apply'
/usr/share/ruby/vendor_ruby/puppet/util/log.rb:153:in `with_destination'
/usr/share/ruby/vendor_ruby/puppet/transaction/report.rb:112:in `as_logging_destination'
/usr/share/ruby/vendor_ruby/puppet/resource/catalog.rb:166:in `apply'
/usr/share/ruby/vendor_ruby/puppet/indirector/resource/ral.rb:41:in `save'
/usr/share/ruby/vendor_ruby/puppet/indirector/indirection.rb:283:in `save'
/usr/share/ruby/vendor_ruby/puppet/application/resource.rb:219:in `find_or_save_resources'
/usr/share/ruby/vendor_ruby/puppet/application/resource.rb:143:in `main'
/usr/share/ruby/vendor_ruby/puppet/application.rb:379:in `run_command'
/usr/share/ruby/vendor_ruby/puppet/application.rb:371:in `block (2 levels) in run'
/usr/share/ruby/vendor_ruby/puppet/application.rb:477:in `plugin_hook'
/usr/share/ruby/vendor_ruby/puppet/application.rb:371:in `block in run'
/usr/share/ruby/vendor_ruby/puppet/util.rb:479:in `exit_on_fail'
/usr/share/ruby/vendor_ruby/puppet/application.rb:371:in `run'
/usr/share/ruby/vendor_ruby/puppet/util/command_line.rb:137:in `run'
/usr/share/ruby/vendor_ruby/puppet/util/command_line.rb:91:in `execute'
/usr/bin/puppet:8:in `<main>'
Error: /Physical_volume[/dev/sdc]/ensure: change from absent to present failed: Execution of '/sbin/pvcreate /dev/sdc' returned 5: Physical volume /dev/sdc not found
  Device /dev/sdc not found (or ignored by filtering).
Debug: Finishing transaction 21676060
Debug: Storing state
Debug: Stored state in 0.00 seconds
Debug: Executing '/sbin/pvs /dev/sdc'
physical_volume { '/dev/sdc':
  ensure => 'absent',
}

[walkamongus]

Does your vagrant box have a /dev/sdc disk available? Just to be clear -- the original issue is that an error is thrown when the device is present and available. If the device doesn't exist then receiving the error is correct behavior.

[iankronquist]

I suspect that is the problem. Sorry, I'm not that knowledgeable about lvm. Do you know an easy way to mount a generic sdc device on a vagrant vm? If not I'll go try to learn how.

[walkamongus]

I think this gist will do it: https://gist.github.com/leifg/4713995

If for whatever reason the device shows up as /dev/sdb, /dev/sdd, etc. just sub that in for sdc.

[DavidS]

The change looks good to me, but please squash the two commits into one so that the fix of the test coincides with the functional change. This will make the history so much cleaner.

Squashing: https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#Squashing-Commits

[walkamongus]

squashed!

[DavidS]

Excellent, thanks!