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
(PUP-7807) Add mount options on AIX remount #6283
(PUP-7807) Add mount options on AIX remount #6283
Conversation
Prior to this commit, the mount resource on AIX would not remount NFS shares with the mount options specified. Since AIX did not read the NFS options from the `/etc/filesystems`, the share would be remounted without options. This commit adds a simple check to ensure all AIX remounts are done with the specified options including JFS2.
…retlavallee/puppet into PUP-7807/master/aix_remount_options
Building on prior work, this commit DRYs up the code in mount a bit, sharing behavior between bsd and aix, and breaking out a couple utility methods. A test is added to assert that on AIX, if remounts is true and options are specified, we pass the specified options with the appropriate 'remount' command. Signed-off-by: Moses Mendoza <moses@puppet.com>
@adrienthebo should be good, but can you validate my little refactor/added spec? |
CLA signed by all contributors. |
lib/puppet/provider/mount.rb
Outdated
else | ||
unmount | ||
mount | ||
end | ||
end | ||
|
||
def remount_with_option(option) |
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 method would benefit from documentation indicating that it'll both remount with a specific option, as well as the contents of the options
parameter if the field is set.
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.
agreed. update pushed
end | ||
|
||
def using_explicit_options? | ||
!self.options.nil? && !self.options.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.
I'll give you extra points if you use DeMorgan's law to convert this to !(self.options.nil? || self.options.empty?)
- that is if you think this is more clear.
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.
While that would be correct, I was trying to mimic the thought flow - ie, we're really saying "as long as this thing isn't nil, and as long as it's not empty", so this made the most sense to me.
Clarify what's happening in the #remount_with_option command. Signed-off-by: Moses Mendoza <moses@puppet.com>
thanks again @jarretlavallee |
Contains the content of PR #6105, but with added test and a bit of refactor/DRYup. See that PR for context.