Merge pull request #3197 from hkenney/issue/stable/pup-3334_fix_environment_conf_not_changing_issue

(PUP-3334) Fix issue around settings not updating with environment.conf

3 files changed, 71 insertions, 3 deletions

diff --git a/lib/puppet/environments.rb b/lib/puppet/environments.rb
index 1095759fd..ec7a12940 100644
--- a/lib/puppet/environments.rb
+++ b/lib/puppet/environments.rb
@@ -211,11 +211,11 @@ module Puppet::Environments
     # @!macro loader_list
     def list
       valid_directories.collect do |envdir|
-        name = Puppet::FileSystem.basename_string(envdir)
+        name = Puppet::FileSystem.basename_string(envdir).intern
 
         setting_values = Puppet.settings.values(name, Puppet.settings.preferred_run_mode)
         env = Puppet::Node::Environment.create(
-          name.intern,
+          name,
           Puppet::Node::Environment.split_path(setting_values.interpolate(:modulepath)),
           setting_values.interpolate(:manifest),
           setting_values.interpolate(:config_version)
@@ -367,10 +367,12 @@ module Puppet::Environments
     end
 
     # Evicts the entry if it has expired
-    #
+    # Also clears caches in Settings that may prevent the entry from being updated
     def evict_if_expired(name)
       if (result = @cache[name]) && result.expired?
         @cache.delete(name)
+
+        Puppet.settings.clear_environment_settings(name)
       end
     end
 
diff --git a/lib/puppet/settings.rb b/lib/puppet/settings.rb
index 499ee1502..a0f41f1c7 100644
--- a/lib/puppet/settings.rb
+++ b/lib/puppet/settings.rb
@@ -205,6 +205,23 @@ class Puppet::Settings
   end
   private :unsafe_clear
 
+  # Clears all cached settings for a particular environment to ensure
+  # that changes to environment.conf are reflected in the settings if
+  # the environment timeout has expired.
+  #
+  # param [String, Symbol] environment the  name of environment to clear settings for
+  #
+  # @api private
+  def clear_environment_settings(environment)
+
+    if environment.nil?
+      return
+    end
+
+    @cache[environment.to_sym].clear
+    @values[environment.to_sym] = {}
+  end
+
   # Clear @cache, @used and the Environment.
   #
   # Whenever an object is returned by Settings, a copy is stored in @cache.
diff --git a/spec/unit/environments_spec.rb b/spec/unit/environments_spec.rb
index e68abd5ce..fe0814814 100644
--- a/spec/unit/environments_spec.rb
+++ b/spec/unit/environments_spec.rb
@@ -317,6 +317,49 @@ config_version=$vardir/random/scripts
             with_config_version(File.expand_path('/some/script'))
         end
       end
+
+      it "should update environment settings if environment.conf has changed and timeout has expired" 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=0
+            EOF
+          ]),
+        ])
+
+        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
+              ]),
+            ])
+
+            FS.overlay(changed_envdir) do
+              changed_env = loader.get("env3")
+
+              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
+      end
     end
   end
 
@@ -391,6 +434,7 @@ config_version=$vardir/random/scripts
       env.name == name &&
         (!@manifest || @manifest == env.manifest) &&
         (!@modulepath || @modulepath == env.modulepath) &&
+        (!@full_modulepath || @full_modulepath == env.full_modulepath) &&
         (!@config_version || @config_version == env.config_version)
     end
 
@@ -402,6 +446,10 @@ config_version=$vardir/random/scripts
       @modulepath = modulepath
     end
 
+    chain :with_full_modulepath do |full_modulepath|
+      @full_modulepath = full_modulepath
+    end
+
     chain :with_config_version do |config_version|
       @config_version = config_version
     end
@@ -410,6 +458,7 @@ config_version=$vardir/random/scripts
       "environment #{expected}" +
         (@manifest ? " with manifest #{@manifest}" : "") +
         (@modulepath ? " with modulepath [#{@modulepath.join(', ')}]" : "") +
+        (@full_modulepath ? " with full_modulepath [#{@full_modulepath.join(', ')}]" : "") +
         (@config_version ? " with config_version #{@config_version}" : "")
     end