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
Fix backup-volfile-server(s) in mount provider #72
Conversation
when String | ||
',backupvolfile-server=' + new_resource.backup_server | ||
when Array | ||
',backupvolfile-server=' + new_resource.backup_server.join(',backupvolfile-server=') | ||
',backup-volfile-servers=' + new_resource.backup_server.join(':') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
backup-volfile-servers
is not correct per docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be a platform specific addition? man mount.glusterfs (glusterfs-fuse-3.7.15-1.el7.x86_64) has the following parameter:
backup-volfile-servers=SERVERLIST
Provide list of backup volfile servers in the following format [default: None]
I take it is meant for where there is multiple backup servers. Should I remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm interesting, i find no mention of backup-volfile-servers
on https://gluster.readthedocs.io/en/latest/Administrator%20Guide/Setting%20Up%20Clients/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does look like you are correct though
https://github.com/gluster/glusterfs/blob/2e23c62cc50037c8e61bcd9c04348409e7627181/xlators/mount/fuse/utils/mount_glusterfs.in#L266
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -12,6 +12,7 @@ | |||
- **[PR #67](https://github.com/shortdudey123/chef-gluster/pull/67)** - Fix serverspec verification | |||
- **[PR #68](https://github.com/shortdudey123/chef-gluster/pull/68)** - Add kitchen testing for gluster::client recipe | |||
- **[PR #69](https://github.com/shortdudey123/chef-gluster/pull/69)** - Update yum_repository resource | |||
- **[PR #70](https://github.com/shortdudey123/chef-gluster/pull/70)** - Fix backup-volfile-server(s) in mount provider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is PR 72, not 70
@@ -12,6 +12,7 @@ | |||
- **[PR #67](https://github.com/shortdudey123/chef-gluster/pull/67)** - Fix serverspec verification | |||
- **[PR #68](https://github.com/shortdudey123/chef-gluster/pull/68)** - Add kitchen testing for gluster::client recipe | |||
- **[PR #69](https://github.com/shortdudey123/chef-gluster/pull/69)** - Update yum_repository resource | |||
- **[PR #72](https://github.com/shortdudey123/chef-gluster/pull/70)** - Fix backup-volfile-server(s) in mount provider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link and PR number don't match :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link updated as well.
Sorry, one last thing... |
Can you fix the merge conflict?
|
File updated |
can you squash the commits so the sync with master is not in the commit history? thanks! |
c162a3e
to
3d302ec
Compare
Fix case statement and use backupvolfile_servers parameter for multiple servers.