[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 2.6.5-303-gfcfa26a

[Paul Berry]

The following commit has been merged in the upstream branch:
commit 5ef10315705b8e4d69d13b8df86b9585f2bcc29a
Author: Paul Berry <paul at puppetlabs.com>
Date:   Tue Mar 8 11:35:58 2011 -0800

    (#6632) Adding a new mount no longer causes error with umount
    
    There were two problems:
    
    * In lib/puppet/type/mount.rb, we were calling provider.mounted? to
      determine whether we needed to execute "mount" after updating the
      in-memory fstab record.  This wasn't working properly because
      provider.mounted? makes its decision based on the data stored in the
      in-memory fstab record.  Since the fstab record had just been
      updated, provider.mounted? was incorrectly returning true even
      though the device wasn't actually mounted.  Fixed this by checking
      provider.mounted? before updating the in-memory fstab record.
    
    * Calling mount from this point in lib/puppet/type/mount.rb is
      actually too early, because even though the in-memory fstab record
      has been created, its contents have not been written to `/etc/fstab`
      yet.  Fixed this by storing a :needs_mount entry in the
      property_hash and checking it at the end of the flush() method.
    
    Reviewed-by: Jacob Helwig <jacob at puppetlabs.com>

diff --git a/lib/puppet/provider/mount/parsed.rb b/lib/puppet/provider/mount/parsed.rb
index 42e543c..11c5e21 100755
--- a/lib/puppet/provider/mount/parsed.rb
+++ b/lib/puppet/provider/mount/parsed.rb
@@ -97,4 +97,10 @@ Puppet::Type.type(:mount).provide(
     end
     instances
   end
+
+  def flush
+    needs_mount = @property_hash.delete(:needs_mount)
+    super
+    mount if needs_mount
+  end
 end
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index 98a1f25..5b8c5ca 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -74,12 +74,13 @@ module Puppet
       newvalue(:mounted, :event => :mount_mounted) do
         # Create the mount point if it does not already exist.
         current_value = self.retrieve
+        currently_mounted = provider.mounted?
         provider.create if [nil, :absent, :ghost].include?(current_value)
 
         syncothers
 
         # The fs can be already mounted if it was absent but mounted
-        provider.mount unless provider.mounted?
+        provider.property_hash[:needs_mount] = true unless currently_mounted
       end
 
       # insync: mounted   -> present
diff --git a/spec/integration/provider/mount_spec.rb b/spec/integration/provider/mount_spec.rb
index c28707d..518b295 100644
--- a/spec/integration/provider/mount_spec.rb
+++ b/spec/integration/provider/mount_spec.rb
@@ -35,6 +35,7 @@ describe "mount provider (integration)" do
         fail "unexpected umount" unless @umount_permitted
         command.length.should == 2
         command[1].should == '/Volumes/foo_disk'
+        @mounted.should == true # "umount" doesn't work when device not mounted (see #6632)
         @mounted = false
         ''
       else
@@ -77,7 +78,7 @@ describe "mount provider (integration)" do
 
   it "should be able to create and mount a brand new mount point" do
     @mounted = false
-    @umount_permitted = true # Work around bug #6632
+    @umount_permitted = true # Work around bug #6633
     run_in_catalog(:mounted)
     @mounted.should == true
     check_fstab
diff --git a/spec/unit/type/mount_spec.rb b/spec/unit/type/mount_spec.rb
index 7f9a0eb..fdb67f7 100755
--- a/spec/unit/type/mount_spec.rb
+++ b/spec/unit/type/mount_spec.rb
@@ -65,7 +65,8 @@ end
 
 describe Puppet::Type.type(:mount)::Ensure do
   before :each do
-    @provider = stub 'provider', :class => Puppet::Type.type(:mount).defaultprovider, :clear => nil, :satisfies? => true, :name => :mock
+    provider_properties = {}
+    @provider = stub 'provider', :class => Puppet::Type.type(:mount).defaultprovider, :clear => nil, :satisfies? => true, :name => :mock, :property_hash => provider_properties
     Puppet::Type.type(:mount).defaultprovider.stubs(:new).returns(@provider)
     @mount = Puppet::Type.type(:mount).new(:name => "yay", :check => :ensure)
 
@@ -93,11 +94,12 @@ describe Puppet::Type.type(:mount)::Ensure do
       @provider.stubs(:mounted?).returns([:mounted,:ghost].include? options[:from])
       @provider.expects(:create).times(options[:create] || 0)
       @provider.expects(:destroy).times(options[:destroy] || 0)
-      @provider.expects(:mount).times(options[:mount] || 0)
+      @provider.expects(:mount).never
       @provider.expects(:unmount).times(options[:unmount] || 0)
       @ensure.stubs(:syncothers)
       @ensure.should = options[:to]
       @ensure.sync
+      (!!@provider.property_hash[:needs_mount]).should == (!!options[:mount])
    end
 
    it "should create itself when changing from :ghost to :present" do
@@ -285,34 +287,6 @@ describe Puppet::Type.type(:mount), "when modifying an existing mount entry" do
     @catalog.apply
   end
 
-  it "should flush changes before mounting" do
-    syncorder = sequence('syncorder')
-    @mount.provider.expects(:options).returns 'soft'
-    @mount.provider.expects(:ensure).returns :unmounted
-    @mount.provider.expects(:mounted?).returns false
-
-    @mount.provider.expects(:options=).in_sequence(syncorder).with 'hard'
-    @mount.expects(:flush).in_sequence(syncorder) # Have to write with no options
-    @mount.provider.expects(:mount).in_sequence(syncorder)
-    @mount.expects(:flush).in_sequence(syncorder) # Call flush again cause we changed everything
-
-    @mount[:ensure] = :mounted
-    @mount[:options] = 'hard'
-
-    @catalog.apply
-  end
-
-  it "should not flush before mounting if there are no other changes" do
-    syncorder = sequence('syncorder')
-    @mount.provider.expects(:ensure).returns :unmounted
-    @mount.provider.expects(:mounted?).returns false
-    @mount.provider.expects(:mount).in_sequence(syncorder)
-    @mount.expects(:flush).in_sequence(syncorder) # Call flush cause we changed everything
-
-    @mount[:ensure] = :mounted
-    @catalog.apply
-  end
-
   it "should umount before flushing changes to disk" do
     syncorder = sequence('syncorder')
     @mount.provider.expects(:options).returns 'soft'

-- 
Puppet packaging for Debian