From bc7fa7d6a114a37f452d754e726c4087d8478af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20M=C3=BCller?= Date: Thu, 4 May 2017 13:29:41 +0200 Subject: [PATCH] Use structured-facts for array values instead of joining them by , --- README.md | 10 +++++----- lib/facter/gluster.rb | 12 ++++++------ manifests/peer.pp | 2 +- manifests/volume.pp | 8 ++++---- spec/classes/init_spec.rb | 4 ++-- spec/defines/peer_spec.rb | 10 +++++----- spec/defines/volume_spec.rb | 10 +++++----- 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 5cd36371..de24b5c8 100644 --- a/README.md +++ b/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. diff --git a/lib/facter/gluster.rb b/lib/facter/gluster.rb index e3d34e4b..73498ac1 100644 --- a/lib/facter/gluster.rb +++ b/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") @@ -68,13 +68,13 @@ unless volume_bricks.empty? Facter.add(:gluster_volume_list) do setcode do - volume_bricks.keys.join(',') + volume_bricks.keys end end volume_bricks.each do |vol, bricks| Facter.add("gluster_volume_#{vol}_bricks".to_sym) do setcode do - bricks.join(',') + bricks end end end @@ -82,7 +82,7 @@ volume_options.each do |vol, opts| Facter.add("gluster_volume_#{vol}_options".to_sym) do setcode do - opts.join(',') + opts 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 end end end diff --git a/manifests/peer.pp b/manifests/peer.pp index 55b85f7b..106202c3 100644 --- a/manifests/peer.pp +++ b/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 { diff --git a/manifests/volume.pp b/manifests/volume.pp index b5e81954..3fd835cb 100644 --- a/manifests/volume.pp +++ b/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 = [] } diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb index c8864b64..19aba098 100644 --- a/spec/classes/init_spec.rb +++ b/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 diff --git a/spec/defines/peer_spec.rb b/spec/defines/peer_spec.rb index 0edef1c8..704b3d19 100644 --- a/spec/defines/peer_spec.rb +++ b/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: %w{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: %w{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 diff --git a/spec/defines/volume_spec.rb b/spec/defines/volume_spec.rb index 3443e5c7..d1a0ffd0 100644 --- a/spec/defines/volume_spec.rb +++ b/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