Use structured-facts for array values instead of joining them by ,

README.md

@@ -31,11 +31,11 @@ Also provided with this module are a number of custom Gluster-related facts.
 
 * `gluster_binary`: the full pathname of the Gluster CLI command
 * `gluster_peer_count`: the number of peers to which this server is connected in the pool.
-* `gluster_peer_list`: a comma-separated list of peer hostnames
-* `gluster_volume_list`: a comma-separated list of volumes being served by this server
-* `gluster_volume_#{vol}_bricks`: a comma-separated list of bricks in each volume being served by this server
-* `gluster_volume_#{vol}_options`: a comma-separared list of options enabled on each volume
-* `gluster_volume_#{vol}_ports`: a comma-separated list of ports used by the bricks in the specified volume.
+* `gluster_peer_list`: an array of peer hostnames
+* `gluster_volume_list`: an array of volumes being served by this server
+* `gluster_volume_#{vol}_bricks`: an array of bricks in each volume being served by this server
+* `gluster_volume_#{vol}_options`: an array of options enabled on each volume
+* `gluster_volume_#{vol}_ports`: an array of ports used by the bricks in the specified volume.
 
 The `gluster_binary` fact will look for an [external fact](http://docs.puppetlabs.com/guides/custom_facts.html#external-facts) named `gluster_custom_binary`. If this fact is defined, `gluster_binary` will use that value. Otherwise the path will be searched until the gluster command is found.
 

lib/facter/gluster.rb

@@ -1,5 +1,5 @@
 peer_count = 0
-peer_list = ''
+peer_list = []
 volume_bricks = {}
 volume_options = {}
 volume_ports = {}
@@ -28,7 +28,7 @@
   output = Facter::Util::Resolution.exec("#{binary} peer status")
   peer_count = Regexp.last_match[1].to_i if output =~ %r{^Number of Peers: (\d+)$}
   if peer_count > 0
-    peer_list = output.scan(%r{^Hostname: (.+)$}).flatten.join(',')
+    peer_list = output.scan(%r{^Hostname: (.+)$}).flatten
     # note the stderr redirection here
     # `gluster volume list` spits to stderr :(
     output = Facter::Util::Resolution.exec("#{binary} volume list 2>&1")
@@ -61,28 +61,28 @@
   if peer_count > 0
     Facter.add(:gluster_peer_list) do
       setcode do
-        peer_list
+        peer_list.sort
       end
     end
 
     unless volume_bricks.empty?
       Facter.add(:gluster_volume_list) do
         setcode do
-          volume_bricks.keys.join(',')
+          volume_bricks.keys.sort
         end
       end
       volume_bricks.each do |vol, bricks|
         Facter.add("gluster_volume_#{vol}_bricks".to_sym) do
           setcode do
-            bricks.join(',')
+            bricks.sort
           end
         end
       end
       if volume_options
         volume_options.each do |vol, opts|
           Facter.add("gluster_volume_#{vol}_options".to_sym) do
             setcode do
-              opts.join(',')
+              opts.sort
             end
           end
         end
@@ -91,7 +91,7 @@
         volume_ports.each do |vol, ports|
           Facter.add("gluster_volume_#{vol}_ports".to_sym) do
             setcode do
-              ports.join(',')
+              ports.sort
             end
           end
         end

manifests/peer.pp

@@ -56,7 +56,7 @@
       # and we don't want to attach a server that is already a member
       # of the current pool
       if getvar('::gluster_peer_list') {
-        $peers = split($::gluster_peer_list, ',' )
+        $peers = $::gluster_peer_list
         if ! member($peers, $title) {
           $already_in_pool = false
         } else {

manifests/volume.pp

@@ -94,7 +94,7 @@
       $minimal_requirements = false
     }
 
-    if getvar('::gluster_volume_list') and member( split( $::gluster_volume_list, ',' ), $title ) {
+    if getvar('::gluster_volume_list') and member( $::gluster_volume_list, $title ) {
       $already_exists = true
     } else {
       $already_exists = false
@@ -109,7 +109,7 @@
       # first, get a list of unique server names hosting bricks
       $brick_hosts = unique( regsubst( $bricks, '^([^:]+):(.+)$', '\1') )
       # now get a list of all peers, including ourself
-      $pool_members = concat( split( $::gluster_peer_list, ','), [ $::fqdn ] )
+      $pool_members = concat($::gluster_peer_list, [ $::fqdn ] )
       # now see what the difference is
       $missing_bricks = difference( $brick_hosts, $pool_members)
 
@@ -163,7 +163,7 @@
       # this volume exists
 
       # our fact lists bricks comma-separated, but we need an array
-      $vol_bricks = split( getvar( "::gluster_volume_${title}_bricks" ), ',')
+      $vol_bricks = getvar( "::gluster_volume_${title}_bricks" )
       if $bricks != $vol_bricks {
         # this resource's list of bricks does not match the existing
         # volume's list of bricks
@@ -226,7 +226,7 @@
       # did the options change?
       $current_options = getvar("gluster_volume_${title}_options")
       if $current_options {
-        $_current = sort( split($current_options, ',') )
+        $_current = sort( $current_options )
       } else {
         $_current = []
       }

spec/classes/init_spec.rb

@@ -126,8 +126,8 @@
         let :facts do
           super().merge(
             gluster_binary: '/sbin/gluster',
-            gluster_peer_list: 'example1,example2',
-            gluster_volume_list: 'gl1.example.com:/glusterfs/backup,gl2.example.com:/glusterfs/backup'
+            gluster_peer_list: %w[example1 example2],
+            gluster_volume_list: %w[gl1.example.com:/glusterfs/backup gl2.example.com:/glusterfs/backup]
           )
         end
         let :params do

spec/defines/peer_spec.rb

@@ -24,7 +24,7 @@
         {
           gluster_binary: '/usr/sbin/gluster',
           gluster_peer_count: 1,
-          gluster_peer_list: 'peer1.example.com'
+          gluster_peer_list: ['peer1.example.com']
         }
       end
 
@@ -36,7 +36,7 @@
         {
           gluster_binary: '/usr/sbin/gluster',
           gluster_peer_count: 2,
-          gluster_peer_list: 'peer1.example.com,peer2.example.com'
+          gluster_peer_list: %w[peer1.example.com peer2.example.com]
         }
       end
 
@@ -51,7 +51,7 @@
         {
           gluster_binary: '/usr/sbin/gluster',
           gluster_peer_count: 0,
-          gluster_peer_list: ''
+          gluster_peer_list: []
         }
       end
 
@@ -63,7 +63,7 @@
         {
           gluster_binary: '/usr/sbin/gluster',
           gluster_peer_count: 1,
-          gluster_peer_list: 'peer2.example.com'
+          gluster_peer_list: ['peer2.example.com']
         }
       end
 
@@ -75,7 +75,7 @@
         {
           gluster_binary: '/usr/sbin/gluster',
           gluster_peer_count: 2,
-          gluster_peer_list: 'peer2.example.com,peer3.example.com'
+          gluster_peer_list: %w[peer2.example.com peer3.example.com]
         }
       end
 

spec/defines/volume_spec.rb

@@ -37,7 +37,7 @@
       let(:facts) do
         {
           gluster_binary: '/usr/sbin/gluster',
-          gluster_peer_list: 'peer1.example.com,peer2.example.com'
+          gluster_peer_list: %w[peer1.example.com peer2.example.com]
         }
       end
 
@@ -48,8 +48,8 @@
       let(:facts) do
         {
           gluster_binary: '/usr/sbin/gluster',
-          gluster_peer_list: 'peer1.example.com,peer2.example.com',
-          gluster_volume_list: 'gl1.example.com:/glusterfs/backup,gl2.example.com:/glusterfs/backup'
+          gluster_peer_list: %w[peer1.example.com peer2.example.com],
+          gluster_volume_list: %w[gl1.example.com:/glusterfs/backup gl2.example.com:/glusterfs/backup]
         }
       end
 
@@ -60,8 +60,8 @@
     let(:facts) do
       {
         gluster_binary: '/usr/sbin/gluster',
-        gluster_peer_list: 'srv1.local,srv2.local',
-        gluster_volume_list: 'srv1.local:/glusterfs/backup,srv2.local:/glusterfs/backup'
+        gluster_peer_list: %w[srv1.local srv2.local],
+        gluster_volume_list: %w[srv1.local:/glusterfs/backup srv2.local:/glusterfs/backup]
       }
     end