Fixing #113. I added support in the transaction for self-refreshing, which just creates a special trigger for resources that have self-refreshing enabled. Logging is a bit different for them, so it is clear why they are refreshing. I still need to verify the remount methods work in the providers.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2107 980ebf18-57e1-0310-9a29-db15c13687c0

6 files changed, 100 insertions, 8 deletions

diff --git a/lib/puppet/provider/mount.rb b/lib/puppet/provider/mount.rb
index fc3e5acef..bd8693e7d 100644
--- a/lib/puppet/provider/mount.rb
+++ b/lib/puppet/provider/mount.rb
@@ -11,6 +11,16 @@ module Puppet::Provider::Mount
         mountcmd @model[:name]
     end
 
+    def remount
+        info "Remounting"
+        if @model[:remounts] == :true
+            mountcmd "-o", "remount", @model[:name]
+        else
+            unmount()
+            mount()
+        end
+    end
+
     # This only works when the mount point is synced to the fstab.
     def unmount
         umount @model[:name]
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index e184bf128..f3662ccfe 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -83,6 +83,13 @@ class Transaction
             if resource.respond_to?(:flush)
                 resource.flush
             end
+            
+            # And set a trigger for refreshing this resource if it's a self-refresher
+            if resource.self_refresh?
+                # Create an edge with this resource as both the source and target.  The triggering
+                # method treats these specially for logging.
+                set_trigger(Puppet::Relationship.new(resource, resource, :callback => :refresh, :event => :ALL_EVENTS))
+            end
         end
 
         resourceevents
@@ -567,7 +574,9 @@ class Transaction
         return unless method = edge.callback
         return unless edge.target.respond_to?(method)
         if edge.target.respond_to?(:ref)
-            edge.source.info "Scheduling %s of %s" % [edge.callback, edge.target.ref]
+            unless edge.source == edge.target
+                edge.source.info "Scheduling %s of %s" % [edge.callback, edge.target.ref]
+            end
         end
         @targets[edge.target] << edge
     end
@@ -634,9 +643,14 @@ class Transaction
         end
 
         callbacks.each do |callback, subs|
-            message = "Triggering '%s' from %s dependencies" %
-                [callback, subs.length]
+            if subs.length == 1 and subs[0].source == resource
+                message = "Refreshing self"
+            else
+                message = "Triggering '%s' from %s dependencies" %
+                    [callback, subs.length]
+            end
             resource.notice message
+            
             # At this point, just log failures, don't try to react
             # to them in any way.
             begin
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index fb73e0459..26d2df24a 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -337,6 +337,10 @@ class Type < Puppet::Element
     def ref
         "%s[%s]" % [self.class.name.to_s.capitalize, self.title]
     end
+    
+    def self_refresh?
+        self.class.self_refresh
+    end
 
     # Retrieve the title of an object.  If no title was set separately,
     # then use the object's name.
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index d9e894b05..4961e689a 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -1,7 +1,8 @@
 require 'puppet/type/parsedtype'
 
 module Puppet
-    newtype(:mount) do
+    # We want the mount to refresh when it changes.
+    newtype(:mount, :self_refresh => true) do
         # Use the normal parent class, because we actually want to
         # call code when sync() is called.
         newstate(:ensure) do
@@ -152,10 +153,28 @@ module Puppet
                 super
             end
         end
+        
+        newparam(:remounts) do
+            desc "Whether the mount can be remounted  ``mount -o remount``.  If this is false,
+                then the filesystem will be unmounted and remounted manually, which is prone to failure."
+            
+            newvalues(:true, :false)
+            defaultto do
+                case Facter.value(:operatingsystem)
+                when "Darwin": false
+                else
+                    true
+                end
+            end
+        end
 
         @doc = "Manages mounted mounts, including putting mount
             information into the mount table. The actual behavior depends 
             on the value of the 'ensure' parameter."
+        
+        def refresh
+            provider.remount
+        end
 
         def value(name)
             name = symbolize(name)
diff --git a/test/other/transactions.rb b/test/other/transactions.rb
index 308bcfeea..c9d0cec1c 100755
--- a/test/other/transactions.rb
+++ b/test/other/transactions.rb
@@ -1002,6 +1002,31 @@ class TestTransactions < Test::Unit::TestCase
 
         assert_apply(obj)
     end
+    
+    def test_self_refresh_causes_triggering
+        type = Puppet::Type.newtype(:refresher, :self_refresh => true) do
+            attr_accessor :refreshed, :testing
+            newparam(:name) {}
+            newstate(:testing) do
+                def sync
+                    self.is = self.should
+                    :ran_testing
+                end
+            end
+            def refresh
+                @refreshed = true
+            end
+        end
+        cleanup { Puppet::Type.rmtype(:refresher)}
+        
+        obj = type.create(:name => "yay", :testing => "cool")
+        
+        assert(! obj.insync?, "fake object is already in sync")
+        
+        # Now make sure it gets refreshed when the change happens
+        assert_apply(obj)
+        assert(obj.refreshed, "object was not refreshed during transaction")
+    end
 end
 
 # $Id$
diff --git a/test/types/mount.rb b/test/types/mount.rb
index 949253b5e..df1a71d71 100755
--- a/test/types/mount.rb
+++ b/test/types/mount.rb
@@ -94,10 +94,7 @@ class TestMounts < Test::Unit::TestCase
         }
 
         [@mount.validstates, @mount.parameters].flatten.each do |field|
-            next if field == :path
-            next if field == :provider
-            next if field == :target
-            next if field == :ensure
+            next if [:path, :provider, :target, :ensure, :remounts].include?(field)
             unless args.include? field
                 args[field] = "fake%s" % @pcount
             end
@@ -266,6 +263,29 @@ class TestMounts < Test::Unit::TestCase
 
         assert_equal("mount_path", mount[:name], "Name did not get copied over")
     end
+    
+    def test_refresh
+        mount = mkmount
+        
+        remounted = false
+        mount.provider.meta_def(:remount) do
+            remounted = true
+        end
+        
+        # First make sure we correctly call the provider
+        assert_nothing_raised do
+            mount.refresh
+        end
+        assert(remounted, "did not call remount on provider")
+        
+        # then make sure it gets called during transactions
+        remounted = false
+        mount[:device] = "/dev/yayness"
+        
+        assert_apply(mount)
+        
+        assert(remounted, "did not remount when mount changed")
+    end
 end
 
 # $Id$