From f98ee4eff0ba23f1532e85a1f4aada7e3056b47b Mon Sep 17 00:00:00 2001
From: Rune Juhl Jacobsen <runejuhl@petardo.dk>
Date: Fri, 24 May 2019 11:44:09 +0200
Subject: [PATCH] Fix unnecessarily setting gluster volume option repeatedly

Checks Gluster volume options against existing values to avoid repeatedly
setting values

Also adds these types:
+ `Gluster::VolumeName`
+ `Gluster::VolumeOption`

And these functions:
+ `gluster::cmd_volume_get_option`
+ `gluster::onoff`
---
 functions/cmd_volume_get_option.pp | 34 ++++++++++++++++++++++++++++++
 functions/onoff.pp                 |  9 ++++++++
 manifests/volume/option.pp         | 29 +++++++++++++++----------
 types/volumename.pp                |  1 +
 types/volumeoption.pp              |  1 +
 5 files changed, 63 insertions(+), 11 deletions(-)
 create mode 100644 functions/cmd_volume_get_option.pp
 create mode 100644 functions/onoff.pp
 create mode 100644 types/volumename.pp
 create mode 100644 types/volumeoption.pp

diff --git a/functions/cmd_volume_get_option.pp b/functions/cmd_volume_get_option.pp
new file mode 100644
index 00000000..f87a3ded
--- /dev/null
+++ b/functions/cmd_volume_get_option.pp
@@ -0,0 +1,34 @@
+# Create a command string to get option `$opt` from gluster volume `$vol`, and
+# optionally compare it against `$comparison`.
+#
+# @param vol [Gluster::VolumeName] Gluster volume name
+# @param opt [Gluster::OptionName] Gluster volume option name
+# @param comparison [Optional[String]] Optional string to compare the existing
+#   value against
+# @return [String]
+#
+# @example Usage
+#
+#   ```puppet
+#   gluster::cmd_volume_get_option('data', 'nfs.disable', String(true))
+#   ```
+#
+function gluster::cmd_volume_get_option(
+  Gluster::VolumeName   $vol,
+  Gluster::VolumeOption $opt,
+  Optional[Any]         $comparison = undef,
+) {
+  $_cmd = "${::gluster_binary} volume get ${vol} ${opt}"
+
+  unless $comparison {
+    return $_cmd
+  }
+
+  $_comparison = $comparison ? {
+    Undef   => '\(null\)',
+    Boolean => gluster::onoff($comparison),
+    default => $comparison,
+  }
+
+  "${_cmd} | tail -n1 | grep -E '^${opt} +${_comparison} *\$'"
+}
diff --git a/functions/onoff.pp b/functions/onoff.pp
new file mode 100644
index 00000000..4a5e61fd
--- /dev/null
+++ b/functions/onoff.pp
@@ -0,0 +1,9 @@
+function gluster::onoff (
+  Boolean $value,
+) {
+  if $value {
+    'on'
+  } else {
+    'off'
+  }
+}
diff --git a/manifests/volume/option.pp b/manifests/volume/option.pp
index a0538537..8100013e 100644
--- a/manifests/volume/option.pp
+++ b/manifests/volume/option.pp
@@ -33,26 +33,33 @@
 # Copyright 2014 CoverMyMeds, unless otherwise noted
 #
 define gluster::volume::option (
-  $value  = undef,
-  $ensure = true,
+  Optional                         $value  = undef,
+  Variant[Boolean, Enum['absent']] $ensure = true,
 ) {
 
-  $arr = split( $title, ':' )
-  $count = count($arr)
+  $arr = $title.split(':')
   # do we have more than one array element?
-  if $count != 2 {
+  if count($arr) != 2 {
     fail("${title} does not parse as volume:option")
   }
-  $vol = $arr[0]
-  $opt = $arr[1]
+  [$vol, $opt] = $arr
 
-  if $ensure == 'absent' {
-    $cmd = "reset ${vol} ${opt}"
+  $_value = $value ? {
+    Boolean => gluster::onoff($value),
+    default => String($value),
+  }
+
+  $cmd = if $ensure == 'absent' {
+    "reset ${vol} ${opt}"
   } else {
-    $cmd = "set ${vol} ${opt} ${value}"
+    "set ${vol} ${opt} ${_value}"
   }
 
-  exec { "gluster option ${vol} ${opt} ${value}":
+  exec { "gluster option ${vol} ${opt} ${_value}":
+    path    => '/usr/bin:/usr/sbin:/bin',
     command => "${::gluster_binary} volume ${cmd}",
+    unless  => unless $ensure == 'absent' {
+      gluster::cmd_volume_get_option($vol, $opt, $_value)
+    },
   }
 }
diff --git a/types/volumename.pp b/types/volumename.pp
new file mode 100644
index 00000000..a2fb03fd
--- /dev/null
+++ b/types/volumename.pp
@@ -0,0 +1 @@
+type Gluster::VolumeName = Pattern[/^[a-zA-Z0-9_-]+$/]
diff --git a/types/volumeoption.pp b/types/volumeoption.pp
new file mode 100644
index 00000000..d4034ecf
--- /dev/null
+++ b/types/volumeoption.pp
@@ -0,0 +1 @@
+type Gluster::VolumeOption = Pattern[/^[a-z0-9]+\.[a-z0-9-]+$/]