[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-639-g8f94f35

test branch puppet-dev at googlegroups.com
Wed Jul 14 10:34:36 UTC 2010


The following commit has been merged in the upstream branch:
commit 0b95a8528e554df07efe970c9ecfc34535d17c92
Author: Luke Kanies <luke at puppetlabs.com>
Date:   Thu Jun 10 20:31:34 2010 -0700

    Working #3139 - scheduling moved to resource harness
    
    We previously had the schedule checking code in Puppet::Type,
    but it's more of a transactional function, and in order to
    do proper auditing in the transactional area, we need the
    cache checking done there.  Scheduling is one
    of the few functions that actually uses cached data currently.
    
    Signed-off-by: Luke Kanies <luke at puppetlabs.com>

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index e57fe56..d2dbf7a 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -318,12 +318,11 @@ class Puppet::Transaction
 
     # Is the resource currently scheduled?
     def scheduled?(resource)
-        self.ignoreschedules or resource.scheduled?
+        self.ignoreschedules or resource_harness.scheduled?(resource)
     end
 
     # Should this resource be skipped?
     def skip?(resource)
-        skip = false
         if missing_tags?(resource)
             resource.debug "Not tagged with %s" % tags.join(", ")
         elsif ! scheduled?(resource)
@@ -356,11 +355,10 @@ class Puppet::Transaction
 
     # Is this resource tagged appropriately?
     def missing_tags?(resource)
-        not appropriately_tagged?(resource)
-    end
+        return false if ignore_tags?
+        return false if tags.empty?
 
-    def appropriately_tagged?(resource)
-        self.ignore_tags? or tags.empty? or resource.tagged?(*tags)
+        not resource.tagged?(*tags)
     end
 end
 
diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb
index 76148e6..05e569b 100644
--- a/lib/puppet/transaction/resource_harness.rb
+++ b/lib/puppet/transaction/resource_harness.rb
@@ -73,6 +73,28 @@ class Puppet::Transaction::ResourceHarness
         @transaction = transaction
     end
 
+    def scheduled?(resource)
+        return true if Puppet[:ignoreschedules]
+        return true unless schedule = schedule(resource)
+
+        # We use 'checked' here instead of 'synced' because otherwise we'll
+        # end up checking most resources most times, because they will generally
+        # have been synced a long time ago (e.g., a file only gets updated
+        # once a month on the server and its schedule is daily; the last sync time
+        # will have been a month ago, so we'd end up checking every run).
+        return schedule.match?(resource.cached(:checked).to_i)
+    end
+
+    def schedule(resource)
+        unless resource.catalog
+            resource.warning "Cannot schedule without a schedule-containing catalog"
+            return nil
+        end
+
+        return nil unless name = resource[:schedule]
+        resource.catalog.resource(:schedule, name) || resource.fail("Could not find schedule #{name}")
+    end
+
     private
 
     def absent_and_not_being_created?(current, param)
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index dde3428..4b5a65e 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1669,45 +1669,6 @@ class Type
         end.flatten.reject { |r| r.nil? }
     end
 
-    ###############################
-    # All of the scheduling code.
-
-    # Look up the schedule and set it appropriately.  This is done after
-    # the instantiation phase, so that the schedule can be anywhere in the
-    # file.
-    def schedule
-         unless catalog
-             warning "Cannot schedule without a schedule-containing catalog"
-             return nil
-         end
-
-        unless defined? @schedule
-            if name = self[:schedule]
-                if sched = catalog.resource(:schedule, name)
-                    @schedule = sched
-                else
-                    self.fail "Could not find schedule %s" % name
-                end
-            else
-                @schedule = nil
-            end
-        end
-        @schedule
-    end
-
-    # Check whether we are scheduled to run right now or not.
-    def scheduled?
-        return true if Puppet[:ignoreschedules]
-        return true unless schedule = self.schedule
-
-        # We use 'checked' here instead of 'synced' because otherwise we'll
-        # end up checking most resources most times, because they will generally
-        # have been synced a long time ago (e.g., a file only gets updated
-        # once a month on the server and its schedule is daily; the last sync time
-        # will have been a month ago, so we'd end up checking every run).
-        return schedule.match?(self.cached(:checked).to_i)
-    end
-
     # Define the initial list of tags.
     def tags=(list)
         tag(self.class.name)
@@ -1895,10 +1856,6 @@ class Type
 
     # Set up all of our autorequires.
     def finish
-        # Scheduling has to be done when the whole config is instantiated, so
-        # that file order doesn't matter in finding them.
-        self.schedule
-
         # Make sure all of our relationships are valid.  Again, must be done
         # when the entire catalog is instantiated.
         self.class.relationship_params.collect do |klass|
diff --git a/spec/integration/transaction.rb b/spec/integration/transaction.rb
index badf561..f682874 100755
--- a/spec/integration/transaction.rb
+++ b/spec/integration/transaction.rb
@@ -181,8 +181,6 @@ describe Puppet::Transaction do
         catalog.apply
         FileTest.should be_exists(fname)
 
-        exec.should_not be_scheduled
-
         # Now remove it, so it can get created again
         File.unlink(fname)
 
@@ -234,4 +232,16 @@ describe Puppet::Transaction do
         FileTest.should_not be_exists(file1[:path])
         FileTest.should_not be_exists(file2[:path])
     end
+
+    # #801 -- resources only checked in noop should be rescheduled immediately.
+    it "should immediately reschedule noop resources" do
+        Puppet::Type.type(:schedule).mkdefaultschedules
+        resource = Puppet::Type.type(:notify).new(:name => "mymessage", :noop => true)
+        catalog = Puppet::Resource::Catalog.new
+        catalog.add_resource resource
+
+        trans = catalog.apply
+
+        trans.resource_harness.should be_scheduled(resource)
+    end
 end
diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb
index b315eb1..0c3d901 100755
--- a/spec/unit/transaction.rb
+++ b/spec/unit/transaction.rb
@@ -241,39 +241,84 @@ describe Puppet::Transaction do
 
     describe "when skipping a resource" do
         before :each do
-            @resource = stub_everything 'res'
+            @resource = Puppet::Type.type(:notify).new :name => "foo"
             @catalog = Puppet::Resource::Catalog.new
+            @resource.catalog = @catalog
             @transaction = Puppet::Transaction.new(@catalog)
         end
 
         it "should skip resource with missing tags" do
             @transaction.stubs(:missing_tags?).returns(true)
-            @transaction.skip?(@resource).should be_true
+            @transaction.should be_skip(@resource)
         end
 
-        it "should ask the resource if it's tagged with any of the tags" do
-            tags = ['one', 'two']
-            @transaction.stubs(:ignore_tags?).returns(false)
-            @transaction.stubs(:tags).returns(tags)
-
-            @resource.expects(:tagged?).with(*tags).returns(true)
-
-            @transaction.missing_tags?(@resource).should be_false
-        end
-
-        it "should skip not scheduled resources" do
+        it "should skip unscheduled resources" do
             @transaction.stubs(:scheduled?).returns(false)
-            @transaction.skip?(@resource).should be_true
+            @transaction.should be_skip(@resource)
         end
 
         it "should skip resources with failed dependencies" do
-            @transaction.stubs(:failed_dependencies?).returns(false)
-            @transaction.skip?(@resource).should be_true
+            @transaction.stubs(:failed_dependencies?).returns(true)
+            @transaction.should be_skip(@resource)
         end
 
         it "should skip virtual resource" do
             @resource.stubs(:virtual?).returns true
-            @transaction.skip?(@resource).should be_true
+            @transaction.should be_skip(@resource)
+        end
+    end
+
+    describe "when determining if tags are missing" do
+        before :each do
+            @resource = Puppet::Type.type(:notify).new :name => "foo"
+            @catalog = Puppet::Resource::Catalog.new
+            @resource.catalog = @catalog
+            @transaction = Puppet::Transaction.new(@catalog)
+
+            @transaction.stubs(:ignore_tags?).returns false
+        end
+
+        it "should not be missing tags if tags are being ignored" do
+            @transaction.expects(:ignore_tags?).returns true
+
+            @resource.expects(:tagged?).never
+
+            @transaction.should_not be_missing_tags(@resource)
+        end
+
+        it "should not be missing tags if the transaction tags are empty" do
+            @transaction.tags = []
+            @resource.expects(:tagged?).never
+            @transaction.should_not be_missing_tags(@resource)
+        end
+
+        it "should otherwise let the resource determine if it is missing tags" do
+            tags = ['one', 'two']
+            @transaction.tags = tags
+            @resource.expects(:tagged?).with(*tags).returns(false)
+            @transaction.should be_missing_tags(@resource)
+        end
+    end
+
+    describe "when determining if a resource should be scheduled" do
+        before :each do
+            @resource = Puppet::Type.type(:notify).new :name => "foo"
+            @catalog = Puppet::Resource::Catalog.new
+            @resource.catalog = @catalog
+            @transaction = Puppet::Transaction.new(@catalog)
+        end
+
+        it "should always schedule resources if 'ignoreschedules' is set" do
+            @transaction.ignoreschedules = true
+            @transaction.resource_harness.expects(:scheduled?).never
+
+            @transaction.should be_scheduled(@resource)
+        end
+
+        it "should let the resource harness determine whether the resource should be scheduled" do
+            @transaction.resource_harness.expects(:scheduled?).with(@resource).returns "feh"
+
+            @transaction.scheduled?(@resource).should == "feh"
         end
     end
 
diff --git a/spec/unit/transaction/resource_harness.rb b/spec/unit/transaction/resource_harness.rb
index 3b9a42a..334dbff 100755
--- a/spec/unit/transaction/resource_harness.rb
+++ b/spec/unit/transaction/resource_harness.rb
@@ -265,4 +265,65 @@ describe Puppet::Transaction::ResourceHarness do
             @harness.should_not be_allow_changes(@resource)
         end
     end
+
+    describe "when finding the schedule" do
+        before do
+            @catalog = Puppet::Resource::Catalog.new
+            @resource.catalog = @catalog
+        end
+
+        it "should warn and return nil if the resource has no catalog" do
+            @resource.catalog = nil
+            @resource.expects(:warning)
+
+            @harness.schedule(@resource).should be_nil
+        end
+
+        it "should return nil if the resource specifies no schedule" do
+            @harness.schedule(@resource).should be_nil
+        end
+
+        it "should fail if the named schedule cannot be found" do
+            @resource[:schedule] = "whatever"
+            @resource.expects(:fail)
+            @harness.schedule(@resource)
+        end
+
+        it "should return the named schedule if it exists" do
+            sched = Puppet::Type.type(:schedule).new(:name => "sched")
+            @catalog.add_resource(sched)
+            @resource[:schedule] = "sched"
+            @harness.schedule(@resource).to_s.should == sched.to_s
+        end
+    end
+
+    describe "when determining if a resource is scheduled" do
+        before do
+            @catalog = Puppet::Resource::Catalog.new
+            @resource.catalog = @catalog
+        end
+
+        it "should return true if 'ignoreschedules' is set" do
+            Puppet[:ignoreschedules] = true
+            @resource[:schedule] = "meh"
+            @harness.should be_scheduled(@resource)
+        end
+
+        it "should return true if the resource has no schedule set" do
+            @harness.should be_scheduled(@resource)
+        end
+
+        it "should return the result of matching the schedule with the cached 'checked' time if a schedule is set" do
+            t = Time.now
+            @resource.expects(:cached).with(:checked).returns(t)
+
+            sched = Puppet::Type.type(:schedule).new(:name => "sched")
+            @catalog.add_resource(sched)
+            @resource[:schedule] = "sched"
+
+            sched.expects(:match?).with(t.to_i).returns "feh"
+
+            @harness.scheduled?(@resource).should == "feh"
+        end
+    end
 end
diff --git a/test/ral/manager/type.rb b/test/ral/manager/type.rb
index 8b146b6..e373cde 100755
--- a/test/ral/manager/type.rb
+++ b/test/ral/manager/type.rb
@@ -315,54 +315,4 @@ class TestType < Test::Unit::TestCase
         exec = Puppet::Type.newexec(:title => "yay", :command => "/bin/echo yay")
         assert_equal("Exec[yay]", exec.ref)
     end
-
-    # Partially test #704, but also cover the rest of the schedule management bases.
-    def test_schedule
-        schedule = Puppet::Type.type(:schedule).new(:name => "maint")
-        catalog = mk_catalog(schedule)
-
-        {"maint" => true, nil => false, :fail => :fail}.each do |name, should|
-            args = {:name => tempfile, :ensure => :file}
-            if name
-                args[:schedule] = name
-            end
-            resource = Puppet::Type.type(:file).new(args)
-            catalog.add_resource(resource)
-
-            if should == :fail
-                assert_raise(Puppet::Error, "Did not fail on missing schedule") do
-                    resource.schedule
-                end
-            elsif should == false
-                assert_nil(resource.schedule, "Set the schedule tho it is set to nil")
-            else
-                sched = nil
-                assert_nothing_raised("Failed when schedule was %s" % sched) do
-                    sched = resource.schedule
-                end
-
-                assert(sched, "Did not find schedule %s" % sched.inspect)
-
-                if should
-                    assert_equal(name, sched.name, "did not get correct schedule back")
-                end
-            end
-            catalog.remove_resource(resource)
-        end
-    end
-
-    # #801 -- resources only checked in noop should be rescheduled immediately.
-    def test_reschedule_when_noop
-        Puppet::Type.type(:schedule).mkdefaultschedules
-        file = Puppet::Type.type(:file).new(:path => "/tmp/whatever", :mode => "755", :noop => true, :schedule => :daily, :ensure => :file)
-        catalog = Puppet::Resource::Catalog.new
-        catalog.add_resource
-
-        assert(file.noop?, "File not considered in noop")
-        assert(file.scheduled?, "File is not considered scheduled")
-
-        catalog.apply
-
-        assert(file.scheduled?, "File is not considered scheduled even though only a noop run was made")
-    end
 end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list