Mounts work again, at least with the parsedfile provider. I still need to create a netinfo provider, but it should be short and easy. And, painfully, I still need to port the other six or so subclasses to this new provider.

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

2 files changed, 133 insertions, 185 deletions

diff --git a/lib/puppet/provider/mount/parsed.rb b/lib/puppet/provider/mount/parsed.rb
index 086cd5cd9..af67ca6ff 100755
--- a/lib/puppet/provider/mount/parsed.rb
+++ b/lib/puppet/provider/mount/parsed.rb
@@ -1,150 +1,43 @@
 require 'puppet/provider/parsedfile'
 
-Puppet::Type.type(:mount).provide :parsed, :parent => Puppet::Provider::ParsedFile do
-    @filetype = Puppet::FileType.filetype(:flat)
-
-    commands :mountcmd => "mount", :umount => "umount", :df => "df"
-
-    def self.init
-        @platform = Facter["operatingsystem"].value
-        case @platform
-        when "Solaris":
-            @path = "/etc/vfstab"
-            @fields = [:device, :blockdevice, :path, :fstype, :pass, :atboot,
-                       :options]
-            @fielddefaults = [ nil ] * @fields.size
-        when "Darwin":
-            @filetype = Puppet::FileType.filetype(:netinfo)
-            @filetype.format = "fstab"
-            @path = "mounts"
-            @fields = [:device, :path, :fstype, :options, :dump, :pass]
-            @fielddefaults = [ nil ] * @fields.size
-
-            # How to map the dumped table to what we want
-            @fieldnames = {
-                "name" => :device,
-                "dir" => :path,
-                "dump_freq" => :dump,
-                "passno" => :pass,
-                "vfstype" => :fstype,
-                "opts" => :options
-            }
-        else
-            @path = "/etc/fstab"
-            @fields = [:device, :path, :fstype, :options, :dump, :pass]
-            @fielddefaults = [ nil ] * 4 + [ "0", "2" ]
-        end
-
-        # Allow Darwin to override the default filetype
-        unless defined? @filetype
-            @filetype = Puppet::FileType.filetype(:flat)
-        end
-    end
-
-    init
-
-    # XXX This needs to change to being a netinfo provider
-    unless @platform == "Darwin"
-        confine :exists => @path
-    end
-
-    def self.clear
-        init
-        super
-    end
-
-    # Parse a mount tab.
-    #
-    # This method also stores existing comments, and it stores all
-    # mounts in order, mostly so that comments are retained in the
-    # order they were written and in proximity to the same fses.
-    def self.parse(text)
-        # provide a single exception for darwin & netinfo
-        if @filetype == Puppet::FileType.filetype(:netinfo)
-            return parseninfo(text)
-        end
-        count = 0
-
-        instances = []
-        text.chomp.split("\n").each { |line|
-            hash = {}
-            case line
-            when /^#/, /^\s*$/:
-                # add comments and blank lines to the list as they are
-                instances << line
-                comment(line)
-            else
-                values = line.split(/\s+/)
-                if @fields.length < values.length
-                    raise Puppet::Error, "Could not parse line %s" % line
-                end
+fstab = nil
+case Facter.value(:operatingsystem)
+when "Solaris": fstab = "/etc/vfstab"
+else
+    fstab = "/etc/fstab"
+end
 
-                values = @fielddefaults.zip(values).collect { |d, v| v || d }
-                unless @fields.length == values.length
-                    raise Puppet::Error, "Could not parse line %s" % line
-                end
-                
-                @fields.zip(values).each do |field, value|
-                    hash[field] = value
-                end
+Puppet::Type.type(:mount).provide(:parsed,
+    :parent => Puppet::Provider::ParsedFile,
+    :default_target => fstab,
+    :filetype => :flat
+) do
 
-                instances << hash
-                count += 1
-            end
-        }
+    commands :mountcmd => "mount", :umount => "umount", :df => "df"
 
-        return instances
+    @platform = Facter["operatingsystem"].value
+    case @platform
+    when "Solaris":
+        @fields = [:device, :blockdevice, :name, :fstype, :pass, :atboot,
+               :options]
+    else
+        @fields = [:device, :name, :fstype, :options, :dump, :pass]
+        @fielddefaults = [ nil ] * 4 + [ "0", "2" ]
     end
 
-    # Parse a netinfo table.
-    def self.parseninfo(text)
-        array = @fileobj.to_array(text)
+    text_line :comment, :match => /^\s*#/
+    text_line :blank, :match => /^\s*$/
 
-        instances = []
-        array.each do |record|
-            hash = {}
-            @fieldnames.each do |name, field|
-                if value = record[name]
-                    if field == :options
-                        hash[field] = value.join(",")
-                    else
-                        hash[field] = value[0]
-                    end
-                else
-                    raise ArgumentError, "Field %s was not provided" % [name]
-                end
-            end
-
-            instances << hash
-        end
-
-        return instances
-    end
-
-    # Convert the current object into an fstab-style string.
-    def self.to_record(hash)
-        self.fields.collect do |field|
-            if value = hash[field]
-                value
-            else
-                raise Puppet::Error,
-                    "Could not retrieve value for %s" % field
-            end
-        end.join("\t")
-    end
+    record_line self.name, :fields => @fields, :separator => /\s+/, :joiner => "\t"
 
     # This only works when the mount point is synced to the fstab.
     def mount
-        mountcmd @model[:path]
+        mountcmd @model[:name]
     end
 
     # This only works when the mount point is synced to the fstab.
     def unmount
-        output = %x{#{command(:umount)} #{@model[:path]}}
-
-        unless $? == 0
-            raise Puppet::Error, "Could not unmount %s" % @model[:path]
-        end
+        umount @model[:name]
     end
 
     # Is the mount currently mounted?
@@ -156,13 +49,12 @@ Puppet::Type.type(:mount).provide :parsed, :parent => Puppet::Provider::ParsedFi
         when "Solaris": df = "#{command(:df)} -k"
         end
         %x{#{df}}.split("\n").find do |line|
-            p line
             fs = line.split(/\s+/)[-1]
             if platform == "Darwin"
-                fs == "/private/var/automount" + @model[:path] or
-                    fs == @model[:path]
+                fs == "/private/var/automount" + @model[:name] or
+                    fs == @model[:name]
             else
-                fs == @model[:path]
+                fs == @model[:name]
             end
         end
     end
diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index f9e98d96a..5539679d0 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -13,17 +13,34 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
     extend Puppet::Util::FileParsing
 
     class << self
-        attr_reader :filetype
         attr_accessor :default_target
     end
 
     attr_accessor :state_hash
 
+    def self.clean(hash)
+        newhash = hash.dup
+        [:record_type, :on_disk].each do |p|
+            if newhash.include?(p)
+                newhash.delete(p)
+            end
+        end
+
+        return newhash
+    end
+
     def self.clear
         @target_objects.clear
         @records.clear
     end
 
+    def self.filetype
+        unless defined? @filetype
+            @filetype = Puppet::FileType.filetype(:flat)
+        end
+        return @filetype
+    end
+
     def self.filetype=(type)
         if type.is_a?(Class)
             @filetype = type
@@ -34,25 +51,37 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         end
     end
 
-    # Flush all of the targets for which there are modified records.
+    # Flush all of the targets for which there are modified records.  The only
+    # reason we pass a record here is so that we can add it to the stack if
+    # necessary -- it's passed from the instance calling 'flush'.
     def self.flush(record)
-        return unless defined?(@modified) and ! @modified.empty?
-
         # Make sure this record is on the list to be flushed.
         unless record[:on_disk]
             record[:on_disk] = true
             @records << record
+
+            # If we've just added the record, then make sure our
+            # target will get flushed.
+            modified(record[:target] || default_target)
         end
 
+        return unless defined?(@modified) and ! @modified.empty?
+
+        flushed = []
         @modified.sort { |a,b| a.to_s <=> b.to_s }.uniq.each do |target|
             Puppet.debug "Flushing %s provider target %s" % [@model.name, target]
             flush_target(target)
+            flushed << target
         end
+
+        @modified.reject! { |t| flushed.include?(t) }
     end
 
     # Flush all of the records relating to a specific target.
     def self.flush_target(target)
-        target_object(target).write(to_file(target_records(target)))
+        target_object(target).write(to_file(target_records(target).reject { |r|
+            r[:ensure] == :absent
+        }))
     end
 
     # Return the header placed at the top of each generated file, warning
@@ -66,20 +95,43 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
     # Add another type var.
     def self.initvars
         @records = []
+        @target_objects = {}
 
         # Default to flat files
         @filetype = Puppet::FileType.filetype(:flat)
         super
     end
 
+    # Return a list of all of the records we can find.
+    def self.list
+        prefetch()
+        @records.find_all { |r| r[:record_type] == self.name }.collect { |r|
+            clean(r)
+        }
+    end
+
+    def self.list_by_name
+        list.collect { |r| r[:name] }
+    end
+
     # Create attribute methods for each of the model's non-metaparam attributes.
     def self.model=(model)
         [model.validstates, model.parameters].flatten.each do |attr|
-            define_method(attr) do @state_hash[attr] end
+            attr = symbolize(attr)
+            define_method(attr) do
+                # If it's not a valid field for this record type (which can happen
+                # when different platforms support different fields), then just
+                # return the should value, so the model shuts up.
+                if @state_hash[attr] or self.class.valid_attr?(self.class.name, attr)
+                    @state_hash[attr] || :absent
+                else
+                    @model.should(attr)
+                end
+            end
 
             define_method(attr.to_s + "=") do |val|
                 # Mark that this target was modified.
-                modeltarget = @model[:target]
+                modeltarget = @model[:target] || self.class.default_target
 
                 # If they're the same, then just mark that one as modified
                 if @state_hash[:target] and @state_hash[:target] == modeltarget
@@ -94,7 +146,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
                     end
                     @state_hash[:target] = modeltarget
                 end
-                @state_hash[attr] = val.to_s
+                @state_hash[attr] = val
             end
         end
         @model = model
@@ -104,7 +156,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
     # used within the attr= methods.
     def self.modified(target)
         @modified ||= []
-        @modified << target
+        @modified << target unless @modified.include?(target)
     end
 
     # Retrieve all of the data from disk.  There are three ways to know
@@ -127,9 +179,10 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         @records += retrieve(target).each do |r|
             r[:on_disk] = true
             r[:target] = target
+            r[:ensure] = :present
         end
 
-        # Retrieve the current state of this target.
+        # Set current state on any existing resource instances.
         target_records(target).find_all { |i| i.is_a?(Hash) }.each do |record|
             # Find any model instances whose names match our instances.
             if instance = self.model[record[:name]]
@@ -140,11 +193,14 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
                     instance.provider.state_hash = record
                 end
             end
-            # Any unmatched records are assumed to be unmanaged, so
-            # we just leave them alone.
         end
     end
 
+    # Is there an existing record with this name?
+    def self.record?(name)
+        @records.find { |r| r[:name] == name }
+    end
+
     # Retrieve the text for the file. Returns nil in the unlikely
     # event that it doesn't exist.
     def self.retrieve(path)
@@ -169,7 +225,6 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
 
     # Initialize the object if necessary.
     def self.target_object(target)
-        @target_objects ||= {}
         @target_objects[target] ||= @filetype.new(target)
 
         @target_objects[target]
@@ -198,55 +253,56 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
             targets << model[:target]
         end
 
-        targets.uniq
+        targets.uniq.reject { |t| t.nil? }
     end
 
-    # Write our data to disk.
-    def flush
-        # Make sure we've got a target and name set.
-        @state_hash[:target] ||= @model[:target]
-        @state_hash[:name] ||= @model[:name]
-
-        self.class.flush(@state_hash)
+    def create
+        @model.class.validstates.each do |state|
+            if value = @model.should(state)
+                @state_hash[state] = value
+            end
+        end
+        self.class.modified(@state_hash[:target] || self.class.default_target)
     end
 
-    def initialize(model)
-        super
-
-        # Initialize our data hash with the record type.  The record type
-        # in the subclass has to create a record matching its name.
-        @state_hash = {:record_type => self.class.name}
+    def destroy
+        # We use the method here so it marks the target as modified.
+        self.ensure = :absent
     end
 
-    # Have we been modified since the last flush?
-    def modified?
-        @state[:flush_needed]
+    def exists?
+        if @state_hash[:ensure] == :absent or @state_hash[:ensure].nil?
+            return false
+        else
+            return true
+        end
     end
 
-    def store(hash = nil)
-        hash ||= self.model.to_hash
+    # Write our data to disk.
+    def flush
+        # Make sure we've got a target and name set.
 
-        unless @instances
-            self.hash
+        # If the target isn't set, then this is our first modification, so
+        # mark it for flushing.
+        unless @state_hash[:target]
+            @state_hash[:target] = @model[:target] || self.class.default_target
+            self.class.modified(@state_hash[:target])
         end
+        @state_hash[:name] ||= @model.name
 
-        if hash.empty? 
-            @me.clear
-        else
-            hash.each do |name, value|
-                if @me[name] != hash[name]
-                    @me[name] = hash[name]
-                end
-            end
+        self.class.flush(@state_hash)
+    end
 
-            @me.each do |name, value|
-                unless hash.has_key? name
-                    @me.delete(name)
-                end
-            end
-        end
+    def initialize(model)
+        super
 
-        self.class.store(@instances)
+        # See if there's already a matching state_hash in the records list;
+        # else, use a default value.
+        # We provide a default for 'ensure' here, because the provider will
+        # override it if the thing exists, but it won't touch it if it doesn't
+        # exist.
+        @state_hash = self.class.record?(model[:name]) ||
+            {:record_type => self.class.name, :ensure => :absent}
     end
 end