On Fri, Feb 18, 2011 at 03:40:11PM -0800, Jacob Helwig wrote:
> Previously the mount type would only check if anything was mounted at the
> desired point, when 'ensure => mounted' was specified.  Now we check not
> only whether something is mounted at the desired point, but also that it
> is the thing we wish to be mounted there.
> 
> There is also a chance that the mount point directory could be
> "automagically" removed for us, when unmounting incorrect devices, so we
> attempt to re-create the directory after unmounting to give the mount of
> the correct device a better chance at succeeding.
> 
> Paired-with: Matt Robinson <[email protected]>
> Paired-with: Nick Lewis <[email protected]>
> Paired-with: Jesse Wolfe <[email protected]>
> Signed-off-by: Jacob Helwig <[email protected]>
> ---
> 
> [...]
> +  # Is the desired thing mounted at this point?
> +  def correctly_mounted?
> +    platform = Facter.value("operatingsystem")
> +    name = resource[:name]
> +    device = resource[:device]
> +    mounts = mountcmd.split("\n").find do |line|
> +      case platform
> +      when "Darwin"
> +        line =~ /^#{device} on #{name} / or line =~ %r{^#{device} on 
> /private/var/automount#{name}}
> +      when "Solaris", "HP-UX"
> +        # Yes, Solaris does list mounts as "mount_point on device"
> +        line =~ /^#{name} on #{device}/
> +      when "AIX"
> +        line.split(/\s+/)[2] == name &&
> +          line.split(/\s+/)[1] == device
> +      else
> +        line =~ /^#{device} on #{name} /
> +      end
> +    end
> +  end
>  end

This can go wrong on Linux since mount does resolve symlinks. I saw a
bunch of RedHat Boxes with LVM that have /dev/diskgroup/volume in fstab
and mount shows /dev/mapper/diskgroup-volume. I'm not sure but it is
possible that this is even redhat/lvm default.

> diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
> index da9a70b..10eed53 100755
> --- a/lib/puppet/type/mount.rb
> +++ b/lib/puppet/type/mount.rb
> @@ -29,7 +29,7 @@ module Puppet
>        aliasvalue :present, :defined
>  
>        newvalue(:unmounted) do
> -        if provider.mounted?
> +        if provider.anything_mounted?
>            syncothers
>            provider.unmount
>            return :mount_unmounted
> @@ -40,20 +40,15 @@ module Puppet
>        end
>  
>        newvalue(:absent, :event => :mount_deleted) do
> -        provider.unmount if provider.mounted?
> +        provider.unmount if provider.anything_mounted?
>  
>          provider.destroy
>        end
>  
>        newvalue(:mounted, :event => :mount_mounted) do
> -        # Create the mount point if it does not already exist.
> -        current_value = self.retrieve
> -        provider.create if current_value.nil? or current_value == :absent
> -
>          syncothers
>  
> -        # The fs can be already mounted if it was absent but mounted
> -        provider.mount unless provider.mounted?
> +        provider.mount
>        end
>  
>        def insync?(is)
> @@ -70,7 +65,7 @@ module Puppet
>          curval = super()
>          if curval == :absent
>            return :absent
> -        elsif provider.mounted?
> +        elsif provider.correctly_mounted?
>            return :mounted
>          else
>            return :unmounted
> @@ -210,7 +205,7 @@ module Puppet
>  

This will resolve mounts that are not correctly mounted as `unmounted`.
In my opinion this is just wrong. If I now specify `ensure => unmounted`
nothing happens. In every other situation the mountpoint is the only key
attribute to identify a resource.


> [...]

-Stefan

Attachment: pgpgmkcNwt5D0.pgp
Description: PGP signature

Reply via email to