The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #18187

Cached *root* environment isn't cleared when Puppet::Node::Environment.clear is called

Added by Dominic Cleal over 3 years ago. Updated about 3 years ago.

Status:RejectedStart date:
Priority:NormalDue date:
Assignee:Dominic Cleal% Done:

0%

Category:autoloader
Target version:3.1.1
Affected Puppet version:2.7.20 Branch:https://github.com/puppetlabs/puppet/pull/1455
Keywords:autoloader environments type regression

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com


Description

2.7.20 included a backport of a performance fix, which included a change to how types are autoloaded.

commit 8173a6e6c199426381f1f9fb8d0a71e0d0c12f2a

commit 8173a6e6c199426381f1f9fb8d0a71e0d0c12f2a
Author: Daniel Pittman <daniel@puppetlabs.com>
Date:   Mon Jul 16 23:40:31 2012 -0700

    Avoid object creation/destruction when possible.

    Object creation and destruction, even over a short time-frame, is expensive,
    so where we can avoid it we should.

[..]
diff --git a/lib/puppet/metatype/manager.rb b/lib/puppet/metatype/manager.rb
index 597a89f..dfcc74d 100644
--- a/lib/puppet/metatype/manager.rb
+++ b/lib/puppet/metatype/manager.rb
@@ -108,17 +108,24 @@ module Manager
[..]
+    # Try loading the type.
+    if typeloader.load(name, Puppet::Node::Environment.current)
+      Puppet.warning "Loaded puppet/type/#{name} but no class was created" unless @types.include? name
     end

This happens to fix #13858, but now makes the autoloader for types depend on the current environment. I run a module spec and load types from a modulepath (set up in my module’s spec_helper for third party modules). The spec_helper looks a bit like this:

require 'puppetlabs_spec_helper/puppetlabs_spec_helper'
Puppet[:modulepath] = File.join(dir, '..', 'modules')

Puppet is initialised, settings are loaded by the pl_spec_helper / testhelper, then I specify a new modulepath, then begin loading types and testing them.

The autoloader calls:

real_env = Puppet::Node::Environment.new(env)

With 2.7.19, env would have been nil (since none was passed in) and so Puppet::Node::Environment returns the “production” environment. 2.7.20 (see commit above) gets the “root” environment from Puppet::Node::Environment.current and passes it in.

Both TestHelper and Puppet::Util::Settings call Puppet::Node::Environment.clear when things change (i.e. the modulepath I’m setting), but this doesn’t clear the special *root* environment. This causes the old modulepath to remain cached in both the root environment itself and the autoloader (which has a thread cache based on the environment object), therefore types in the new modulepath can’t be found.

With 3.1.0 and Josh’s fix in #15529, this isn’t a problem since the modulepath won’t get cached until app_defaults_initialized becomes true, which is just before the new modulepath is set, so all is OK.


Related issues

Related to Puppet - Bug #13858: Custom types in environments require loading into master'... Closed 04/08/2012
Related to Puppet - Bug #15529: Puppet settings should be initialized prior to running an... Closed 07/15/2012

History

#1 Updated by Dominic Cleal over 3 years ago

  • Description updated (diff)

#2 Updated by Dominic Cleal over 3 years ago

  • Status changed from Unreviewed to In Topic Branch Pending Review

#3 Updated by Dominic Cleal over 3 years ago

  • Branch set to https://github.com/puppetlabs/puppet/pull/1339

#4 Updated by Anonymous over 3 years ago

  • Target version deleted (2.7.x)

As the 2.7.x line is winding down, I am removing the target at 2.7.x from tickets in the system. The 2.7 line should only receive fixes for major problems (crashes, for instance) or security problems.

#5 Updated by Dominic Cleal about 3 years ago

  • Branch changed from https://github.com/puppetlabs/puppet/pull/1339 to https://github.com/puppetlabs/puppet/pull/1455

#6 Updated by Adrien Thebo about 3 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release
  • Target version set to 3.1.1

Since it was decided that this is a valuable contribution but we’re trying to wrap up the 2.7.x line, I rebased this onto 3.1.x and merged it as 7bd6a90 into 3.1.x and master. It should be released into 3.1.1.

#7 Updated by Adrien Thebo about 3 years ago

  • Status changed from Merged - Pending Release to Code Insufficient

The implementation added in pull request 18187 has some pretty severe impacts with respect to the Puppet logging parser functions. Specifically those functions are attached to the *root* environment only, and when the root environment is cleared they are not re-added. If the *root* environment is going to be cleared along with all other environments then the logging functions need to be extracted from the Puppet::Parser::Functions module and implemented in a standalone manner, or somehow re-attached to the root environment.

#8 Updated by Adrien Thebo about 3 years ago

  • Status changed from Code Insufficient to Rejected

Also available in: Atom PDF