[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:30:34 UTC 2010
The following commit has been merged in the upstream branch:
commit ad90900e68a2c406f0e95dba3f780ad135415b14
Author: Luke Kanies <luke at madstop.com>
Date: Thu Oct 29 09:21:34 2009 -0700
Random code cleanup
Signed-off-by: Luke Kanies <luke at madstop.com>
diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index 753c0d7..e246eb7 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -152,8 +152,8 @@ class Puppet::Property < Puppet::Parameter
end
# Figure out which event to return.
- def event(name, event = nil)
- if value_event = self.class.value_option(name, :event)
+ def default_event_name(value, event = nil)
+ if value_event = self.class.value_option(value, :event)
return value_event
end
@@ -315,7 +315,7 @@ class Puppet::Property < Puppet::Parameter
devfail "Cannot use obsolete :call value '%s' for property '%s'" % [call, self.class.name]
end
- return event(name, event)
+ return default_event_name(name, event)
end
# If there's a shadowing metaparam, instantiate it now.
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 6671fb1..a5feecb 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -409,7 +409,7 @@ class Puppet::Transaction
end
if restarted
- queue_events(Puppet::Transaction::Event.new(:restarted, resource))
+ queue_event(resource, Puppet::Transaction::Event.new(:restarted, resource))
@resourcemetrics[:restarted] += 1
end
@@ -417,12 +417,8 @@ class Puppet::Transaction
# Queue events for other resources to respond to. All of these events have
# to be from the same resource.
- def queue_events(*events)
- events.flatten!
-
- @events += events
-
- resource = events[0].resource
+ def queue_event(resource, event)
+ @events << event
# Collect the targets of any subscriptions to those events. We pass
# the parent resource in so it will override the source in the events,
@@ -431,20 +427,20 @@ class Puppet::Transaction
next unless method = edge.callback
next unless edge.target.respond_to?(method)
- queue_events_for_resource(resource, edge.target, method, events)
+ queue_event_for_resource(resource, edge.target, method, event)
end
if resource.self_refresh? and ! resource.deleting?
- queue_events_for_resource(resource, resource, :refresh, events)
+ queue_event_for_resource(resource, resource, :refresh, event)
end
end
- def queue_events_for_resource(source, target, callback, events)
+ def queue_event_for_resource(source, target, callback, event)
source.info "Scheduling #{callback} of #{target}"
@event_queues[target] ||= {}
@event_queues[target][callback] ||= []
- @event_queues[target][callback] += events
+ @event_queues[target][callback] << event
end
def queued_events(resource)
@@ -461,13 +457,8 @@ class Puppet::Transaction
# Roll all completed changes back.
def rollback
@changes.reverse.collect do |change|
- # skip changes that were never actually run
- unless change.changed
- Puppet.debug "%s was not changed" % change.to_s
- next
- end
begin
- events = change.backward
+ event = change.backward
rescue => detail
Puppet.err("%s rollback failed: %s" % [change,detail])
if Puppet[:trace]
@@ -483,7 +474,7 @@ class Puppet::Transaction
end
# And queue the events
- queue_events(events)
+ queue_event(change.resource, event)
# Now check to see if there are any events for this child.
process_events(change.property.resource)
@@ -542,21 +533,14 @@ class Puppet::Transaction
def apply_change(resource, change)
@changes << change
- # use an array, so that changes can return more than one
- # event if they want
- events = [change.forward].flatten.reject { |e| e.nil? }
+ event = change.forward
- # Mark that our change happened, so it can be reversed
- # if we ever get to that point
- change.changed = true
- @resourcemetrics[:applied] += 1
- queue_events(events)
- rescue => detail
- puts detail.backtrace if Puppet[:trace]
- is = change.property.is_to_s(change.is)
- should = change.property.should_to_s(change.should)
- change.property.err "change from #{is} to #{should} failed: #{detail}"
- @failures[resource] += 1
+ if event.status == "success"
+ @resourcemetrics[:applied] += 1
+ else
+ @failures[resource] += 1
+ end
+ queue_event(resource, event)
end
def process_callback(resource, callback, events)
@@ -577,7 +561,7 @@ class Puppet::Transaction
resource.notice "Would have triggered '#{callback}' from #{events.length} events"
# And then add an event for it.
- queue_events(Puppet::Transaction::Event.new(:noop, resource))
+ queue_event(resource, Puppet::Transaction::Event.new(:noop, resource))
true # so the 'and if' works
end
end
diff --git a/lib/puppet/transaction/change.rb b/lib/puppet/transaction/change.rb
index b710ee0..5e1aff1 100644
--- a/lib/puppet/transaction/change.rb
+++ b/lib/puppet/transaction/change.rb
@@ -4,7 +4,7 @@ require 'puppet/transaction/event'
# Handle all of the work around performing an actual change,
# including calling 'sync' on the properties and producing events.
class Puppet::Transaction::Change
- attr_accessor :is, :should, :path, :property, :changed, :proxy
+ attr_accessor :is, :should, :property, :proxy
# Switch the goals of the property, thus running the change in reverse.
def backward
@@ -15,26 +15,22 @@ class Puppet::Transaction::Change
return self.go
end
- def changed?
- self.changed
- end
-
# Create our event object.
- def event(name)
+ def event(event_name)
+ event_name ||= property.default_event_name(should)
+
# default to a simple event type
- unless name.is_a?(Symbol)
- @property.warning("Property '%s' returned invalid event '%s'; resetting to default" %
- [@property.class, name])
+ unless event_name.is_a?(Symbol)
+ @property.warning "Property '#{property.class}' returned invalid event '#{event_name}'; resetting to default"
- name = @property.event(should)
+ event_name = property.default_event_name(should)
end
- Puppet::Transaction::Event.new(name, self.resource)
+ Puppet::Transaction::Event.new(event_name, resource.ref, property.name, is, should)
end
def initialize(property, currentvalue)
@property = property
- @path = [property.path,"change"].flatten
@is = currentvalue
@should = property.should
@@ -47,25 +43,30 @@ class Puppet::Transaction::Change
def go
if self.noop?
@property.log "is %s, should be %s (noop)" % [property.is_to_s(@is), property.should_to_s(@should)]
- return [event(:noop)]
+ return event(:noop)
end
# The transaction catches any exceptions here.
- events = @property.sync
- if events.nil?
- events = [(@property.name.to_s + "_changed").to_sym]
- elsif events.is_a?(Array)
- if events.empty?
- events = [(@property.name.to_s + "_changed").to_sym]
- end
- else
- events = [events]
- end
-
- return events.collect { |name|
- @report = @property.log(@property.change_to_s(@is, @should))
- event(name)
- }
+ event_name = @property.sync
+
+ # Use the first event only, if multiple are provided.
+ # This might result in the event_name being nil,
+ # which is fine.
+ event_name = event_name.shift if event_name.is_a?(Array)
+
+ event = event(event_name)
+ event.log = @property.notice @property.change_to_s(@is, @should)
+ event.status = "success"
+ event
+ rescue => detail
+ puts detail.backtrace if Puppet[:trace]
+ event = event(nil)
+ event.status = "failure"
+
+ is = property.is_to_s(is)
+ should = property.should_to_s(should)
+ event.log = property.err "change from #{is} to #{should} failed: #{detail}"
+ event
end
def forward
@@ -79,7 +80,7 @@ class Puppet::Transaction::Change
# The resource that generated this change. This is used for handling events,
# and the proxy resource is used for generated resources, since we can't
- # send an event to a resource we don't have a direct relationship. If we
+ # send an event to a resource we don't have a direct relationship with. If we
# have a proxy resource, then the events will be considered to be from
# that resource, rather than us, so the graph resolution will still work.
def resource
diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb
index a797d94..abb3621 100755
--- a/spec/unit/transaction.rb
+++ b/spec/unit/transaction.rb
@@ -47,37 +47,34 @@ describe Puppet::Transaction do
describe "when applying changes" do
before do
@transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
- @transaction.stubs(:queue_events)
+ @transaction.stubs(:queue_event)
@resource = stub 'resource'
@property = stub 'property', :is_to_s => "is", :should_to_s => "should"
- @change = stub 'change', :property => @property, :changed= => nil, :forward => nil, :is => "is", :should => "should"
+
+ @event = stub 'event', :status => "success"
+ @change = stub 'change', :property => @property, :changed= => nil, :forward => @event, :is => "is", :should => "should"
end
it "should apply each change" do
c1 = stub 'c1', :property => @property, :changed= => nil
- c1.expects(:forward)
+ c1.expects(:forward).returns @event
c2 = stub 'c2', :property => @property, :changed= => nil
- c2.expects(:forward)
+ c2.expects(:forward).returns @event
@transaction.apply_changes(@resource, [c1, c2])
end
it "should queue the events from each change" do
- c1 = stub 'c1', :forward => "e1", :property => @property, :changed= => nil
- c2 = stub 'c2', :forward => "e2", :property => @property, :changed= => nil
+ c1 = stub 'c1', :forward => stub("event1", :status => "success"), :property => @property, :changed= => nil
+ c2 = stub 'c2', :forward => stub("event2", :status => "success"), :property => @property, :changed= => nil
- @transaction.expects(:queue_events).with(["e1"])
- @transaction.expects(:queue_events).with(["e2"])
+ @transaction.expects(:queue_event).with(@resource, c1.forward)
+ @transaction.expects(:queue_event).with(@resource, c2.forward)
@transaction.apply_changes(@resource, [c1, c2])
end
- it "should mark the change as 'changed'" do
- @change.expects(:changed=).with true
- @transaction.apply_changes(@resource, [@change])
- end
-
it "should store the change in the transaction's change list" do
@transaction.apply_changes(@resource, [@change])
@transaction.changes.should include(@change)
@@ -90,22 +87,17 @@ describe Puppet::Transaction do
describe "and a change fails" do
before do
- @change.expects(:forward).raises "a failure"
- @change.property.stubs(:err)
+ @event.stubs(:status).returns "failure"
end
- it "should rescue the exception" do
- lambda { @transaction.apply_changes(@resource, [@change]) }.should_not raise_error
- end
-
- it "should log the failure with the change's property" do
- @change.property.expects(:err)
+ it "should increment the failures" do
@transaction.apply_changes(@resource, [@change])
+ @transaction.should be_any_failed
end
- it "should increment the failures" do
+ it "should queue the event" do
+ @transaction.expects(:queue_event).with(@resource, @event)
@transaction.apply_changes(@resource, [@change])
- @transaction.should be_any_failed
end
end
end
@@ -114,38 +106,30 @@ describe Puppet::Transaction do
before do
@transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
- @graph = stub 'graph', :matching_edges => []
+ @resource = stub("resource", :self_refresh? => false, :deleting => false)
+
+ @graph = stub 'graph', :matching_edges => [], :resource => @resource
@transaction.stubs(:relationship_graph).returns @graph
- @resource = stub("resource", :self_refresh? => false, :deleting => false)
@event = Puppet::Transaction::Event.new(:foo, @resource)
end
it "should store each event in its event list" do
- @transaction.queue_events(@event)
+ @transaction.queue_event(@resource, @event)
@transaction.events.should include(@event)
end
- it "should use the resource from the first edge as the source of all of the events" do
- event1 = Puppet::Transaction::Event.new(:foo, @resource)
- event2 = Puppet::Transaction::Event.new(:foo, stub('resource2', :self_refresh? => false, :deleting? => false))
-
- @graph.expects(:matching_edges).with { |events, resource| resource == event1.resource }.returns []
-
- @transaction.queue_events(event1, event2)
- end
-
it "should queue events for the target and callback of any matching edges" do
edge1 = stub("edge1", :callback => :c1, :source => stub("s1"), :target => stub("t1", :c1 => nil))
edge2 = stub("edge2", :callback => :c2, :source => stub("s2"), :target => stub("t2", :c2 => nil))
@graph.expects(:matching_edges).with { |events, resource| events == [@event] }.returns [edge1, edge2]
- @transaction.expects(:queue_events_for_resource).with(@resource, edge1.target, edge1.callback, [@event])
- @transaction.expects(:queue_events_for_resource).with(@resource, edge2.target, edge2.callback, [@event])
+ @transaction.expects(:queue_event_for_resource).with(@resource, edge1.target, edge1.callback, @event)
+ @transaction.expects(:queue_event_for_resource).with(@resource, edge2.target, edge2.callback, @event)
- @transaction.queue_events(@event)
+ @transaction.queue_event(@resource, @event)
end
it "should queue events for the changed resource if the resource is self-refreshing and not being deleted" do
@@ -153,9 +137,9 @@ describe Puppet::Transaction do
@resource.expects(:self_refresh?).returns true
@resource.expects(:deleting?).returns false
- @transaction.expects(:queue_events_for_resource).with(@resource, @resource, :refresh, [@event])
+ @transaction.expects(:queue_event_for_resource).with(@resource, @resource, :refresh, @event)
- @transaction.queue_events(@event)
+ @transaction.queue_event(@resource, @event)
end
it "should not queue events for the changed resource if the resource is not self-refreshing" do
@@ -163,9 +147,9 @@ describe Puppet::Transaction do
@resource.expects(:self_refresh?).returns false
@resource.stubs(:deleting?).returns false
- @transaction.expects(:queue_events_for_resource).never
+ @transaction.expects(:queue_event_for_resource).never
- @transaction.queue_events(@event)
+ @transaction.queue_event(@resource, @event)
end
it "should not queue events for the changed resource if the resource is being deleted" do
@@ -173,9 +157,9 @@ describe Puppet::Transaction do
@resource.expects(:self_refresh?).returns true
@resource.expects(:deleting?).returns true
- @transaction.expects(:queue_events_for_resource).never
+ @transaction.expects(:queue_event_for_resource).never
- @transaction.queue_events(@event)
+ @transaction.queue_event(@resource, @event)
end
it "should ignore edges that don't have a callback" do
@@ -183,9 +167,9 @@ describe Puppet::Transaction do
@graph.expects(:matching_edges).returns [edge1]
- @transaction.expects(:queue_events_for_resource).never
+ @transaction.expects(:queue_event_for_resource).never
- @transaction.queue_events(@event)
+ @transaction.queue_event(@resource, @event)
end
it "should ignore targets that don't respond to the callback" do
@@ -193,9 +177,9 @@ describe Puppet::Transaction do
@graph.expects(:matching_edges).returns [edge1]
- @transaction.expects(:queue_events_for_resource).never
+ @transaction.expects(:queue_event_for_resource).never
- @transaction.queue_events(@event)
+ @transaction.queue_event(@resource, @event)
end
end
@@ -212,7 +196,7 @@ describe Puppet::Transaction do
target = stub("target")
2.times do |i|
- @transaction.queue_events_for_resource(stub("source", :info => nil), target, "callback#{i}", ["event#{i}"])
+ @transaction.queue_event_for_resource(stub("source", :info => nil), target, "callback#{i}", ["event#{i}"])
end
@transaction.queued_events(target) { |callback, events| }
@@ -223,7 +207,7 @@ describe Puppet::Transaction do
source = stub 'source'
source.expects(:info)
- @transaction.queue_events_for_resource(source, target, "callback", ["event"])
+ @transaction.queue_event_for_resource(source, target, "callback", ["event"])
@transaction.queued_events(target) { |callback, events| }
end
@@ -232,7 +216,7 @@ describe Puppet::Transaction do
describe "when processing events for a given resource" do
before do
@transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
- @transaction.stubs(:queue_events)
+ @transaction.stubs(:queue_event)
@resource = stub 'resource', :notice => nil
@event = Puppet::Transaction::Event.new(:event, @resource)
@@ -262,8 +246,8 @@ describe Puppet::Transaction do
@resource.stubs(:callback1)
- @transaction.expects(:queue_events).with do |event|
- event.name == :restarted and event.resource == @resource
+ @transaction.expects(:queue_event).with do |resource, event|
+ event.name == :restarted and resource == @resource
end
@transaction.process_events(@resource)
@@ -298,7 +282,7 @@ describe Puppet::Transaction do
end
it "should queue a new noop event" do
- @transaction.expects(:queue_events).with { |event| event.name == :noop and event.resource == @resource }
+ @transaction.expects(:queue_event).with { |resource, event| event.name == :noop and resource == @resource }
@transaction.process_events(@resource)
end
@@ -324,7 +308,7 @@ describe Puppet::Transaction do
end
it "should not queue a 'restarted' event" do
- @transaction.expects(:queue_events).never
+ @transaction.expects(:queue_event).never
@transaction.process_events(@resource)
end
diff --git a/spec/unit/transaction/change.rb b/spec/unit/transaction/change.rb
index 9e0cedc..b42f069 100755
--- a/spec/unit/transaction/change.rb
+++ b/spec/unit/transaction/change.rb
@@ -28,10 +28,6 @@ describe Puppet::Transaction::Change do
# Yay rspec :)
Change.new(@property, "value").should.should == @property.should
end
-
- it "should set its path to the path of the property plus 'change'" do
- Change.new(@property, "value").path.should == [@property.path, "change"]
- end
end
describe "when an instance" do
@@ -55,33 +51,65 @@ describe Puppet::Transaction::Change do
@change.resource.should == :myresource
end
- it "should have a method for marking that it's been execution" do
- @change.changed = true
- @change.changed?.should be_true
- end
-
describe "and creating an event" do
before do
- @property.stubs(:resource).returns "myresource"
+ @resource = stub 'resource', :ref => "My[resource]"
+ @property.stubs(:resource).returns @resource
+ @property.stubs(:name).returns :myprop
+ end
+
+ it "should set the event name to the provided name" do
+ @change.event(:foo).name.should == :foo
+ end
+
+ it "should use the property's default event if the event name is nil" do
+ @property.expects(:default_event_name).with(@change.should).returns :myevent
+ @change.event(nil).name.should == :myevent
end
it "should produce a warning if the event name is not a symbol" do
@property.expects(:warning)
- @property.stubs(:event).returns :myevent
+ @property.stubs(:default_event_name).returns :myevent
@change.event("a string")
end
it "should use the property to generate the event name if the provided name is not a symbol" do
@property.stubs(:warning)
- @property.expects(:event).with(@change.should).returns :myevent
+ @property.expects(:default_event_name).with(@change.should).returns :myevent
- Puppet::Transaction::Event.expects(:new).with { |name, source| name == :myevent }
+ @change.event("a string").name.should == :myevent
+ end
- @change.event("a string")
+ it "should set the resource to the resource reference" do
+ @change.resource.expects(:ref).returns "Foo[bar]"
+ @change.event(:foo).resource.should == "Foo[bar]"
+ end
+
+ it "should set the property to the property name" do
+ @change.property.expects(:name).returns :myprop
+ @change.event(:foo).property.should == :myprop
+ end
+
+ it "should set 'previous_value' from the change's 'is'" do
+ @change.event(:foo).previous_value.should == @change.is
+ end
+
+ it "should set 'desired_value' from the change's 'should'" do
+ @change.event(:foo).desired_value.should == @change.should
end
end
describe "and executing" do
+ before do
+ @event = Puppet::Transaction::Event.new(:myevent)
+ @change.stubs(:noop?).returns false
+ @change.stubs(:event).returns @event
+
+ @property.stub_everything
+ @property.stubs(:resource).returns "myresource"
+ @property.stubs(:name).returns :myprop
+ end
+
describe "in noop mode" do
before { @change.stubs(:noop?).returns true }
@@ -99,56 +127,64 @@ describe Puppet::Transaction::Change do
@change.expects(:event).with(:noop).returns :noop_event
- @change.forward.should == [:noop_event]
+ @change.forward.should == :noop_event
end
end
- describe "without noop" do
- before do
- @change.stubs(:noop?).returns false
- @property.stub_everything
- @property.stubs(:resource).returns "myresource"
- @property.stubs(:name).returns :myprop
- end
+ it "should sync the property" do
+ @property.expects(:sync)
- it "should sync the property" do
- @property.expects(:sync)
+ @change.forward
+ end
- @change.forward
- end
+ it "should return the default event if syncing the property returns nil" do
+ @property.stubs(:sync).returns nil
- it "should return the default event if syncing the property returns nil" do
- @property.stubs(:sync).returns nil
+ @change.expects(:event).with(nil).returns @event
- @change.expects(:event).with(:myprop_changed).returns :myevent
+ @change.forward.should == @event
+ end
- @change.forward.should == [:myevent]
- end
+ it "should return the default event if syncing the property returns an empty array" do
+ @property.stubs(:sync).returns []
- it "should return the default event if syncing the property returns an empty array" do
- @property.stubs(:sync).returns []
+ @change.expects(:event).with(nil).returns @event
- @change.expects(:event).with(:myprop_changed).returns :myevent
+ @change.forward.should == @event
+ end
- @change.forward.should == [:myevent]
- end
+ it "should log the change" do
+ @property.expects(:sync).returns [:one]
- it "should log the change" do
- @property.expects(:sync).returns [:one]
+ @property.expects(:notice).returns "my log"
- @property.expects(:log)
- @property.expects(:change_to_s)
+ @change.forward
+ end
- @change.forward
- end
+ it "should set the event's log to the log" do
+ @property.expects(:notice).returns "my log"
+ @change.forward.log.should == "my log"
+ end
+
+ it "should set the event's status to 'success'" do
+ @change.forward.status.should == "success"
+ end
- it "should return an array of events" do
- @property.expects(:sync).returns [:one, :two]
+ describe "and the change fails" do
+ before { @property.expects(:sync).raises "an exception" }
- @change.expects(:event).with(:one).returns :uno
- @change.expects(:event).with(:two).returns :dos
+ it "should catch the exception and log the err" do
+ @property.expects(:err)
+ lambda { @change.forward }.should_not raise_error
+ end
- @change.forward.should == [:uno, :dos]
+ it "should mark the event status as 'failure'" do
+ @change.forward.status.should == "failure"
+ end
+
+ it "should set the event log to a failure log" do
+ @property.expects(:err).returns "my failure"
+ @change.forward.log.should == "my failure"
end
end
@@ -177,9 +213,9 @@ describe Puppet::Transaction::Change do
@change.backward
end
- it "should execute" do
- @change.expects(:go)
- @change.backward
+ it "should execute and return the resulting event" do
+ @change.expects(:go).returns :myevent
+ @change.backward.should == :myevent
end
end
end
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list