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

(PUP-7807) Add mount options on AIX remount #6283

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 25 additions & 8 deletions lib/puppet/provider/mount.rb
Expand Up @@ -26,21 +26,38 @@ def mount
def remount
#TRANSLATORS refers to remounting a file system
info _("Remounting")
if resource[:remounts] == :true
os = Facter.value(:operatingsystem)
supports_remounts = (resource[:remounts] == :true)
if supports_remounts && os == 'AIX'
remount_with_option("remount")
elsif os.match(/^(FreeBSD|DragonFly|OpenBSD)$/)
remount_with_option("update")
elsif supports_remounts
mountcmd "-o", "remount", resource[:name]
elsif ["FreeBSD", "DragonFly", "OpenBSD"].include?(Facter.value(:operatingsystem))
if self.options && !self.options.empty?
options = self.options + ",update"
else
options = "update"
end
mountcmd "-o", options, resource[:name]
else
unmount
mount
end
end

# Remount by appending the supplied param "option" to any existing explicitly
# defined options. If resource has no explicitly defined options, will mount
# with only "option".
# @param [String] option A remount option to use or append with existing options
#
def remount_with_option(option)
if using_explicit_options?
options = self.options + "," + option
else
options = option
end
mountcmd "-o", options, resource[:name]
end

def using_explicit_options?
!self.options.nil? && !self.options.empty?
Copy link
Contributor

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.

Copy link
Contributor Author

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.

end

# This only works when the mount point is synced to the fstab.
def unmount
umount(resource[:name])
Expand Down
23 changes: 18 additions & 5 deletions spec/unit/provider/mount_spec.rb
Expand Up @@ -72,11 +72,24 @@

describe Puppet::Provider::Mount, " when remounting" do

it "should use '-o remount' if the resource specifies it supports remounting" do
@mounter.stubs(:info)
@resource.stubs(:[]).with(:remounts).returns(:true)
@mounter.expects(:mountcmd).with("-o", "remount", @name)
@mounter.remount
context "if the resource supports remounting" do
context "given explicit options on AIX" do
it "should combine the options with 'remount'" do
@mounter.stubs(:info)
@mounter.stubs(:options).returns('ro')
@resource.stubs(:[]).with(:remounts).returns(:true)
Facter.expects(:value).with(:operatingsystem).returns 'AIX'
@mounter.expects(:mountcmd).with("-o", "ro,remount", @name)
@mounter.remount
end
end

it "should use '-o remount'" do
@mounter.stubs(:info)
@resource.stubs(:[]).with(:remounts).returns(:true)
@mounter.expects(:mountcmd).with("-o", "remount", @name)
@mounter.remount
end
end

it "should mount with '-o update' on OpenBSD" do
Expand Down