On Tue, 22 Feb 2011 18:24:11 +0100, Stefan Schulte wrote: > > 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?
*snip* > > + 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. > Ugh. Annoying, but that doesn't sound too terrible to deal with. We'll have to fully resolve any symlinks in the device specified for the resource. Does that sound sufficient to you, or am I grossly over-simplifying this? > > 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 *snip* > > @@ -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. > With the device unspecified in the resource, I'd agree that this is the wrong thing to do. If the device is specified however, I'd argue that it's the right thing to do, since it's stating "this device should not be mounted here". Either way, you're right: This needs to be changed (at the very least) to handle the case where the device is unspecified. -- Jacob Helwig
signature.asc
Description: Digital signature
