Merge pull request #1888 from opscode/lcg/1738

Lcg/1738

2 files changed, 111 insertions, 64 deletions

diff --git a/lib/chef/provider/mount/solaris.rb b/lib/chef/provider/mount/solaris.rb
index 462fa32b71..cf04150322 100644
--- a/lib/chef/provider/mount/solaris.rb
+++ b/lib/chef/provider/mount/solaris.rb
@@ -1,4 +1,4 @@
-#
+# Encoding: utf-8
 # Author:: Hugo Fichter
 # Author:: Lamont Granquist (<lamont@getchef.com>)
 # Author:: Joshua Timberman (<joshua@getchef.com>)
@@ -25,14 +25,16 @@ require 'forwardable'
 class Chef
   class Provider
     class Mount
+      # Mount Solaris File systems
       class Solaris < Chef::Provider::Mount
         extend Forwardable
 
-        VFSTAB = "/etc/vfstab".freeze
+        VFSTAB = '/etc/vfstab'.freeze
 
         def_delegator :@new_resource, :device, :device
         def_delegator :@new_resource, :device_type, :device_type
         def_delegator :@new_resource, :dump, :dump
+        def_delegator :@new_resource, :fsck_device, :fsck_device
         def_delegator :@new_resource, :fstype, :fstype
         def_delegator :@new_resource, :mount_point, :mount_point
         def_delegator :@new_resource, :options, :options
@@ -42,6 +44,7 @@ class Chef
           @current_resource = Chef::Resource::Mount.new(new_resource.name)
           current_resource.mount_point(mount_point)
           current_resource.device(device)
+          current_resource.fsck_device(fsck_device)
           current_resource.device_type(device_type)
           update_current_resource_state
         end
@@ -53,6 +56,14 @@ class Chef
             a.whyrun("Assuming device #{device} would have been created")
           end
 
+          unless fsck_device == '-'
+            requirements.assert(:mount, :remount) do |a|
+              a.assertion { ::File.exist?(fsck_device) }
+              a.failure_message(Chef::Exceptions::Mount, "Device #{fsck_device} does not exist")
+              a.whyrun("Assuming device #{fsck_device} would have been created")
+            end
+          end
+
           requirements.assert(:mount, :remount) do |a|
             a.assertion { ::File.exist?(mount_point) }
             a.failure_message(Chef::Exceptions::Mount, "Mount point #{mount_point} does not exist")
@@ -62,7 +73,7 @@ class Chef
 
         def mount_fs
           actual_options = options || []
-          actual_options.delete("noauto")
+          actual_options.delete('noauto')
           command = "mount -F #{fstype}"
           command << " -o #{actual_options.join(',')}" unless actual_options.empty?
           command << " #{device} #{mount_point}"
@@ -74,59 +85,28 @@ class Chef
         end
 
         def remount_fs
-          # FIXME: what about options like "-o remount,logging" to enable logging on a UFS device?
-          shell_out!("mount -o remount #{mount_point}")
+          # FIXME: Should remount always do the remount or only if the options change?
+          actual_options = options || []
+          actual_options.delete('noauto')
+          mount_options = actual_options.empty? ? '' : ",#{actual_options.join(',')}" 
+          shell_out!("mount -o remount#{mount_options} #{mount_point}")
         end
 
         def enable_fs
-          if !mount_options_unchanged?
+          unless mount_options_unchanged?
             # we are enabling because our options have changed, so disable first then re-enable.
             # XXX: this should be refactored to be the responsibility of the caller
             disable_fs if current_resource.enabled
           end
 
-          auto = options.nil? || ! options.include?("noauto")
-          actual_options = unless options.nil?
-                             options.delete("noauto")
-                             options
-                           end
-
-          autostr = auto ? 'yes' : 'no'
-          passstr = pass == 0 ? "-" : pass
-          optstr = (actual_options.nil? || actual_options.empty?) ? "-" : actual_options.join(',')
-
-          etc_tempfile do |f|
-            f.write(IO.read(VFSTAB).chomp)
-            f.puts("\n#{device}\t-\t#{mount_point}\t#{fstype}\t#{passstr}\t#{autostr}\t#{optstr}")
-            f.close
-            # move, preserving modes of destination file
-            mover = Chef::FileContentManagement::Deploy.strategy(true)
-            mover.deploy(f.path, VFSTAB)
-          end
-
+          vfstab_write(merge_vfstab_entry)
         end
 
         def disable_fs
-          contents = []
-
-          found = false
-          ::File.readlines(VFSTAB).reverse_each do |line|
-            if !found && line =~ /^#{device_regex}\s+\S+\s+#{Regexp.escape(mount_point)}/
-              found = true
-              Chef::Log.debug("#{new_resource} is removed from vfstab")
-              next
-            end
-            contents << line
-          end
+          contents, found = delete_vfstab_entry
 
           if found
-            etc_tempfile do |f|
-              f.write(contents.reverse.join(''))
-              f.close
-              # move, preserving modes of destination file
-              mover = Chef::FileContentManagement::Deploy.strategy(true)
-              mover.deploy(f.path, VFSTAB)
-            end
+            vfstab_write(contents.reverse)
           else
             # this is likely some kind of internal error, since we should only call disable_fs when there
             # the filesystem we want to disable is enabled.
@@ -135,19 +115,24 @@ class Chef
         end
 
         def etc_tempfile
-          yield Tempfile.open("vfstab", "/etc")
+          yield Tempfile.open('vfstab', '/etc')
         end
 
         def mount_options_unchanged?
-          current_resource.fstype == fstype and
-            current_resource.options == options and
-            current_resource.dump == dump and
-            current_resource.pass == pass
+          new_options = options_remove_noauto(options)
+          current_options = options_remove_noauto(current_resource.nil? ? nil : current_resource.options)
+
+          current_resource.fsck_device == fsck_device &&
+            current_resource.fstype == fstype &&
+            current_options == new_options &&
+            current_resource.dump == dump &&
+            current_resource.pass == pass &&
+            current_resource.options.include?('noauto') == !mount_at_boot?
         end
 
         def update_current_resource_state
           current_resource.mounted(mounted?)
-          ( enabled, fstype, options, pass ) = read_vfstab_status
+          (enabled, fstype, options, pass) = read_vfstab_status
           current_resource.enabled(enabled)
           current_resource.fstype(fstype)
           current_resource.options(options)
@@ -158,17 +143,18 @@ class Chef
           read_vfstab_status[0]
         end
 
+        # Check for the device in mounttab.
+        # <device> on <mountpoint> type <fstype> <options> on <date>
+        # /dev/dsk/c1t0d0s0 on / type ufs read/write/setuid/devices/intr/largefiles/logging/xattr/onerror=panic/dev=700040 on Tue May  1 11:33:55 2012
         def mounted?
           mounted = false
-          shell_out!("mount -v").stdout.each_line do |line|
-            # <device> on <mountpoint> type <fstype> <options> on <date>
-            # /dev/dsk/c1t0d0s0 on / type ufs read/write/setuid/devices/intr/largefiles/logging/xattr/onerror=panic/dev=700040 on Tue May  1 11:33:55 2012
+          shell_out!('mount -v').stdout.each_line do |line|
             case line
             when /^#{device_regex}\s+on\s+#{Regexp.escape(mount_point)}\s+/
               Chef::Log.debug("Special device #{device} is mounted as #{mount_point}")
               mounted = true
             when /^([\/\w]+)\son\s#{Regexp.escape(mount_point)}\s+/
-              Chef::Log.debug("Special device #{$1} is mounted as #{mount_point}")
+              Chef::Log.debug("Special device #{Regexp.last_match[1]} is mounted as #{mount_point}")
               mounted = false
             end
           end
@@ -178,7 +164,7 @@ class Chef
         private
 
         def read_vfstab_status
-          # Check to see if there is a entry in /etc/vfstab. Last entry for a volume wins.
+          # Check to see if there is an entry in /etc/vfstab. Last entry for a volume wins.
           enabled = false
           fstype = options = pass = nil
           ::File.foreach(VFSTAB) do |line|
@@ -190,18 +176,18 @@ class Chef
               # to mount       to fsck         point           type    pass    at boot options
             when /^#{device_regex}\s+[-\/\w]+\s+#{Regexp.escape(mount_point)}\s+(\S+)\s+(\S+)\s+(\S+)\s+(\S+)/
               enabled = true
-              fstype = $1
-              options = $4
+              fstype = Regexp.last_match[1]
+              options = Regexp.last_match[4]
               # Store the 'mount at boot' column from vfstab as the 'noauto' option
               # in current_resource.options (linux style)
-              if $3 == "yes"
+              if Regexp.last_match[3] == 'no'
                 if options.nil? || options.empty?
-                  options = "noauto"
+                  options = 'noauto'
                 else
-                  options += ",noauto"
+                  options += ',noauto'
                 end
               end
-              pass = ( $2 == "-" ) ? 0 : $2.to_i
+              pass = (Regexp.last_match[2] == '-') ? 0 : Regexp.last_match[2].to_i
               Chef::Log.debug("Found mount #{device} to #{mount_point} in #{VFSTAB}")
               next
             when /^[-\/\w]+\s+[-\/\w]+\s+#{Regexp.escape(mount_point)}\s+/
@@ -210,21 +196,73 @@ class Chef
               Chef::Log.debug("Found conflicting mount point #{mount_point} in #{VFSTAB}")
             end
           end
-          [ enabled, fstype, options, pass ]
+          [enabled, fstype, options, pass]
         end
 
         def device_should_exist?
-          !%w{tmpfs nfs ctfs proc mntfs objfs sharefs fd smbfs}.include?(fstype)
+          !%w(tmpfs nfs ctfs proc mntfs objfs sharefs fd smbfs vxfs).include?(fstype)
+        end
+
+        def mount_at_boot?
+          options.nil? || !options.include?('noauto')
+        end
+
+        def vfstab_write(contents)
+          etc_tempfile do |f|
+            f.write(contents.join(''))
+            f.close
+            # move, preserving modes of destination file
+            mover = Chef::FileContentManagement::Deploy.strategy(true)
+            mover.deploy(f.path, VFSTAB)
+          end
+        end
+
+        def vfstab_entry
+          actual_options = unless options.nil?
+            tempops = options.dup
+            tempops.delete('noauto')
+            tempops
+          end
+          autostr = mount_at_boot? ? 'yes' : 'no'
+          passstr = pass == 0 ? '-' : pass
+          optstr = (actual_options.nil? || actual_options.empty?) ? '-' : actual_options.join(',')
+          "\n#{device}\t#{fsck_device}\t#{mount_point}\t#{fstype}\t#{passstr}\t#{autostr}\t#{optstr}\n"
+        end
+
+        def delete_vfstab_entry
+          contents = []
+          found = false
+          ::File.readlines(VFSTAB).reverse_each do |line|
+            if !found && line =~ /^#{device_regex}\s+\S+\s+#{Regexp.escape(mount_point)}/
+              found = true
+              Chef::Log.debug("#{new_resource} is removed from vfstab")
+              next
+            end
+            contents << line
+          end
+          [contents, found]
+        end
+
+        def merge_vfstab_entry
+          contents = ::File.readlines(VFSTAB)
+          contents[-1].chomp!
+          contents << vfstab_entry
+        end
+
+        def options_remove_noauto(temp_options)
+          new_options = []
+          new_options += temp_options.nil? ? [] : temp_options
+          new_options.delete('noauto')
+          new_options
         end
 
         def device_regex
           if ::File.symlink?(device)
-            "(?:#{Regexp.escape(device)}|#{Regexp.escape(::File.expand_path(::File.readlink(device),::File.dirname(device)))})"
+            "(?:#{Regexp.escape(device)}|#{Regexp.escape(::File.expand_path(::File.readlink(device), ::File.dirname(device)))})"
           else
             Regexp.escape(device)
           end
         end
-
       end
     end
   end
diff --git a/lib/chef/resource/mount.rb b/lib/chef/resource/mount.rb
index 9eafe07253..275c069f61 100644
--- a/lib/chef/resource/mount.rb
+++ b/lib/chef/resource/mount.rb
@@ -33,6 +33,7 @@ class Chef
         @mount_point = name
         @device = nil
         @device_type = :device
+        @fsck_device = '-'
         @fstype = "auto"
         @options = ["defaults"]
         @dump = 0
@@ -77,6 +78,14 @@ class Chef
         )
       end
 
+      def fsck_device(arg=nil)
+        set_or_return(
+          :fsck_device,
+          arg,
+          :kind_of => [ String ]
+        )
+      end
+
       def fstype(arg=nil)
         set_or_return(
           :fstype,