Revamping collections to get what is hopefully more reasonable behaviour when they are used in combination with defined resource types. You should now be able to combine them in just about any way and get "correct" behaviour, which in this case means that you can have virtual definitions or definitions wrapping virtual resources and the resources will still all get realized.

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

2 files changed, 54 insertions, 24 deletions

diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index d64e76132..9eae4cc4a 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -62,16 +62,25 @@ class Puppet::Parser::Collector
             "realize() is not yet implemented for exported resources"
     end
 
+    # Collect resources directly; this is the result of using 'realize',
+    # which specifies resources, rather than using a normal collection.
     def collect_virtual_resources
-        @resources.collect do |ref|
+        result = @resources.dup.collect do |ref|
             if res = @scope.findresource(ref.to_s)
+                @resources.delete(ref)
                 res
-            else
-                raise Puppet::ParseError, "Could not find resource %s" % ref
             end
-        end.each do |res|
+        end.reject { |r| r.nil? }.each do |res|
             res.virtual = false
         end
+
+        # If there are no more resources to find, delete this from the list
+        # of collections.
+        if @resources.empty?
+            @scope.collections.delete(self)
+        end
+
+        return result
     end
 
     # Collect just virtual objects, from our local configuration.
@@ -90,24 +99,20 @@ class Puppet::Parser::Collector
     # and then delete this object from the list of collections to evaluate.
     def evaluate
         if self.resources
-            # We don't want to get rid of the collection unless it actually
-            # finds something, so that the collection will keep trying until
-            # all of the definitions are evaluated.
-            unless objects = collect_resources
-                return
+            if objects = collect_resources and ! objects.empty?
+                return objects
+            else
+                return false
             end
         else
             method = "collect_#{@form.to_s}"
-            objects = send(method).each do |obj|
-                obj.virtual = false
+            objects = send(method).each { |obj| obj.virtual = false }
+            if objects.empty?
+                return false
+            else
+                return objects
             end
         end
-
-        # And then remove us from the list of collections, since we've
-        # now been evaluated.
-        @scope.collections.delete(self)
-
-        objects
     end
 
     def initialize(scope, type, equery, vquery, form)
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index 3b268c16c..24de0904e 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -97,12 +97,32 @@ class Puppet::Parser::Interpreter
         return @ldap
     end
 
+    # Make sure we don't have any remaining collections that specifically
+    # look for resources, because we want to consider those to be
+    # parse errors.
+    def check_resource_collections(scope)
+        remaining = []
+        scope.collections.each do |coll|
+            if r = coll.resources
+                if r.is_a?(Array)
+                    remaining += r
+                else
+                    remaining << r
+                end
+            end
+        end
+        unless remaining.empty?
+            raise Puppet::ParseError, "Failed to find virtual resources %s" %
+                remaining.join(', ')
+        end
+    end
+
     def clear
         initparsevars
     end
 
-    # Iteratively evaluate all of the objects.  This finds all of the
-    # objects that represent definitions and evaluates the definitions appropriately.
+    # Iteratively evaluate all of the objects.  This finds all of the objects
+    # that represent definitions and evaluates the definitions appropriately.
     # It also adds defaults and overrides as appropriate.
     def evaliterate(scope)
         count = 0
@@ -113,10 +133,13 @@ class Puppet::Parser::Interpreter
             if coll = scope.collections and ! coll.empty?
                 exceptwrap do
                     coll.each do |c|
-                        c.evaluate
+                        # Only keep the loop going if we actually successfully
+                        # collected something.
+                        if c.evaluate
+                            done = false
+                        end
                     end
                 end
-                done = false
             end
             
             # Then evaluate any defined types.
@@ -124,6 +147,7 @@ class Puppet::Parser::Interpreter
                 ary.each do |resource|
                     resource.evaluate
                 end
+                # If we evaluated, then loop through again.
                 done = false
             end
             break if done
@@ -240,14 +264,15 @@ class Puppet::Parser::Interpreter
     # Fail if there any overrides left to perform.
     def failonleftovers(scope)
         overrides = scope.overrides
-        if overrides.empty?
-            return nil
-        else
+        unless overrides.empty?
             fail Puppet::ParseError,
                 "Could not find object(s) %s" % overrides.collect { |o|
                     o.ref
                 }.join(", ")
         end
+
+        # Now check that there aren't any extra resource collections.
+        check_resource_collections(scope)
     end
 
     # Find a class definition, relative to the current namespace.