Use structured-facts for array values instead of joining them by , #120
Conversation
coder-hugo
force-pushed
the
feature/structured-facts
branch
from
bc7fa7d
to
26cac62
Compare
coder-hugo
force-pushed
the
feature/structured-facts
branch
from
26cac62
to
f60cea4
Compare
manifests/volume.pp
Outdated
| @@ -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 { | |||
There was a problem hiding this comment.
@coder-hugo Do you think the arrays can be sorted before they're compared (and operated on)?
This would fix #121 ?
There was a problem hiding this comment.
I'd say it would be even better to sort the arrays already when generating the facts. I'll just try this now.
There was a problem hiding this comment.
I added a patch that sorts all array facts and also the $bricks array given to the gluster::volume.
|
Looks good to me, but I'm not in a position to actually test it. I'll have an ask around for volunteers. Have you been using this branch yourself? |
|
At the moment we are using the fix-port-facts branch for our setup. This is a combination of this PR and #118. And it works pretty well. |
lib/facter/gluster.rb
Outdated
| end | ||
| end | ||
| volume_bricks.each do |vol, bricks| | ||
| Facter.add("gluster_volume_#{vol}_bricks".to_sym) do | ||
| setcode do | ||
| bricks | ||
| bricks.sort |
There was a problem hiding this comment.
Having read a bit more, I'm not actually sure this is the correct thing to do. Brick order is important?
See #121 (comment)
|
Should this create a complex |
|
I agree with @ekohl. @coder-hugo can you please take a look at the suggestions? |
|
Dear @coder-hugo, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Dear @coder-hugo, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
This PR converts all custom facts that contain array values from string to structured-facts.
Btw. this PR will conflict with #118 as both are touching line 31 of
lib/facts/gluster.rb. This PR just touches this line as thepeer_listfact is already joined within the "calculation" code and not like for the others in theFacter.addsection. If #118 will be merged first I'll rebase the PR and fix the merge conflict.