-
Notifications
You must be signed in to change notification settings - Fork 47
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
Allow use gluster recipe on already existed filesystem #77
Conversation
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.
Can you also add a changelog entry?
@@ -4,7 +4,7 @@ | |||
license 'Apache 2.0' | |||
description 'Installs and configures Gluster servers and clients' | |||
long_description IO.read(File.join(File.dirname(__FILE__), 'README.md')) | |||
version '5.1.0' | |||
version '5.1.1' |
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.
Revert
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.
why? I think that's a "feature" fix, so it's easier to bump version to point which version works :)
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.
Version bumps happen when a release it cut
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.
ok
size volume_values['size'] | ||
filesystem filesystem | ||
mount_point "#{node['gluster']['server']['brick_mount_path']}/#{volume_name}" | ||
if node['gluster']['server']['disks'].any? |
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.
Can you add an else
to this with a warning log saying that no LVM setup will be done?
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.
ok
end | ||
end | ||
|
||
directory "#{node['gluster']['server']['brick_mount_path']}/#{volume_name}" do |
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.
Have you confirmed this will not have any affect on LVM based mounts?
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.
i'ts just creating directory. If directory already exists - nothing will change. But - i can put it into conditional block.
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.
Cool, just confirming
@@ -92,6 +103,7 @@ | |||
# Create option string | |||
force = false | |||
options = '' | |||
force = true if node['gluster']['server']['disks'].empty? |
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.
Why is force need here? What error does gluster throw?
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.
force is needed only when gluster is creating on root partition/volume (for example on kitchen ;). But i dont know how to easy check if there is that case. I think about conditional "force" option for volume configuration.
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.
df /path/to/dir | tail -1
then check the last part of the array
There is probably other solutions too
Example outside the context of chef
[3] pry(main)> `df /etc | tail -1`.chomp.split[-1]
=> "/"
[4] pry(main)>
Is that better? :-) |
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.
Much better! Nice work
Can you squash the commits then i will merge?
squashed :-) |
No description provided.