improve zfsonlinux support

lib/puppet/provider/zpool/zpool.rb

@@ -39,9 +39,9 @@ def process_zpool_data(pool_array)
         sym = :log
       when 'cache'
         sym = :cache
-      when %r{^mirror|^raidz1|^raidz2}
-        sym = (value =~ %r{^mirror}) ? :mirror : :raidz
-        pool[:raid_parity] = 'raidz2' if %r{^raidz2}.match?(value)
+      when %r{^((mirror|raidz)\d?)}
+        sym = Regexp.last_match(2).to_sym
+        pool[:raid_parity] = Regexp.last_match(1) if sym == :raidz
       else
         # get full drive name if the value is a partition (Linux only)
         tmp << if Facter.value(:kernel) == 'Linux' && value =~ %r{/dev/(:?[a-z]+1|disk/by-id/.+-part1)$}
@@ -58,6 +58,10 @@ def process_zpool_data(pool_array)
       end
     end
 
+    if pool.key?(:mirror) && pool.key?(:raidz)
+      pool[:force] = true
+    end
+
     pool
   end
 
@@ -113,27 +117,38 @@ def build_vdevs
     mirror = @resource[:mirror]
     raidz = @resource[:raidz]
 
+    vdevs = []
+
     if disk
-      disk.map { |d| d.split(' ') }.flatten
-    elsif mirror
-      handle_multi_arrays('mirror', mirror)
-    elsif raidz
-      handle_multi_arrays(raidzarity, raidz)
+      vdevs << disk.map { |d| d.split(' ') }.flatten
+    else
+      if mirror
+        vdevs << handle_multi_arrays('mirror', mirror)
+      end
+      if raidz
+        vdevs << handle_multi_arrays(raidzarity, raidz)
+      end
     end
+
+    vdevs
   end
 
   def add_pool_properties
     properties = []
-    [:ashift, :autoexpand, :failmode].each do |property|
+    [:force, :ashift, :autoexpand, :failmode].each do |property|
       if (value = @resource[property]) && value != ''
-        properties << '-o' << "#{property}=#{value}"
+        if property == :force
+          properties << '-f'
+        else
+          properties << '-o' << "#{property}=#{value}"
+        end
       end
     end
     properties
   end
 
   def create
-    zpool(*([:create] + add_pool_properties + [@resource[:pool]] + build_vdevs + build_named('spare') + build_named('log') + build_named('cache')))
+    zpool(*([:create] + add_pool_properties + [@resource[:pool]] + build_vdevs.flatten + build_named('spare') + build_named('log') + build_named('cache')))
   end
 
   def destroy
@@ -148,7 +163,7 @@ def exists?
     end
   end
 
-  [:disk, :mirror, :raidz, :log, :spare, :cache].each do |field|
+  [:disk, :mirror, :raidz, :log, :spare, :cache, :raid_parity, :force].each do |field|
     define_method(field) do
       current_pool[field]
     end

lib/puppet/type/zpool.rb

@@ -115,13 +115,22 @@ def insync?(is)
       isnamevar
     end
 
-    newparam(:raid_parity) do
+    newproperty(:raid_parity, array_matching: :all, parent: Puppet::Property::MultiVDev) do
       desc 'Determines parity when using the `raidz` parameter.'
     end
 
+    newproperty(:force, boolean: false) do
+      desc "Forces use of vdevs, even if they appear in use or specify a conflicting replication level.
+      Not all devices can be overridden in this manner.
+      WARNING: this is an advanced option that should be used with caution! Ignores safety checks, may OVERWRITE DATA!"
+
+      defaultto false
+      newvalues(:true, :false)
+    end
+
     validate do
       has_should = [:disk, :mirror, :raidz].select { |prop| should(prop) }
-      raise _('You cannot specify %{multiple_props} on this type (only one)') % { multiple_props: has_should.join(' and ') } if has_should.length > 1
+      raise _('You cannot specify %{multiple_props} on this type (only one)') % { multiple_props: has_should.join(' and ') } if has_should.length > 1 && !should(:force)
     end
   end
 end

spec/acceptance/lib/solaris_util.rb

@@ -1,5 +1,9 @@
 OPTS = { poolpath: '/ztstpool', pool: 'tstpool', fs: 'tstfs' }.freeze
 
+def os_mkfile_command(agent)
+  agent['platform'].include?('solaris') ? 'mkfile' : 'truncate --size'
+end
+
 def zfs_clean(agent, o = {})
   o = OPTS.merge(o)
   on agent, "zfs destroy -f -r #{o[:pool]}/#{o[:fs]} ||:"
@@ -11,7 +15,7 @@ def zfs_setup(agent, o = {})
   o = OPTS.merge(o)
   on agent, "mkdir -p #{o[:poolpath]}/mnt"
   on agent, "mkdir -p #{o[:poolpath]}/mnt2"
-  on agent, "mkfile 64m #{o[:poolpath]}/dsk"
+  on agent, "#{os_mkfile_command(agent)} 64m #{o[:poolpath]}/dsk"
   on agent, "zpool create #{o[:pool]} #{o[:poolpath]}/dsk"
 end
 
@@ -24,6 +28,6 @@ def zpool_clean(agent, o = {})
 def zpool_setup(agent, o = {})
   o = OPTS.merge(o)
   on agent, "mkdir -p #{o[:poolpath]}/mnt||:"
-  on agent, "mkfile 100m #{o[:poolpath]}/dsk1 #{o[:poolpath]}/dsk2 #{o[:poolpath]}/dsk3 #{o[:poolpath]}/dsk5 ||:"
-  on agent, "mkfile 50m #{o[:poolpath]}/dsk4 ||:"
+  on agent, "#{os_mkfile_command} 100m #{o[:poolpath]}/dsk1 #{o[:poolpath]}/dsk2 #{o[:poolpath]}/dsk3 #{o[:poolpath]}/dsk5 ||:"
+  on agent, "#{os_mkfile_command} 50m #{o[:poolpath]}/dsk4 ||:"
 end

spec/acceptance/nodesets/default.yml

@@ -12,6 +12,18 @@ HOSTS:
     roles:
     - agent
     - default
+  ubuntu22-64-1:
+    pe_dir:
+    pe_ver:
+    pe_upgrade_dir:
+    pe_upgrade_ver:
+    hypervisor: vmpooler
+    platform: ubuntu-22.04-amd64
+    packaging_platform: ubuntu-22.04-amd64
+    template: ubuntu-2204-x86_64
+    roles:
+    - agent
+    - default
 CONFIG:
   type: agent
   nfs_server: none

spec/acceptance/tests/zpool/basic_tests_spec.rb

@@ -3,19 +3,19 @@
 RSpec.context 'ZPool: Basic Tests' do
   before(:all) do
     # ZPool: setup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_setup agent
     end
   end
 
   after(:all) do
     # ZPool: cleanup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_clean agent
     end
   end
 
-  solaris_agents.each do |agent|
+  agents.each do |agent|
     it 'can create an idempotent zpool resource' do
       # ZPool: create zpool disk
       apply_manifest_on(agent, "zpool{ tstpool: ensure=>present, disk=>'/ztstpool/dsk1' }") do

spec/acceptance/tests/zpool/should_be_idempotent_spec.rb

@@ -3,19 +3,19 @@
 RSpec.context 'ZPool: Should be Idempotent' do
   before(:all) do
     # ZPool: setup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_setup agent
     end
   end
 
   after(:all) do
     # ZPool: cleanup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_clean agent
     end
   end
 
-  solaris_agents.each do |agent|
+  agents.each do |agent|
     it 'creates an idempotent resource' do
       # ZPool: ensure create
       apply_manifest_on(agent, "zpool{ tstpool: ensure=>present, disk=>'/ztstpool/dsk1' }") do

spec/acceptance/tests/zpool/should_create_spec.rb

@@ -3,19 +3,19 @@
 RSpec.context 'ZPool: Should Create' do
   before(:all) do
     # ZPool: setup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_setup agent
     end
   end
 
   after(:all) do
     # ZPool: cleanup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_clean agent
     end
   end
 
-  solaris_agents.each do |agent|
+  agents.each do |agent|
     it 'creates a zpool resource' do
       # ZPool: ensure create
       apply_manifest_on(agent, "zpool{ tstpool: ensure=>present, disk=>'/ztstpool/dsk1' }") do

spec/acceptance/tests/zpool/should_query_spec.rb

@@ -3,19 +3,19 @@
 RSpec.context 'ZPool: Should Query' do
   before(:all) do
     # ZPool: setup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_setup agent
     end
   end
 
   after(:all) do
     # ZPool: cleanup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_clean agent
     end
   end
 
-  solaris_agents.each do |agent|
+  agents.each do |agent|
     it 'queries both manages and unmanages zpool resources' do
       # ZPool: ensure create
       apply_manifest_on(agent, "zpool{ tstpool: ensure=>present, disk=>'/ztstpool/dsk1' }") do

spec/acceptance/tests/zpool/should_remove_spec.rb

@@ -3,19 +3,19 @@
 RSpec.context 'ZPool: Should Remove' do
   before(:all) do
     # ZPool: setup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_setup agent
     end
   end
 
   after(:all) do
     # ZPool: cleanup
-    solaris_agents.each do |agent|
+    agents.each do |agent|
       zpool_clean agent
     end
   end
 
-  solaris_agents.each do |agent|
+  agents.each do |agent|
     it 'removes a specified resource' do
       # ZPool: create
       on(agent, 'zpool create tstpool /ztstpool/dsk1')

spec/spec_helper_acceptance.rb

@@ -15,6 +15,10 @@ def solaris_agents
   agents.select { |agent| agent['platform'].include?('solaris') }
 end
 
+def linux_agents
+  agents.select { |agent| agent['platform'].include?('ubuntu') }
+end
+
 RSpec.configure do |c|
   c.before :suite do
     unless ENV['BEAKER_provision'] == 'no'
@@ -55,6 +59,9 @@ def solaris_agents
 
       # Until solaris gets new image we need to add to the cert chain on solaris, call a beaker-puppet setup script to handle this
       hosts.each do |host|
+        if hosts.platform.include?('ubuntu')
+          on(host, 'apt -y install zfsutils-linux')
+        end
         next unless host.platform.match? %r{solaris-11(\.2)?-(i386|sparc)}
         create_remote_file(host, 'DigiCertTrustedRootG4.crt.pem', DIGICERT)
         on(host, 'chmod a+r /root/DigiCertTrustedRootG4.crt.pem')

spec/unit/provider/zpool/zpool_spec.rb

@@ -170,6 +170,24 @@
         expect(pool[:raid_parity]).to eq('raidz2')
       end
     end
+
+    describe 'when the vdev is a raidz3 on linux' do
+      it 'calls create_multi_array with raidz3 and set the raid_parity' do
+        zpool_data = ['mirrorpool', 'raidz3-0', 'disk1', 'disk2']
+        pool = provider.process_zpool_data(zpool_data)
+        expect(pool[:raidz]).to eq(['disk1 disk2'])
+        expect(pool[:raid_parity]).to eq('raidz3')
+      end
+    end
+
+    describe 'when there are mixed vdev replication levels' do
+      it 'calls create_multi_array with mirror and raidz1' do
+        zpool_data = ['mirrorpool', 'mirror-0', 'disk1', 'disk2', 'raidz1-1', 'disk3', 'disk4']
+        pool = provider.process_zpool_data(zpool_data)
+        expect(pool[:mirror]).to eq(['disk1 disk2'])
+        expect(pool[:raidz]).to eq(['disk3 disk4'])
+      end
+    end
   end
 
   describe 'when calling the getters and setters for configurable options' do
@@ -209,7 +227,7 @@
   end
 
   describe 'when calling the getters and setters' do
-    [:disk, :mirror, :raidz, :log, :spare, :cache].each do |field|
+    [:disk, :mirror, :raidz, :log, :spare, :cache, :raid_parity, :force].each do |field|
       describe "when calling #{field}" do
         it "gets the #{field} value from the current_pool hash" do
           pool_hash = {}

spec/unit/type/zpool_spec.rb

@@ -1,20 +1,30 @@
 require 'spec_helper'
 
 describe 'zpool' do
-  describe Puppet::Type.type(:zpool) do
-    properties = [:ensure, :disk, :mirror, :raidz, :spare, :log, :autoexpand, :failmode, :ashift, :cache]
+  describe Puppet::Type.type(:zpool), type: :type do
+    properties = [:ensure, :disk, :mirror, :raidz, :spare, :log, :autoexpand, :failmode, :ashift, :cache, :raid_parity]
     properties.each do |property|
       it "has a #{property} property" do
         expect(described_class.attrclass(property).ancestors).to be_include(Puppet::Property)
       end
     end
 
-    parameters = [:pool, :raid_parity]
+    parameters = [:pool]
     parameters.each do |parameter|
       it "has a #{parameter} parameter" do
         expect(described_class.attrclass(parameter).ancestors).to be_include(Puppet::Parameter)
       end
     end
+
+    it 'is invalid when multiple vdev replications specified' do
+      expect {
+        described_class.new(name: 'dummy', disk: 'disk1', mirror: 'disk2 disk3')
+      }.to raise_error(RuntimeError, 'You cannot specify disk and mirror on this type (only one)')
+    end
+
+    it 'is valid when multiple vdev replications are forced' do
+      described_class.new(name: 'dummy', disk: 'disk1', mirror: 'disk2 disk3', force: true)
+    end
   end
 
   describe Puppet::Property::VDev do