summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThom May <thom@chef.io>2018-03-09 18:28:47 +0000
committerThom May <thom@chef.io>2018-03-19 14:00:31 +0000
commit7bb7391688c973388db85efb1156e995d654cfb2 (patch)
treeca93739346ee3fa64fcda9e5ba0d546d3f2f8525
parent15b9a7ad9b9ddf53139479112cb16960b0d61392 (diff)
downloadchef-7bb7391688c973388db85efb1156e995d654cfb2.tar.gz
update mount to use properties and fix 6851
Signed-off-by: Thom May <thom@chef.io>
-rw-r--r--lib/chef/provider/mount.rb10
-rw-r--r--lib/chef/provider/mount/mount.rb15
-rw-r--r--lib/chef/resource/mount.rb141
-rw-r--r--spec/unit/provider/mount/mount_spec.rb16
-rw-r--r--spec/unit/resource/mount_spec.rb4
5 files changed, 47 insertions, 139 deletions
diff --git a/lib/chef/provider/mount.rb b/lib/chef/provider/mount.rb
index 4d0d273ea9..533dd2514b 100644
--- a/lib/chef/provider/mount.rb
+++ b/lib/chef/provider/mount.rb
@@ -83,7 +83,7 @@ class Chef
end
def action_enable
- unless current_resource.enabled && mount_options_unchanged?
+ unless current_resource.enabled && mount_options_unchanged? && device_unchanged?
converge_by("enable #{current_resource.device}") do
enable_fs
Chef::Log.info("#{new_resource} enabled")
@@ -120,6 +120,14 @@ class Chef
raise Chef::Exceptions::UnsupportedAction, "#{self} does not implement #mount_options_unchanged?"
end
+ # It's entirely plausible that a site might prefer UUIDs or labels, so
+ # we need to be able to update fstab to conform with their wishes
+ # without necessarily needing to remount the device.
+ # See #6851 for more.
+ def device_unchanged?
+ @current_resource.device == @new_resource.device
+ end
+
#
# NOTE: for the following methods, this superclass will already have checked if the filesystem is
# enabled and/or mounted and they will be called in converge_by blocks, so most defensive checking
diff --git a/lib/chef/provider/mount/mount.rb b/lib/chef/provider/mount/mount.rb
index c3b1cc0141..1435a75ec3 100644
--- a/lib/chef/provider/mount/mount.rb
+++ b/lib/chef/provider/mount/mount.rb
@@ -57,12 +57,13 @@ class Chef
case line
when /^[#\s]/
next
- when /^#{device_fstab_regex}\s+#{Regexp.escape(@new_resource.mount_point)}\s+(\S+)\s+(\S+)\s+(\S+)\s+(\S+)/
+ when /^(#{device_fstab_regex})\s+#{Regexp.escape(@new_resource.mount_point)}\s+(\S+)\s+(\S+)\s+(\S+)\s+(\S+)/
enabled = true
- @current_resource.fstype($1)
- @current_resource.options($2)
- @current_resource.dump($3.to_i)
- @current_resource.pass($4.to_i)
+ @current_resource.device($1)
+ @current_resource.fstype($2)
+ @current_resource.options($3)
+ @current_resource.dump($4.to_i)
+ @current_resource.pass($5.to_i)
Chef::Log.debug("Found mount #{device_fstab} to #{@new_resource.mount_point} in /etc/fstab")
next
when /^[\/\w]+\s+#{Regexp.escape(@new_resource.mount_point)}\s+/
@@ -147,7 +148,7 @@ class Chef
end
def enable_fs
- if @current_resource.enabled && mount_options_unchanged?
+ if @current_resource.enabled && mount_options_unchanged? && device_unchanged?
Chef::Log.debug("#{@new_resource} is already enabled - nothing to do")
return nil
end
@@ -253,7 +254,7 @@ class Chef
if @new_resource.device_type == :device
device_mount_regex
else
- device_fstab
+ Regexp.union(device_fstab, device_mount_regex)
end
end
diff --git a/lib/chef/resource/mount.rb b/lib/chef/resource/mount.rb
index 1a1f8c0565..fca99886b7 100644
--- a/lib/chef/resource/mount.rb
+++ b/lib/chef/resource/mount.rb
@@ -24,10 +24,6 @@ class Chef
# Use the mount resource to manage a mounted file system.
class Mount < Chef::Resource
- identity_attr :device
-
- state_attrs :mount_point, :device_type, :fstype, :username, :password, :domain
-
default_action :mount
allowed_actions :mount, :umount, :unmount, :remount, :enable, :disable
@@ -37,130 +33,27 @@ class Chef
property :password, String, sensitive: true
- def initialize(name, run_context = nil)
- super
- @mount_point = name
- @device = nil
- @device_type = :device
- @fsck_device = "-"
- @fstype = "auto"
- @options = ["defaults"]
- @dump = 0
- @pass = 2
- @mounted = false
- @enabled = false
- @username = nil
- @password = nil
- @domain = nil
- end
-
- def mount_point(arg = nil)
- set_or_return(
- :mount_point,
- arg,
- :kind_of => [ String ]
- )
- end
-
- def device(arg = nil)
- set_or_return(
- :device,
- arg,
- :kind_of => [ String ]
- )
- end
-
- def device_type(arg = nil)
- real_arg = arg.kind_of?(String) ? arg.to_sym : arg
- valid_devices = if RUBY_PLATFORM =~ /solaris/i
- [ :device ]
- else
- [ :device, :label, :uuid ]
- end
- set_or_return(
- :device_type,
- real_arg,
- :equal_to => valid_devices
- )
- end
-
- def fsck_device(arg = nil)
- set_or_return(
- :fsck_device,
- arg,
- :kind_of => [ String ]
- )
- end
-
- def fstype(arg = nil)
- set_or_return(
- :fstype,
- arg,
- :kind_of => [ String ]
- )
- end
-
- def options(arg = nil)
- ret = set_or_return(
- :options,
- arg,
- :kind_of => [ Array, String ]
- )
-
- if ret.is_a? String
- ret.tr(",", " ").split(/ /)
- else
- ret
- end
- end
-
- def dump(arg = nil)
- set_or_return(
- :dump,
- arg,
- :kind_of => [ Integer, FalseClass ]
- )
- end
+ property :mount_point, String, name_property: true
+ property :device, String, identity: true
- def pass(arg = nil)
- set_or_return(
- :pass,
- arg,
- :kind_of => [ Integer, FalseClass ]
- )
- end
-
- def mounted(arg = nil)
- set_or_return(
- :mounted,
- arg,
- :kind_of => [ TrueClass, FalseClass ]
- )
- end
+ property :device_type, [String, Symbol],
+ coerce: proc { |arg| arg.kind_of?(String) ? arg.to_sym : arg },
+ default: :device,
+ equal_to: RUBY_PLATFORM =~ /solaris/i ? %i{ device } : %i{ device label uuid }
- def enabled(arg = nil)
- set_or_return(
- :enabled,
- arg,
- :kind_of => [ TrueClass, FalseClass ]
- )
- end
+ property :fsck_device, String, default: "-"
+ property :fstype, String, default: "auto"
- def username(arg = nil)
- set_or_return(
- :username,
- arg,
- :kind_of => [ String ]
- )
- end
+ property :options, [Array, String],
+ coerce: proc { |arg| arg.kind_of?(String) ? arg.split(",") : arg },
+ default: %w{defaults}
- def domain(arg = nil)
- set_or_return(
- :domain,
- arg,
- :kind_of => [ String ]
- )
- end
+ property :dump, [Integer, FalseClass], default: 0
+ property :pass, [Integer, FalseClass], default: 2
+ property :mounted, [TrueClass, FalseClass], default: false
+ property :enabled, [TrueClass, FalseClass], default: false
+ property :username, String
+ property :domain, String
private
diff --git a/spec/unit/provider/mount/mount_spec.rb b/spec/unit/provider/mount/mount_spec.rb
index b4ce10d861..ca6cf59d71 100644
--- a/spec/unit/provider/mount/mount_spec.rb
+++ b/spec/unit/provider/mount/mount_spec.rb
@@ -475,26 +475,28 @@ describe Chef::Provider::Mount::Mount do
# we should not create two mount lines, but update the existing one
context "when the device is described differently" do
it "should update the existing line" do
+ @current_resource.enabled(true)
status = double(:stdout => "/dev/sdz1\n", :exitstatus => 1)
expect(@provider).to receive(:shell_out).with("/sbin/findfs UUID=d21afe51-a0fe-4dc6-9152-ac733763ae0a").and_return(status)
filesystems = [%q{/dev/sdy1 /tmp/foo ext3 defaults 1 2},
%q{/dev/sdz1 /tmp/foo ext3 defaults 1 2}].join("\n")
- fstab = StringIO.new
- fstab.puts filesystems
+ fstab = StringIO.new filesystems
+
+ fstab_write = StringIO.new
allow(::File).to receive(:readlines).with("/etc/fstab").and_return(fstab.readlines)
- allow(::File).to receive(:open).with("/etc/fstab", "w").and_yield(fstab)
- allow(::File).to receive(:open).with("/etc/fstab", "a").and_yield(fstab)
+ allow(::File).to receive(:open).with("/etc/fstab", "w").and_yield(fstab_write)
+ allow(::File).to receive(:open).with("/etc/fstab", "a").and_yield(fstab_write)
@new_resource.device_type :uuid
@new_resource.device "d21afe51-a0fe-4dc6-9152-ac733763ae0a"
@new_resource.dump 1
@provider.enable_fs
- expect(fstab.string).to match(%r{/dev/sdy1\s+/tmp/foo\s+ext3\s+defaults\s+1\s+2})
- expect(fstab.string).to match(%r{UUID=d21afe51-a0fe-4dc6-9152-ac733763ae0a\s+/tmp/foo\s+ext3\s+defaults\s+1\s+2})
- expect(fstab.string).not_to match(%r{/dev/sdz1\s+/tmp/foo\s+ext3\s+defaults\s+1\s+2})
+ expect(fstab_write.string).to match(%r{/dev/sdy1\s+/tmp/foo\s+ext3\s+defaults\s+1\s+2})
+ expect(fstab_write.string).to match(%r{UUID=d21afe51-a0fe-4dc6-9152-ac733763ae0a\s+/tmp/foo\s+ext3\s+defaults\s+1\s+2})
+ expect(fstab_write.string).not_to match(%r{/dev/sdz1\s+/tmp/foo\s+ext3\s+defaults\s+1\s+2})
end
end
end
diff --git a/spec/unit/resource/mount_spec.rb b/spec/unit/resource/mount_spec.rb
index da181b7f8c..1d4cc880d9 100644
--- a/spec/unit/resource/mount_spec.rb
+++ b/spec/unit/resource/mount_spec.rb
@@ -66,6 +66,10 @@ describe Chef::Resource::Mount do
expect(resource.fstype).to eql("nfs")
end
+ it "sets fstype to 'auto' by default" do
+ expect(resource.fstype).to eql("auto")
+ end
+
it "allows you to set the dump attribute" do
resource.dump 1
expect(resource.dump).to eql(1)