Issue 1804 VDev with the same devices should be in sync

Added VDev and MultiVDev properties to the ZPool type to handle logic
Vdevs with the same devices are now in sync even if the order changes

5 files changed, 158 insertions, 16 deletions

diff --git a/CHANGELOG b/CHANGELOG
index d6596627b..99768cb77 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 0.24.7
+    Fixed #1804 - Added VDev and MultiVDev properties to the ZPool type
+
     Fixed #1496 - nagios_servicedependency needs a unique host_name?
 
     Fixed #1420 - nagios_serviceescalation not allowing host_name more than one type
diff --git a/lib/puppet/provider/zpool/solaris.rb b/lib/puppet/provider/zpool/solaris.rb
index d680a5f63..aaa79c15f 100644
--- a/lib/puppet/provider/zpool/solaris.rb
+++ b/lib/puppet/provider/zpool/solaris.rb
@@ -9,25 +9,29 @@ Puppet::Type.type(:zpool).provide(:solaris) do
             return Hash.new(:absent)
         end
         #get the name and get rid of it
-        pool = Hash.new([])
+        pool = Hash.new
         pool[:pool] = pool_array[0]
         pool_array.shift
 
-        #order matters here :(
         tmp = []
 
-        pool_array.reverse.each_with_index do |value, i|
+        #order matters here :(
+        pool_array.reverse.each do |value|
+            sym = nil
             case value
-            when "spares": pool[:spare] = tmp.reverse and tmp.clear
-            when "logs": pool[:log] = tmp.reverse and tmp.clear
+            when "spares": sym = :spare
+            when "logs": sym = :log
             when "mirror", "raidz1", "raidz2":
                 sym = value == "mirror" ? :mirror : :raidz
-                pool[sym].unshift(tmp.reverse.join(' '))
                 pool[:raid_parity] = "raidz2" if value == "raidz2"
-                tmp.clear
             else
                 tmp << value
-                pool[:disk] = tmp.reverse if i == 0
+                sym = :disk if value == pool_array.first
+            end
+
+            if sym
+                pool[sym] = pool[sym] ? pool[sym].unshift(tmp.reverse.join(' ')) : [tmp.reverse.join(' ')]
+                tmp.clear
             end
         end
 
diff --git a/lib/puppet/type/zpool.rb b/lib/puppet/type/zpool.rb
index 6d589a0fe..11618256f 100755
--- a/lib/puppet/type/zpool.rb
+++ b/lib/puppet/type/zpool.rb
@@ -1,4 +1,37 @@
 module Puppet
+    class Property
+
+        class VDev < Property
+
+            def flatten_and_sort(array)
+                array.collect { |a| a.split(' ') }.flatten.sort
+            end
+
+            def insync?(is)
+                return true unless self.should
+
+                return @should == [:absent] if is == :absent
+
+                flatten_and_sort(is) == flatten_and_sort(@should)
+            end
+        end
+
+        class MultiVDev < VDev
+            def insync?(is)
+                return true unless self.should
+
+                return @should == [:absent] if is == :absent
+
+                return false unless is.length == @should.length
+
+                is.each_with_index { |list, i| return false unless flatten_and_sort(list) == flatten_and_sort(@should[i]) }
+
+                #if we made it this far we are in sync
+                true
+            end
+        end
+    end
+
     newtype(:zpool) do
         @doc = "Manage zpools. Create and delete zpools. The provider WILL NOT SYNC, only report differences.
 
@@ -6,11 +39,11 @@ module Puppet
 
         ensurable
 
-        newproperty(:disk, :array_matching => :all) do
+        newproperty(:disk, :array_matching => :all, :parent => Puppet::Property::VDev) do
             desc "The disk(s) for this pool. Can be an array or space separated string"
         end
 
-        newproperty(:mirror, :array_matching => :all) do
+        newproperty(:mirror, :array_matching => :all, :parent => Puppet::Property::MultiVDev) do
             desc "List of all the devices to mirror for this pool. Each mirror should be a space separated string.
                   mirror => [\"disk1 disk2\", \"disk3 disk4\"]"
 
@@ -21,7 +54,7 @@ module Puppet
             end
         end
 
-        newproperty(:raidz, :array_matching => :all) do
+        newproperty(:raidz, :array_matching => :all, :parent => Puppet::Property::MultiVDev) do
             desc "List of all the devices to raid for this pool. Should be an array of space separated strings.
                   raidz => [\"disk1 disk2\", \"disk3 disk4\"]"
 
@@ -32,11 +65,11 @@ module Puppet
             end
         end
 
-        newproperty(:spare, :array_matching => :all) do
+        newproperty(:spare, :array_matching => :all, :parent => Puppet::Property::VDev) do
             desc "Spare disk(s) for this pool."
         end
 
-        newproperty(:log, :array_matching => :all) do
+        newproperty(:log, :array_matching => :all, :parent => Puppet::Property::VDev) do
             desc "Log disks for this pool. (doesn't support mirroring yet)"
         end
 
diff --git a/spec/unit/provider/zpool/solaris.rb b/spec/unit/provider/zpool/solaris.rb
index af4db88c1..fc8336f32 100755
--- a/spec/unit/provider/zpool/solaris.rb
+++ b/spec/unit/provider/zpool/solaris.rb
@@ -42,20 +42,41 @@ describe provider_class do
         end
 
         describe "when there is a spare" do
-            it "should add the spare disk to the hash and strip the array" do
+            it "should add the spare disk to the hash" do
                 @zpool_data += ["spares", "spare_disk"]
                 @provider.process_zpool_data(@zpool_data)[:spare].should == ["spare_disk"]
             end
         end
 
+        describe "when there are two spares" do
+            it "should add the spare disk to the hash as a single string" do
+                @zpool_data += ["spares", "spare_disk", "spare_disk2"]
+                @provider.process_zpool_data(@zpool_data)[:spare].should == ["spare_disk spare_disk2"]
+            end
+        end
+
         describe "when there is a log" do
-            it "should add the log disk to the hash and strip the array" do
+            it "should add the log disk to the hash" do
                 @zpool_data += ["logs", "log_disk"]
                 @provider.process_zpool_data(@zpool_data)[:log].should == ["log_disk"]
             end
         end
 
-        describe "when the vdev is a mirror" do
+        describe "when there are two logs" do
+            it "should add the log disks to the hash as a single string" do
+                @zpool_data += ["spares", "spare_disk", "spare_disk2"]
+                @provider.process_zpool_data(@zpool_data)[:spare].should == ["spare_disk spare_disk2"]
+            end
+        end
+
+        describe "when the vdev is a single mirror" do
+            it "should call create_multi_array with mirror" do
+                @zpool_data = ["mirrorpool", "mirror", "disk1", "disk2"]
+                @provider.process_zpool_data(@zpool_data)[:mirror].should == ["disk1 disk2"]
+            end
+        end
+
+        describe "when the vdev is a double mirror" do
             it "should call create_multi_array with mirror" do
                 @zpool_data = ["mirrorpool", "mirror", "disk1", "disk2", "mirror", "disk3", "disk4"]
                 @provider.process_zpool_data(@zpool_data)[:mirror].should == ["disk1 disk2", "disk3 disk4"]
diff --git a/spec/unit/type/zpool.rb b/spec/unit/type/zpool.rb
index 6477d061d..c957e5684 100755
--- a/spec/unit/type/zpool.rb
+++ b/spec/unit/type/zpool.rb
@@ -26,3 +26,85 @@ describe zpool do
         end
     end
 end
+
+vdev_property = Puppet::Property::VDev
+
+describe vdev_property do
+    before do
+        vdev_property.initvars
+        @resource = stub 'resource', :[]= => nil, :property => nil
+        @property = vdev_property.new(:resource => @resource)
+    end
+
+    it "should be insync if the devices are the same" do
+        @property.should = ["dev1 dev2"]
+        @property.insync?(["dev2 dev1"]).must be_true
+    end
+
+    it "should be out of sync if the devices are not the same" do
+        @property.should = ["dev1 dev3"]
+        @property.insync?(["dev2 dev1"]).must be_false
+    end
+
+    it "should be insync if the devices are the same and the should values are comma seperated" do
+        @property.should = ["dev1", "dev2"]
+        @property.insync?(["dev2 dev1"]).must be_true
+    end
+
+    it "should be out of sync if the device is absent and should has a value" do
+        @property.should = ["dev1", "dev2"]
+        @property.insync?(:absent).must be_false
+    end
+
+    it "should be insync if the device is absent and should is absent" do
+        @property.should = [:absent]
+        @property.insync?(:absent).must be_true
+    end
+end
+
+multi_vdev_property = Puppet::Property::MultiVDev
+
+describe multi_vdev_property do
+    before do
+        multi_vdev_property.initvars
+        @resource = stub 'resource', :[]= => nil, :property => nil
+        @property = multi_vdev_property.new(:resource => @resource)
+    end
+
+    it "should be insync if the devices are the same" do
+        @property.should = ["dev1 dev2"]
+        @property.insync?(["dev2 dev1"]).must be_true
+    end
+
+    it "should be out of sync if the devices are not the same" do
+        @property.should = ["dev1 dev3"]
+        @property.insync?(["dev2 dev1"]).must be_false
+    end
+
+    it "should be out of sync if the device is absent and should has a value" do
+        @property.should = ["dev1", "dev2"]
+        @property.insync?(:absent).must be_false
+    end
+
+    it "should be insync if the device is absent and should is absent" do
+        @property.should = [:absent]
+        @property.insync?(:absent).must be_true
+    end
+
+    describe "when there are multiple lists of devices" do
+        it "should be in sync if each group has the same devices" do
+            @property.should = ["dev1 dev2", "dev3 dev4"]
+            @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_true
+        end
+
+        it "should be out of sync if any group has the different devices" do
+            @property.should = ["dev1 devX", "dev3 dev4"]
+            @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_false
+        end
+
+        it "should be out of sync if devices are in the wrong group" do
+            @property.should = ["dev1 dev2", "dev3 dev4"]
+            @property.insync?(["dev2 dev3", "dev1 dev4"]).must be_false
+        end
+    end
+end