Make it possible to skip parted+mkfs

Previously, each run puppet runs a parted and a xfs_admin command,
which itself triggers udev rules. In production, and during a
rebalance, this may trigger a *very* high load on the storage nodes.

This patch introduces 2 a new options: "manage_partition" and
"manage_filesystem" which are "true" by default (not changing the
past behavior), so administrators can, once the cluster is setup
and the new disks are in, set this variable to "false", or manage
partitions and formating out of puppet.

Change-Id: I2740da550157c94cd80805f60321d3cd91381cff

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