diff --git a/lib/puppet/environments.rb b/lib/puppet/environments.rb index aebbdd13ee6..c0390d10f6d 100644 --- a/lib/puppet/environments.rb +++ b/lib/puppet/environments.rb @@ -225,6 +225,9 @@ def get_conf(name) private def create_environment(name) + # interpolated modulepaths may be cached from prior environment instances + Puppet.settings.clear_environment_settings(name) + env_symbol = name.intern setting_values = Puppet.settings.values(env_symbol, Puppet.settings.preferred_run_mode) env = Puppet::Node::Environment.create( @@ -350,7 +353,19 @@ def initialize(loader) # @!macro loader_list def list - @loader.list + # Evict all that have expired, in the same way as `get` + clear_all_expired + + @loader.list.map do |env| + name = env.name + old_entry = @cache[name] + if old_entry + old_entry.value + else + add_entry(name, entry(env)) + env + end + end end # @!macro loader_search_paths diff --git a/lib/puppet/file_system/memory_file.rb b/lib/puppet/file_system/memory_file.rb index 5ca2b43f789..778bfe01022 100644 --- a/lib/puppet/file_system/memory_file.rb +++ b/lib/puppet/file_system/memory_file.rb @@ -7,6 +7,13 @@ def self.a_missing_file(path) new(path, :exist? => false, :executable? => false) end + def self.a_missing_directory(path) + new(path, + :exist? => false, + :executable? => false, + :directory? => true) + end + def self.a_regular_file_containing(path, content) new(path, :exist? => true, :executable? => false, :content => content) end @@ -18,7 +25,7 @@ def self.an_executable(path) def self.a_directory(path, children = []) new(path, :exist? => true, - :excutable? => true, + :executable? => true, :directory? => true, :children => children) end diff --git a/lib/puppet/settings/environment_conf.rb b/lib/puppet/settings/environment_conf.rb index 94219adc532..0775d2ca347 100644 --- a/lib/puppet/settings/environment_conf.rb +++ b/lib/puppet/settings/environment_conf.rb @@ -29,6 +29,7 @@ def self.load_from(path_to_env, global_module_path) section = config.sections[:main] rescue Errno::ENOENT # environment.conf is an optional file + Puppet.debug { "Path to #{path_to_env} does not exist, using default environment.conf" } end new(path_to_env, section, global_module_path) diff --git a/spec/integration/indirector/direct_file_server_spec.rb b/spec/integration/indirector/direct_file_server_spec.rb index 1a158dd7a84..d46d16058d2 100644 --- a/spec/integration/indirector/direct_file_server_spec.rb +++ b/spec/integration/indirector/direct_file_server_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'matchers/include' require 'puppet/indirector/file_content/file' require 'puppet/indirector/file_metadata/file' @@ -30,7 +29,6 @@ describe Puppet::Indirector::DirectFileServer, " when interacting with FileServing::Fileset and the model" do include PuppetSpec::Files - include Matchers::Include matcher :file_with_content do |name, content| match do |actual| @@ -52,7 +50,7 @@ terminus = Puppet::Indirector::FileContent::File.new request = terminus.indirection.request(:search, Puppet::Util.path_to_uri(path).to_s, nil, :recurse => true) - expect(terminus.search(request)).to include_in_any_order( + expect(terminus.search(request)).to contain_exactly( file_with_content(File.join(path, "one"), "one content"), file_with_content(File.join(path, "two"), "two content"), directory_named(path)) diff --git a/spec/lib/matchers/include.rb b/spec/lib/matchers/include.rb deleted file mode 100644 index 35606a61590..00000000000 --- a/spec/lib/matchers/include.rb +++ /dev/null @@ -1,27 +0,0 @@ -module Matchers; module Include - extend RSpec::Matchers::DSL - - matcher :include_in_any_order do |*matchers| - match do |enumerable| - @not_matched = [] - expected_as_array.each do |matcher| - if enumerable.empty? - break - end - - if found = enumerable.find { |elem| matcher.matches?(elem) } - enumerable = enumerable.reject { |elem| elem == found } - else - @not_matched << matcher - end - end - - - @not_matched.empty? && enumerable.empty? - end - - failure_message do |enumerable| - "did not match #{@not_matched.collect(&:description).join(', ')} in #{enumerable.inspect}: <#{@not_matched.collect(&:failure_message).join('>, <')}>" - end - end -end; end diff --git a/spec/lib/matchers/include_spec.rb b/spec/lib/matchers/include_spec.rb deleted file mode 100644 index 7a55e90f8e3..00000000000 --- a/spec/lib/matchers/include_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'spec_helper' -require 'matchers/include' - -describe "include matchers" do - include Matchers::Include - - context :include_in_any_order do - it "matches an empty list" do - expect([]).to include_in_any_order() - end - - it "matches a list with a single element" do - expect([1]).to include_in_any_order(eq(1)) - end - - it "does not match when an expected element is missing" do - expect([1]).to_not include_in_any_order(eq(2)) - end - - it "matches a list with 2 elements in a different order from the expectation" do - expect([1, 2]).to include_in_any_order(eq(2), eq(1)) - end - - it "does not match when there are more than just the expected elements" do - expect([1, 2]).to_not include_in_any_order(eq(1)) - end - - it "matches multiple, equal elements when there are multiple, equal exepectations" do - expect([1, 1]).to include_in_any_order(eq(1), eq(1)) - end - end -end diff --git a/spec/unit/environments_spec.rb b/spec/unit/environments_spec.rb index c5d77d74de9..f914458af62 100644 --- a/spec/unit/environments_spec.rb +++ b/spec/unit/environments_spec.rb @@ -1,13 +1,8 @@ require 'spec_helper' require 'puppet/environments' require 'puppet/file_system' -require 'matchers/include' -require 'matchers/include_in_order' -module PuppetEnvironments describe Puppet::Environments do - include Matchers::Include - FS = Puppet::FileSystem before(:each) do @@ -49,7 +44,7 @@ module PuppetEnvironments loader_from(:filesystem => [directory_tree, global_path_1, global_path_2], :directory => directory_tree.children.first, :modulepath => [global_path_1_location, global_path_2_location]) do |loader| - expect(loader.list).to include_in_any_order( + expect(loader.list).to contain_exactly( environment(:an_environment). with_manifest("#{FS.path_string(directory_tree)}/envdir/an_environment/manifests"). with_modulepath(["#{FS.path_string(directory_tree)}/envdir/an_environment/modules", @@ -87,7 +82,7 @@ module PuppetEnvironments loader_from(:filesystem => [envdir], :directory => envdir) do |loader| - expect(loader.list).to include_in_any_order(environment(:env1), environment(:env2)) + expect(loader.list).to contain_exactly(environment(:env1), environment(:env2)) end end @@ -406,33 +401,29 @@ module PuppetEnvironments ]), ]) - FS.overlay(original_envdir) do - dir_loader = Puppet::Environments::Directories.new(original_envdir, []) - loader = Puppet::Environments::Cached.new(dir_loader) - Puppet.override(:environments => loader) do - original_env = loader.get("env3") # force the environment.conf to be read - - changed_envdir = FS::MemoryFile.a_directory(base_dir, [ - FS::MemoryFile.a_directory("env3", [ - FS::MemoryFile.a_regular_file_containing("environment.conf", <<-EOF) - manifest=/manifest_changed - modulepath=/modules_changed - environment_timeout=0 - EOF - ]), - ]) + cached_loader_from(:filesystem => [original_envdir], :directory => original_envdir) do |loader| + original_env = loader.get("env3") # force the environment.conf to be read - FS.overlay(changed_envdir) do - changed_env = loader.get("env3") + changed_envdir = FS::MemoryFile.a_directory(base_dir, [ + FS::MemoryFile.a_directory("env3", [ + FS::MemoryFile.a_regular_file_containing("environment.conf", <<-EOF) + manifest=/manifest_changed + modulepath=/modules_changed + environment_timeout=0 + EOF + ]), + ]) - expect(original_env).to environment(:env3). - with_manifest(File.expand_path("/manifest_orig")). - with_full_modulepath([File.expand_path("/modules_orig")]) + FS.overlay(changed_envdir) do + changed_env = loader.get("env3") - expect(changed_env).to environment(:env3). - with_manifest(File.expand_path("/manifest_changed")). - with_full_modulepath([File.expand_path("/modules_changed")]) - end + expect(original_env).to environment(:env3). + with_manifest(File.expand_path("/manifest_orig")). + with_full_modulepath([File.expand_path("/modules_orig")]) + + expect(changed_env).to environment(:env3). + with_manifest(File.expand_path("/manifest_changed")). + with_full_modulepath([File.expand_path("/modules_changed")]) end end end @@ -558,24 +549,49 @@ module PuppetEnvironments describe "cached loaders" do it "lists environments" do - loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| - expect(Puppet::Environments::Cached.new(loader).list).to include_in_any_order( + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.list).to contain_exactly( environment(:an_environment), environment(:another_environment), environment(:symlinked_environment)) end end + it "returns the same cached environment object for list and get methods" do + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + env = loader.list.find { |e| e.name == :an_environment } + + expect(env).to equal(loader.get(:an_environment)) # same object + end + end + + it "returns the same cached environment object for multiple list calls" do + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.list.first).to equal(loader.list.first) # same object + end + end + + it "expires environments and returns a new environment object with the same value" do + Puppet[:environment_timeout] = "0" + + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + a = loader.list.first + b = loader.list.first + expect(a).to eq(b) # same value + expect(a).to_not equal(b) # not same object + end + end + it "has search_paths" do - loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| - expect(Puppet::Environments::Cached.new(loader).search_paths).to eq(["file://#{directory_tree.children.first}"]) + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.search_paths).to eq(["file://#{directory_tree.children.first}"]) end end context "#get" do it "gets an environment" do - loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| - expect(Puppet::Environments::Cached.new(loader).get(:an_environment)).to environment(:an_environment) + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.get(:an_environment)).to environment(:an_environment) end end @@ -592,16 +608,16 @@ module PuppetEnvironments end it "returns nil if env not found" do - loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| - expect(Puppet::Environments::Cached.new(loader).get(:doesnotexist)).to be_nil + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.get(:doesnotexist)).to be_nil end end end context "#get!" do it "gets an environment" do - loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| - expect(Puppet::Environments::Cached.new(loader).get!(:an_environment)).to environment(:an_environment) + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.get!(:an_environment)).to environment(:an_environment) end end @@ -618,14 +634,41 @@ module PuppetEnvironments end it "raises error if environment is not found" do - loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| expect do - Puppet::Environments::Cached.new(loader).get!(:doesnotexist) + loader.get!(:doesnotexist) end.to raise_error(Puppet::Environments::EnvironmentNotFound) end end end + context "#get_conf" do + it "loads environment.conf" do + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.get_conf(:an_environment)).to match_environment_conf(:an_environment). + with_env_path(directory_tree.children.first). + with_global_module_path([]) + end + end + + it "always reloads environment.conf" do + env = Puppet::Node::Environment.create(:cached, []) + mocked_loader = double('loader') + expect(mocked_loader).to receive(:get_conf).with(:cached).and_return(Puppet::Settings::EnvironmentConf.static_for(env, 20)).twice + + cached = Puppet::Environments::Cached.new(mocked_loader) + + cached.get_conf(:cached) + cached.get_conf(:cached) + end + + it "returns nil if environment is not found" do + cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| + expect(loader.get_conf(:doesnotexist)).to be_nil + end + end + end + context "expiration policies" do let(:service) { ReplayExpirationService.new } @@ -647,8 +690,6 @@ module PuppetEnvironments end it "evicts an expired environment" do - service = ReplayExpirationService.new - expect(service).to receive(:expired?).and_return(true) with_environment_loaded(service) do |cached| @@ -703,13 +744,15 @@ module PuppetEnvironments expect(service.created_envs).to eq([:an_environment, :an_environment]) expect(service.evicted_envs).to eq([:an_environment]) end - end - it "gets an environment.conf" do - loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader| - expect(Puppet::Environments::Cached.new(loader).get_conf(:an_environment)).to match_environment_conf(:an_environment). - with_env_path(directory_tree.children.first). - with_global_module_path([]) + it "evicts expired environments when listing" do + expect(service).to receive(:expired?).with(:an_environment).and_return(true) + + with_environment_loaded(service) do |cached| + cached.list + end + + expect(service.evicted_envs).to eq([:an_environment]) end end @@ -727,6 +770,30 @@ module PuppetEnvironments context '#clear_all' do let(:service) { ReplayExpirationService.new } + let(:envdir) { File.expand_path("envdir") } + let(:default_dir) { File.join(envdir, "cached_env", "modules") } + let(:expected_dir) { File.join(envdir, "cached_env", "site") } + + let(:base_dir) do + FS::MemoryFile.a_directory(envdir, [ + FS::MemoryFile.a_directory("cached_env", [ + FS::MemoryFile.a_missing_file("environment.conf") + ]) + ]) + end + + let(:updated_dir) do + FS::MemoryFile.a_directory(envdir, [ + FS::MemoryFile.a_directory("cached_env", [ + FS::MemoryFile.a_directory("site"), + FS::MemoryFile.a_missing_directory("modules"), + FS::MemoryFile.a_regular_file_containing("environment.conf", <<-EOF) + modulepath=site + environment_timeout=unlimited + EOF + ]) + ]) + end it 'evicts all environments' do with_environment_loaded(service) do |cached| @@ -738,48 +805,44 @@ module PuppetEnvironments end end - it 'clears cached environment settings' do - base_dir = File.expand_path("envdir") - original_envdir = FS::MemoryFile.a_directory(base_dir, [ - FS::MemoryFile.a_directory("env3", [ - FS::MemoryFile.a_regular_file_containing("environment.conf", <<-EOF) - manifest=/manifest_orig - modulepath=/modules_orig - environment_timeout=60 - EOF - ]), - ]) + it "recomputes modulepath if 'get' is called before 'clear_all'" do + cached_loader_from(:filesystem => [base_dir], :directory => base_dir) do |loader| + loader.get(:cached_env) - FS.overlay(original_envdir) do - dir_loader = Puppet::Environments::Directories.new(original_envdir, []) - loader = Puppet::Environments::Cached.new(dir_loader) - Puppet.override(:environments => loader) do - original_env = loader.get("env3") # force the environment.conf to be read - - changed_envdir = FS::MemoryFile.a_directory(base_dir, [ - FS::MemoryFile.a_directory("env3", [ - FS::MemoryFile.a_regular_file_containing("environment.conf", <<-EOF) - manifest=/manifest_changed - modulepath=/modules_changed - environment_timeout=60 - EOF - ]), - ]) + expect(Puppet.settings.value(:modulepath, :cached_env)).to eq(default_dir) - #Clear all cached environments + FS.overlay(updated_dir) do loader.clear_all - FS.overlay(changed_envdir) do - changed_env = loader.get("env3") + expect(loader.get(:cached_env).modulepath).to contain_exactly(expected_dir) + end + end + end + + it "recomputes modulepath if 'list' is called before 'clear_all'" do + cached_loader_from(:filesystem => [base_dir], :directory => base_dir) do |loader| + loader.list - expect(original_env).to environment(:env3). - with_manifest(File.expand_path("/manifest_orig")). - with_full_modulepath([File.expand_path("/modules_orig")]) + expect(Puppet.settings.value(:modulepath, :cached_env)).to eq(default_dir) - expect(changed_env).to environment(:env3). - with_manifest(File.expand_path("/manifest_changed")). - with_full_modulepath([File.expand_path("/modules_changed")]) - end + FS.overlay(updated_dir) do + loader.clear_all + + expect(loader.get(:cached_env).modulepath).to contain_exactly(expected_dir) + end + end + end + + it "recomputes modulepath if 'get_conf' is called before 'clear_all'" do + cached_loader_from(:filesystem => [base_dir], :directory => base_dir) do |loader| + loader.get_conf(:cached_env) + + expect(Puppet.settings.value(:modulepath, :cached_env)).to eq(default_dir) + + FS.overlay(updated_dir) do + loader.clear_all + + expect(loader.get(:cached_env).modulepath).to contain_exactly(expected_dir) end end end @@ -865,6 +928,20 @@ module PuppetEnvironments end end + def cached_loader_from(options, &block) + FS.overlay(*options[:filesystem]) do + environments = Puppet::Environments::Cached.new( + Puppet::Environments::Directories.new( + options[:directory], + options[:modulepath] || [] + ) + ) + Puppet.override(:environments => environments) do + yield environments + end + end + end + def loader_from(options, &block) FS.overlay(*options[:filesystem]) do environments = Puppet::Environments::Directories.new( @@ -917,4 +994,3 @@ def evicted(env_name) end end end -end