From c6739078a07a2c39e6453ff1a0aaad93914eed2c Mon Sep 17 00:00:00 2001 From: Benjamin Merot Date: Thu, 19 Nov 2015 19:00:25 +0100 Subject: [PATCH 1/4] Check for presence of Fact gluster_volume_list On a node with no pre-existing Gluster volume, this module will not populate the Fact 'gluster_volume_list' there for we should check if the current volume already exists ONLY if the nodes already has some volumes. Work on conditional statement readability Puppet DSL doesn't allow variable re-assignment Of course! So reformating conditional statement to make everything checked within a single 'if'. Adding checks for setup from scratch As discussed in https://github.com/voxpupuli/puppet-gluster/pull/24#issuecomment-158412322 extra check added to handle creation of volumes from scratch on a Gluster pool. Added check for undef fact Ensure first run doesn't throw an error because Fact for gluster_peer_list may return as undef. Related to https://github.com/voxpupuli/puppet-gluster/issues/23 Handle different state/member in pool Adding check to enable creation of pool and volume from scratch by the module. https://github.com/voxpupuli/puppet-gluster/issues/23 Removing blocking check for dry run Fact $::gluster_volume_list presence should not be checked on L 105 as it may be undef on very first run in a pool without pre-existing volumes Correcting typo in var ref and pattern in regsubst Prevent error being thrown by pattern in regsubst() function. As explained and corrected in https://github.com/voxpupuli/puppet-gluster/pull/31 --- manifests/peer.pp | 19 +++++++++++++++---- manifests/volume.pp | 24 ++++++++++++++++++------ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/manifests/peer.pp b/manifests/peer.pp index cd490af3..35465a6d 100644 --- a/manifests/peer.pp +++ b/manifests/peer.pp @@ -56,12 +56,23 @@ # and we don't want to attach a server that is already a member # of the current pool - $peers = split($::gluster_peer_list, ',' ) - if ! member($peers, $title) { - exec { "gluster peer probe ${title}": - command => "${binary} peer probe ${title}", + if $::gluster_peer_list != undef { + $peers = split($::gluster_peer_list, ',' ) + if ! member($peers, $title) { + $already_in_pool = false + } else { + $already_in_pool = true } + } else { + $already_in_pool = false + } + + if !$already_in_pool { + exec { "gluster peer probe ${title}": + command => "${binary} peer probe ${title}", + } } + } } } diff --git a/manifests/volume.pp b/manifests/volume.pp index 6a709748..a0920116 100644 --- a/manifests/volume.pp +++ b/manifests/volume.pp @@ -101,8 +101,20 @@ $binary = $::gluster_binary if $binary{ # we need the Gluster binary to do anything! + + if $::gluster_peer_list != undef{ + $minimal_requirements = true + } else { + $minimal_requirements = false + } + + if $::gluster_volume_list != undef and member( split( $::gluster_volume_list, ',' ), $title ) { + $already_exists = true + } else { + $already_exists = false + } - if ! member( split( $::gluster_volume_list, ',' ), $title ) { + if $minimal_requirements and $already_exists == false { # this volume has not yet been created # before we can create it, we need to ensure that all the @@ -139,7 +151,7 @@ # volume:option $vol_opts = prefix( $_options, "${title}:" ) # now we make some YAML, and then parse that to get a Puppet hash - $yaml = join( regsubst( $vol_opts, ': ', ":\n value: ", G), "\n") + $yaml = join( regsubst( $vol_opts, ': ', ":\n value: ", 'G'), "\n") $hoh = parseyaml($yaml) # safety check @@ -161,7 +173,7 @@ } } - } else { + } elsif $already_exists { # this volume exists # our fact lists bricks comma-separated, but we need an array @@ -179,7 +191,7 @@ # number of bricks to add is a factor of that value if $stripe { if ( count($new_bricks) % $stripe ) != 0 { - fail("Number of bricks to add is not a multiple of stripe count ${stipe}") + fail("Number of bricks to add is not a multiple of stripe count ${stripe}") } $s = "stripe ${stripe}" } else { @@ -256,7 +268,7 @@ # so build up the hash correctly # $remove_opts = prefix( $to_remove, "${title}:" ) - $remove_yaml = join( regsubst( $remove_opts, ': .+$', ":\n ensure: absent", G ), "\n" ) + $remove_yaml = join( regsubst( $remove_opts, ': .+$', ":\n ensure: absent", 'G' ), "\n" ) $remove = parseyaml($remove_yaml) if $remove_options { create_resources( ::gluster::volume::option, $remove ) @@ -268,7 +280,7 @@ if ! empty($to_add) { # we have some options defined that are not active. Add them $add_opts = prefix( $to_add, "${title}:" ) - $add_yaml = join( regsubst( $add_opts, ': ', ":\n value: ", G ), "\n" ) + $add_yaml = join( regsubst( $add_opts, ': ', ":\n value: ", 'G' ), "\n" ) $add = parseyaml($add_yaml) create_resources( ::gluster::volume::option, $add ) } From 01269431ca37a62e5e932e99e89cb5361e777e0c Mon Sep 17 00:00:00 2001 From: Benjamin Merot Date: Wed, 2 Mar 2016 10:51:41 +0100 Subject: [PATCH 2/4] Removing trailing whitespace --- manifests/peer.pp | 4 ++-- manifests/volume.pp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/manifests/peer.pp b/manifests/peer.pp index 35465a6d..15fdd80b 100644 --- a/manifests/peer.pp +++ b/manifests/peer.pp @@ -66,13 +66,13 @@ } else { $already_in_pool = false } - + if !$already_in_pool { exec { "gluster peer probe ${title}": command => "${binary} peer probe ${title}", } } - + } } } diff --git a/manifests/volume.pp b/manifests/volume.pp index a0920116..9c01dc24 100644 --- a/manifests/volume.pp +++ b/manifests/volume.pp @@ -101,13 +101,13 @@ $binary = $::gluster_binary if $binary{ # we need the Gluster binary to do anything! - + if $::gluster_peer_list != undef{ $minimal_requirements = true } else { $minimal_requirements = false } - + if $::gluster_volume_list != undef and member( split( $::gluster_volume_list, ',' ), $title ) { $already_exists = true } else { From affc60272e9d81bbb2634d404ac36daf783693df Mon Sep 17 00:00:00 2001 From: Benjamin Merot Date: Wed, 2 Mar 2016 10:54:42 +0100 Subject: [PATCH 3/4] Removing trailing whitespace --- manifests/peer.pp | 4 ++-- manifests/volume.pp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/manifests/peer.pp b/manifests/peer.pp index 15fdd80b..62f3bb06 100644 --- a/manifests/peer.pp +++ b/manifests/peer.pp @@ -66,13 +66,13 @@ } else { $already_in_pool = false } - + if !$already_in_pool { exec { "gluster peer probe ${title}": command => "${binary} peer probe ${title}", } } - + } } } diff --git a/manifests/volume.pp b/manifests/volume.pp index 9c01dc24..759277f6 100644 --- a/manifests/volume.pp +++ b/manifests/volume.pp @@ -101,13 +101,13 @@ $binary = $::gluster_binary if $binary{ # we need the Gluster binary to do anything! - + if $::gluster_peer_list != undef{ $minimal_requirements = true } else { $minimal_requirements = false } - + if $::gluster_volume_list != undef and member( split( $::gluster_volume_list, ',' ), $title ) { $already_exists = true } else { From 1f2e26fa5fa7bc12681c62c7709d01e27e6d0b0f Mon Sep 17 00:00:00 2001 From: Benjamin Merot Date: Wed, 2 Mar 2016 10:51:41 +0100 Subject: [PATCH 4/4] Removing trailing whitespace --- manifests/peer.pp | 4 ++-- manifests/volume.pp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/manifests/peer.pp b/manifests/peer.pp index 35465a6d..62f3bb06 100644 --- a/manifests/peer.pp +++ b/manifests/peer.pp @@ -66,13 +66,13 @@ } else { $already_in_pool = false } - + if !$already_in_pool { exec { "gluster peer probe ${title}": command => "${binary} peer probe ${title}", } } - + } } } diff --git a/manifests/volume.pp b/manifests/volume.pp index a0920116..759277f6 100644 --- a/manifests/volume.pp +++ b/manifests/volume.pp @@ -101,13 +101,13 @@ $binary = $::gluster_binary if $binary{ # we need the Gluster binary to do anything! - + if $::gluster_peer_list != undef{ $minimal_requirements = true } else { $minimal_requirements = false } - + if $::gluster_volume_list != undef and member( split( $::gluster_volume_list, ',' ), $title ) { $already_exists = true } else {