[puppet/el6] Add slightly modified upstream patches to bugs in notify/restart (#1038041)

Sam Kottler skottler at fedoraproject.org
Fri Dec 6 15:42:36 UTC 2013


commit 4c9678e8d2dc92a19ce44357e247f937746381f1
Author: Sam Kottler <shk at redhat.com>
Date:   Fri Dec 6 10:42:01 2013 -0500

    Add slightly modified upstream patches to bugs in notify/restart (#1038041)

 ...e-event-dequeues-when-target-resource-has.patch |   88 ++++++++
 ...date-refreshes-after-services-are-started.patch |  218 ++++++++++++++++++++
 puppet.spec                                        |   16 ++-
 3 files changed, 321 insertions(+), 1 deletions(-)
---
diff --git a/0001-20128-Handle-event-dequeues-when-target-resource-has.patch b/0001-20128-Handle-event-dequeues-when-target-resource-has.patch
new file mode 100644
index 0000000..b28520c
--- /dev/null
+++ b/0001-20128-Handle-event-dequeues-when-target-resource-has.patch
@@ -0,0 +1,88 @@
+From 013f0ee5f1e946456060907fa2873d62ca9dbe8a Mon Sep 17 00:00:00 2001
+From: Dominic Cleal <dcleal at redhat.com>
+Date: Mon, 8 Apr 2013 16:37:01 +0100
+Subject: [PATCH] (#20128) Handle event dequeues when target resource has no
+ events registered
+
+In the case where a resource triggers an event with invalidate_refreshes
+enabled (i.e. a service starts), the dequeue process would attempt to
+invalidate other events registered for this resource.  If no other
+notifications had been received on the resource already, then the hash element
+for the resource would be nil.  The resource would then fail:
+
+  Error: /Stage[main]//Service[service]: Could not evaluate: undefined method `[]=' for nil:NilClass
+
+Cherry-pick of af6eaaa0 and 977644da from Puppet 3.2 to 2.6.
+---
+ lib/puppet/transaction/event_manager.rb     |  2 +-
+ spec/unit/transaction/event_manager_spec.rb | 29 ++++++++++++++++++-----------
+ 2 files changed, 19 insertions(+), 12 deletions(-)
+
+diff --git a/lib/puppet/transaction/event_manager.rb b/lib/puppet/transaction/event_manager.rb
+index 39403d1..ed1d10f 100644
+--- a/lib/puppet/transaction/event_manager.rb
++++ b/lib/puppet/transaction/event_manager.rb
+@@ -62,7 +62,7 @@ class Puppet::Transaction::EventManager
+ 
+   def dequeue_events_for_resource(target, callback)
+     target.info "Unscheduling #{callback} on #{target}"
+-    @event_queues[target][callback] = {}
++    @event_queues[target][callback] = {} if @event_queues[target]
+   end
+ 
+   def queue_events_for_resource(source, target, callback, events)
+diff --git a/spec/unit/transaction/event_manager_spec.rb b/spec/unit/transaction/event_manager_spec.rb
+index eef6fc9..76deb4a 100755
+--- a/spec/unit/transaction/event_manager_spec.rb
++++ b/spec/unit/transaction/event_manager_spec.rb
+@@ -273,29 +273,36 @@ describe Puppet::Transaction::EventManager do
+       @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
+       @manager = Puppet::Transaction::EventManager.new(@transaction)
+ 
+-      @graph = stub 'graph', :matching_edges => [], :resource => @resource
++      @resource = Puppet::Type.type(:file).new :path => "/my/file"
++      @target = Puppet::Type.type(:file).new :path => "/your/file"
++
++      @graph = stub 'graph'
+       @graph.stubs(:matching_edges).returns []
++      @graph.stubs(:matching_edges).with(anything, @resource).returns [stub('edge', :target => @target, :callback => :refresh)]
+       @manager.stubs(:relationship_graph).returns @graph
+ 
+-      @resource = Puppet::Type.type(:file).new :path => "/my/file"
+-      @resource.expects(:self_refresh?).returns true
+-      @resource.expects(:deleting?).returns false
+-      @resource.expects(:info).with { |msg| msg.include?("Scheduling refresh") }
+-      @event = Puppet::Transaction::Event.new(:name => :foo, :resource => @resource)
++      @event  = Puppet::Transaction::Event.new(:name => :notify, :resource => @target)
++      @event2 = Puppet::Transaction::Event.new(:name => :service_start, :resource => @target, :invalidate_refreshes => true)
++    end
++
++    it "should succeed when there's no invalidated event" do
++      @manager.queue_events(@target, [@event2])
+     end
+ 
+     describe "and the events were dequeued/invalidated" do
+       before do
+-        @event2 = Puppet::Transaction::Event.new(:name => :foo, :resource => @resource, :invalidate_refreshes => true)
+-        @resource.expects(:info).with { |msg| msg.include?("Unscheduling") }
++        @resource.expects(:info).with { |msg| msg.include?("Scheduling refresh") }
++        @target.expects(:info).with { |msg| msg.include?("Unscheduling") }
+       end
+ 
+       it "should not run an event or log" do
+-        @resource.expects(:notice).with { |msg| msg.include?("Would have triggered 'refresh'") }.never
+-        @resource.expects(:refresh).never
++        @target.expects(:notice).with { |msg| msg.include?("Would have triggered 'refresh'") }.never
++        @target.expects(:refresh).never
+ 
+-        @manager.queue_events(@resource, [@event, @event2])
++        @manager.queue_events(@resource, [@event])
++        @manager.queue_events(@target, [@event2])
+         @manager.process_events(@resource)
++        @manager.process_events(@target)
+       end
+     end
+   end
+-- 
+1.8.1.4
+
diff --git a/0001-7165-Invalidate-refreshes-after-services-are-started.patch b/0001-7165-Invalidate-refreshes-after-services-are-started.patch
new file mode 100644
index 0000000..a75087e
--- /dev/null
+++ b/0001-7165-Invalidate-refreshes-after-services-are-started.patch
@@ -0,0 +1,218 @@
+commit fe846f2c1d55c3ef243a8f0d61c67c8e29a2b442
+Author: Dominic Cleal <dcleal at redhat.com>
+Date:   Tue Mar 19 10:30:34 2013 +0000
+
+    (#7165) Invalidate refreshes after services are started
+    
+    Backport of 0dfc0b8e and 996380b8 from Puppet 3.2 to 2.6.
+
+diff --git a/acceptance/tests/ticket_7165_no_refresh_after_starting_service.rb b/acceptance/tests/ticket_7165_no_refresh_after_starting_service.rb
+new file mode 100644
+index 0000000..f17caad
+--- /dev/null
++++ b/acceptance/tests/ticket_7165_no_refresh_after_starting_service.rb
+@@ -0,0 +1,41 @@
++test_name "Bug #7165: Don't refresh service immediately after starting it"
++
++confine :except, :platform => 'windows'
++
++agents.each do |host|
++  dir = host.tmpdir('7165-no-refresh')
++
++manifest = %Q{
++  file { "#{dir}/notify":
++    content => "foo",
++    notify  => Service["service"],
++  }
++
++  service { "service":
++    ensure     => running,
++    hasstatus  => true,
++    hasrestart => true,
++    status     => "test -e #{dir}/service",
++    start      => "touch #{dir}/service",
++    stop       => "rm -f #{dir}/service",
++    restart    => "touch #{dir}/service_restarted",
++    require    => File["#{dir}/notify"],
++    provider   => base,
++  }
++}
++
++  apply_manifest_on(host, manifest) do
++    on(host, "test -e #{dir}/service")
++    # service should not restart, since it wasn't running to begin with
++    on(host, "test -e #{dir}/service_restarted", :acceptable_exit_codes => [1])
++  end
++
++  # will trigger a notify next time as the file changes
++  on(host, "echo bar > #{dir}/notify")
++
++  apply_manifest_on(host, manifest) do
++    on(host, "test -e #{dir}/service")
++    # service should restart this time
++    on(host, "test -e #{dir}/service_restarted")
++  end
++end
+diff --git a/lib/puppet/parameter/value.rb b/lib/puppet/parameter/value.rb
+index d9bfbaf..64065cf 100644
+--- a/lib/puppet/parameter/value.rb
++++ b/lib/puppet/parameter/value.rb
+@@ -3,7 +3,7 @@ require 'puppet/parameter/value_collection'
+ # An individual Value class.
+ class Puppet::Parameter::Value
+   attr_reader :name, :options, :event
+-  attr_accessor :block, :call, :method, :required_features
++  attr_accessor :block, :call, :method, :required_features, :invalidate_refreshes
+ 
+   # Add an alias for this value.
+   def alias(name)
+diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
+index 12f496a..aab892c 100644
+--- a/lib/puppet/property.rb
++++ b/lib/puppet/property.rb
+@@ -132,7 +132,11 @@ class Puppet::Property < Puppet::Parameter
+ 
+   # Return a modified form of the resource event.
+   def event
+-    resource.event :name => event_name, :desired_value => should, :property => self, :source_description => path
++    attrs = { :name => event_name, :desired_value => should, :property => self, :source_description => path }
++    if should and value = self.class.value_collection.match?(should)
++      attrs[:invalidate_refreshes] = true if value.invalidate_refreshes
++    end
++    resource.event attrs
+   end
+ 
+   attr_reader :shadow
+diff --git a/lib/puppet/transaction/event.rb b/lib/puppet/transaction/event.rb
+index cd695cf..500ddae 100644
+--- a/lib/puppet/transaction/event.rb
++++ b/lib/puppet/transaction/event.rb
+@@ -7,7 +7,7 @@ class Puppet::Transaction::Event
+   include Puppet::Util::Tagging
+   include Puppet::Util::Logging
+ 
+-  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :file, :line, :source_description, :audited]
++  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :file, :line, :source_description, :audited, :invalidate_refreshes]
+   YAML_ATTRIBUTES = %w{@audited @property @previous_value @desired_value @historical_value @message @name @status @time}
+   attr_accessor *ATTRIBUTES
+   attr_writer :tags
+diff --git a/lib/puppet/transaction/event_manager.rb b/lib/puppet/transaction/event_manager.rb
+index 3ebb0a9..39403d1 100644
+--- a/lib/puppet/transaction/event_manager.rb
++++ b/lib/puppet/transaction/event_manager.rb
+@@ -56,6 +56,13 @@ class Puppet::Transaction::EventManager
+ 
+       queue_events_for_resource(resource, resource, :refresh, [event]) if resource.self_refresh? and ! resource.deleting?
+     end
++
++    dequeue_events_for_resource(resource, :refresh) if events.detect { |e| e.invalidate_refreshes }
++  end
++
++  def dequeue_events_for_resource(target, callback)
++    target.info "Unscheduling #{callback} on #{target}"
++    @event_queues[target][callback] = {}
+   end
+ 
+   def queue_events_for_resource(source, target, callback, events)
+@@ -69,7 +76,7 @@ class Puppet::Transaction::EventManager
+   def queued_events(resource)
+     return unless callbacks = @event_queues[resource]
+     callbacks.each do |callback, events|
+-      yield callback, events
++      yield callback, events unless events.empty?
+     end
+   end
+ 
+diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
+index 3658e28..2d292b8 100644
+--- a/lib/puppet/type/service.rb
++++ b/lib/puppet/type/service.rb
+@@ -62,7 +62,7 @@ module Puppet
+         provider.stop
+       end
+ 
+-      newvalue(:running, :event => :service_started) do
++      newvalue(:running, :event => :service_started, :invalidate_refreshes => true) do
+         provider.start
+       end
+ 
+diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
+index 56e643b..3b9699a 100755
+--- a/spec/unit/property_spec.rb
++++ b/spec/unit/property_spec.rb
+@@ -145,6 +145,17 @@ describe Puppet::Property do
+       property.stubs(:path).returns "/my/param"
+       property.event.source_description.should == "/my/param"
+     end
++
++    it "should have the 'invalidate_refreshes' value set if set on a value" do
++      property.stubs(:event_name).returns :my_event
++      property.stubs(:should).returns "foo"
++      foo = mock()
++      foo.expects(:invalidate_refreshes).returns(true)
++      collection = mock()
++      collection.expects(:match?).with("foo").returns(foo)
++      property.class.stubs(:value_collection).returns(collection)
++      property.event.invalidate_refreshes.should be_true
++    end
+   end
+
+   describe "when shadowing metaparameters" do
+diff --git a/spec/unit/transaction/event_manager_spec.rb b/spec/unit/transaction/event_manager_spec.rb
+index eeb3d33..eef6fc9 100755
+--- a/spec/unit/transaction/event_manager_spec.rb
++++ b/spec/unit/transaction/event_manager_spec.rb
+@@ -101,6 +101,16 @@ describe Puppet::Transaction::EventManager do
+ 
+       @manager.queue_events(@resource, [@event])
+     end
++
++    it "should dequeue events for the changed resource if an event with invalidate_refreshes is processed" do
++      @event2 = Puppet::Transaction::Event.new(:name => :foo, :resource => @resource, :invalidate_refreshes => true)
++
++      @graph.stubs(:matching_edges).returns []
++
++      @manager.expects(:dequeue_events_for_resource).with(@resource, :refresh)
++
++      @manager.queue_events(@resource, [@event, @event2])
++    end
+   end
+ 
+   describe "when queueing events for a resource" do
+@@ -257,4 +267,36 @@ describe Puppet::Transaction::EventManager do
+       end
+     end
+   end
++
++  describe "when queueing then processing events for a given resource" do
++    before do
++      @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
++      @manager = Puppet::Transaction::EventManager.new(@transaction)
++
++      @graph = stub 'graph', :matching_edges => [], :resource => @resource
++      @graph.stubs(:matching_edges).returns []
++      @manager.stubs(:relationship_graph).returns @graph
++
++      @resource = Puppet::Type.type(:file).new :path => "/my/file"
++      @resource.expects(:self_refresh?).returns true
++      @resource.expects(:deleting?).returns false
++      @resource.expects(:info).with { |msg| msg.include?("Scheduling refresh") }
++      @event = Puppet::Transaction::Event.new(:name => :foo, :resource => @resource)
++    end
++
++    describe "and the events were dequeued/invalidated" do
++      before do
++        @event2 = Puppet::Transaction::Event.new(:name => :foo, :resource => @resource, :invalidate_refreshes => true)
++        @resource.expects(:info).with { |msg| msg.include?("Unscheduling") }
++      end
++
++      it "should not run an event or log" do
++        @resource.expects(:notice).with { |msg| msg.include?("Would have triggered 'refresh'") }.never
++        @resource.expects(:refresh).never
++
++        @manager.queue_events(@resource, [@event, @event2])
++        @manager.process_events(@resource)
++      end
++    end
++  end
+ end
diff --git a/puppet.spec b/puppet.spec
index c16b897..751b338 100644
--- a/puppet.spec
+++ b/puppet.spec
@@ -13,7 +13,7 @@
 
 Name:           puppet
 Version:        2.7.23
-Release:        1%{?dist}
+Release:        2%{?dist}
 Vendor:         %{?_host_vendor}
 Summary:        A network tool for managing many disparate systems
 License:        ASL 2.0
@@ -50,6 +50,15 @@ Requires(preun): chkconfig
 Requires(preun): initscripts
 Requires(postun): initscripts
 
+# http://projects.puppetlabs.com/issues/7165
+# https://bugzilla.redhat.com/show_bug.cgi?id=908655
+# https://github.com/puppetlabs/puppet/commit/3da4dc9
+Patch0: 0001-7165-Invalidate-refreshes-after-services-are-started.patch
+# http://projects.puppetlabs.com/issues/20128
+# https://bugzilla.redhat.com/show_bug.cgi?id=908655#c11
+# https://github.com/puppetlabs/puppet/commit/3e5cea2
+Patch1: 0001-20128-Handle-event-dequeues-when-target-resource-has.patch
+
 %description
 Puppet lets you centrally manage every important aspect of your system using a
 cross-platform specification language that manages all the separate elements
@@ -72,6 +81,8 @@ The server can also function as a certificate authority and file server.
 %prep
 %setup -q -n %{name}-%{version}
 patch -s -p1 < conf/redhat/rundir-perms.patch
+%patch0 -p1
+%patch1 -p1
 
 
 %build
@@ -307,6 +318,9 @@ fi
 rm -rf %{buildroot}
 
 %changelog
+* Fri Dec 6 2013 Sam Kottler <skottler at fedoraproject.org> - 2.7.23-2
+- Add slightly modified upstream patches to fix bugs in notify/restart (#1038041)
+
 * Sun Oct 6 2013 Sam Kottler <skottler at fedoraproject.org> - 2.7.23-1
 - Update to the 2.7.x series
 


More information about the scm-commits mailing list