Fixed instanced method for logical_volumes, enables puppet resource

Author: Dylan Ratcliffe

lib/puppet/provider/logical_volume/lvm.rb

@@ -21,7 +21,10 @@
     def self.instances
       get_logical_volumes.collect do |logical_volumes_line|
         logical_volumes_properties = get_logical_volume_properties(logical_volumes_line)
-        new(logical_volumes_properties)
+        instance = new(logical_volumes_properties)
+        # Save the volume group in the provider so the type can find it
+        instance.volume_group = logical_volumes_properties[:volume_group]
+        instance
       end
     end
 
@@ -44,13 +47,21 @@ def self.get_logical_volume_properties(logical_volumes_line)
       output_array = logical_volumes_line.gsub(/\s+/m, ' ').strip.split(" ")
 
       # Assign properties based on headers
-      # Just doing name for now...
-      logical_volumes_properties[:ensure]     = :present
-      logical_volumes_properties[:name]       = output_array[0]
+      logical_volumes_properties[:ensure]       = :present
+      logical_volumes_properties[:name]         = output_array[0]
+      logical_volumes_properties[:volume_group] = output_array[1]
 
       logical_volumes_properties
     end
 
+    # Just assume that the volume group is correct, we don't support changing
+    # it anyway.
+    attr_writer :volume_group
+
+    def volume_group
+        @resource ? @resource[:volume_group] : @volume_group
+    end
+
     def create
         args = []
 
@@ -146,12 +157,23 @@ def destroy
     end
 
     def exists?
-        lvs(@resource[:volume_group]) =~ lvs_pattern
+        begin
+            lvs(@resource[:volume_group]) =~ /#{@resource[:name]}/
+        rescue Puppet::ExecutionFailure
+            # lvs fails if we give it an empty volume group name, as would
+            # happen if we were running `puppet resource`. This should be
+            # interpreted as "The resource does not exist"
+            nil
+        end
     end
 
     def size
         if @resource[:size] =~ /^\d+\.?\d{0,2}([KMGTPE])/i
             unit = $1.downcase
+        else
+            # If we are getting the size initially we don't know what the
+            # units will be, default to GB
+            unit = 'g'
         end
 
         raw = lvs('--noheading', '--unit', unit, path)
@@ -301,9 +323,6 @@ def mirrorlog=( new_mirror_log_location )
         end
     end
 
-
-
-
     private
 
     def lvs_pattern

lib/puppet/type/logical_volume.rb

@@ -13,9 +13,19 @@
         raise ArgumentError, "Volume names must be entirely unqualified"
       end
     end
+
+    # This is a bit of a hack, drags the volume_group up from the provider if
+    # it was specified, allowing `puppet resource` and other things that rely
+    # on an indirector search to work
+    munge do |name|
+      if @resource.original_parameters[:provider].respond_to? :volume_group
+        @resource[:volume_group] = @resource.original_parameters[:provider].volume_group
+      end
+      name
+    end
   end
 
-  newparam(:volume_group) do
+  newproperty(:volume_group) do
     desc "The volume group name associated with this logical volume.  This will automatically
             set this volume group as a dependency, but it must be defined elsewhere using the
             volume_group resource type."

spec/unit/classes/lvm_spec.rb

@@ -21,7 +21,7 @@
               'backup' => {
                 'size'              => '5G',
                 'mountpath'         => '/var/backups',
-                'mountpath_require' => true
+                'mountpath_require' => false
               }
             }
           }
@@ -62,7 +62,7 @@
                 'size'              => '5G',
                 'mounted'           => false,
                 'mountpath'         => '/mnt/not_mounted',
-                'mountpath_require' => true
+                'mountpath_require' => false
               }
             }
           }

spec/unit/puppet/provider/logical_volume/lvm_spec.rb

@@ -36,9 +36,8 @@
       expect(@provider.exists?).to be > 10
     end
     it "should return 'nil', lv 'data' in vg 'myvg' does not exist" do
-      @resource.expects(:[]).with(:name).returns('data')
       @resource.expects(:[]).with(:volume_group).returns('myvg').at_least_once
-      @provider.class.stubs(:lvs).with('myvg').returns(lvs_output)
+      @provider.class.stubs(:lvs).with('myvg').raises(Puppet::ExecutionFailure, 'Execution of \'/sbin/lvs myvg\' returned 5')
       expect(@provider.exists?).to be_nil
     end
   end
@@ -205,6 +204,7 @@
             @resource.expects(:[]).with(:volume_group).returns('myvg').at_least_once
             @resource.expects(:[]).with(:size).returns('1g').at_least_once
             @resource.expects(:[]).with(:thinpool).returns('mythinpool').at_least_once
+            @provider.expects(:blkid).with('/dev/myvg/mylv').returns('TYPE=ext4')
             @provider.expects(:lvcreate).with('-n', 'mylv', '--virtualsize', '1g', '--thin', 'myvg/mythinpool')
             @provider.create
             @provider.expects(:lvs).with('--noheading', '--unit', 'g', '/dev/myvg/mylv').returns(' 1.00g').at_least_once