volume_group type does not handle passing of physical_volumes as a hash

[dacron]

The documentation (README) indicates that we can do this:

class { 'lvm':
  volume_groups => {
    'vg' => {
      createonly => true,
      physical_volumes => {
        '/dev/sdb' => {
          unless_vg => 'vg',
        },
      },
    },
  },
}

However, this results in a message like so:

2018-11-08 12:23:02 +0000 Puppet (err): Execution of '/sbin/vgcreate vg {"/dev/sdb"=>{"unless_vg"=>"vg"}}' returned 5: Device {"/dev/sdb"=>{"unless_vg"=>"vg"}} not found.
2018-11-08 12:23:02 +0000 /Stage[main]/Lvm/Lvm::Volume_group[vg]/Volume_group[vg]/ensure (err): change from 'absent' to 'present' failed: Execution of '/sbin/vgcreate vg {"/dev/sdb"=>{"unless_vg"=>"vg"}}' returned 5: Device {"/dev/sdb"=>{"unless_vg"=>"vg"}} not found.

The issue is that although the defines permit the passing of physical_volumes as a hash, the type/provider does not.

This pull request attempts to fix that.

[dn1s]

I'd love to see this getting merged.

[mmogylenko]

What's preventing this change from being merged? We need a corrective release as well. Example from README.md where physical_volumes represented as a hash is not working.

[dashcheulov]

I wish somebody merged this fix. It's obviously bug being spotted 9 month ago. Example from README.md doesn't work and nobody cares.

[dacron]

Yeah. It sucks. Guess its time to switch to Ansible.

[dacron]

@rodjek is there anyone you can lean on to get this reviewed and - if appropriately fixed - merged?

[rodjek]

Hi @dacron, I'm sorry that this PR has lingered so long without action :( I've pinged some people internally to have a look at it and get back to you.

[tphoney]

Hi @dacron thanks for the fix !

[dashcheulov]

Hi, @tphoney.
Unfortunately it doesn't work for me after merge. I updated the module.
mod 'lvm', :git => 'https://github.com/puppetlabs/puppetlabs-lvm', :commit => '109ed00'

Here is part of my config in Hiera.

lvm::volume_groups:
  vg0:
    followsymlinks: true
    physical_volumes:
      '/dev/disk/by-id/nvme-eui.0025388191bd020c':
        unless_vg: 'vg0'

Puppet throws error.

Error: no implicit conversion of Array into String
Error: /Stage[main]/Lvm/Lvm::Volume_group[vg0]/Volume_group[vg0]/physical_volumes: change from ['/dev/nvme1n1p2'] to [
  ['/dev/disk/by-id/nvme-eui.0025388491bd81c0-part2']] failed: no implicit conversion of Array into String
Error: no implicit conversion of Array into String
Error: /Stage[main]/Lvm/Lvm::Volume_group[data]/Volume_group[data]/physical_volumes: change from ['/dev/nvme0n1'] to [
  ['/dev/disk/by-id/nvme-eui.0025388191bd020c']] failed: no implicit conversion of Array into String

We have embedded array [ ['/dev/disk/by-id/nvme-eui.0025388191bd020c']]. It happens only for one-key hash of physical_volumes.

It works properly in this case (one key) with the patch.

diff --git a/lib/puppet/type/volume_group.rb b/lib/puppet/type/volume_group.rb
index 6e3afe3..8b852d4 100644
--- a/lib/puppet/type/volume_group.rb
+++ b/lib/puppet/type/volume_group.rb
@@ -13,7 +13,7 @@ Puppet::Type.newtype(:volume_group) do
 
         munge do |value|
           if value.is_a?(Hash) then
-            value.keys.sort
+            value.keys.sort[0]
           else
             value
           end

[tphoney]

Thanks @dacron i have reverted this PR, we need some testing around this PR. Looking into it

[michaeltlombardi]

@tphoney, @dacron, can you try this branch on my fork?

https://github.com/michaeltlombardi/puppetlabs-lvm/tree/maint/master/volume_group-hash-handling

Got a WIP PR up with (some) testing last night:

#237