Add data type validation for device handling

... and also makes behavior of the `device` parameters consistent.

Change-Id: I5f34a91aa4c9f3bebf6e41b19fbf5d41eb7ecf17

manifests/storage/disk.pp

@@ -61,12 +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          = '',
-  $manage_partition  = true,
-  $manage_filesystem = true,
+  Stdlib::Absolutepath $base_dir     = '/dev',
+  Stdlib::Absolutepath $mnt_base_dir = '/srv/node',
+  $byte_size                         = '1024',
+  $ext_args                          = '',
+  Boolean $manage_partition          = true,
+  Boolean $manage_filesystem         = true,
 ) {
 
   include swift::deps

manifests/storage/ext4.pp

@@ -6,7 +6,8 @@
 # === Parameters:
 #
 # [*device*]
-#   (mandatory) An array of devices (prefixed or not by /dev)
+#   (optional) Path to the device.
+#   Defaults to "/dev/${name}"
 #
 # [*mnt_base_dir*]
 #   (optional) The directory where the flat files that store the file system
@@ -23,10 +24,10 @@
 #   Defaults to false.
 #
 define swift::storage::ext4(
-  $device,
-  $byte_size    = '1024',
-  $mnt_base_dir = '/srv/node',
-  $loopback     = false
+  Stdlib::Absolutepath $device       = "/dev/${name}",
+  $byte_size                         = '1024',
+  Stdlib::Absolutepath $mnt_base_dir = '/srv/node',
+  Boolean $loopback                  = false
 ) {
 
   include swift::deps

manifests/storage/mount.pp

@@ -1,12 +1,9 @@
 #
-# Usage
-#   swift::storage::mount
-#
-#
 # === Parameters:
 #
 # [*device*]
-#   (mandatory) An array of devices (prefixed or not by /dev)
+#   (optional) Path to the device.
+#   Defaults to "/dev/${name}"
 #
 # [*mnt_base_dir*]
 #   (optional) The directory where the flat files that store the file system
@@ -22,7 +19,7 @@
 #   Defaults to 'xfs'.
 #
 define swift::storage::mount(
-  $device,
+  Stdlib::Absolutepath $device       = "/dev/${name}",
   Stdlib::Absolutepath $mnt_base_dir = '/srv/node',
   Boolean $loopback                  = false,
   String[1] $fstype                  = 'xfs'

manifests/storage/xfs.pp

@@ -2,7 +2,8 @@
 # === Parameters:
 #
 # [*device*]
-#   (mandatory) An array of devices (prefixed or not by /dev)
+#   (optional) Path to the device.
+#   Defaults to "/dev/${name}"
 #
 # [*mnt_base_dir*]
 #   (optional) The directory where the flat files that store the file system
@@ -42,11 +43,11 @@
 # it already has an XFS FS, and mounts de FS in /srv/node/sdX
 #
 define swift::storage::xfs(
-  $device                            = '',
+  Stdlib::Absolutepath $device       = "/dev/${name}",
   $byte_size                         = '1024',
   Stdlib::Absolutepath $mnt_base_dir = '/srv/node',
   Boolean $loopback                  = false,
-  $mount_type                        = 'path',
+  Enum['path', 'uuid'] $mount_type   = 'path',
   Boolean $manage_filesystem         = true,
 ) {
 
@@ -54,23 +55,21 @@ define swift::storage::xfs(
   include swift::params
   include swift::xfs
 
-  if $device == '' {
-    $target_device = "/dev/${name}"
-  } else {
-    $target_device = $device
-  }
-
   # Currently, facter doesn't support to fetch the device's uuid, only the partition's.
   # If you want to mount device by uuid, you should set $ext_args to 'mkpart primary 0% 100%'
   # in swift::storage::disk to make a partition. Also, the device name should change accordingly.
   # For example: from 'sda' to 'sda1'.
   # The code does NOT work in existing Swift cluster.
   case $mount_type {
-    'path': { $mount_device = $target_device }
-    'uuid': { $mount_device = dig44($facts, ['partitions', $target_device, 'uuid'])
-              unless $mount_device { fail("Unable to fetch uuid of ${target_device}") }
-            }
-    default: { fail("Unsupported mount_type parameter value: '${mount_type}'. Should be 'path' or 'uuid'.") }
+    'uuid': {
+      $mount_device = dig44($facts, ['partitions', $device, 'uuid'])
+      if !$mount_device {
+        fail("Unable to fetch uuid of ${device}")
+      }
+    }
+    default: { # path
+      $mount_device = $device
+    }
   }
 
   if(!defined(File[$mnt_base_dir])) {
@@ -89,9 +88,9 @@ define swift::storage::xfs(
   # So we do NOT touch it.
   if $manage_filesystem {
     exec { "mkfs-${name}":
-      command => "mkfs.xfs -f -i size=${byte_size} ${target_device}",
+      command => "mkfs.xfs -f -i size=${byte_size} ${device}",
       path    => ['/sbin/', '/usr/sbin/'],
-      unless  => "xfs_admin -l ${target_device}",
+      unless  => "xfs_admin -l ${device}",
       before  => Anchor['swift::config::end'],
     }
     Package<| title == 'xfsprogs' |>

releasenotes/notes/device-path-validation-331b52cbc9c70ae6.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    The following parameters are now optional and default to ``/dev/${name}``.
+
+    - ``swift::storage::ext4::device``
+    - ``swift::storage::mount::device``
+
+upgrade:
+  - |
+    The ``swift::storage::xfs::device`` parameter no longer accepts an empty
+    string. Use the default value or give an actual device path.

spec/defines/swift_storage_xfs_spec.rb

@@ -19,9 +19,9 @@ describe 'swift::storage::xfs' do
 
       [{},
        {
-         :device       => 'some_device',
+         :device       => '/dev/foo',
          :byte_size    => 1,
-         :mnt_base_dir => '/mnt/foo',
+         :mnt_base_dir => '/mnt/bar',
          :loopback     => true
        }
       ].each do |param_set|