Fixing cron support (I hope). It now uses providers, and seems to work, at least on my os x box.

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

3 files changed, 130 insertions, 26 deletions

diff --git a/lib/puppet/provider/cron/crontab.rb b/lib/puppet/provider/cron/crontab.rb
index c50bc2680..b7caa4a21 100755
--- a/lib/puppet/provider/cron/crontab.rb
+++ b/lib/puppet/provider/cron/crontab.rb
@@ -6,49 +6,130 @@ tab = case Facter.value(:operatingsystem)
         :crontab
     end
 
+
 Puppet::Type.type(:cron).provide(:crontab,
     :parent => Puppet::Provider::ParsedFile,
+    :default_target => ENV["USER"],
     :filetype => tab
 ) do
     commands :crontab => "crontab"
 
-    attr_accessor :target
-
     text_line :comment, :match => %r{^#}, :post_parse => proc { |hash|
         if hash[:line] =~ /Puppet Name: (.+)\s*$/
             hash[:name] = $1
         end
     }
 
-    text_line :blank, :match => /^\s+/
+    text_line :blank, :match => %r{^\s+}
+
+    text_line :environment, :match => %r{^\w+=}
 
-    text_line :environment, :match => %r{/^\w+=/}
+    record_line :freebsd_special, :fields => %w{special command},
+        :match => %r{^@(\w+)\s+(.+)$}, :pre_gen => proc { |hash|
+            hash[:special] = "@" + hash[:special]
+        }
 
-    record_line :crontab, :fields => %w{minute hour weekday month monthday command},
+    crontab = record_line :crontab, :fields => %w{minute hour monthday month weekday command},
         :optional => %w{minute hour weekday month monthday}, :absent => "*"
 
-    #record_line :freebsd_special, :fields => %w{special command},
-    #    :match => %r{^@(\w+)\s+(.+)}
-    # Apparently Freebsd will "helpfully" add a new TZ line to every
-    # single cron line, but not in all cases (e.g., it doesn't do it
-    # on my machine.  This is my attempt to fix it so the TZ lines don't
-    # multiply.
-    #if tab =~ /^TZ=.+$/
-    #    return tab.sub(/\n/, "\n" + self.header)
-    #else
-    #    return self.header() + tab
-    #end
+    class << crontab
+        def numeric_fields
+            fields - [:command]
+        end
+        # Do some post-processing of the parsed record.  Basically just
+        # split the numeric fields on ','.
+        def post_parse(details)
+            numeric_fields.each do |field|
+                if val = details[field] and val != :absent
+                    details[field] = details[field].split(",")
+                end
+            end
+        end
+
+        # Join the fields back up based on ','.
+        def pre_gen(details)
+            numeric_fields.each do |field|
+                if vals = details[field] and vals.is_a?(Array)
+                    details[field] = vals.join(",")
+                end
+            end
+        end
+
+
+        # Add name and environments as necessary.
+        def to_line(details)
+            str = ""
+            if details[:name]
+                str = "# Puppet Name: %s\n" % details[:name]
+            end
+            if details[:environment] and details[:environment] != :absent
+                details[:environment].each do |env|
+                    str += env + "\n"
+                end
+            end
+
+            str += join(details)
+            str
+        end
+    end
 
 
     # Return the header placed at the top of each generated file, warning
     # users that modifying this file manually is probably a bad idea.
     def self.header
-%{# HEADER This file was autogenerated at #{Time.now} by puppet.  While it
-# HEADER can still be managed manually, it is definitely not recommended.
+%{# HEADER This file was autogenerated at #{Time.now} by puppet.
+# HEADER While it can still be managed manually, it is definitely notrecommended.
 # HEADER Note particularly that the comments starting with 'Puppet Name' should
 # HEADER not be deleted, as doing so could cause duplicate cron jobs.\n}
     end
 
+    # See if we can match the hash against an existing cron job.
+    def self.match(hash)
+        model.find_all { |obj|
+            obj.value(:user) == hash[:user] and obj.value(:command) == hash[:command]
+        }.each do |obj|
+            # we now have a cron job whose command exactly matches
+            # let's see if the other fields match
+
+            # First check the @special stuff
+            if hash[:special]
+                next unless obj.value(:special) == hash[:special]
+            end
+
+            # Then the normal fields.
+            matched = true
+            record_type(hash[:record_type]).fields().each do |field|
+                next if field == :command
+                if hash[field] and ! obj.value(field)
+                    Puppet.info "Cron is missing %s: %s and %s" %
+                        [field, hash[field].inspect, obj.value(field).inspect]
+                    matched = false
+                    break
+                end
+
+                if ! hash[field] and obj.value(field)
+                    Puppet.info "Hash is missing %s: %s and %s" %
+                        [field, obj.value(field).inspect, hash[field].inspect]
+                    matched = false
+                    break
+                end
+
+                # FIXME It'd be great if I could somehow reuse how the
+                # fields are turned into text, but....
+                next if (hash[field] == :absent and obj.value(field) == "*")
+                next if (hash[field].join(",") == obj.value(field))
+                Puppet.info "Did not match %s: %s vs %s" %
+                    [field, obj.value(field).inspect, hash[field].inspect]
+                matched = false 
+                break
+            end
+            next unless matched
+            return obj
+        end
+
+        return false
+    end
+
     # Collapse name and env records.
     def self.prefetch_hook(records)
         name = nil
@@ -78,12 +159,27 @@ Puppet::Type.type(:cron).provide(:crontab,
         }.reject { |record| record[:skip] }
     end
 
+    def self.to_file(records)
+        text = super
+        # Apparently Freebsd will "helpfully" add a new TZ line to every
+        # single cron line, but not in all cases (e.g., it doesn't do it
+        # on my machine).  This is my attempt to fix it so the TZ lines don't
+        # multiply.
+        if text =~ /(^TZ=.+\n)/
+            tz = $1 
+            text.sub!(tz, '')
+            text = tz + text
+        end
+        return text
+    end
+
     def user=(user)
-        self.target = target
+        @property_hash[:user] = user
+        @property_hash[:target] = user
     end
 
     def user
-        self.target
+        @property_hash[:user] || @property_hash[:target]
     end
 end
 
diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index 56474e273..bb6546095 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -80,9 +80,10 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
 
     # Flush all of the records relating to a specific target.
     def self.flush_target(target)
-        target_object(target).write(to_file(target_records(target).reject { |r|
+        records = target_records(target).reject { |r|
             r[:ensure] == :absent
-        }))
+        }
+        target_object(target).write(to_file(records))
     end
 
     # Return the header placed at the top of each generated file, warning
@@ -101,7 +102,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         @target = nil
 
         # Default to flat files
-        @filetype = Puppet::Util::FileType.filetype(:flat)
+        @filetype ||= Puppet::Util::FileType.filetype(:flat)
         super
     end
 
@@ -191,7 +192,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         end
 
         if respond_to?(:prefetch_hook)
-            prefetch_hook(target_records)
+            target_records = prefetch_hook(target_records)
         end
 
         @records += target_records
@@ -202,8 +203,8 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
             if instance = self.model[record[:name]]
                 next unless instance.provider.is_a?(self)
                 instance.provider.property_hash = record
-            elsif self.respond_to?(:match)
-                if instance = self.match(record)
+            elsif respond_to?(:match)
+                if instance = match(record)
                     record[:name] = instance[:name]
                     instance.provider.property_hash = record
                 end
@@ -269,6 +270,12 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         targets.uniq.reject { |t| t.nil? }
     end
 
+    def self.to_file(records)
+        text = super
+        header + text
+    end
+
+
     def create
         @model.class.validproperties.each do |property|
             if value = @model.should(property)
diff --git a/lib/puppet/provider/service/smf.rb b/lib/puppet/provider/service/smf.rb
index 08f2bd731..e2e5c989a 100755
--- a/lib/puppet/provider/service/smf.rb
+++ b/lib/puppet/provider/service/smf.rb
@@ -54,6 +54,7 @@ Puppet::Type.type(:service).provide :smf, :parent => :base do
                 value = $2
             else
                 Puppet.err "Could not match %s" % line.inspect
+                next
             end
             case var
             when "state":