Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Volume creation has race condition with fact gluster_volume_list #23

Closed
tux-o-matic opened this issue Nov 19, 2015 · 4 comments
Closed

Volume creation has race condition with fact gluster_volume_list #23

tux-o-matic opened this issue Nov 19, 2015 · 4 comments

Comments

@tux-o-matic
Copy link
Contributor

My attempt to create a volume with two nodes seems stuck at the volume creation.

Evaluation Error: Error while evaluating a Function Call, function 'split' called with mis-matched arguments

Then points to volume.pp line 105. The split function there depends on the Fact 'gluster_volume_list' except that one is undef at this stage cause it is only populated if the cluster pool has a volume.

Or am I missing something?
@skpy
Copy link
Contributor

skpy commented Nov 19, 2015

Which version of Puppet are you using?

I just tried with open source Puppet 4.3.0 and can confirm that I get the same error. I suspect a change to the split function has occurred somewhere since when we published this module, and now. Maybe PUP-4463?

@tux-o-matic
Copy link
Contributor Author

Puppet 4.2 and stdlib 4.5.1.
I'll be testing a change to handle missing the undef Fact in those versions and then create a PR.

@tux-o-matic tux-o-matic mentioned this issue Nov 20, 2015
Merged
@tux-o-matic
Copy link
Contributor Author

Issue with the 'gluster_volume_list' Fact and the split() function is now by passed in #24.
Side note, regsubst() complains about a tuple as first argument while parsing the options which in my case only contained "nfs.disable: true"

tux-o-matic pushed a commit to tux-o-matic/puppet-gluster that referenced this issue Jan 5, 2016
Added check for undef fact
b460640 
Ensure first run doesn't throw an error because Fact for gluster_peer_list may return as undef.
Related to voxpupuli#23
tux-o-matic pushed a commit to tux-o-matic/puppet-gluster that referenced this issue Jan 5, 2016
Handle different state/member in pool
cb119b6 
Adding check to enable creation of pool and volume from scratch by the module.
voxpupuli#23
tux-o-matic pushed a commit to tux-o-matic/puppet-gluster that referenced this issue Mar 2, 2016
Check for presence of Fact gluster_volume_list
c673907 
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 voxpupuli#24 (comment) 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 voxpupuli#23

Handle different state/member in pool

Adding check to enable creation of pool and volume from scratch by the module.
voxpupuli#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 voxpupuli#31
@tux-o-matic
Copy link
Contributor Author

With #24 merged, this is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@skpy @tux-o-matic