Adding tracing to prefetch failures, and Fixing the environment support in the cron type (#669).

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

5 files changed, 77 insertions, 10 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7ccd0dce0..963772850 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,5 @@
+    Fixed environment handling in the crontab provider (#669).
+
     Added patch by trombik in #572, supporting old-style
     freebsd init scripts with '.sh' endings.
 
diff --git a/lib/puppet/provider/cron/crontab.rb b/lib/puppet/provider/cron/crontab.rb
index 23fa9201c..7599b1bdf 100755
--- a/lib/puppet/provider/cron/crontab.rb
+++ b/lib/puppet/provider/cron/crontab.rb
@@ -62,7 +62,7 @@ Puppet::Type.type(:cron).provide(:crontab,
             if details[:name]
                 str = "# Puppet Name: %s\n" % details[:name]
             end
-            if details[:environment] and details[:environment] != :absent
+            if details[:environment] and details[:environment] != :absent and details[:environment] != [:absent]
                 details[:environment].each do |env|
                     str += env + "\n"
                 end
@@ -84,10 +84,9 @@ Puppet::Type.type(:cron).provide(:crontab,
     end
 
     # See if we can match the hash against an existing cron job.
-    def self.match(hash)
-        resource_type.find_all { |obj|
-            obj.value(:user) == hash[:user] and obj.value(:command) == hash[:command]
-        }.each do |obj|
+    def self.match(hash, resources)
+        resources.each do |name, obj|
+            p hash
             # we now have a cron job whose command exactly matches
             # let's see if the other fields match
 
@@ -152,10 +151,7 @@ Puppet::Type.type(:cron).provide(:crontab,
                     record[:name] = name
                     name = nil
                 end
-                unless envs.empty?
-                    record[:environment] = envs
-                    envs = []
-                end
+                record[:environment] = envs
             end
         }.reject { |record| record[:skip] }
     end
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index bb756740d..d0997d4df 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -487,6 +487,9 @@ class Transaction
             begin
                 provider.prefetch(resources)
             rescue => detail
+                if Puppet[:trace]
+                    puts detail.backtrace
+                end
                 Puppet.err "Could not prefetch % provider %s: %s" % [resources[0].class.name, provider.name, detail]
             end
         end
diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb
index ff0ef8f7d..debf8c85f 100755
--- a/lib/puppet/type/cron.rb
+++ b/lib/puppet/type/cron.rb
@@ -287,7 +287,7 @@ Puppet::Type.newtype(:cron) do
             the crontab, e.g., ``PATH=/bin:/usr/bin:/usr/sbin``."
 
         validate do |value|
-            unless value =~ /^\s*(\w+)\s*=\s*(.+)\s*$/
+            unless value =~ /^\s*(\w+)\s*=\s*(.+)\s*$/ or value == :absent or value == "absent"
                 raise ArgumentError, "Invalid environment setting %s" %
                     value.inspect
             end
@@ -301,9 +301,25 @@ Puppet::Type.newtype(:cron) do
             end
         end
 
+        def is_to_s(newvalue)
+            if newvalue
+                newvalue.join(",")
+            else
+                nil
+            end
+        end
+
         def should
             @should
         end
+
+        def should_to_s(newvalue = @should)
+            if newvalue
+                newvalue.join(",")
+            else
+                nil
+            end
+        end
     end
 
     newparam(:name) do
diff --git a/test/ral/providers/cron/crontab.rb b/test/ral/providers/cron/crontab.rb
index 53327be9a..790a63de8 100755
--- a/test/ral/providers/cron/crontab.rb
+++ b/test/ral/providers/cron/crontab.rb
@@ -342,6 +342,56 @@ class TestCronParsedProvider < Test::Unit::TestCase
             @provider.clear
         end
     end
+
+    def test_prefetch
+        cron = @type.create :command => "/bin/echo yay", :name => "test", :hour => 4
+
+        assert_nothing_raised("Could not prefetch cron") do
+            cron.provider.class.prefetch("test" => cron)
+        end
+    end
+
+    # Testing #669.
+    def test_environment_settings
+        @provider.filetype = :ram
+        setme
+
+        target = @provider.target_object(@me)
+
+        # First with no env settings
+        cron = @type.create :command => "/bin/echo yay", :name => "test", :hour => 4
+
+        assert_apply(cron)
+
+        props = cron.retrieve.inject({}) { |hash, ary| hash[ary[0]] = ary[1]; hash }
+
+        # Now set the env
+        cron[:environment] = "TEST=foo"
+        assert_apply(cron)
+
+        props = cron.retrieve.inject({}) { |hash, ary| hash[ary[0]] = ary[1]; hash }
+        assert(target.read.include?("TEST=foo"), "Did not get environment setting")
+        #assert_equal(["TEST=foo"], props[:environment], "Did not get environment setting")
+
+        # Modify it
+        cron[:environment] = ["TEST=foo", "BLAH=yay"]
+        assert_apply(cron)
+
+        props = cron.retrieve.inject({}) { |hash, ary| hash[ary[0]] = ary[1]; hash }
+        assert(target.read.include?("TEST=foo"), "Did not keep environment setting")
+        assert(target.read.include?("BLAH=yay"), "Did not get second environment setting")
+        #assert_equal(["TEST=foo", "BLAH=yay"], props[:environment], "Did not modify environment setting")
+
+        # And remove it
+        cron[:environment] = :absent
+        assert_apply(cron)
+
+        props = cron.retrieve.inject({}) { |hash, ary| hash[ary[0]] = ary[1]; hash }
+        assert(! target.read.include?("TEST=foo"), "Did not remove environment setting")
+        assert(! target.read.include?("BLAH=yay"), "Did not remove second environment setting")
+        #assert_nil(props[:environment], "Did not modify environment setting")
+
+    end
 end
 
 # $Id$