From e4aa5d024f8a8f6e347d3db4bc91430b59685421 Mon Sep 17 00:00:00 2001 From: Jeffrey Clark Date: Fri, 11 Nov 2022 04:49:03 -0600 Subject: [PATCH 1/4] support raidz3 parity --- lib/puppet/provider/zpool/zpool.rb | 6 +++--- spec/unit/provider/zpool/zpool_spec.rb | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/puppet/provider/zpool/zpool.rb b/lib/puppet/provider/zpool/zpool.rb index 80c5675..0c55311 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)$} diff --git a/spec/unit/provider/zpool/zpool_spec.rb b/spec/unit/provider/zpool/zpool_spec.rb index f773707..05d2f66 100644 --- a/spec/unit/provider/zpool/zpool_spec.rb +++ b/spec/unit/provider/zpool/zpool_spec.rb @@ -170,6 +170,15 @@ 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 end describe 'when calling the getters and setters for configurable options' do From 8ee54fe7ae2bda331ffef5f3b7025b7148e17112 Mon Sep 17 00:00:00 2001 From: Jeffrey Clark Date: Fri, 11 Nov 2022 04:50:53 -0600 Subject: [PATCH 2/4] fix raid_parity --- lib/puppet/provider/zpool/zpool.rb | 2 +- lib/puppet/type/zpool.rb | 2 +- spec/unit/provider/zpool/zpool_spec.rb | 2 +- spec/unit/type/zpool_spec.rb | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/puppet/provider/zpool/zpool.rb b/lib/puppet/provider/zpool/zpool.rb index 0c55311..13ac091 100644 --- a/lib/puppet/provider/zpool/zpool.rb +++ b/lib/puppet/provider/zpool/zpool.rb @@ -148,7 +148,7 @@ def exists? end end - [:disk, :mirror, :raidz, :log, :spare, :cache].each do |field| + [:disk, :mirror, :raidz, :log, :spare, :cache, :raid_parity].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..a8b5730 100644 --- a/lib/puppet/type/zpool.rb +++ b/lib/puppet/type/zpool.rb @@ -115,7 +115,7 @@ 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 diff --git a/spec/unit/provider/zpool/zpool_spec.rb b/spec/unit/provider/zpool/zpool_spec.rb index 05d2f66..766d8de 100644 --- a/spec/unit/provider/zpool/zpool_spec.rb +++ b/spec/unit/provider/zpool/zpool_spec.rb @@ -218,7 +218,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].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..b0e46b3 100644 --- a/spec/unit/type/zpool_spec.rb +++ b/spec/unit/type/zpool_spec.rb @@ -2,14 +2,14 @@ describe 'zpool' do describe Puppet::Type.type(:zpool) do - properties = [:ensure, :disk, :mirror, :raidz, :spare, :log, :autoexpand, :failmode, :ashift, :cache] + 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) From ab1b10f94c7131d8376e6e1e8af9bf40212094b9 Mon Sep 17 00:00:00 2001 From: Jeffrey Clark Date: Fri, 11 Nov 2022 05:16:46 -0600 Subject: [PATCH 3/4] support forcing mixed vdev types --- lib/puppet/provider/zpool/zpool.rb | 33 +++++++++++++++++++------- lib/puppet/type/zpool.rb | 11 ++++++++- spec/unit/provider/zpool/zpool_spec.rb | 11 ++++++++- spec/unit/type/zpool_spec.rb | 12 +++++++++- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/lib/puppet/provider/zpool/zpool.rb b/lib/puppet/provider/zpool/zpool.rb index 13ac091..62bf45d 100644 --- a/lib/puppet/provider/zpool/zpool.rb +++ b/lib/puppet/provider/zpool/zpool.rb @@ -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, :raid_parity].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 a8b5730..ea63637 100644 --- a/lib/puppet/type/zpool.rb +++ b/lib/puppet/type/zpool.rb @@ -119,9 +119,18 @@ def insync?(is) 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/unit/provider/zpool/zpool_spec.rb b/spec/unit/provider/zpool/zpool_spec.rb index 766d8de..46dd2d8 100644 --- a/spec/unit/provider/zpool/zpool_spec.rb +++ b/spec/unit/provider/zpool/zpool_spec.rb @@ -179,6 +179,15 @@ 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 @@ -218,7 +227,7 @@ end describe 'when calling the getters and setters' do - [:disk, :mirror, :raidz, :log, :spare, :cache, :raid_parity].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 b0e46b3..c612160 100644 --- a/spec/unit/type/zpool_spec.rb +++ b/spec/unit/type/zpool_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe 'zpool' do - describe Puppet::Type.type(:zpool) do + 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 @@ -15,6 +15,16 @@ 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 From 9c30c475faabedaca7261eaf871889e87c3c8a33 Mon Sep 17 00:00:00 2001 From: Jeffrey Clark Date: Fri, 12 Jan 2024 23:58:16 -0600 Subject: [PATCH 4/4] support acceptance tests on ubuntu --- spec/acceptance/lib/solaris_util.rb | 10 +++++++--- spec/acceptance/nodesets/default.yml | 12 ++++++++++++ spec/acceptance/tests/zpool/basic_tests_spec.rb | 6 +++--- .../tests/zpool/should_be_idempotent_spec.rb | 6 +++--- spec/acceptance/tests/zpool/should_create_spec.rb | 6 +++--- spec/acceptance/tests/zpool/should_query_spec.rb | 6 +++--- spec/acceptance/tests/zpool/should_remove_spec.rb | 6 +++--- spec/spec_helper_acceptance.rb | 7 +++++++ 8 files changed, 41 insertions(+), 18 deletions(-) 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')