From 9778f2a47922a66e59d571c1c98552223a817ce1 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 23 Jul 2010 17:04:22 -0700 Subject: [#4242] Fixed recursion due to parents including their children Resources mark themselves as evaluated to prevent being evaluated again. Unfortunately, they were not marking themselves until after they had finished being completely evaluated. Thus, there was nothing actually stopping recursive evaluations. This patch just makes resources mark themselves as evaluated when they start evaluating, and adds tests. The original setting of evaluated was done in an ensure block, so this doesn't change the behavior of a resource which fails to evaluate. The only places evaluated? is checked aren't affected by this change, as they wouldn't want to evaluate it when it's already being evaluated anyway. --- lib/puppet/parser/resource.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index c956a1106..3c451929e 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -64,6 +64,7 @@ class Puppet::Parser::Resource < Puppet::Resource # Retrieve the associated definition and evaluate it. def evaluate + @evaluated = true if klass = resource_type and ! builtin_type? finish return klass.evaluate_code(self) @@ -72,8 +73,6 @@ class Puppet::Parser::Resource < Puppet::Resource else self.fail "Cannot find definition #{type}" end - ensure - @evaluated = true end # Mark this resource as both exported and virtual, -- cgit From 23830504dfeb48b2d162e44b84b6f9dfa97e4e7e Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Thu, 22 Jul 2010 20:36:24 +0200 Subject: Fix #4302 - Compilation speed regression compared to 2.6 Each time the compiler was accessing the loaded types, we were checking if the manifests had changed. This incurred a large performance cost compared to 0.25 and introduced race conditions if manifests changed while a thread was in the middle of a compilation. This tentative fix, based on Brice's, makes sure each thread will get access to the same loaded types collection for the durration of a compilation, even if the manifests change. We now only check for changed files at the start of a compilation or if the environment changes, and we maintain a per environment thread lock so that only one thread at a time can be reloading any particular environment (and the need-check is done inside the synchronize block so that only the first will actually load it). As long as the manifests don't change, the threads will share the same collection, so there is only duplication in memory for a brief window surrounding a change. Signed-off-by: Brice Figureau Second-author: Markus Roberts --- lib/puppet/parser/compiler.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index a901c0dd6..760d5a75a 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -15,6 +15,11 @@ class Puppet::Parser::Compiler include Puppet::Resource::TypeCollectionHelper def self.compile(node) + # At the start of a new compile we don't assume anything about + # known_resouce_types; we'll get these from the environment and + # cache them in a thread variable for the duration of the + # compilation. + Thread.current[:known_resource_types] = nil new(node).compile.to_resource rescue => detail puts detail.backtrace if Puppet[:trace] -- cgit From 1d494a3104e9794cc09ba27c701ced68a74fa398 Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Fri, 23 Jul 2010 08:47:48 -0700 Subject: Tweak to fix for #4302--dangling ref to known_resource_types Since we were clearing the thread variable containing the compiler's reference to it's environment's known resource types at the start of each compile the reference remaining at the end of a compilation could never be used and was thus just garbage that we were arbitrarily retaining. This patch moves the clearing of the thread var to the _end_ of compilation so that it's always nil except in the middle of a compile. This raises an interesting question; should the ref just live on the compiler object and we could dispense with the thread-var? It might require things that now only know about the environment to need a ref to the compiler and introduce other thread issues (e.g. we might just end up needing a :current_compiler thread variable, for no net gain in simplicity). --- lib/puppet/parser/compiler.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index 760d5a75a..61bb13cb6 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -15,16 +15,15 @@ class Puppet::Parser::Compiler include Puppet::Resource::TypeCollectionHelper def self.compile(node) - # At the start of a new compile we don't assume anything about - # known_resouce_types; we'll get these from the environment and - # cache them in a thread variable for the duration of the - # compilation. - Thread.current[:known_resource_types] = nil new(node).compile.to_resource rescue => detail puts detail.backtrace if Puppet[:trace] raise Puppet::Error, "#{detail} on node #{node.name}" - end + ensure + # We get these from the environment and only cache them in a thread + # variable for the duration of the compilation. + Thread.current[:known_resource_types] = nil + end attr_reader :node, :facts, :collections, :catalog, :node_scope, :resources, :relationships -- cgit From 000fd1e83782c70fc9d9b032b52d96800cab2121 Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Fri, 23 Jul 2010 11:31:40 -0700 Subject: Fix for #4303 -- reverting to old escaping in '-strings Single quoted used to allow escape on single quotes and pass all other characters through without comment; now the do again. --- lib/puppet/parser/lexer.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb index 1e10ff96c..aa04f17a0 100644 --- a/lib/puppet/parser/lexer.rb +++ b/lib/puppet/parser/lexer.rb @@ -221,7 +221,7 @@ class Puppet::Parser::Lexer TOKENS.add_token :RETURN, "\n", :skip => true, :incr_line => true, :skip_text => true TOKENS.add_token :SQUOTE, "'" do |lexer, value| - [TOKENS[:STRING], lexer.slurpstring(value).first ] + [TOKENS[:STRING], lexer.slurpstring(value,["'"],:ignore_invalid_esapes).first ] end DQ_initial_token_types = {'$' => :DQPRE,'"' => :STRING} @@ -517,8 +517,7 @@ class Puppet::Parser::Lexer # we've encountered the start of a string... # slurp in the rest of the string and return it - Valid_escapes_in_strings = %w{ \\ $ ' " n t s }+["\n"] - def slurpstring(terminators) + def slurpstring(terminators,escapes=%w{ \\ $ ' " n t s }+["\n"],ignore_invalid_escapes=false) # we search for the next quote that isn't preceded by a # backslash; the caret is there to match empty strings str = @scanner.scan_until(/([^\\]|^)[#{terminators}]/) or lex_error "Unclosed quote after '#{last}' in '#{rest}'" @@ -529,10 +528,10 @@ class Puppet::Parser::Lexer when 't'; "\t" when 's'; " " else - if Valid_escapes_in_strings.include? ch and not (ch == '"' and terminators == "'") + if escapes.include? ch ch else - Puppet.warning "Unrecognised escape sequence '\\#{ch}'#{file && " in file #{file}"}#{line && " at line #{line}"}" + Puppet.warning "Unrecognised escape sequence '\\#{ch}'#{file && " in file #{file}"}#{line && " at line #{line}"}" unless ignore_invalid_escapes "\\#{ch}" end end -- cgit From 636079f114034f5dced1772da206d1b1b1a537ed Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Fri, 23 Jul 2010 02:32:50 +1000 Subject: Fixed #4304 - Changed logging level for auto import message --- lib/puppet/parser/type_loader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb index 35ad49593..09aa636e1 100644 --- a/lib/puppet/parser/type_loader.rb +++ b/lib/puppet/parser/type_loader.rb @@ -88,7 +88,7 @@ class Puppet::Parser::TypeLoader nil end if result = yield(filename) - Puppet.info "Automatically imported #{name} from #{filename} into #{environment}" + Puppet.debug "Automatically imported #{name} from #{filename} into #{environment}" result.module_name = modname if modname and result.respond_to?(:module_name=) return result end -- cgit From 13c71b97d93edf033897dc09093861f68c9dd23a Mon Sep 17 00:00:00 2001 From: Jesse Wolfe Date: Fri, 23 Jul 2010 13:59:40 -0700 Subject: extlookup() is a builtin This patch promotes extlookup() to being a builtin function. It also adds test and makes some minor tweaks to the code. The behavior of extlookup has been left unchanged. --- lib/puppet/parser/functions/extlookup.rb | 173 +++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 lib/puppet/parser/functions/extlookup.rb (limited to 'lib/puppet/parser') diff --git a/lib/puppet/parser/functions/extlookup.rb b/lib/puppet/parser/functions/extlookup.rb new file mode 100644 index 000000000..ee230e7ce --- /dev/null +++ b/lib/puppet/parser/functions/extlookup.rb @@ -0,0 +1,173 @@ +# Puppet External Data Sources +# +# This is a parser function to read data from external files, this version +# uses CSV files but the concept can easily be adjust for databases, yaml +# or any other queryable data source. +# +# The object of this is to make it obvious when it's being used, rather than +# magically loading data in when an module is loaded I prefer to look at the code +# and see statements like: +# +# $snmp_contact = extlookup("snmp_contact") +# +# The above snippet will load the snmp_contact value from CSV files, this in its +# own is useful but a common construct in puppet manifests is something like this: +# +# case $domain { +# "myclient.com": { $snmp_contact = "John Doe " } +# default: { $snmp_contact = "My Support " } +# } +# +# Over time there will be a lot of this kind of thing spread all over your manifests +# and adding an additional client involves grepping through manifests to find all the +# places where you have constructs like this. +# +# This is a data problem and shouldn't be handled in code, a using this function you +# can do just that. +# +# First you configure it in site.pp: +# $extlookup_datadir = "/etc/puppet/manifests/extdata" +# $extlookup_precedence = ["%{fqdn}", "domain_%{domain}", "common"] +# +# The array tells the code how to resolve values, first it will try to find it in +# web1.myclient.com.csv then in domain_myclient.com.csv and finally in common.csv +# +# Now create the following data files in /etc/puppet/manifests/extdata +# +# domain_myclient.com.csv: +# snmp_contact,John Doe +# root_contact,support@%{domain} +# client_trusted_ips,192.168.1.130,192.168.10.0/24 +# +# common.csv: +# snmp_contact,My Support +# root_contact,support@my.com +# +# Now you can replace the case statement with the simple single line to achieve +# the exact same outcome: +# +# $snmp_contact = extlookup("snmp_contact") +# +# The obove code shows some other features, you can use any fact or variable that +# is in scope by simply using %{varname} in your data files, you can return arrays +# by just having multiple values in the csv after the initial variable name. +# +# In the event that a variable is nowhere to be found a critical error will be raised +# that will prevent your manifest from compiling, this is to avoid accidentally putting +# in empty values etc. You can however specify a default value: +# +# $ntp_servers = extlookup("ntp_servers", "1.${country}.pool.ntp.org") +# +# In this case it will default to "1.${country}.pool.ntp.org" if nothing is defined in +# any data file. +# +# You can also specify an additional data file to search first before any others at use +# time, for example: +# +# $version = extlookup("rsyslog_version", "present", "packages") +# +# package{"rsyslog": ensure => $version } +# +# This will look for a version configured in packages.csv and then in the rest as configured +# by $extlookup_precedence if it's not found anywhere it will default to "present", this kind +# of use case makes puppet a lot nicer for managing large amounts of packages since you do not +# need to edit a load of manifests to do simple things like adjust a desired version number. +# +# For more information on installing and writing your own custom functions see: +# http://docs.puppetlabs.com/guides/custom_functions.html +# +# For further help contact Volcane on #puppet +require 'csv' + +module Puppet::Parser::Functions + newfunction(:extlookup, :type => :rvalue) do |args| + key = args[0] + + default = args[1] + datafile = args[2] + + raise Puppet::ParseError, ("extlookup(): wrong number of arguments (#{args.length}; must be <= 3)") if args.length > 3 + + extlookup_datadir = lookupvar('extlookup_datadir') + extlookup_precedence = Array.new + + # precedence values can have variables embedded in them + # in the form %{fqdn}, you could for example do + # + # $extlookup_precedence = ["hosts/%{fqdn}", "common"] + # + # this will result in /path/to/extdata/hosts/your.box.com.csv + # being searched. + # + # we parse the precedence here because the best place to specify + # it would be in site.pp but site.pp is only evaluated at startup + # so $fqdn etc would have no meaning there, this way it gets evaluated + # each run and has access to the right variables for that run + lookupvar('extlookup_precedence').each do |prec| + while prec =~ /%\{(.+?)\}/ + prec.gsub!(/%\{#{$1}\}/, lookupvar($1)) + end + + extlookup_precedence << prec + end + + + datafiles = Array.new + + # if we got a custom data file, put it first in the array of search files + if datafile != "" + datafiles << extlookup_datadir + "/#{datafile}.csv" if File.exists?(extlookup_datadir + "/#{datafile}.csv") + end + + extlookup_precedence.each do |d| + datafiles << extlookup_datadir + "/#{d}.csv" + end + + desired = nil + + datafiles.each do |file| + parser = Puppet::Parser::Parser.new(environment) + parser.watch_file(file) if File.exists?(file) + + if desired.nil? + if File.exists?(file) + result = CSV.read(file).find_all do |r| + r[0] == key + end + + + # return just the single result if theres just one, + # else take all the fields in the csv and build an array + if result.length > 0 + if result[0].length == 2 + val = result[0][1].to_s + + # parse %{}'s in the CSV into local variables using lookupvar() + while val =~ /%\{(.+?)\}/ + val.gsub!(/%\{#{$1}\}/, lookupvar($1)) + end + + desired = val + elsif result[0].length > 1 + length = result[0].length + cells = result[0][1,length] + + # Individual cells in a CSV result are a weird data type and throws + # puppets yaml parsing, so just map it all to plain old strings + desired = cells.map do |c| + # parse %{}'s in the CSV into local variables using lookupvar() + while c =~ /%\{(.+?)\}/ + c.gsub!(/%\{#{$1}\}/, lookupvar($1)) + end + + c.to_s + end + end + end + end + end + end + + desired || default or raise Puppet::ParseError, "No match found for '#{key}' in any data file during extlookup()" + end +end -- cgit