diff --git a/manifests/storage/disk.pp b/manifests/storage/disk.pp index 74278625..b71576e7 100644 --- a/manifests/storage/disk.pp +++ b/manifests/storage/disk.pp @@ -33,6 +33,19 @@ # (optional) The external command that will be used in parted command. # Default to ''. For making partitions, it would be 'mkpart primary 0% 100%'. # +# [*manage_partition*] +# (optional) If set to false, skip calling parted which can, in some cases, +# increase the load on the server. This is to set to false only after the +# server is fully setup or if the partition was created outside of puppet. +# Defaults to true. +# +# [*manage_filesystem*] +# (optional) If set to false, skip calling xfs_admin -l to check if a +# partition needs to be formated with mkfs.xfs, which can, in some cases, +# increase the load on the server. This is to set to false only after the +# server is fully setup, or if the filesystem was created outside of puppet. +# Defaults to true. +# # =Example= # # Simply add one disk sdb: @@ -48,10 +61,12 @@ # TODO(yuxcer): maybe we can remove param $base_dir # define swift::storage::disk( - $base_dir = '/dev', - $mnt_base_dir = '/srv/node', - $byte_size = '1024', - $ext_args = '', + $base_dir = '/dev', + $mnt_base_dir = '/srv/node', + $byte_size = '1024', + $ext_args = '', + $manage_partition = true, + $manage_filesystem = true, ) { include swift::deps @@ -66,19 +81,22 @@ define swift::storage::disk( } } - exec { "create_partition_label-${name}": - command => "parted -s ${base_dir}/${name} mklabel gpt ${ext_args}", - path => ['/usr/bin/', '/sbin','/bin'], - onlyif => ["test -b ${base_dir}/${name}","parted ${base_dir}/${name} print|tail -1|grep 'Error'"], - before => Anchor['swift::config::end'], + if $manage_partition { + exec { "create_partition_label-${name}": + command => "parted -s ${base_dir}/${name} mklabel gpt ${ext_args}", + path => ['/usr/bin/', '/sbin','/bin'], + onlyif => ["test -b ${base_dir}/${name}","parted ${base_dir}/${name} print|tail -1|grep 'Error'"], + before => Anchor['swift::config::end'], + } + Exec["create_partition_label-${name}"] ~> Swift::Storage::Xfs<| title == $name |> } swift::storage::xfs { $name: - device => "${base_dir}/${name}", - mnt_base_dir => $mnt_base_dir, - byte_size => $byte_size, - loopback => false, - subscribe => Exec["create_partition_label-${name}"], + device => "${base_dir}/${name}", + mnt_base_dir => $mnt_base_dir, + byte_size => $byte_size, + loopback => false, + manage_filesystem => $manage_filesystem, } } diff --git a/manifests/storage/xfs.pp b/manifests/storage/xfs.pp index 1dee0425..fda02f36 100644 --- a/manifests/storage/xfs.pp +++ b/manifests/storage/xfs.pp @@ -21,6 +21,13 @@ # (optional) Define if the device is mounted by the device partition path or UUID. # Defaults to 'path'. # +# [*manage_filesystem*] +# (optional) If set to false, skip calling xfs_admin -l to check if a +# partition needs to be formated with mkfs.xfs, which can, in some cases, +# increase the load on the server. This is to set to false only after the +# server is fully setup, or if the filesystem was created outside of puppet. +# Defaults to true. +# # Sample usage: # # swift::storage::xfs { @@ -34,11 +41,12 @@ # it already has an XFS FS, and mounts de FS in /srv/node/sdX # define swift::storage::xfs( - $device = '', - $byte_size = '1024', - $mnt_base_dir = '/srv/node', - $loopback = false, - $mount_type = 'path', + $device = '', + $byte_size = '1024', + $mnt_base_dir = '/srv/node', + $loopback = false, + $mount_type = 'path', + $manage_filesystem = true, ) { include swift::deps @@ -77,11 +85,19 @@ define swift::storage::xfs( # If it's not a valid XFS FS, command will return 1 # so we format it. If device has a valid XFS FS, command returns 0 # So we do NOT touch it. - exec { "mkfs-${name}": - command => "mkfs.xfs -f -i size=${byte_size} ${target_device}", - path => ['/sbin/', '/usr/sbin/'], - unless => "xfs_admin -l ${target_device}", - before => Anchor['swift::config::end'], + if $manage_filesystem { + exec { "mkfs-${name}": + command => "mkfs.xfs -f -i size=${byte_size} ${target_device}", + path => ['/sbin/', '/usr/sbin/'], + unless => "xfs_admin -l ${target_device}", + before => Anchor['swift::config::end'], + } + Package<| title == 'xfsprogs' |> + ~> Exec<| title == "mkfs-${name}" |> + ~> Swift::Storage::Mount<| title == $name |> + } else { + Package<| title == 'xfsprogs' |> + ~> Swift::Storage::Mount<| title == $name |> } swift::storage::mount { $name: @@ -90,8 +106,5 @@ define swift::storage::xfs( loopback => $loopback, } - Package<| title == 'xfsprogs' |> - ~> Exec<| title == "mkfs-${name}" |> - ~> Swift::Storage::Mount<| title == $name |> } diff --git a/releasenotes/notes/manage_partition_and_filesystem-64cd4086d1bb5b0a.yaml b/releasenotes/notes/manage_partition_and_filesystem-64cd4086d1bb5b0a.yaml new file mode 100644 index 00000000..ee8e1438 --- /dev/null +++ b/releasenotes/notes/manage_partition_and_filesystem-64cd4086d1bb5b0a.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + The class swift::storage::disk now has 2 new parameters called + manage_partition and manage_filesystem. The swift::storage::xfs now has + a new parameter manage_filesystem. Both have a value of true by default. + These new options may be used to skip running parted and xfs_admin once + the storage disks are setup. The goal is to avoid the kernel to flush + the HDD buffer, which can seriously increase the load and IO wait on a + (busy) production server. These parameters should be left to true when + setting-up a new server, or when changing a disk, and can be set to false + otherwise, or always set to false if the formating is done outside of + puppet. diff --git a/spec/defines/swift_storage_disk_spec.rb b/spec/defines/swift_storage_disk_spec.rb index 7d2e20bb..a17e7563 100644 --- a/spec/defines/swift_storage_disk_spec.rb +++ b/spec/defines/swift_storage_disk_spec.rb @@ -28,7 +28,6 @@ describe 'swift::storage::disk' do :device => '/dev/sdb', :mnt_base_dir => '/srv/node', :byte_size => '1024', - :subscribe => 'Exec[create_partition_label-sdb]', :loopback => false ) } end