Multiple environment support now works, and I have even tested it in real life. This commit is mostly a bug-fix commit, resulting from the difference between real-life testing and unit testing.

8 files changed, 118 insertions, 52 deletions

diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb
index fd1ee86ed..b2b16d022 100644
--- a/lib/puppet/network/handler/configuration.rb
+++ b/lib/puppet/network/handler/configuration.rb
@@ -92,6 +92,10 @@ class Puppet::Network::Handler
             end
 
             # Ask the interpreter to compile the configuration.
+            str = "Compiled configuration for %s" % node.name
+            if node.environment
+                str += " in environment %s" % node.environment
+            end
             config = nil
             benchmark(level, "Compiled configuration for %s" % node.name) do
                 begin
@@ -117,8 +121,8 @@ class Puppet::Network::Handler
             # Allow specification of a code snippet or of a file
             if code = options[:Code]
                 args[:Code] = code
-            else
-                args[:Manifest] = options[:Manifest] || Puppet[:manifest]
+            elsif options[:Manifest]
+                args[:Manifest] = options[:Manifest]
             end
 
             args[:Local] = local?
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index b6c61d202..93a4bc170 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -67,6 +67,9 @@ class Puppet::Parser::Interpreter
                 parser.string = self.code
             elsif self.file
                 parser.file = self.file
+            else
+                file = Puppet.config.value(:manifest, environment)
+                parser.file = file
             end
             parser.parse
             return parser
diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb
index 401b5b1c0..660fa8169 100644
--- a/lib/puppet/parser/parser_support.rb
+++ b/lib/puppet/parser/parser_support.rb
@@ -15,7 +15,7 @@ class Puppet::Parser::Parser
 
     AST = Puppet::Parser::AST
 
-    attr_reader :file, :version
+    attr_reader :version, :environment
     attr_accessor :files
 
 
@@ -85,6 +85,10 @@ class Puppet::Parser::Parser
         raise except
     end
 
+    def file
+        @lexer.file
+    end
+
     def file=(file)
         unless FileTest.exists?(file)
             unless file =~ /\.pp$/
diff --git a/lib/puppet/util/config.rb b/lib/puppet/util/config.rb
index b6831ba9b..fb1c01d56 100644
--- a/lib/puppet/util/config.rb
+++ b/lib/puppet/util/config.rb
@@ -38,7 +38,7 @@ class Puppet::Util::Config
                 @name = nil
             end
             @values[:memory][param] = value
-            @values[:cache].clear
+            @cache.clear
         end
 
         return value
@@ -113,12 +113,14 @@ class Puppet::Util::Config
             @used = []
         end
 
+        @cache.clear
+
         @name = nil
     end
 
     # This is mostly just used for testing.
     def clearused
-        @values[:cache].clear
+        @cache.clear
         @used = []
     end
 
@@ -173,6 +175,7 @@ class Puppet::Util::Config
 
     # Handle a command-line argument.
     def handlearg(opt, value = nil)
+        clear(true)
         value = munge_value(value) if value
         str = opt.sub(/^--/,'')
         bool = true
@@ -215,6 +218,9 @@ class Puppet::Util::Config
         # Keep track of set values.
         @values = Hash.new { |hash, key| hash[key] = {} }
 
+        # And keep a per-environment cache
+        @cache = Hash.new { |hash, key| hash[key] = {} }
+
         # A central concept of a name.
         @name = nil
     end
@@ -448,9 +454,9 @@ class Puppet::Util::Config
     # The order in which to search for values.
     def searchpath(environment = nil)
         if environment
-            [:cache, :cli, :memory, environment, :name, :main]
+            [:cli, :memory, environment, :name, :main]
         else
-            [:cache, :cli, :memory, :name, :main]
+            [:cli, :memory, :name, :main]
         end
     end
 
@@ -730,7 +736,15 @@ Generated on #{Time.now}.
         # Yay, recursion.
         self.reparse() unless param == :filetimeout
 
+        # Check the cache first.  It needs to be a per-environment
+        # cache so that we don't spread values from one env
+        # to another.
+        if @cache[environment||"none"].include?(param)
+            return @cache[environment||"none"][param]
+        end
+
         # See if we can find it within our searchable list of values
+        val = nil
         searchpath(environment).each do |source|
             # Modify the source as necessary.
             source = case source
@@ -740,21 +754,24 @@ Generated on #{Time.now}.
                 source
             end
 
-            # Look for the value.
+            # Look for the value.  We have to test the hash for whether
+            # it exists, because the value might be false.
             if @values[source].include?(param)
                 val = @values[source][param]
-                # Cache the value, because we do so many parameter lookups.
-                unless source == :cache
-                    val = convert(val)
-                    @values[:cache][param] = val
-                end
-                return val
+                break
             end
         end
 
-        # No normal source, so get the default and cache it
-        val = convert(@config[param].default)
-        @values[:cache][param] = val
+        # If we didn't get a value, use the default
+        if val.nil?
+            val = @config[param].default
+        end
+
+        # Convert it if necessary
+        val = convert(val)
+
+        # And cache it
+        @cache[environment||"none"][param] = val
         return val
     end
 
diff --git a/spec/unit/parser/interpreter.rb b/spec/unit/parser/interpreter.rb
index 7328e2651..ebb7d4cbf 100755
--- a/spec/unit/parser/interpreter.rb
+++ b/spec/unit/parser/interpreter.rb
@@ -44,29 +44,51 @@ describe Puppet::Parser::Interpreter, " when creating parser instances" do
 
     it "should create a parser with code passed in at initialization time" do
         @interp.code = :some_code
-        @parser.expects(:code=).with(:some_code)
+        @parser.expects(:string=).with(:some_code)
         @parser.expects(:parse)
-        Puppet::Parser::Parser.expects(:new).with(:environment).returns(@parser)
-        @interp.send(:create_parser, :environment).object_id.should equal(@parser.object_id)
+        Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).returns(@parser)
+        @interp.send(:create_parser, :myenv).object_id.should equal(@parser.object_id)
     end
 
     it "should create a parser with a file passed in at initialization time" do
         @interp.file = :a_file
         @parser.expects(:file=).with(:a_file)
         @parser.expects(:parse)
-        Puppet::Parser::Parser.expects(:new).with(:environment).returns(@parser)
-        @interp.send(:create_parser, :environment).should equal(@parser)
+        Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).returns(@parser)
+        @interp.send(:create_parser, :myenv).should equal(@parser)
     end
 
-    it "should create a parser when passed neither code nor file" do
+    it "should create a parser with the main manifest when passed neither code nor file" do
         @parser.expects(:parse)
-        Puppet::Parser::Parser.expects(:new).with(:environment).returns(@parser)
-        @interp.send(:create_parser, :environment).should equal(@parser)
+        @parser.expects(:file=).with(Puppet[:manifest])
+        Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).returns(@parser)
+        @interp.send(:create_parser, :myenv).should equal(@parser)
     end
 
     it "should return nothing when new parsers fail" do
-        Puppet::Parser::Parser.expects(:new).with(:environment).raises(ArgumentError)
-        @interp.send(:create_parser, :environment).should be_nil
+        Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).raises(ArgumentError)
+        @interp.send(:create_parser, :myenv).should be_nil
+    end
+
+    it "should create parsers with environment-appropriate manifests" do
+        # Set our per-environment values.  We can't just stub :value, because
+        # it's called by too much of the rest of the code.
+        text = "[env1]\nmanifest = /t/env1.pp\n[env2]\nmanifest = /t/env2.pp"
+        file = mock 'file'
+        file.stubs(:changed?).returns(true)
+        file.stubs(:file).returns("/whatever")
+        Puppet.config.stubs(:read_file).with(file).returns(text)
+        Puppet.config.parse(file)
+
+        parser1 = mock 'parser1'
+        Puppet::Parser::Parser.expects(:new).with(:environment => :env1).returns(parser1)
+        parser1.expects(:file=).with("/t/env1.pp")
+        @interp.send(:create_parser, :env1)
+
+        parser2 = mock 'parser2'
+        Puppet::Parser::Parser.expects(:new).with(:environment => :env2).returns(parser2)
+        parser2.expects(:file=).with("/t/env2.pp")
+        @interp.send(:create_parser, :env2)
     end
 end
 
@@ -77,16 +99,16 @@ describe Puppet::Parser::Interpreter, " when managing parser instances" do
     end
 
     it "it should an exception when nothing is there and nil is returned" do
-        @interp.expects(:create_parser).with(:environment).returns(nil)
-        lambda { @interp.send(:parser, :environment) }.should raise_error(Puppet::Error)
+        @interp.expects(:create_parser).with(:myenv).returns(nil)
+        lambda { @interp.send(:parser, :myenv) }.should raise_error(Puppet::Error)
     end
 
     it "should create and return a new parser and use the same parser when the parser does not need reparsing" do
-        @interp.expects(:create_parser).with(:environment).returns(@parser)
-        @interp.send(:parser, :environment).should equal(@parser)
+        @interp.expects(:create_parser).with(:myenv).returns(@parser)
+        @interp.send(:parser, :myenv).should equal(@parser)
 
         @parser.expects(:reparse?).returns(false)
-        @interp.send(:parser, :environment).should equal(@parser)
+        @interp.send(:parser, :myenv).should equal(@parser)
     end
 
     it "should create a new parser when reparse is true" do
@@ -95,25 +117,25 @@ describe Puppet::Parser::Interpreter, " when managing parser instances" do
         oldparser.expects(:reparse?).returns(true)
         oldparser.expects(:clear)
 
-        @interp.expects(:create_parser).with(:environment).returns(oldparser)
-        @interp.send(:parser, :environment).should equal(oldparser)
-        @interp.expects(:create_parser).with(:environment).returns(newparser)
-        @interp.send(:parser, :environment).should equal(newparser)
+        @interp.expects(:create_parser).with(:myenv).returns(oldparser)
+        @interp.send(:parser, :myenv).should equal(oldparser)
+        @interp.expects(:create_parser).with(:myenv).returns(newparser)
+        @interp.send(:parser, :myenv).should equal(newparser)
     end
 
     it "should keep the old parser if create_parser doesn't return anything." do
         # Get the first parser in the hash.
-        @interp.expects(:create_parser).with(:environment).returns(@parser)
-        @interp.send(:parser, :environment).should equal(@parser)
+        @interp.expects(:create_parser).with(:myenv).returns(@parser)
+        @interp.send(:parser, :myenv).should equal(@parser)
 
         # Have it indicate something has changed
         @parser.expects(:reparse?).returns(true)
 
         # But fail to create a new parser
-        @interp.expects(:create_parser).with(:environment).returns(nil)
+        @interp.expects(:create_parser).with(:myenv).returns(nil)
 
         # And make sure we still get the old valid parser
-        @interp.send(:parser, :environment).should equal(@parser)
+        @interp.send(:parser, :myenv).should equal(@parser)
     end
 
     it "should use different parsers for different environments" do
@@ -140,7 +162,7 @@ describe Puppet::Parser::Interpreter, " when compiling configurations" do
         parser = mock 'parser'
         @interp.expects(:parser).with(:myenv).returns(parser)
         @interp.expects(:usenodes?).returns(true)
-        Puppet::Parser::Configuration.expects(:new).with(node, parser, :ast_nodes => true).returns(compile)
+        Puppet::Parser::Compile.expects(:new).with(node, parser, :ast_nodes => true).returns(compile)
         @interp.compile(node)
 
         # Now try it when usenodes is true
@@ -149,7 +171,7 @@ describe Puppet::Parser::Interpreter, " when compiling configurations" do
         compile.expects(:compile).returns(:config)
         @interp.expects(:parser).with(:myenv).returns(parser)
         @interp.expects(:usenodes?).returns(false)
-        Puppet::Parser::Configuration.expects(:new).with(node, parser, :ast_nodes => false).returns(compile)
+        Puppet::Parser::Compile.expects(:new).with(node, parser, :ast_nodes => false).returns(compile)
         @interp.compile(node).should equal(:config)
     end
 end
diff --git a/spec/unit/util/config.rb b/spec/unit/util/config.rb
index 7f9b64c94..0b3b65c28 100755
--- a/spec/unit/util/config.rb
+++ b/spec/unit/util/config.rb
@@ -139,6 +139,18 @@ describe Puppet::Util::Config, " when returning values" do
         @config[:two].should == "one TWO"
     end
 
+    it "should not cache values such that information from one environment is returned for another environment" do
+        text = "[env1]\none = oneval\n[env2]\none = twoval\n"
+        file = mock 'file'
+        file.stubs(:changed?).returns(true)
+        file.stubs(:file).returns("/whatever")
+        @config.stubs(:read_file).with(file).returns(text)
+        @config.parse(file)
+
+        @config.value(:one, "env1").should == "oneval"
+        @config.value(:one, "env2").should == "twoval"
+    end
+
     it "should have a name determined by the 'name' parameter" do
         @config.setdefaults(:whatever, :name => ["something", "yayness"])
         @config.name.should == :something
diff --git a/test/network/handler/configuration.rb b/test/network/handler/configuration.rb
index a34952208..072fdc053 100755
--- a/test/network/handler/configuration.rb
+++ b/test/network/handler/configuration.rb
@@ -52,7 +52,7 @@ class TestHandlerConfiguration < Test::Unit::TestCase
         args = {}
 
         # Try it first with defaults.
-        Puppet::Parser::Interpreter.expects(:new).with(:Local => config.local?, :Manifest => Puppet[:manifest]).returns(:interp)
+        Puppet::Parser::Interpreter.expects(:new).with(:Local => config.local?).returns(:interp)
         assert_equal(:interp, config.send(:create_interpreter, args), "Did not return the interpreter")
 
         # Now reset it and make sure a specified manifest passes through
@@ -93,10 +93,12 @@ class TestHandlerConfiguration < Test::Unit::TestCase
         config = Config.new
 
         # First do a local
-        node = Object.new
-        node.expects(:name).returns(:mynode)
+        node = mock 'node'
+        node.stubs(:name).returns(:mynode)
+        node.stubs(:environment).returns(:myenv)
 
-        interp = Object.new
+        interp = mock 'interpreter'
+        interp.stubs(:environment)
         interp.expects(:compile).with(node).returns(:config)
         config.expects(:interpreter).returns(interp)
 
@@ -105,13 +107,15 @@ class TestHandlerConfiguration < Test::Unit::TestCase
         assert_equal(:config, config.send(:compile, node), "Did not return config")
         
         # Now try it non-local
-        config = Config.new(:Local => true)
-
-        node = Object.new
-        node.expects(:name).returns(:mynode)
+        node = mock 'node'
+        node.stubs(:name).returns(:mynode)
+        node.stubs(:environment).returns(:myenv)
 
-        interp = Object.new
+        interp = mock 'interpreter'
+        interp.stubs(:environment)
         interp.expects(:compile).with(node).returns(:config)
+
+        config = Config.new(:Local => true)
         config.expects(:interpreter).returns(interp)
 
         assert_equal(:config, config.send(:compile, node), "Did not return config")
diff --git a/test/network/handler/node.rb b/test/network/handler/node.rb
index f7cbf6017..6b8ab9290 100755
--- a/test/network/handler/node.rb
+++ b/test/network/handler/node.rb
@@ -531,7 +531,7 @@ class LdapNodeTest < PuppetTest::TestCase
     end
 
     def ldaphost(name)
-        node = NodeDef.new(:name => name)
+        node = Puppet::Node.new(name)
         parent = nil
         found = false
         @ldap.search( "ou=hosts, dc=madstop, dc=com", 2,