[PATCH/puppet 1/1] (#7188) Add a mounttab type and provider

46 views
Skip to first unread message

Stefan Schulte

unread,
May 4, 2011, 6:47:05 PM5/4/11
to puppe...@googlegroups.com
Add a new type mounttab that does only ensure present/absent in fstab.
This is a much more simpler type than the original mounttype.

A few other things were changed, too:

* property options is an array now
this means that options cannot be specified as a comma separated
list and it allows us to treat options in sync if just the order
of options is different.
* defaultvalue for the pass-property is now '-' on Solaris. It is still
0 on other OS
* defaultvalue for blockdevice is now '-' if fstype is NFS (#4689)
* property atboot now doesnt only support yes/no but also true/false
(and yes/no will still be put in vfstab)
* no property supports whitespace so we wont write currupt fstab
entries to disk (#6409)

Signed-off-by: Stefan Schulte <stefan....@taunusstein.net>
---
Local-branch: feature/2.7.x/mounttab
lib/puppet/provider/mounttab/parsed.rb | 36 ++++
lib/puppet/type/mounttab.rb | 139 +++++++++++++
spec/unit/provider/mounttab/parsed_spec.rb | 105 ++++++++++
spec/unit/type/mounttab_spec.rb | 295 ++++++++++++++++++++++++++++
4 files changed, 575 insertions(+), 0 deletions(-)
create mode 100755 lib/puppet/provider/mounttab/parsed.rb
create mode 100755 lib/puppet/type/mounttab.rb
create mode 100755 spec/unit/provider/mounttab/parsed_spec.rb
create mode 100755 spec/unit/type/mounttab_spec.rb

diff --git a/lib/puppet/provider/mounttab/parsed.rb b/lib/puppet/provider/mounttab/parsed.rb
new file mode 100755
index 0000000..740c413
--- /dev/null
+++ b/lib/puppet/provider/mounttab/parsed.rb
@@ -0,0 +1,36 @@
+require 'puppet/provider/parsedfile'
+
+fstab = case Facter.value(:operatingsystem)
+when "Solaris"
+ "/etc/vfstab"
+else
+ "/etc/fstab"
+end
+
+Puppet::Type.type(:mounttab).provide(:parsed, :parent => Puppet::Provider::ParsedFile, :default_target => fstab, :filetype => :flat) do
+
+ case Facter.value(:operatingsystem)
+ when "Solaris"
+ @fields = [:device, :blockdevice, :name, :fstype, :pass, :atboot, :options]
+ else
+ @fields = [:device, :name, :fstype, :options, :dump, :pass]
+ @fielddefaults = [ nil ] * 4 + [ "0", "2" ]
+ end
+
+ text_line :comment, :match => /^\s*#/
+ text_line :blank, :match => /^\s*$/
+
+ optional_fields = @fields - [:device, :name, :blockdevice]
+ mandatory_fields = @fields - optional_fields
+
+ # fstab will ignore lines that have fewer than the mandatory number of columns,
+ # so we should, too.
+ field_pattern = '(\s*(?>\S+))'
+ text_line :incomplete, :match => /^(?!#{field_pattern}{#{mandatory_fields.length}})/
+
+ record_line :parsed,
+ :fields => @fields,
+ :separator => /\s+/,
+ :joiner => "\t",
+ :optional => optional_fields
+end
diff --git a/lib/puppet/type/mounttab.rb b/lib/puppet/type/mounttab.rb
new file mode 100755
index 0000000..940d879
--- /dev/null
+++ b/lib/puppet/type/mounttab.rb
@@ -0,0 +1,139 @@
+require 'puppet/property/list'
+require 'puppet/provider/parsedfile'
+
+module Puppet
+ newtype(:mounttab) do
+ @doc = "Manages entries in the filesystem table."
+
+ ensurable
+
+ newproperty(:device) do
+ desc "The device providing the mount. This can be whatever
+ device is supporting by the mount, including network
+ devices or devices specified by UUID rather than device
+ path, depending on the operating system."
+
+ validate do |value|
+ raise Puppet::Error, "device must not contain whitespace: #{value}" if value =~ /\s/
+ end
+ end
+
+ newproperty(:blockdevice) do
+ desc "The device to fsck. This is property is only valid
+ on Solaris, and in most cases will default to the correct
+ value."
+
+ defaultto do
+ if Facter.value(:operatingsystem) == "Solaris"
+ if device = @resource[:device] and device =~ %r{/dsk/}
+ device.sub(%r{/dsk/}, "/rdsk/")
+ elsif fstype = resource[:fstype] and fstype.downcase == 'nfs'
+ '-'
+ else
+ nil
+ end
+ else
+ nil
+ end
+ end
+
+ validate do |value|
+ raise Puppet::Error, "blockdevice must not contain whitespace: #{value}" if value =~ /\s/
+ end
+
+ end
+
+ newproperty(:fstype) do
+ desc "The mount type. Valid values depend on the
+ operating system. This is a required option."
+
+ validate do |value|
+ raise Puppet::Error, "fstype must not contain whitespace: #{value}" if value =~ /\s/
+ end
+ end
+
+ newproperty(:options, :parent => Puppet::Property::List) do
+ desc "Mount options for the mount. More than one option should
+ be specified as an array"
+
+ def delimiter
+ ","
+ end
+
+ def inclusive?
+ true
+ end
+
+ validate do |value|
+ raise Puppet::Error, "multiple options have to be specified as an array not a comma separated list" if value =~ /,/
+ raise Puppet::Error, "option must not contain whitespace: #{value}" if value =~ /\s/
+ end
+
+ end
+
+ newproperty(:pass) do
+ desc "The pass in which the mount is checked."
+
+ defaultto do
+ if resource.managed?
+ if Facter.value(:operatingsystem) == 'Solaris'
+ '-'
+ else
+ 0
+ end
+ end
+ end
+ end
+
+ newproperty(:atboot) do
+ desc "Whether to mount the mount at boot. Not all platforms
+ support this."
+
+ newvalues :yes, :no
+ aliasvalue :true, :yes
+ aliasvalue :false, :no
+ end
+
+ newproperty(:dump) do
+ desc "Whether to dump the mount. Not all platform support this.
+ Valid values are `1` or `0`. or `2` on FreeBSD, Default is `0`."
+
+ if Facter.value(:operatingsystem) == "FreeBSD"
+ newvalue(%r{(0|1|2)})
+ else
+ newvalue(%r{(0|1)})
+ end
+
+ defaultto do
+ 0 if resource.managed?
+ end
+
+ end
+
+ newproperty(:target) do
+ desc "The file in which to store the mount table. Only used by
+ those providers that write to disk."
+
+ defaultto do
+ if @resource.class.defaultprovider.ancestors.include?(Puppet::Provider::ParsedFile)
+ @resource.class.defaultprovider.default_target
+ else
+ nil
+ end
+ end
+ end
+
+ newparam(:name) do
+ desc "The mount path for the mount."
+
+ isnamevar
+
+ validate do |value|
+ raise Puppet::Error, "mount name must not contain whitespace: #{value}" if value =~ /\s/
+ # Except of root / a trailing slash can cause problems
+ raise Puppet::Error, "mount should be specified without a trailing slash: #{value}" if value =~ /.+\/$/
+ end
+ end
+
+ end
+end
diff --git a/spec/unit/provider/mounttab/parsed_spec.rb b/spec/unit/provider/mounttab/parsed_spec.rb
new file mode 100755
index 0000000..7b25e47
--- /dev/null
+++ b/spec/unit/provider/mounttab/parsed_spec.rb
@@ -0,0 +1,105 @@
+#!/usr/bin/env ruby
+
+require 'spec_helper'
+
+describe Puppet::Type.type(:mounttab).provider(:parsed) do
+
+ before :each do
+ @mounttab_class = Puppet::Type.type(:mounttab)
+ @provider = @mounttab_class.provider(:parsed)
+ @provider.stubs(:suitable?).returns true
+ end
+
+ # LAK:FIXME I can't mock Facter because this test happens at parse-time.
+ it "should default to /etc/vfstab on Solaris" do
+ pending "This test only works on Solaris" unless Facter.value(:operatingsystem) == 'Solaris'
+ @provider.default_target.should == '/etc/vfstab'
+ end
+
+ it "should default to /etc/fstab on anything else" do
+ pending "This test does not work on Solaris" if Facter.value(:operatingsystem) == 'Solaris'
+ @provider.default_target.should == '/etc/fstab'
+ end
+
+ describe "when parsing a line" do
+
+ it "should not crash on incomplete lines in fstab" do
+ parse = @provider.parse <<-FSTAB
+/dev/incomplete
+/dev/device name
+FSTAB
+ lambda{ @provider.to_line(parse[0]) }.should_not raise_error
+ end
+
+
+ describe "on Solaris", :if => Facter.value(:operatingsystem) == 'Solaris' do
+
+ before :each do
+ @example_line = "/dev/dsk/c0d0s0 /dev/rdsk/c0d0s0 \t\t / \t ufs 1 no\t-"
+ end
+
+ it "should extract device from the first field" do
+ @provider.parse_line(@example_line)[:device].should == '/dev/dsk/c0d0s0'
+ end
+
+ it "should extract blockdevice from second field" do
+ @provider.parse_line(@example_line)[:blockdevice].should == "/dev/rdsk/c0d0s0"
+ end
+
+ it "should extract name from third field" do
+ @provider.parse_line(@example_line)[:name].should == "/"
+ end
+
+ it "should extract fstype from fourth field" do
+ @provider.parse_line(@example_line)[:fstype].should == "ufs"
+ end
+
+ it "should extract pass from fifth field" do
+ @provider.parse_line(@example_line)[:pass].should == "1"
+ end
+
+ it "should extract atboot from sixth field" do
+ @provider.parse_line(@example_line)[:atboot].should == "no"
+ end
+
+ it "should extract options from seventh field" do
+ @provider.parse_line(@example_line)[:options].should == "-"
+ end
+
+ end
+
+ describe "on other platforms than Solaris", :if => Facter.value(:operatingsystem) != 'Solaris' do
+
+ before :each do
+ @example_line = "/dev/vg00/lv01\t/spare \t \t ext3 defaults\t1 2"
+ end
+
+ it "should extract device from the first field" do
+ @provider.parse_line(@example_line)[:device].should == '/dev/vg00/lv01'
+ end
+
+ it "should extract name from second field" do
+ @provider.parse_line(@example_line)[:name].should == "/spare"
+ end
+
+ it "should extract fstype from third field" do
+ @provider.parse_line(@example_line)[:fstype].should == "ext3"
+ end
+
+ it "should extract options from fourth field" do
+ @provider.parse_line(@example_line)[:options].should == "defaults"
+ end
+
+ it "should extract dump from fifth field" do
+ @provider.parse_line(@example_line)[:dump].should == "1"
+ end
+
+ it "should extract options from sixth field" do
+ @provider.parse_line(@example_line)[:pass].should == "2"
+ end
+
+ end
+
+ end
+
+end
diff --git a/spec/unit/type/mounttab_spec.rb b/spec/unit/type/mounttab_spec.rb
new file mode 100755
index 0000000..ca4bb6a
--- /dev/null
+++ b/spec/unit/type/mounttab_spec.rb
@@ -0,0 +1,295 @@
+#!/usr/bin/env ruby
+
+require 'spec_helper'
+
+describe Puppet::Type.type(:mounttab) do
+
+ before do
+ @class = described_class
+ @provider_class = @class.provide(:fake) { mk_resource_methods }
+ @provider = @provider_class.new
+ @resource = stub 'resource', :resource => nil, :provider => @provider
+
+ @class.stubs(:defaultprovider).returns @provider_class
+ @class.any_instance.stubs(:provider).returns @provider
+ end
+
+ it "should have :name as its keyattribute" do
+ @class.key_attributes.should == [:name]
+ end
+
+ describe "when validating attributes" do
+
+ [:name, :provider].each do |param|
+ it "should have a #{param} parameter" do
+ @class.attrtype(param).should == :param
+ end
+ end
+
+ [:ensure, :device, :blockdevice, :fstype, :options, :pass, :dump, :atboot, :target].each do |param|
+ it "should have a #{param} property" do
+ @class.attrtype(param).should == :property
+ end
+ end
+
+ end
+
+ describe "when validating values" do
+
+ describe "for name" do
+
+ it "should support absolute paths" do
+ proc { @class.new(:name => "/foo", :ensure => :present) }.should_not raise_error
+ end
+
+ it "should not support whitespace" do
+ proc { @class.new(:name => "/foo bar", :ensure => :present) }.should raise_error(Puppet::Error, /name.*whitespace/)
+ end
+
+ it "should not allow trailing slashes" do
+ proc { @class.new(:name => "/foo/", :ensure => :present) }.should raise_error(Puppet::Error, /mount should be specified without a trailing slash/)
+ proc { @class.new(:name => "/foo//", :ensure => :present) }.should raise_error(Puppet::Error, /mount should be specified without a trailing slash/)
+ end
+
+ end
+
+ describe "for ensure" do
+ it "should support present as a value for ensure" do
+ proc { @class.new(:name => "/foo", :ensure => :present) }.should_not raise_error
+ end
+
+ it "should support absent as a value for ensure" do
+ proc { @class.new(:name => "/foo", :ensure => :absent) }.should_not raise_error
+ end
+ end
+
+ describe "for device" do
+
+ it "should support normal /dev paths for device" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => '/dev/hda1') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => '/dev/dsk/c0d0s0') }.should_not raise_error
+ end
+
+ it "should support labels for device" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => 'LABEL=/boot') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => 'LABEL=SWAP-hda6') }.should_not raise_error
+ end
+
+ it "should support pseudo devices for device" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => 'ctfs') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => 'swap') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => 'sysfs') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => 'proc') }.should_not raise_error
+ end
+
+ it "should not support whitespace in device" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => '/dev/my dev/foo') }.should raise_error Puppet::Error, /device.*whitespace/
+ proc { @class.new(:name => "/foo", :ensure => :present, :device => "/dev/my\tdev/foo") }.should raise_error Puppet::Error, /device.*whitespace/
+ end
+
+ end
+
+ describe "for blockdevice" do
+
+ before :each do
+ Facter.stubs(:value).with(:operatingsystem).returns 'Solaris'
+ end
+
+ it "should support normal /dev/rdsk paths for blockdevice" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => '/dev/rdsk/c0d0s0') }.should_not raise_error
+ end
+
+ it "should support a dash for blockdevice" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => '-') }.should_not raise_error
+ end
+
+ it "should not support whitespace in blockdevice" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => '/dev/my dev/foo') }.should raise_error Puppet::Error, /blockdevice.*whitespace/
+ proc { @class.new(:name => "/foo", :ensure => :present, :blockdevice => "/dev/my\tdev/foo") }.should raise_error Puppet::Error, /blockdevice.*whitespace/
+ end
+
+ it "should default to /dev/rdsk/DEVICE if device is /dev/dsk/DEVICE" do
+ obj = @class.new(:name => "/foo", :device => '/dev/dsk/c0d0s0')
+ obj[:blockdevice].should == '/dev/rdsk/c0d0s0'
+ end
+
+ it "should default to - if it is an nfs-share" do
+ obj = @class.new(:name => "/foo", :device => "server://share", :fstype => 'nfs')
+ obj[:blockdevice].should == '-'
+ end
+
+ it "should have no default otherwise" do
+ @class.new(:name => "/foo")[:blockdevice].should == nil
+ @class.new(:name => "/foo", :device => "/foo")[:blockdevice].should == nil
+ end
+
+ it "should overwrite any default if blockdevice is explicitly set" do
+ @class.new(:name => "/foo", :device => '/dev/dsk/c0d0s0', :blockdevice => '/foo')[:blockdevice].should == '/foo'
+ @class.new(:name => "/foo", :device => "server://share", :fstype => 'nfs', :blockdevice => '/foo')[:blockdevice].should == '/foo'
+ end
+
+ end
+
+ describe "for fstype" do
+
+ it "should support valid fstypes" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'ext3') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'proc') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'sysfs') }.should_not raise_error
+ end
+
+ it "should support auto as a special fstype" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'auto') }.should_not raise_error
+ end
+
+ it "should not support whitespace in fstype" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :fstype => 'ext 3') }.should raise_error Puppet::Error, /fstype.*whitespace/
+ end
+
+ end
+
+ describe "for options" do
+
+ it "should support a single option" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :options => 'ro') }.should_not raise_error
+ end
+
+ it "should support muliple options as an array" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :options => ['ro','rsize=4096']) }.should_not raise_error
+ end
+
+ it "should support an empty array as options" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :options => []) }.should_not raise_error
+ end
+
+ it "should not support a comma separated option" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :options => ['ro','foo,bar','intr']) }.should raise_error Puppet::Error, /option.*have to be specified as an array/
+ end
+
+ it "should not support blanks in options" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :options => ['ro','foo bar','intr']) }.should raise_error Puppet::Error, /option.*whitespace/
+ end
+
+ end
+
+ describe "for pass" do
+
+ it "should support numeric values" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :pass => '0') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :pass => '1') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :present, :pass => '2') }.should_not raise_error
+ end
+
+ it "should support - on Solaris" do
+ Facter.stubs(:value).with(:operatingsystem).returns 'Solaris'
+ proc { @class.new(:name => "/foo", :ensure => :present, :pass => '-') }.should_not raise_error
+ end
+
+ it "should default to 0 on non Solaris" do
+ Facter.stubs(:value).with(:operatingsystem).returns 'HP-UX'
+ @class.new(:name => "/foo", :ensure => :present)[:pass].should == 0
+ end
+
+ it "should default to - on Solaris" do
+ Facter.stubs(:value).with(:operatingsystem).returns 'Solaris'
+ @class.new(:name => "/foo", :ensure => :present)[:pass].should == '-'
+ end
+
+ end
+
+ describe "for dump" do
+
+ it "should support 0 as a value for dump" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :dump => '0') }.should_not raise_error
+ end
+
+ it "should support 1 as a value for dump" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :dump => '1') }.should_not raise_error
+ end
+
+ # stefan: Looks like I'm unable to stub facter here
+ it "should support 2 as a value for dump on FreeBSD", :if => Facter.value(:operatingsystem) == 'FreeBSD' do
+ proc { @class.new(:name => "/foo", :ensure => :present, :dump => '2') }.should_not raise_error
+ end
+
+ # stefan: Looks like I'm unable to stub facter here
+ it "should not support 2 as a value for dump when not on FreeBSD", :if => Facter.value(:operatingsystem) != 'FreeBSD' do
+ proc { @class.new(:name => "/foo", :ensure => :present, :dump => '2') }.should raise_error Puppet::Error, /Invalid value/
+ end
+
+ it "should default to 0" do
+ @class.new(:name => "/foo", :ensure => :present)[:dump].should == 0
+ end
+
+ end
+
+ describe "for atboot" do
+
+ it "should support true as a value for atboot" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :true) }.should_not raise_error
+ end
+
+ it "should support false as a value for atboot" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :false) }.should_not raise_error
+ end
+
+ it "should support yes as a value for atboot" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :yes) }.should_not raise_error
+ end
+
+ it "should support no as a value for atboot" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :no) }.should_not raise_error
+ end
+
+ it "should alias true to yes" do
+ @class.new(:name => "/foo", :ensure => :present, :atboot => :true)[:atboot].should == :yes
+ end
+
+ it "should alias false to no" do
+ @class.new(:name => "/foo", :ensure => :present, :atboot => :false)[:atboot].should == :no
+ end
+
+ it "should not support other values for atboot" do
+ proc { @class.new(:name => "/foo", :ensure => :present, :atboot => :please_dont) }.should raise_error Puppet::Error, /Invalid value/
+ end
+
+ end
+
+ end
+
+ describe "when syncing options" do
+
+ before :each do
+ @options = @class.attrclass(:options).new(:resource => @resource, :should => %w{rw rsize=2048 wsize=2048})
+ end
+
+ it "should pass the sorted joined array to the provider" do
+ @provider.expects(:options=).with('rsize=2048,rw,wsize=2048')
+ @options.sync
+ end
+
+ it "should report out of sync if one option is missing" do
+ @options.insync?(%w{rw rsize=2048}).should == false
+ end
+
+ it "should report out of sync if there is an unwanted option" do
+ @options.insync?(%w{rw rsize=2048 wsize=2048 intr}).should == false
+ end
+
+ it "should report out of sync if at least one option is incorrect" do
+ @options.insync?(%w{rw rsize=1024 wsize=2048}).should == false
+ end
+
+ it "should not care about the order of options" do
+ @options.insync?(%w{rw rsize=2048 wsize=2048}).should == true
+ @options.insync?(%w{rw wsize=2048 rsize=2048}).should == true
+ @options.insync?(%w{rsize=2048 rw wsize=2048}).should == true
+ @options.insync?(%w{rsize=2048 wsize=2048 rw}).should == true
+ @options.insync?(%w{wsize=2048 rw rsize=2048}).should == true
+ @options.insync?(%w{wsize=2048 rsize=2048 rw}).should == true
+ @options.insync?(%w{rw rsize=2048 wsize=2048}).should == true
+ end
+
+ end
+
+end
--
1.7.5.rc3

Stefan Schulte

unread,
May 23, 2011, 4:30:06 PM5/23/11
to puppe...@googlegroups.com

As requested my attempt for a mountpoint type/provider for the splitted
mount type. The type should be usable but the provider is probably not
and doesnt have any tests yet.

One thought about a mountpoint provider in general:

How to handle mount options? I we specify options as parameters then
we don't detect if a device is mounted with wrong options.

If we want to handle options as properties I can see two possible
solutions:

1) handle options as an array.

Drawback:
Retrieving options in a way that they match the specified options can
become quite hard because mounting with '-o default' will become 'rw' when
running mount afterwards. Or you can see options when running mount that
are probably not even valid with mount -o option. E.g. when running a
Solaris zone you'll see a mountoption "zone=my_zone" when running mount but
you will never (and probably can't) specify such an option when running
mount -o options /dev/foo /foo

Possible Solution:
options are always treated in sync if AT LEAST the user's options appear
in the output. This means if the user specified

mountpoint {'/mnt/foo':
ensure => mounted,
device => '/dev/foo'
options => ['rw','rsize=8192'],
}

options can be 'rw,rsize=8192' or 'rw,rsize=8192,xattr' etc and are always
treated in sync. But then we have to be carefull that 'rsize=8192' and
'rsize=1024' are not two independet options. And if mount reveals that /mnt/foo
is currently mounted 'ro' we should not try to mount 'rw,rsize=8192,ro' because
rw and ro are also not independent.

2) Handle options as different properties
Only support a subset of possible options, handled by their own properties. A
possible mount definition would now look like

mountpoint { '/foo':
ensure => mounted,
device => '/dev/foo',
readonly => no,
suid => yes,
acl => no,
atime => no,
}

Drawback:
The user is limited to the options that the type supports.

-Stefan

Stefan Schulte

unread,
May 23, 2011, 4:30:07 PM5/23/11
to puppe...@googlegroups.com
Add a new type called mountpoint. This one should only ensure that a
device on a mountpoint is mounted/not mounted.

Valid properties are device, fstype and options but they are not
required because it's likely that there already is an fstab entry for
the described mount.

Mountoptions are currently always treated in sync because checking can
be very hard:
* plain mount command can show different options, e.g. to mount
read-only you say »mount -o ro« but running a plain mount will report
options as »read only« on Solaris.
* there may be options that show up when calling `mount` that you havent
specified explicitly. E.g mounting with `mount -o defaults` or without
any option will become 'rw' when running `mount` afterwards

Signed-off-by: Stefan Schulte <stefan....@taunusstein.net>
---

Local-branch: feature/next/mounttab
lib/puppet/type/mountpoint.rb | 124 +++++++++++++++++++++
spec/unit/type/mountpoint_spec.rb | 220 +++++++++++++++++++++++++++++++++++++
2 files changed, 344 insertions(+), 0 deletions(-)
create mode 100755 lib/puppet/type/mountpoint.rb
create mode 100755 spec/unit/type/mountpoint_spec.rb

diff --git a/lib/puppet/type/mountpoint.rb b/lib/puppet/type/mountpoint.rb
new file mode 100755
index 0000000..171d683
--- /dev/null
+++ b/lib/puppet/type/mountpoint.rb
@@ -0,0 +1,124 @@
+require 'puppet/property/list'
+module Puppet
+ newtype(:mountpoint) do
+ @doc = "Manages mounted filesystems, but does not create any information
+ in the fstab. You have to use the mounttab type for that. The type is
+ currently not able to manage mount options. So if you want to have non-
+ default mountoptions make sure you also use the mounttab type to create
+ an fstab entry first.
+
+ Note that if a `mountpoint` receives an event from another resource,
+ it will try to remount the filesystems if it is currently mounted. Depending
+ on the operating system, the provider and the value of the remount parameter
+ this can be a simple mount -o remount or mount/remount."
+
+ feature :refreshable, "The provider can remount the filesystem.",
+ :methods => [:remount]
+
+ newproperty(:ensure) do
+ desc "Control wether the mountpoint should be mounted or not. You can
+ use `mounted` to mount the device or `unmounted` to make sure that
+ no device is mounted on the specified mountpoint"
+
+ newvalue :mounted
+ newvalue :unmounted
+
+ end
+


+ newproperty(:device) do
+ desc "The device providing the mount. This can be whatever
+ device is supporting by the mount, including network
+ devices or devices specified by UUID rather than device

+ path, depending on the operating system. If you already have an entry
+ in your fstab (or you use the mounttab type to create such an entry),
+ it is generally not necessary to specify the device explicitly"


+
+ validate do |value|
+ raise Puppet::Error, "device must not contain whitespace: #{value}" if value =~ /\s/
+ end
+

+ end
+
+ newproperty(:fstype) do
+ desc "The mount type. Valid values depend on the

+ operating system. If you already have an entry in your fstab (or you use
+ the mounttab type to create such an entry), it is generally not necessary to
+ specify the fstype explicitly"


+
+ validate do |value|
+ raise Puppet::Error, "fstype must not contain whitespace: #{value}" if value =~ /\s/
+ end
+
+ end
+
+ newproperty(:options, :parent => Puppet::Property::List) do

+ desc "Mount options for the mount. This property is currently just used
+ when mounting the device. It is always treated as in sync so if the
+ desired device is already mounted but mounted with wrong options,
+ puppet will not correct it. This limitation is there because options
+ reported by the mount command are often different from the options
+ you may find in fstab"
+
+ def insync?(is = nil)


+ true
+ end
+
+ def delimiter
+ ","
+ end
+
+ def inclusive?
+ true
+ end
+
+ validate do |value|
+ raise Puppet::Error, "multiple options have to be specified as an array not a comma separated list" if value =~ /,/

+ raise Puppet::Error, "options must not contain whitespace: #{value}" if value =~ /\s/


+ end
+
+ end
+

+ newparam(:name) do
+ desc "The mount path for the mount."
+
+ isnamevar
+
+ validate do |value|
+ raise Puppet::Error, "mount name must not contain whitespace: #{value}" if value =~ /\s/

+ # Except of root / a trailing slash can cause problems (#6793)


+ raise Puppet::Error, "mount should be specified without a trailing slash: #{value}" if value =~ /.+\/$/
+ end
+

+ end
+
+ newparam(:remounts) do
+ desc "Whether the mount can be remounted with `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 "FreeBSD", "Darwin", "AIX"
+ :false
+ when "Solaris", "HP-UX"
+ if fstype = @resource[:fstype] and fstype.downcase == 'nfs'
+ # According to mount_nfs, remount can just change options from ro to rw
+ # but nothing more
+ :false
+ else
+ :true
+ end
+ else
+ :true
+ end


+ end
+ end
+

+ def refresh
+ # Only remount if we're supposed to be mounted.
+ provider.remount if provider.get(:fstype) != "swap" and provider.get(:ensure) == :mounted


+ end
+
+ end
+end

diff --git a/spec/unit/type/mountpoint_spec.rb b/spec/unit/type/mountpoint_spec.rb
new file mode 100755
index 0000000..2c40519
--- /dev/null
+++ b/spec/unit/type/mountpoint_spec.rb
@@ -0,0 +1,220 @@


+#!/usr/bin/env ruby
+
+require 'spec_helper'
+

+describe Puppet::Type.type(:mountpoint) do


+
+ before do
+ @class = described_class
+ @provider_class = @class.provide(:fake) { mk_resource_methods }
+ @provider = @provider_class.new
+ @resource = stub 'resource', :resource => nil, :provider => @provider
+
+ @class.stubs(:defaultprovider).returns @provider_class
+ @class.any_instance.stubs(:provider).returns @provider
+ end
+
+ it "should have :name as its keyattribute" do
+ @class.key_attributes.should == [:name]
+ end
+
+ describe "when validating attributes" do
+

+ [:name, :provider, :remounts].each do |param|


+ it "should have a #{param} parameter" do
+ @class.attrtype(param).should == :param
+ end
+ end
+

+ [:ensure, :device, :fstype, :options ].each do |property|
+ it "should have a #{property} property" do
+ @class.attrtype(property).should == :property


+ end
+ end
+
+ end
+
+ describe "when validating values" do
+
+ describe "for name" do
+
+ it "should support absolute paths" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted) }.should_not raise_error
+ end
+
+ it "should support root /" do
+ proc { @class.new(:name => "/", :ensure => :mounted) }.should_not raise_error


+ end
+
+ it "should not support whitespace" do

+ proc { @class.new(:name => "/foo bar", :ensure => :mounted) }.should raise_error(Puppet::Error, /name.*whitespace/)


+ end
+
+ it "should not allow trailing slashes" do

+ proc { @class.new(:name => "/foo/", :ensure => :mounted) }.should raise_error(Puppet::Error, /mount should be specified without a trailing slash/)
+ proc { @class.new(:name => "/foo//", :ensure => :mounted) }.should raise_error(Puppet::Error, /mount should be specified without a trailing slash/)


+ end
+
+ end
+
+
+ describe "for ensure" do
+

+ it "should support mounted as a value for ensure" do
+ proc { @class.new(:name => "/foo", :ensure => :mounted) }.should_not raise_error
+ end
+
+ it "should support unmounted as a value for ensure" do
+ proc { @class.new(:name => "/foo", :ensure => :unmounted) }.should_not raise_error
+ end
+


+ end
+
+ describe "for device" do
+
+ it "should support normal /dev paths for device" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => '/dev/hda1') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => '/dev/dsk/c0d0s0') }.should_not raise_error


+ end
+
+ it "should support labels for device" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => 'LABEL=/boot') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => 'LABEL=SWAP-hda6') }.should_not raise_error


+ end
+
+ it "should support pseudo devices for device" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => 'ctfs') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => 'swap') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => 'sysfs') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => 'proc') }.should_not raise_error


+ end
+
+ it "should not support whitespace in device" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => '/dev/my dev/foo') }.should raise_error Puppet::Error, /device.*whitespace/
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :device => "/dev/my\tdev/foo") }.should raise_error Puppet::Error, /device.*whitespace/


+ end
+
+ end
+

+ describe "for fstype" do
+
+ it "should support valid fstypes" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :fstype => 'ext3') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :fstype => 'proc') }.should_not raise_error
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :fstype => 'sysfs') }.should_not raise_error


+ end
+
+ it "should support auto as a special fstype" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :fstype => 'auto') }.should_not raise_error


+ end
+
+ it "should not support whitespace in fstype" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :fstype => 'ext 3') }.should raise_error Puppet::Error, /fstype.*whitespace/


+ end
+
+ end
+
+ describe "for options" do
+
+ it "should support a single option" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :options => 'ro') }.should_not raise_error


+ end
+
+ it "should support muliple options as an array" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :options => ['ro','rsize=4096']) }.should_not raise_error


+ end
+
+ it "should support an empty array as options" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :options => []) }.should_not raise_error


+ end
+
+ it "should not support a comma separated option" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :options => ['ro','foo,bar','intr']) }.should raise_error Puppet::Error, /option.*have to be specified as an array/


+ end
+
+ it "should not support blanks in options" do

+ proc { @class.new(:name => "/foo", :ensure => :mounted, :options => ['ro','foo bar','intr']) }.should raise_error Puppet::Error, /option.*whitespace/


+ end
+
+ end
+

+ describe "for remounts" do
+
+ it "should support true as a value for remounts" do
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :remounts => :true) }.should_not raise_error
+ end
+
+ it "should support false as value for remounts" do
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :remounts => :true) }.should_not raise_error
+ end
+
+ it "should not support anything else as a value" do
+ proc { @class.new(:name => "/foo", :ensure => :mounted, :remounts => :truu) }.should raise_error Puppet::Error, /remounts failed.*Invalid value/
+ end
+
+ ["FreeBSD", "Darwin","AIX"].each do |os|
+ describe "when on #{os}" do
+
+ it "should always default to false" do
+ Facter.stubs(:value).with(:operatingsystem).returns os
+ @class.new(:name => "/mnt/foo", :fstype => 'ext3', :ensure => :mounted)[:remounts].should == :false
+ @class.new(:name => "/mnt/foo", :fstype => 'nfs', :ensure => :mounted)[:remounts].should == :false
+ end
+


+ end
+ end
+

+ ["Solaris", "HP-UX"].each do |os|
+ describe "when on #{os}" do


+
+ before :each do

+ Facter.stubs(:value).with(:operatingsystem).returns os
+ end
+
+ it "should default to false for nfs mounts" do
+ @class.new(:name => "/mnt/foo", :fstype => 'nfs', :ensure => :mounted)[:remounts].should == :false
+ end
+
+ it "should default to true for other fstypes" do
+ @class.new(:name => "/mnt/foo", :fstype => 'ext3', :ensure => :mounted)[:remounts].should == :true


+ end
+
+ end

+ end
+
+ ["Linux"].each do |os|
+ describe "when on #{os}" do
+ it "should alway default to true" do
+ Facter.stubs(:value).with(:operatingsystem).returns os
+ @class.new(:name => "/mnt/foo", :fstype => 'ext3', :ensure => :mounted)[:remounts].should == :true
+ @class.new(:name => "/mnt/foo", :fstype => 'nfs', :ensure => :mounted)[:remounts].should == :true
+ end


+ end
+ end
+
+ end
+

+ end
+
+ describe "when syncing options" do
+
+ before :each do
+ @options = @class.attrclass(:options).new(:resource => @resource, :should => %w{rw rsize=2048 wsize=2048})
+ end
+
+ it "should pass the sorted joined array to the provider" do
+ @provider.expects(:options=).with('rsize=2048,rw,wsize=2048')
+ @options.sync
+ end
+

+ # Good look fixing this one ;-) -- stefan
+ it "should always treat options insync" do
+ @options.insync?(%w{rw rsize=2048}).should == true
+ @options.insync?(%w{rw rsize=2048 wsize=2048 intr}).should == true
+ @options.insync?(%w{rw rsize=1024 wsize=2048}).should == true
+ @options.insync?(%w{foo}).should == true

Stefan Schulte

unread,
May 23, 2011, 4:30:08 PM5/23/11
to puppe...@googlegroups.com
Add a provider called mount that uses the output of mount to prefetch
current mounted devices and uses mount/umount to perform changes.

It may not work on any other System than Linux and even on this platform
it is not really well tested so far. Especially prefetching options and
the fstype may fail.

Signed-off-by: Stefan Schulte <stefan....@taunusstein.net>
---

Local-branch: feature/next/mounttab
lib/puppet/provider/mountpoint/mount.rb | 135 +++++++++++++++++++++++++++++++
1 files changed, 135 insertions(+), 0 deletions(-)
create mode 100755 lib/puppet/provider/mountpoint/mount.rb

diff --git a/lib/puppet/provider/mountpoint/mount.rb b/lib/puppet/provider/mountpoint/mount.rb
new file mode 100755
index 0000000..c9ffea1
--- /dev/null
+++ b/lib/puppet/provider/mountpoint/mount.rb
@@ -0,0 +1,135 @@
+Puppet::Type.type(:mountpoint).provide(:mount) do
+
+ commands :mount => '/bin/mount'
+ commands :umount => '/bin/umount'
+
+ self::REGEX = case Facter.value(:operatingsystem)
+ when "Darwin"
+ /^(.*?) on (?:\/private\/var\/automount)?(\S*) \(([A-Za-z]*),?([^\)]*)\)$/
+ when "Solaris"
+ # when run mount -p: device blockdevice name fstype pass atboot option
+ /^(\S*)\s*\S*\s*(\S*)\s*(\S*)\s*\S*\s*\S*\s*(\S*)/
+ when "HP-UX"
+ # when run mount -p: device name fstype options dump pass
+ /^(\S*)\s*(\S*)\s*(\S*)\s*(\S*)/
+ else
+ /^(\S*) on (\S*) type (\S*) \((.*)\)$/
+ end
+ self::FIELDS = [:device,:name,:fstype,:options]
+
+ def self.parse_line(line)
+ if match = self::REGEX.match(line)
+ hash = {}
+ self::FIELDS.zip(match.captures) do |field,value|
+ if field == :options and Facter.value(:operatingsystem) == 'Darwin'
+ value.gsub!(/,\s*/,',') # Darwin adds spaces between options
+ end
+ hash[field] = value
+ hash[:ensure] = :mounted
+ Puppet.debug hash.inspect
+ end
+ hash
+ else
+ Puppet.warning "Failed to match mount line #{line}"


+ nil
+ end
+ end
+

+ def self.instances
+ args = []
+ if ['HP-UX','Solaris'].include?(Facter.value(:operatingsystem))
+ args << '-p' # output like fstab to be able to parse fstype
+ end
+ mountpoints = []
+ mount(*args).split("\n").each do |line|
+ if mountpoint = parse_line(line)
+ mountpoints << new(mountpoint)
+ end
+ end
+ mountpoints
+ end
+
+ def self.prefetch(mountpoints)
+ instances.each do |prov|
+ if mountpoint = mountpoints[prov.name]
+ mountpoint.provider = prov


+ end
+ end
+ end
+

+ def perform_mount
+ args = []
+ args << resource[:device] if resource[:device]
+ case Facter.value(:operatingsystem)
+ when 'HP-UX', 'Solaris'
+ args << '-F' << resource[:fstype] if resource[:fstype]
+ else
+ args << '-t' << resource[:fstype] if resource[:fstype]
+ end
+ args << '-o' << resource[:options] if resource[:options]
+ args << resource[:name]
+ mount *args
+ end
+
+ def perform_remount
+ options = "remount"
+ options += ",#{resource[:options]}" if resource[:options]
+ args = []
+ args << '-o' << options
+ args << resource[:name]
+ mount *args
+ end
+
+ def perform_umount
+ umount resource[:name]
+ end
+
+ remount_sufficient = [:options]
+ [:device, :fstype, :options].each do |property|
+ define_method(property) do
+ @property_hash[property] || :absent
+ end
+ define_method("#{property}=") do |val|
+ @property_hash[property] = val
+ if remount_sufficient.include?(property)
+ @flush_need_remount = true
+ else
+ @flush_need_umount_mount = true


+ end
+ end
+ end
+

+ def remount(tryremount = true)
+ info "Remounting"
+ if resource[:remounts] == :true and tryremount
+ perform_remount
+ else
+ perform_umount
+ perform_mount


+ end
+ end
+

+ def ensure
+ @property_hash[:ensure] || :unmounted
+ end
+
+ def ensure=(value)
+ if value == :mounted
+ perform_mount
+ else
+ perform_umount
+ end
+ @property_hash[:ensure] == value
+ end
+
+ def flush
+ if @flush_need_umount_mount
+ remount(false)
+ elsif @flush_need_remount
+ remount(true)
+ end
+ @flush_need_umount_mount = false
+ @flush_need_remount = false

Reply all
Reply to author
Forward
0 new messages