Merge "xfs: Support mount by label"

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'],

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,

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'],

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.

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
       )}

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
 

types/mountdevice.pp

@@ -0,0 +1,4 @@
+type Swift::MountDevice = Variant[
+  Stdlib::Absolutepath,
+  Pattern[/^LABEL=.+$/]
+]