Setting volume properties should be race-free.

This isn't essential, as ensuring this is race-free is really up to glusterfs, but with this patch you reduce the likelihood to ~0% that you'll see a: "volume set: failed: Another transaction is in progress." error. The error isn't harmful, but now we'll see less unnecessary red.
purpleidea · Mar 17, 2014 · 05576cd · 05576cd
1 parent fbe9e9c
commit 05576cd
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 14 deletions.
diff --git a/manifests/simple.pp b/manifests/simple.pp
@@ -176,7 +176,8 @@
 	if "${setgroup}" != '' {
 		$setgroup_yaml = inline_template("<%= @valid_volumes.inject(Hash.new) { |h,i| {i+'#'+@setgroup => {}}.merge(h) }.to_yaml %>")
 		$setgroup_hash = parseyaml($setgroup_yaml)
-		create_resources('gluster::volume::property::group', $setgroup_hash)
+		$setgroup_defaults = {'vip' => "${vip}"}
+		create_resources('gluster::volume::property::group', $setgroup_hash, $setgroup_defaults)
 	}
 }
 

diff --git a/manifests/volume/property.pp b/manifests/volume/property.pp
@@ -19,6 +19,7 @@
 
 define gluster::volume::property(
 	$value,
+	$vip = '',		# vip of the cluster (optional but recommended)
 	$autotype = true		# set to false to disable autotyping
 ) {
 	include gluster::xml
@@ -152,18 +153,29 @@
 		}
 	}
 
-	# volume set <VOLNAME> <KEY> <VALUE>
-	# set a volume property only if value doesn't match what is available
-	# FIXME: check that the value we're setting isn't the default
-	# FIXME: you can check defaults with... gluster volume set help | ...
-	exec { "/usr/sbin/gluster volume set ${volume} ${key} ${safe_value}":
-		unless => "/usr/bin/test \"`/usr/sbin/gluster volume --xml info ${volume} | ${vardir}/xml.py property --key '${key}'`\" = '${safe_value}'",
-		onlyif => "/usr/sbin/gluster volume list | /bin/grep -qxF '${volume}' -",
-		logoutput => on_failure,
-		require => [
-			Gluster::Volume[$volume],
-			File["${vardir}/xml.py"],
-		],
+	$valid_vip = "${vip}" ? {
+		'' => $::gluster::server::vip,
+		default => "${vip}",
+	}
+
+	# returns interface name that has vip, or '' if none are found.
+	$vipif = inline_template("<%= @interfaces.split(',').find_all {|x| '${valid_vip}' == scope.lookupvar('ipaddress_'+x) }[0,1].join('') %>")
+
+	# run if vip not defined (bypass mode) or if vip exists on this machine
+	if ("${valid_vip}" == '' or "${vipif}" != '') {
+		# volume set <VOLNAME> <KEY> <VALUE>
+		# set a volume property only if value doesn't match what is available
+		# FIXME: check that the value we're setting isn't the default
+		# FIXME: you can check defaults with... gluster volume set help | ...
+		exec { "/usr/sbin/gluster volume set ${volume} ${key} ${safe_value}":
+			unless => "/usr/bin/test \"`/usr/sbin/gluster volume --xml info ${volume} | ${vardir}/xml.py property --key '${key}'`\" = '${safe_value}'",
+			onlyif => "/usr/sbin/gluster volume list | /bin/grep -qxF '${volume}' -",
+			logoutput => on_failure,
+			require => [
+				Gluster::Volume[$volume],
+				File["${vardir}/xml.py"],
+			],
+		}
 	}
 }
 

diff --git a/manifests/volume/property/group.pp b/manifests/volume/property/group.pp
@@ -23,6 +23,7 @@
 # NOTE: this does the equivalent of: gluster volume set <VOLNAME> group <GROUP>
 
 define gluster::volume::property::group(
+	$vip = '',		# vip of the cluster (optional but recommended)
 ) {
 	include gluster::xml
 	include gluster::vardir
@@ -62,8 +63,10 @@
 		$group_data_yaml = inline_template("<%= @group_data_list.inject(Hash.new) { |h,i| { '${volume}#'+((i.split('=').length == 2) ? i.split('=')[0] : '') => {'value' => ((i.split('=').length == 2) ? i.split('=')[1] : '')} }.merge(h) }.to_yaml %>")
 		# build into a hash
 		$group_data_hash = parseyaml($group_data_yaml)
+		# pass through the vip
+		$group_data_defaults = {'vip' => "${vip}"}
 		# create the properties
-		create_resources('gluster::volume::property', $group_data_hash)
+		create_resources('gluster::volume::property', $group_data_hash, $group_data_defaults)
 	}
 }