From 9efd303bcbc8b32e6156ef0fff0eb5c571123eee Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Wed, 21 Feb 2024 14:02:54 +0900 Subject: [PATCH] xfs: Support mount by label ... in addition to uuid and device name, as a more static but more flexible way to assign devices to mount points. Change-Id: I92e3f5d09c071c48e8b51026a2cda2394cbe33cf --- manifests/storage/ext4.pp | 2 +- manifests/storage/mount.pp | 8 ++--- manifests/storage/xfs.pp | 35 +++++++++++++------ ...xfs-label-mount-type-1f80ea3bb0a32e0c.yaml | 8 +++++ spec/defines/swift_storage_mount_spec.rb | 2 +- spec/defines/swift_storage_xfs_spec.rb | 29 +++++++++++++-- types/mountdevice.pp | 4 +++ 7 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/xfs-label-mount-type-1f80ea3bb0a32e0c.yaml create mode 100644 types/mountdevice.pp diff --git a/manifests/storage/ext4.pp b/manifests/storage/ext4.pp index 2f475a30..c8f7462e 100644 --- a/manifests/storage/ext4.pp +++ b/manifests/storage/ext4.pp @@ -34,7 +34,7 @@ define swift::storage::ext4( # does this have to be refreshonly? # how can I know if this drive has been formatted? exec { "mkfs-${name}": - command => "mkfs.ext4 -I ${byte_size} -F ${device}", + command => ['mkfs.ext4', '-I', $byte_size, '-F', $device], path => ['/sbin/'], refreshonly => true, before => Anchor['swift::config::end'], diff --git a/manifests/storage/mount.pp b/manifests/storage/mount.pp index e42a4311..eb163bc9 100644 --- a/manifests/storage/mount.pp +++ b/manifests/storage/mount.pp @@ -19,7 +19,7 @@ # Defaults to 'xfs'. # define swift::storage::mount( - Stdlib::Absolutepath $device = "/dev/${name}", + Swift::MountDevice $device = "/dev/${name}", Stdlib::Absolutepath $mnt_base_dir = '/srv/node', Boolean $loopback = false, String[1] $fstype = 'xfs' @@ -50,7 +50,7 @@ define swift::storage::mount( # Make root own the mount point to prevent swift processes from writing files # when the disk device is not mounted exec { "fix_mountpoint_permissions_${name}": - command => "chown -R root:root ${mnt_base_dir}/${name}", + command => ['chown', '-R', 'root:root', "${mnt_base_dir}/${name}"], path => ['/usr/bin', '/bin'], before => Anchor['swift::config::end'], unless => "grep ${mnt_base_dir}/${name} /etc/mtab", @@ -75,7 +75,7 @@ define swift::storage::mount( $group = $::swift::params::group exec { "fix_mount_permissions_${name}": - command => "chown -R ${user}:${group} ${mnt_base_dir}/${name}", + command => ['chown', '-R', "${user}:${group}", "${mnt_base_dir}/${name}"], path => ['/usr/bin', '/bin'], refreshonly => true, before => Anchor['swift::config::end'], @@ -98,7 +98,7 @@ define swift::storage::mount( # systems :( if (str2bool($facts['os']['selinux']['enabled']) == true) { exec { "restorecon_mount_${name}": - command => "restorecon ${mnt_base_dir}/${name}", + command => ['restorecon', "${mnt_base_dir}/${name}"], path => ['/usr/sbin', '/sbin'], before => Anchor['swift::config::end'], refreshonly => true, diff --git a/manifests/storage/xfs.pp b/manifests/storage/xfs.pp index 00d5e84d..fab3bf63 100644 --- a/manifests/storage/xfs.pp +++ b/manifests/storage/xfs.pp @@ -12,15 +12,16 @@ # # [*byte_size*] # (optional) Byte size to use for every inode in the created filesystem. -# Defaults to '1024'. It is recommended to use 1024 to ensure that the metadata can fit -# in a single inode. +# Defaults to '1024'. It is recommended to use 1024 to ensure that +# the metadata can fit in a single inode. # # [*loopback*] # (optional) Define if the device must be mounted as a loopback or not # Defaults to false. # # [*mount_type*] -# (optional) Define if the device is mounted by the device partition path or UUID. +# (optional) Define if the device is mounted by the device partition path, +# UUID, or filesystem label. # Defaults to 'path'. # # [*manage_filesystem*] @@ -30,6 +31,10 @@ # server is fully setup, or if the filesystem was created outside of puppet. # Defaults to true. # +# [*label*] +# (optional) Filesystem label. +# Defaults to $name. +# # Sample usage: # # swift::storage::xfs { @@ -43,12 +48,13 @@ # it already has an XFS FS, and mounts de FS in /srv/node/sdX # define swift::storage::xfs( - Stdlib::Absolutepath $device = "/dev/${name}", - $byte_size = '1024', - Stdlib::Absolutepath $mnt_base_dir = '/srv/node', - Boolean $loopback = false, - Enum['path', 'uuid'] $mount_type = 'path', - Boolean $manage_filesystem = true, + Stdlib::Absolutepath $device = "/dev/${name}", + $byte_size = '1024', + Stdlib::Absolutepath $mnt_base_dir = '/srv/node', + Boolean $loopback = false, + Enum['path', 'uuid', 'label'] $mount_type = 'path', + Boolean $manage_filesystem = true, + String[1] $label = $name, ) { include swift::deps @@ -67,6 +73,9 @@ define swift::storage::xfs( fail("Unable to fetch uuid of ${device}") } } + 'label': { + $mount_device = "LABEL=${label}" + } default: { # path $mount_device = $device } @@ -87,8 +96,14 @@ define swift::storage::xfs( # so we format it. If device has a valid XFS FS, command returns 0 # So we do NOT touch it. if $manage_filesystem { + $mkfs_command = ['mkfs.xfs', '-f', '-i', "size=${byte_size}"] + $mkfs_label_opt = $mount_type ? { + 'label' => ['-L', $label], + default => [] + } + exec { "mkfs-${name}": - command => "mkfs.xfs -f -i size=${byte_size} ${device}", + command => $mkfs_command + $mkfs_label_opt + [$device], path => ['/sbin/', '/usr/sbin/'], unless => "xfs_admin -l ${device}", before => Anchor['swift::config::end'], diff --git a/releasenotes/notes/xfs-label-mount-type-1f80ea3bb0a32e0c.yaml b/releasenotes/notes/xfs-label-mount-type-1f80ea3bb0a32e0c.yaml new file mode 100644 index 00000000..ee82f5dc --- /dev/null +++ b/releasenotes/notes/xfs-label-mount-type-1f80ea3bb0a32e0c.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + The ``swift::storage::xfs`` defined resource type now supports + ``type => 'label'``, which uses filesystem label for mount. + When this is used, the defined type adds the label, according to the new + ``label`` parameter, which defaults to the title, when creating a XFS file + system. diff --git a/spec/defines/swift_storage_mount_spec.rb b/spec/defines/swift_storage_mount_spec.rb index 340f2717..66bd461c 100644 --- a/spec/defines/swift_storage_mount_spec.rb +++ b/spec/defines/swift_storage_mount_spec.rb @@ -47,7 +47,7 @@ describe 'swift::storage::mount' do end it { is_expected.to contain_exec("restorecon_mount_dans_mount_point").with( - :command => "restorecon /srv/node/dans_mount_point", + :command => ['restorecon', '/srv/node/dans_mount_point'], :path => ['/usr/sbin', '/sbin'], :refreshonly => true )} diff --git a/spec/defines/swift_storage_xfs_spec.rb b/spec/defines/swift_storage_xfs_spec.rb index 6419b19a..f11754f4 100644 --- a/spec/defines/swift_storage_xfs_spec.rb +++ b/spec/defines/swift_storage_xfs_spec.rb @@ -36,8 +36,9 @@ describe 'swift::storage::xfs' do end it { is_expected.to contain_exec("mkfs-foo").with( - :command => "mkfs.xfs -f -i size=#{param_hash[:byte_size]} #{param_hash[:device]}", - :path => ['/sbin/', '/usr/sbin/'], + :command => ['mkfs.xfs', '-f', '-i', "size=#{param_hash[:byte_size]}", param_hash[:device]], + :path => ['/sbin/', '/usr/sbin/'], + :unless => "xfs_admin -l #{param_hash[:device]}", )} it { is_expected.to contain_swift__storage__mount(title).with( @@ -47,6 +48,30 @@ describe 'swift::storage::xfs' do )} end end + + context 'with mount type label' do + let :params do + { + :mount_type => :label + } + end + + let :param_hash do + default_params.merge(params) + end + + it { is_expected.to contain_exec("mkfs-foo").with( + :command => ['mkfs.xfs', '-f', '-i', "size=#{param_hash[:byte_size]}", '-L', title, param_hash[:device]], + :path => ['/sbin/', '/usr/sbin/'], + :unless => "xfs_admin -l #{param_hash[:device]}", + )} + + it { is_expected.to contain_swift__storage__mount(title).with( + :device => "LABEL=#{title}", + :mnt_base_dir => param_hash[:mnt_base_dir], + :loopback => param_hash[:loopback], + )} + end end end diff --git a/types/mountdevice.pp b/types/mountdevice.pp new file mode 100644 index 00000000..41642d6d --- /dev/null +++ b/types/mountdevice.pp @@ -0,0 +1,4 @@ +type Swift::MountDevice = Variant[ + Stdlib::Absolutepath, + Pattern[/^LABEL=.+$/] +]