Merge "Make it possible to skip parted+mkfs"

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

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

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.

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