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.

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

Add force binary #227

Closed
wants to merge 16 commits into from
Closed

Conversation

Kotty666
Copy link

needed if we've to deploy the whole gluster server in one Run.

Pull Request (PR) description

We needed the posibility to deploy the glusterfs-server at the first run, this won't work actually. So I've added the variable to force the binary path and disables the getvar() for the first run. With this we are able to add the Peers and Volume at the first run.

This Pull Request (PR) fixes the following issues

n/a

Philipp Frik added 3 commits April 30, 2021 09:31
this options helps to deploy the glusterfs-server with volume creation
on its first puppet run.
@Kotty666
Copy link
Author

Kotty666 commented May 3, 2021

@bastelfreak are you able to Aprove the Workflow?

manifests/peer.pp Outdated Show resolved Hide resolved
manifests/volume.pp Outdated Show resolved Hide resolved
@Kotty666
Copy link
Author

Kotty666 commented May 4, 2021

@bastelfreak i've reviewed the spec test errors and fixed them. Also i removed CentOS6 from the metadata.json becaus the spec test will always fail, CentOS 6 is EOL since 4 Years and the Maintancen Updates also have been remove at Nov 2020.

) {
# we can't do much without the Gluster binary
# but we don't necessarily want the Puppet run to fail if the
# gluster_binary fact is absent!
if getvar('::gluster_binary') {
if($force_binary) {
$real_binary = getvar('::gluster_binary') ? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose of this workaround? The fact we have in https://github.com/voxpupuli/puppet-gluster/blob/master/lib/facter/gluster.rb checks if the binary gluster exists in $PATH.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This getvar() stuff do not work at the first puppet run. So if the Server should be deployed with only 1 puppet agent run the glusterfs volume and peers won't be createt. With this workaround the binary variable will be set to the correct value so at the first run the peers an volumes could be created.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this approach might works, it's really tricky to read and I think we don't use a manual lookup() + hiera in modules just to get a single value. Instead of all this code we could add another parameter like this:

Stdlib::Absolutepath $gluster_binary = pick($facts['gluster_binary'], '/usr/sbin/gluster')

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try the Stdlib version in my Test ENV.
I'll added the hiera stuff as a beginning to start removing the cases stuff from the pramas classes. But this i would add in extra branches/PRs

@Kotty666 Kotty666 closed this Jun 23, 2021
@Kotty666 Kotty666 deleted the add_force_binary branch June 23, 2021 11:09
@Kotty666
Copy link
Author

@bastelfreak i've closed here and added a new PR (#228) in that there is a full hiera tree without params.pp and also added RHEL8 support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants