I know I assigned the bug to Luke, but I had 5 spare minutes tonight, and
wanted to try to solve this as this is affecting me.
So far it seems to work fine.
Please comment and review,
Brice
Original commit msg:
While evaluating the AST, catalog vertices are not always ordered
the same way on different run, leading to some tags (which should
have been applied in evaluation order) to not be associated with
some underlying resources.
This changeset change all accesses to resources inside the compiler
to always use an ordered (in evaluation order) list of added
resources.
Signed-off-by: Brice Figureau <brice-pup...@daysofwonder.com>
---
lib/puppet/parser/compiler.rb | 19 ++++++++++---------
spec/unit/parser/compiler.rb | 27 ++++++++++++++++++++++++++-
2 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 9f5548d..7dcd502 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -10,7 +10,7 @@ require 'puppet/util/errors'
class Puppet::Parser::Compiler
include Puppet::Util
include Puppet::Util::Errors
- attr_reader :parser, :node, :facts, :collections, :catalog, :node_scope
+ attr_reader :parser, :node, :facts, :collections, :catalog, :node_scope,
:resources
# Add a collection to the global list.
def add_collection(coll)
@@ -31,6 +31,8 @@ class Puppet::Parser::Compiler
# Store a resource in our resource table.
def add_resource(scope, resource)
+ @resources << resource
+
# Note that this will fail if the resource is not unique.
@catalog.add_resource(resource)
@@ -204,11 +206,6 @@ class Puppet::Parser::Compiler
@resource_overrides[resource.ref]
end
- # Return a list of all resources.
- def resources
- @catalog.vertices
- end
-
# The top scope is usually the top-level scope, but if we're using AST
nodes,
# then it is instead the node's scope.
def topscope
@@ -310,6 +307,7 @@ class Puppet::Parser::Compiler
@main_resource = Puppet::Parser::Resource.new(:type => "class", :title
=> :main, :scope => @topscope, :source => @main)
@topscope.resource = @main_resource
+ @resources << @main_resource
@catalog.add_resource(@main_resource)
@main_resource.evaluate
@@ -366,7 +364,7 @@ class Puppet::Parser::Compiler
# Make sure all of our resources and such have done any last work
# necessary.
def finish
- @catalog.vertices.each do |resource|
+ resources.each do |resource|
# Add in any resource overrides.
if overrides = resource_overrides(resource)
overrides.each do |over|
@@ -414,6 +412,9 @@ class Puppet::Parser::Compiler
# For maintaining the relationship between scopes and their resources.
@catalog = Puppet::Resource::Catalog.new(@node.name)
@catalog.version = @parser.version
+
+ # local resource array to maintain resource ordering
+ @resources = []
end
# Set the node's parameters into the top-scope as variables.
@@ -436,7 +437,7 @@ class Puppet::Parser::Compiler
# We used to have hooks here for forking and saving, but I don't
# think it's worth retaining at this point.
- store_to_active_record(@node, @catalog.vertices)
+ store_to_active_record(@node, resources)
end
# Do the actual storage.
@@ -459,7 +460,7 @@ class Puppet::Parser::Compiler
# Return an array of all of the unevaluated resources. These will be
definitions,
# which need to get evaluated into native resources.
def unevaluated_resources
- ary = @catalog.vertices.reject { |resource| resource.builtin? or
resource.evaluated? }
+ ary = resources.reject { |resource| resource.builtin? or
resource.evaluated? }
if ary.empty?
return nil
diff --git a/spec/unit/parser/compiler.rb b/spec/unit/parser/compiler.rb
index f36b6fd..855a8c7 100755
--- a/spec/unit/parser/compiler.rb
+++ b/spec/unit/parser/compiler.rb
@@ -239,6 +239,31 @@ describe Puppet::Parser::Compiler do
@compiler.send(:finish)
end
+ it "should call finish() in add_resource order" do
+ resources = sequence('resources')
+
+ resource1 = Puppet::Parser::Resource.new :scope => @scope, :type
=> "file", :title => "finish1"
+ resource1.expects(:finish).in_sequence(resources)
+
+ @compiler.add_resource(@scope, resource1)
+
+ resource2 = Puppet::Parser::Resource.new :scope => @scope, :type
=> "file", :title => "finish2"
+ resource2.expects(:finish).in_sequence(resources)
+
+ @compiler.add_resource(@scope, resource2)
+
+ @compiler.send(:finish)
+ end
+
+ it "should return added resources in add order" do
+ resource1 = stub "1", :ref => "File[yay]"
+ @compiler.add_resource(@scope, resource1)
+ resource2 = stub "2", :ref => "File[youpi]"
+ @compiler.add_resource(@scope, resource2)
+
+ @compiler.resources.should == [resource1, resource2]
+ end
+
it "should add resources that do not conflict with existing resources"
do
resource = stub "noconflict", :ref => "File[yay]"
@compiler.add_resource(@scope, resource)
@@ -484,7 +509,7 @@ describe Puppet::Parser::Compiler do
Puppet.features.expects(:rails?).returns(true)
Puppet::Rails.expects(:connect)
- @compiler.catalog.expects(:vertices).returns(:resources)
+ @compiler.expects(:resources).returns(:resources)
@compiler.expects(:store_to_active_record).with(@node, :resources)
@compiler.send(:store)
--
1.6.0.2
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
