diff --git a/lib/puppet/provider/zpool/zpool.rb b/lib/puppet/provider/zpool/zpool.rb index 80c5675..62bf45d 100644 --- a/lib/puppet/provider/zpool/zpool.rb +++ b/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 diff --git a/lib/puppet/type/zpool.rb b/lib/puppet/type/zpool.rb index 955ab1d..ea63637 100644 --- a/lib/puppet/type/zpool.rb +++ b/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 diff --git a/spec/acceptance/lib/solaris_util.rb b/spec/acceptance/lib/solaris_util.rb index 1adee9b..eb43613 100644 --- a/spec/acceptance/lib/solaris_util.rb +++ b/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 diff --git a/spec/acceptance/nodesets/default.yml b/spec/acceptance/nodesets/default.yml index 64c1990..5adb000 100644 --- a/spec/acceptance/nodesets/default.yml +++ b/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 diff --git a/spec/acceptance/tests/zpool/basic_tests_spec.rb b/spec/acceptance/tests/zpool/basic_tests_spec.rb index 40886db..b555c2d 100644 --- a/spec/acceptance/tests/zpool/basic_tests_spec.rb +++ b/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 diff --git a/spec/acceptance/tests/zpool/should_be_idempotent_spec.rb b/spec/acceptance/tests/zpool/should_be_idempotent_spec.rb index 6059803..7ea492b 100644 --- a/spec/acceptance/tests/zpool/should_be_idempotent_spec.rb +++ b/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 diff --git a/spec/acceptance/tests/zpool/should_create_spec.rb b/spec/acceptance/tests/zpool/should_create_spec.rb index b987b6c..f1f55f3 100644 --- a/spec/acceptance/tests/zpool/should_create_spec.rb +++ b/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 diff --git a/spec/acceptance/tests/zpool/should_query_spec.rb b/spec/acceptance/tests/zpool/should_query_spec.rb index a74719c..58c50a8 100644 --- a/spec/acceptance/tests/zpool/should_query_spec.rb +++ b/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 diff --git a/spec/acceptance/tests/zpool/should_remove_spec.rb b/spec/acceptance/tests/zpool/should_remove_spec.rb index a634e52..44f357f 100644 --- a/spec/acceptance/tests/zpool/should_remove_spec.rb +++ b/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') diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb index 0b71fcd..7dec486 100644 --- a/spec/spec_helper_acceptance.rb +++ b/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') diff --git a/spec/unit/provider/zpool/zpool_spec.rb b/spec/unit/provider/zpool/zpool_spec.rb index f773707..46dd2d8 100644 --- a/spec/unit/provider/zpool/zpool_spec.rb +++ b/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 = {} diff --git a/spec/unit/type/zpool_spec.rb b/spec/unit/type/zpool_spec.rb index 9860355..c612160 100644 --- a/spec/unit/type/zpool_spec.rb +++ b/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