Add support for thin provisioning and setting poolmetadatasize (#154)

* Add support for thin provisioning and setting poolmetadatasize

* Additional fixes and testing/docs

This commit fixes the comments raised in PR/154 and adds testing and
documentation;

* Inherit the thin attribute from Puppet::Parameter::Boolean
* Conditionally add -n flag to provider args rather than drop later
* Fixed hard tab spaces in provider
* Added documentation for new attributes
* Added rspec tests

* added thin and poolmetadatasize to lvm::volume_group defined type

* Changed thin attribute to thinpool

* changed error message for poolmetadatasize validation

* Document that poolmetadatasize currently only sets on creation

* changed $thin to $thinpool, ommitted after attribute rename (#3)

Author: afalko

README.md

@@ -167,12 +167,14 @@ resources out yourself.
 * mounted - If puppet should mount the volume. This only affects what puppet will do, and not what will be mounted at boot-time.
 * no_sync (Parameter) - An optimization in lvcreate, at least on Linux.
 * persistent (Parameter) - Set to true to make the block device persistent
+* poolmetadatasize (Parameter) - Set the initial size of the logical volume pool metadata on creation
 * readahead (Parameter) - The readahead count to use for the new logical volume.
 * region_size (Parameter) - A mirror is divided into regions of this size (in MB), the mirror log uses this granularity to track which regions are in sync. CAN NOT BE CHANGED on already mirrored volume. Take your mirror size in terabytes and round up that number to the next power of 2, using that number as the -R argument.
 * size (Property) - The size of the logical volume. Set to undef to use all available space
 * size_is_minsize (Parameter) Default value: `:false` - Set to true if the ‘size’ parameter specified, is just the minimum size you need (if the LV found is larger then the size requests this is just logged not causing a FAIL)
 * stripes (Parameter) - The number of stripes to allocate for the new logical volume.
 * stripesize (Parameter) - The stripesize to use for the new logical volume.
+* thinpool (Parameter) - Default value: `:false` - Set to true to create a thin pool
 * volume_group (Parameter) - 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.
 
 ### physical_volume

lib/puppet/provider/logical_volume/lvm.rb

@@ -52,7 +52,10 @@ def self.get_logical_volume_properties(logical_volumes_line)
     end
 
     def create
-        args = ['-n', @resource[:name]]
+        args = []
+
+        args.push('-n', @resource[:name]) unless @resource[:thinpool]
+
         if @resource[:size]
             args.push('--size', @resource[:size])
         elsif @resource[:initial_size]
@@ -74,6 +77,11 @@ def create
             args.push('--stripesize', @resource[:stripesize])
         end
 
+	
+
+        if @resource[:poolmetadatasize]
+            args.push('--poolmetadatasize', @resource[:poolmetadatasize])
+        end
 
         if @resource[:mirror]
             args.push('--mirrors', @resource[:mirror])
@@ -110,7 +118,12 @@ def create
             args.push('--type', @resource[:type])
         end
 
-        args << @resource[:volume_group]
+        if @resource[:thinpool]
+            args.push('--thin')
+            args << @resource[:volume_group] + "/" + @resource[:name]
+        else
+            args << @resource[:volume_group]
+        end
         lvcreate(*args)
     end
 

lib/puppet/type/logical_volume.rb

@@ -1,3 +1,5 @@
+require 'puppet/parameter/boolean'
+
 Puppet::Type.newtype(:logical_volume) do
 
   ensurable
@@ -74,6 +76,20 @@ def insync?(is)
     end
   end
 
+  newparam(:thinpool, :boolean => true, :parent => Puppet::Parameter::Boolean) do
+    desc "Set to true to create a thin pool"
+    defaultto false 
+  end
+
+  newparam(:poolmetadatasize) do
+    desc "Change the size of logical volume pool metadata"
+    validate do |value|
+      unless value =~ /^[0-9]+(\.[0-9]+)?[KMGTPE]/i
+        raise ArgumentError , "#{value} is not a valid size for pool metadata"
+      end
+    end
+  end
+
   newparam(:minor) do
     desc "Set the minor number"
     validate do |value|

manifests/logical_volume.pp

@@ -21,6 +21,8 @@
   $range             = undef,
   $size_is_minsize   = undef,
   $type              = undef,
+  $thinpool          = undef,
+  $poolmetadatasize  = undef,
   $mirror            = undef,
   $mirrorlog         = undef,
   $no_sync           = undef,
@@ -76,22 +78,24 @@
   }
 
   logical_volume { $name:
-    ensure          => $ensure,
-    volume_group    => $volume_group,
-    size            => $size,
-    initial_size    => $initial_size,
-    stripes         => $stripes,
-    stripesize      => $stripesize,
-    readahead       => $readahead,
-    extents         => $extents,
-    range           => $range,
-    size_is_minsize => $size_is_minsize,
-    type            => $type,
-    mirror          => $mirror,
-    mirrorlog       => $mirrorlog,
-    no_sync         => $no_sync,
-    region_size     => $region_size,
-    alloc           => $alloc
+    ensure           => $ensure,
+    volume_group     => $volume_group,
+    size             => $size,
+    initial_size     => $initial_size,
+    stripes          => $stripes,
+    stripesize       => $stripesize,
+    readahead        => $readahead,
+    extents          => $extents,
+    range            => $range,
+    size_is_minsize  => $size_is_minsize,
+    type             => $type,
+    thinpool         => $thinpool,
+    poolmetadatasize => $poolmetadatasize,
+    mirror           => $mirror,
+    mirrorlog        => $mirrorlog,
+    no_sync          => $no_sync,
+    region_size      => $region_size,
+    alloc            => $alloc
   }
 
   if $createfs {

spec/unit/puppet/type/logical_volume_spec.rb

@@ -12,6 +12,8 @@
       :size_is_minsize => :false,
       :persistent => :false,
       :minor => 100,
+      :thinpool => false,
+      :poolmetadatasize => '10M',
     }
     stub_default_provider!
       end
@@ -47,6 +49,7 @@
     end
   end
 
+
   describe "when specifying the 'size_is_minsize' parameter" do
     it "should exist" do
       @type.attrclass(:size_is_minsize).should_not be_nil
@@ -90,6 +93,49 @@
 
   end
 
+  describe "when specifying the 'thinpool' parameter" do
+    it "should exist" do
+      @type.attrclass(:thinpool).should_not be_nil
+    end
+    it 'should support setting a value' do
+      with(valid_params)[:thinpool].should == valid_params[:thinpool]
+    end
+    it "should support 'true' as a value" do
+      with(valid_params.merge(:thinpool => :true)) do |resource|
+        resource[:thinpool].should == true
+        end
+      end
+    it "should support 'false' as a value" do
+      with(valid_params.merge(:thinpool => :false)) do |resource|
+        resource[:thinpool].should == false
+        end
+      end
+    it "should not support other values" do
+      specifying(valid_params.merge(:thinpool => :moep)).should raise_error(Puppet::Error)
+    end
+  end
+
+  describe "when specifying the 'poolmetadatasize' parameter" do
+    it "should exist" do
+      @type.attrclass(:poolmetadatasize).should_not be_nil
+    end
+    it 'should support setting a value' do
+      with(valid_params)[:poolmetadatasize].should == valid_params[:poolmetadatasize]
+    end
+
+    it 'should support K, M, G, T, P and E as extensions' do
+      ['K', 'M', 'G', 'T', 'P', 'E'].each do |ext|
+        with(valid_params.merge(:poolmetadatasize => "10#{ext}")) do |resource|
+          resource[:poolmetadatasize].should == "10#{ext}"
+        end
+      end
+    end
+
+    it "should not support other values" do
+      specifying(valid_params.merge(:poolmetadatasize => "100X")).should raise_error(Puppet::Error)
+    end
+  end
+
   describe "when specifying the 'extents' parameter" do
     it "should exist" do
       @type.attrclass(:extents).should_not be_nil