From 9688175883518d403d6ddaf85533b449f4aae76a Mon Sep 17 00:00:00 2001
From: Toomas Pelberg <toomas.pelberg@playtech.com>
Date: Thu, 12 Aug 2010 21:02:06 +0300
Subject: [PATCH] Only call mountable? when trying to mount - otherwise avoid
 raising errors

---
 chef/lib/chef/provider/mount/mount.rb       | 43 ++++++++++++---------
 chef/spec/unit/provider/mount/mount_spec.rb |  7 ++--
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/chef/lib/chef/provider/mount/mount.rb b/chef/lib/chef/provider/mount/mount.rb
index c0b370b02c4..e1e4077dac9 100644
--- a/chef/lib/chef/provider/mount/mount.rb
+++ b/chef/lib/chef/provider/mount/mount.rb
@@ -36,29 +36,21 @@ def load_current_resource
           @current_resource = Chef::Resource::Mount.new(@new_resource.name)
           @current_resource.mount_point(@new_resource.mount_point)
           @current_resource.device(@new_resource.device)
-          Chef::Log.debug("Checking for mount point #{@current_resource.mount_point}")
-
+          mounted?
+          enabled?
+        end
+        
+        def mountable?
           # only check for existence of non-remote devices
           if (device_should_exist? && !::File.exists?(device_real) )
             raise Chef::Exceptions::Mount, "Device #{@new_resource.device} does not exist"
           elsif( !::File.exists?(@new_resource.mount_point) )
             raise Chef::Exceptions::Mount, "Mount point #{@new_resource.mount_point} does not exist"
           end
-
-          # Check to see if the volume is mounted. Last volume entry wins.
-          mounted = false
-          shell_out!("mount").stdout.each_line do |line|
-            case line
-            when /^#{device_mount_regex}\s+on\s+#{Regexp.escape(@new_resource.mount_point)}/
-              mounted = true
-              Chef::Log.debug("Special device #{device_logstring} mounted as #{@new_resource.mount_point}")
-            when /^([\/\w])+\son\s#{Regexp.escape(@new_resource.mount_point)}\s+/
-              mounted = false
-              Chef::Log.debug("Special device #{$~[1]} mounted as #{@new_resource.mount_point}")
-            end
-          end
-          @current_resource.mounted(mounted)
-
+          return true
+        end
+        
+        def enabled?
           # Check to see if there is a entry in /etc/fstab. Last entry for a volume wins.
           enabled = false
           ::File.foreach("/etc/fstab") do |line|
@@ -79,9 +71,25 @@ def load_current_resource
           end
           @current_resource.enabled(enabled)
         end
+        
+        def mounted?
+          mounted = false
+          shell_out!("mount").stdout.each_line do |line|
+            case line
+            when /^#{device_mount_regex}\s+on\s+#{Regexp.escape(@new_resource.mount_point)}/
+              mounted = true
+              Chef::Log.debug("Special device #{device_logstring} mounted as #{@new_resource.mount_point}")
+            when /^([\/\w])+\son\s#{Regexp.escape(@new_resource.mount_point)}\s+/
+              mounted = false
+              Chef::Log.debug("Special device #{$~[1]} mounted as #{@new_resource.mount_point}")
+            end
+          end
+          @current_resource.mounted(mounted)
+        end
 
         def mount_fs
           unless @current_resource.mounted
+            mountable?
             command = "mount -t #{@new_resource.fstype}"
             command << " -o #{@new_resource.options.join(',')}" unless @new_resource.options.nil? || @new_resource.options.empty?
             command << case @new_resource.device_type
@@ -112,7 +120,6 @@ def umount_fs
         def remount_fs
           if @current_resource.mounted and @new_resource.supports[:remount]
             shell_out!("mount -o remount #{@new_resource.mount_point}")
-
             @new_resource.updated_by_last_action(true)
             Chef::Log.debug("#{@new_resource} is remounted at #{@new_resource.mount_point}")
           elsif @current_resource.mounted
diff --git a/chef/spec/unit/provider/mount/mount_spec.rb b/chef/spec/unit/provider/mount/mount_spec.rb
index d7f15243173..8e1b16f85dc 100644
--- a/chef/spec/unit/provider/mount/mount_spec.rb
+++ b/chef/spec/unit/provider/mount/mount_spec.rb
@@ -56,11 +56,12 @@
       @stdout_findfs = mock("STDOUT", :first => "/dev/sdz1")
       @provider.should_receive(:popen4).with("/sbin/findfs UUID=d21afe51-a0fe-4dc6-9152-ac733763ae0a").and_yield(@pid,@stdin,@stdout_findfs,@stderr).and_return(@status)
       @provider.load_current_resource()
+      @provider.mountable?
     end
 
     it "should raise an error if the mount device does not exist" do
       ::File.stub!(:exists?).with("/dev/sdz1").and_return false
-      lambda { @provider.load_current_resource() }.should raise_error(Chef::Exceptions::Mount)
+      lambda { @provider.load_current_resource();@provider.mountable? }.should raise_error(Chef::Exceptions::Mount)
     end
     
     it "should raise an error if the mount device (uuid) does not exist" do
@@ -70,12 +71,12 @@
       stdout_findfs = mock("STDOUT", :first => nil)
       @provider.should_receive(:popen4).with("/sbin/findfs UUID=d21afe51-a0fe-4dc6-9152-ac733763ae0a").and_yield(@pid,@stdin,stdout_findfs,@stderr).and_return(status_findfs)
       ::File.should_receive(:exists?).with("").and_return(false)
-      lambda { @provider.load_current_resource() }.should raise_error(Chef::Exceptions::Mount)
+      lambda { @provider.load_current_resource();@provider.mountable? }.should raise_error(Chef::Exceptions::Mount)
     end
     
     it "should raise an error if the mount point does not exist" do
       ::File.stub!(:exists?).with("/tmp/foo").and_return false
-      lambda { @provider.load_current_resource() }.should raise_error(Chef::Exceptions::Mount)
+      lambda { @provider.load_current_resource();@provider.mountable? }.should raise_error(Chef::Exceptions::Mount)
     end
 
     it "does not expect the device to exist when it is tmpfs" do