From 1fa591287a4ab921cec628aa0c5bf58d61fbdef2 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 4 Oct 2007 17:07:47 -0500 Subject: Adding the integration tests to the Rakefile for spec, fixing the integration tests, and extending the Classmethods for the indirector so that indirected classes can set the terminus class and cache class. --- lib/puppet/indirector.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'lib/puppet') diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb index 6ff2de1b4..7821e809c 100644 --- a/lib/puppet/indirector.rb +++ b/lib/puppet/indirector.rb @@ -28,6 +28,14 @@ module Puppet::Indirector module ClassMethods attr_reader :indirection + + def cache_class=(klass) + indirection.cache_class = klass + end + + def terminus_class=(klass) + indirection.terminus_class = klass + end def find(*args) indirection.find(*args) -- cgit From 0e336bf62b818aaa31fcc323ab5d31e5eb92eb46 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 5 Oct 2007 00:07:38 -0500 Subject: This commit is focused on getting the 'puppet' executable to work. As a result, it involves a lot of integration-level testing, and a lot of small design changes to make the code actually work. In particular, indirections can now have default termini, so that configurations and facts default to their code terminus Also, I've removed the ability to manually control whether ast nodes are used. I might need to add it back in later, but if so it will be in the form of a global setting, rather than the previous system of passing it through 10 different classes. Instead, the parser detects whether there are AST nodes defined and requires them if so or ignores them if not. About 75 tests are still failing in the main set of tests, but it's going to be a long slog to get them working -- there are significant design issues around them, as most of the failures are a result of tests trying to emulate both the client and server sides of a connection, which normally would have different fact termini but in this case must have the same terminus just because they're in the same process and are global. The next step, then, is to figure that process out, thus finding a way to make this all work. --- lib/puppet/indirector.rb | 13 ++++++- lib/puppet/indirector/code/configuration.rb | 60 +++++++++++++++++------------ lib/puppet/network/client/master.rb | 1 + lib/puppet/node/configuration.rb | 2 +- lib/puppet/node/facts.rb | 2 +- lib/puppet/parser/compile.rb | 4 +- lib/puppet/parser/interpreter.rb | 7 +--- 7 files changed, 53 insertions(+), 36 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb index 7821e809c..7ceaa43e2 100644 --- a/lib/puppet/indirector.rb +++ b/lib/puppet/indirector.rb @@ -15,7 +15,7 @@ module Puppet::Indirector # default, not the value -- if it's the value, then it gets # evaluated at parse time, which is before the user has had a chance # to override it. - def indirects(indirection) + def indirects(indirection, options = {}) raise(ArgumentError, "Already handling indirection for %s; cannot also handle %s" % [@indirection.name, indirection]) if defined?(@indirection) and @indirection # populate this class with the various new methods extend ClassMethods @@ -24,6 +24,17 @@ module Puppet::Indirector # instantiate the actual Terminus for that type and this name (:ldap, w/ args :node) # & hook the instantiated Terminus into this class (Node: @indirection = terminus) @indirection = Puppet::Indirector::Indirection.new(self, indirection) + + unless options.empty? + options.each do |param, value| + case param + when :terminus_class: @indirection.terminus_class = value + else + raise ArgumenError, "Invalid option '%s' to 'indirects'" % param + end + end + end + @indirection end module ClassMethods diff --git a/lib/puppet/indirector/code/configuration.rb b/lib/puppet/indirector/code/configuration.rb index 6d0317204..b3a4c67dd 100644 --- a/lib/puppet/indirector/code/configuration.rb +++ b/lib/puppet/indirector/code/configuration.rb @@ -15,21 +15,12 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code # Compile a node's configuration. def find(key, client = nil, clientip = nil) - # If we want to use the cert name as our key - if Puppet[:node_name] == 'cert' and client - key = client - end - - # Note that this is reasonable, because either their node source should actually - # know about the node, or they should be using the ``none`` node source, which - # will always return data. - unless node = Puppet::Node.search(key) - raise Puppet::Error, "Could not find node '%s'" % key + if key.is_a?(Puppet::Node) + node = key + else + node = find_node(key) end - # Add any external data to the node. - add_node_data(node) - configuration = compile(node) return configuration @@ -47,6 +38,11 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code @interpreter end + # Is our compiler part of a network, or are we just local? + def networked? + $0 =~ /puppetmasterd/ + end + # Return the configuration version. def version(client = nil, clientip = nil) if client and node = Puppet::Node.search(client) @@ -76,22 +72,17 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code end config = nil + loglevel = networked? ? :notice : :none + # LAK:FIXME This should log at :none when our client is # local, since we don't want 'puppet' (vs. puppetmasterd) to # log compile times. - benchmark(:notice, "Compiled configuration for %s" % node.name) do + benchmark(loglevel, "Compiled configuration for %s" % node.name) do begin config = interpreter.compile(node) rescue Puppet::Error => detail - if Puppet[:trace] - puts detail.backtrace - end - unless local? - Puppet.err detail.to_s - end - raise XMLRPC::FaultException.new( - 1, detail.to_s - ) + Puppet.err(detail.to_s) if networked? + raise end end @@ -117,6 +108,27 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code return Puppet::Parser::Interpreter.new(args) end + # Turn our host name into a node object. + def find_node(key) + # If we want to use the cert name as our key + # LAK:FIXME This needs to be figured out somehow, but it requires the routing. + #if Puppet[:node_name] == 'cert' and client + # key = client + #end + + # Note that this is reasonable, because either their node source should actually + # know about the node, or they should be using the ``none`` node source, which + # will always return data. + unless node = Puppet::Node.search(key) + raise Puppet::Error, "Could not find node '%s'" % key + end + + # Add any external data to the node. + add_node_data(node) + + node + end + # Initialize our server fact hash; we add these to each client, and they # won't change while we're running, so it's safe to cache the values. def set_server_facts @@ -150,7 +162,7 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code # LAK:FIXME This method should probably be part of the protocol, but it # shouldn't be here. def translate(config) - if local? + unless networked? config else CGI.escape(config.to_yaml(:UseBlock => true)) diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb index f950a6059..989d6dca2 100644 --- a/lib/puppet/network/client/master.rb +++ b/lib/puppet/network/client/master.rb @@ -544,6 +544,7 @@ class Puppet::Network::Client::Master < Puppet::Network::Client end rescue => detail + puts detail.backtrace Puppet.err "Could not retrieve configuration: %s" % detail unless Puppet[:usecacheonfailure] diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index 3571eecda..084bdf880 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -7,7 +7,7 @@ require 'puppet/external/gratr/digraph' # and the relationships between them. class Puppet::Node::Configuration < Puppet::PGraph extend Puppet::Indirector - indirects :configuration + indirects :configuration, :terminus_class => :code # The host name this is a configuration for. attr_accessor :name diff --git a/lib/puppet/node/facts.rb b/lib/puppet/node/facts.rb index a2e6d9c04..2da6c488e 100755 --- a/lib/puppet/node/facts.rb +++ b/lib/puppet/node/facts.rb @@ -9,7 +9,7 @@ class Puppet::Node::Facts extend Puppet::Indirector # Use the node source as the indirection terminus. - indirects :facts + indirects :facts, :terminus_class => :code attr_accessor :name, :values diff --git a/lib/puppet/parser/compile.rb b/lib/puppet/parser/compile.rb index 6aeebeaae..992b165e5 100644 --- a/lib/puppet/parser/compile.rb +++ b/lib/puppet/parser/compile.rb @@ -16,8 +16,6 @@ class Puppet::Parser::Compile include Puppet::Util::Errors attr_reader :topscope, :parser, :node, :facts, :collections, :configuration - attr_writer :ast_nodes - # Add a collection to the global list. def add_collection(coll) @collections << coll @@ -25,7 +23,7 @@ class Puppet::Parser::Compile # Do we use nodes found in the code, vs. the external node sources? def ast_nodes? - defined?(@ast_nodes) and @ast_nodes + parser.nodes.length > 0 end # Store the fact that we've evaluated a class, and store a reference to diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb index 0b8c035ba..8c727118c 100644 --- a/lib/puppet/parser/interpreter.rb +++ b/lib/puppet/parser/interpreter.rb @@ -26,7 +26,7 @@ class Puppet::Parser::Interpreter # evaluate our whole tree def compile(node) raise Puppet::ParseError, "Could not parse configuration; cannot compile" unless env_parser = parser(node.environment) - return Puppet::Parser::Compile.new(node, env_parser, :ast_nodes => usenodes?).compile + return Puppet::Parser::Compile.new(node, env_parser).compile end # create our interpreter @@ -53,11 +53,6 @@ class Puppet::Parser::Interpreter @parsers = {} end - # Should we parse ast nodes? - def usenodes? - defined?(@usenodes) and @usenodes - end - private # Create a new parser object and pre-parse the configuration. -- cgit From 9c58c476c2ffcf9613f14e5881b1177f01d413a7 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 5 Oct 2007 11:46:35 -0500 Subject: Adding a :code setting for specifying code to run instead of a manifest, and removing all of the ambiguity around whether an interpreter gets its own file specified or uses the central setting. Most of the changes are around fixing existing tests to use this new system. --- lib/puppet/defaults.rb | 3 +++ lib/puppet/indirector/code/configuration.rb | 16 +--------------- lib/puppet/node.rb | 2 +- lib/puppet/parser/interpreter.rb | 19 +++---------------- 4 files changed, 8 insertions(+), 32 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index 6ea4eef4c..ce1411b62 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -292,6 +292,9 @@ module Puppet "Where puppetmasterd looks for its manifests."], :manifest => ["$manifestdir/site.pp", "The entry-point manifest for puppetmasterd."], + :code => ["", "Code to parse directly. This is essentially only used + by ``puppet`, and should only be set if you're writing your own Puppet + executable"], :masterlog => { :default => "$logdir/puppetmaster.log", :owner => "$user", :group => "$group", diff --git a/lib/puppet/indirector/code/configuration.rb b/lib/puppet/indirector/code/configuration.rb index b3a4c67dd..0d06cb029 100644 --- a/lib/puppet/indirector/code/configuration.rb +++ b/lib/puppet/indirector/code/configuration.rb @@ -91,21 +91,7 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code # Create our interpreter object. def create_interpreter - args = {} - - # Allow specification of a code snippet or of a file - if self.code - args[:Code] = self.code - end - - # LAK:FIXME This needs to be handled somehow. - #if options.include?(:UseNodes) - # args[:UseNodes] = options[:UseNodes] - #elsif @local - # args[:UseNodes] = false - #end - - return Puppet::Parser::Interpreter.new(args) + return Puppet::Parser::Interpreter.new end # Turn our host name into a node object. diff --git a/lib/puppet/node.rb b/lib/puppet/node.rb index d71bd507e..9758c895c 100644 --- a/lib/puppet/node.rb +++ b/lib/puppet/node.rb @@ -9,7 +9,7 @@ class Puppet::Node extend Puppet::Indirector # Use the node source as the indirection terminus. - indirects :node + indirects :node, :terminus_class => :null # Add the node-searching methods. This is what people will actually # interact with that will find the node with the list of names or diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb index 8c727118c..87513cb18 100644 --- a/lib/puppet/parser/interpreter.rb +++ b/lib/puppet/parser/interpreter.rb @@ -14,7 +14,6 @@ class Puppet::Parser::Interpreter include Puppet::Util attr_accessor :usenodes - attr_accessor :code, :file include Puppet::Util::Errors @@ -30,17 +29,7 @@ class Puppet::Parser::Interpreter end # create our interpreter - def initialize(options = {}) - if @code = options[:Code] - elsif @file = options[:Manifest] - end - - if options.include?(:UseNodes) - @usenodes = options[:UseNodes] - else - @usenodes = true - end - + def initialize # The class won't always be defined during testing. if Puppet[:storeconfigs] if Puppet.features.rails? @@ -59,10 +48,8 @@ class Puppet::Parser::Interpreter def create_parser(environment) begin parser = Puppet::Parser::Parser.new(:environment => environment) - if self.code - parser.string = self.code - elsif self.file - parser.file = self.file + if code = Puppet.settings.value(:code, environment) and code != "" + parser.string = code else file = Puppet.settings.value(:manifest, environment) parser.file = file -- cgit From f084d83df1abf51766e2dd390e118f1189864346 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 5 Oct 2007 14:25:29 -0500 Subject: Another round of test-fixing around the changes I made to the configuration system. 'puppet' itself still works, even with -e, but I expect that puppetd and puppetmasterd are broken, and there are still quite a few broken tests because the default fact store can't write but that's the default behaviour for a networked configuration master. --- lib/puppet/dsl.rb | 11 +++++-- lib/puppet/network/client/master.rb | 1 + lib/puppet/network/handler/configuration.rb | 45 +++++++++-------------------- lib/puppet/network/handler/master.rb | 15 +--------- 4 files changed, 23 insertions(+), 49 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/dsl.rb b/lib/puppet/dsl.rb index 75bc81b2b..97d06a4ec 100644 --- a/lib/puppet/dsl.rb +++ b/lib/puppet/dsl.rb @@ -251,12 +251,17 @@ module Puppet def scope unless defined?(@scope) - @interp = Puppet::Parser::Interpreter.new :Code => "" + # Set the code to something innocuous; we just need the + # scopes, not the interpreter. Hackish, but true. + Puppet[:code] = " " + @interp = Puppet::Parser::Interpreter.new require 'puppet/node' @node = Puppet::Node.new(Facter.value(:hostname)) + if env = Puppet[:environment] and env == "" + env = nil + end @node.parameters = Facter.to_hash - @interp = Puppet::Parser::Interpreter.new :Code => "" - @compile = Puppet::Parser::Compile.new(@node, @interp.send(:parser, Puppet[:environment])) + @compile = Puppet::Parser::Compile.new(@node, @interp.send(:parser, env)) @scope = @compile.topscope end @scope diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb index 989d6dca2..5408cabe4 100644 --- a/lib/puppet/network/client/master.rb +++ b/lib/puppet/network/client/master.rb @@ -266,6 +266,7 @@ class Puppet::Network::Client::Master < Puppet::Network::Client self.getconfig end rescue => detail + puts detail.backtrace if Puppet[:trace] Puppet.err "Could not retrieve configuration: %s" % detail end diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb index 353693bdc..09c4b971a 100644 --- a/lib/puppet/network/handler/configuration.rb +++ b/lib/puppet/network/handler/configuration.rb @@ -49,10 +49,14 @@ class Puppet::Network::Handler @local = false end - # Just store the options, rather than creating the interpreter - # immediately. Mostly, this is so we can create the interpreter - # on-demand, which is easier for testing. - @options = options + options.each do |param, value| + case param + when :Classes: @classes = value + when :Local: self.local = true + else + raise ArgumentError, "Configuration handler does not accept %s" % param + end + end set_server_facts end @@ -82,8 +86,8 @@ class Puppet::Network::Handler node.merge(@server_facts) # Add any specified classes to the node's class list. - if classes = @options[:Classes] - classes.each do |klass| + if @classes + @classes.each do |klass| node.classes << klass end end @@ -121,37 +125,14 @@ class Puppet::Network::Handler end # Create our interpreter object. - def create_interpreter(options) - args = {} - - # Allow specification of a code snippet or of a file - if code = options[:Code] - args[:Code] = code - elsif options[:Manifest] - args[:Manifest] = options[:Manifest] - end - - args[:Local] = local? - - if options.include?(:UseNodes) - args[:UseNodes] = options[:UseNodes] - elsif @local - args[:UseNodes] = false - end - - # This is only used by the cfengine module, or if --loadclasses was - # specified in +puppet+. - if options.include?(:Classes) - args[:Classes] = options[:Classes] - end - - return Puppet::Parser::Interpreter.new(args) + def create_interpreter + return Puppet::Parser::Interpreter.new end # Create/return our interpreter. def interpreter unless defined?(@interpreter) and @interpreter - @interpreter = create_interpreter(@options) + @interpreter = create_interpreter end @interpreter end diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb index 25c4318b8..1c7f7310e 100644 --- a/lib/puppet/network/handler/master.rb +++ b/lib/puppet/network/handler/master.rb @@ -30,13 +30,6 @@ class Puppet::Network::Handler def initialize(hash = {}) args = {} - # Allow specification of a code snippet or of a file - if code = hash[:Code] - args[:Code] = code - elsif man = hash[:Manifest] - args[:Manifest] = man - end - if hash[:Local] @local = hash[:Local] else @@ -53,12 +46,6 @@ class Puppet::Network::Handler Puppet.debug("Creating interpreter") - if hash.include?(:UseNodes) - args[:UseNodes] = hash[:UseNodes] - elsif @local - args[:UseNodes] = false - end - # This is only used by the cfengine module, or if --loadclasses was # specified in +puppet+. if hash.include?(:Classes) @@ -74,7 +61,7 @@ class Puppet::Network::Handler client, clientip = clientname(client, clientip, facts) # Pass the facts to the fact handler - Puppet::Node::Facts.new(client, facts).save + Puppet::Node::Facts.new(client, facts).save unless local? # And get the configuration from the config handler config = config_handler.configuration(client) -- cgit From 7d7e428df34077062aebb8f08f10eef42d0c8907 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 5 Oct 2007 17:46:08 -0500 Subject: Removing obsolete comment --- lib/puppet/indirector/code/configuration.rb | 3 --- 1 file changed, 3 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/indirector/code/configuration.rb b/lib/puppet/indirector/code/configuration.rb index 0d06cb029..2d8fedcc8 100644 --- a/lib/puppet/indirector/code/configuration.rb +++ b/lib/puppet/indirector/code/configuration.rb @@ -74,9 +74,6 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code loglevel = networked? ? :notice : :none - # LAK:FIXME This should log at :none when our client is - # local, since we don't want 'puppet' (vs. puppetmasterd) to - # log compile times. benchmark(loglevel, "Compiled configuration for %s" % node.name) do begin config = interpreter.compile(node) -- cgit From cdaad286b1fe5fc3c1ab363c890bb6a8a752c9b5 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Fri, 5 Oct 2007 17:46:30 -0500 Subject: Fixing error thrown when the end of the file is encountered unexpectedly --- lib/puppet/parser/parser_support.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb index 8cf0dcfe8..acf3c9f7c 100644 --- a/lib/puppet/parser/parser_support.rb +++ b/lib/puppet/parser/parser_support.rb @@ -366,10 +366,12 @@ class Puppet::Parser::Parser end def on_error(token,value,stack) - #on '%s' at '%s' in\n'%s'" % [token,value,stack] - #error = "line %s: parse error after '%s'" % - # [@lexer.line,@lexer.last] - error = "Syntax error at '%s'" % [value] + if token == 0 # denotes end of file + value = 'end of file' + else + value = "'%s'" % value + end + error = "Syntax error at %s" % [value] if brace = @lexer.expected error += "; expected '%s'" % brace -- cgit From fc9c850414baff17dc97b0184f34e58b4bec5785 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sat, 6 Oct 2007 17:18:30 -0500 Subject: Adding support for versions and freshness-checking to the indirection layers. This should hopefully enable the different application models we need in our different executables. --- lib/puppet/indirector.rb | 4 ++++ lib/puppet/indirector/indirection.rb | 21 ++++++++++++++------ lib/puppet/indirector/terminus.rb | 37 ++++++++++++++++++++++++++++++------ 3 files changed, 50 insertions(+), 12 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb index 7ceaa43e2..a2eb41763 100644 --- a/lib/puppet/indirector.rb +++ b/lib/puppet/indirector.rb @@ -62,6 +62,10 @@ module Puppet::Indirector end module InstanceMethods + # Make it easy for the model to set versions, + # which are used for caching and such. + attr_accessor :version + # these become instance methods def save(*args) self.class.indirection.save(self, *args) diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index 3a6284877..cd71c4d66 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -90,12 +90,18 @@ class Puppet::Indirector::Indirection end end - def find(*args) - terminus.find(*args) + def find(key, *args) + if cache? and terminus.fresh?(key, cache.version(key)) + return cache.find(key, *args) + end + if result = terminus.find(key, *args) + result.version ||= Time.now.utc + cache.save(result, *args) if cache? + return result + end end def destroy(*args) - cache.destroy(*args) if cache? terminus.destroy(*args) end @@ -104,9 +110,12 @@ class Puppet::Indirector::Indirection end # these become instance methods - def save(*args) - cache.save(*args) if cache? - terminus.save(*args) + def save(instance, *args) + instance.version ||= Time.now.utc + dest = cache? ? cache : terminus + return if dest.fresh?(instance.name, instance.version) + cache.save(instance, *args) if cache? + terminus.save(instance, *args) end private diff --git a/lib/puppet/indirector/terminus.rb b/lib/puppet/indirector/terminus.rb index 3e0ea447c..5aeb4c6d7 100644 --- a/lib/puppet/indirector/terminus.rb +++ b/lib/puppet/indirector/terminus.rb @@ -95,25 +95,50 @@ class Puppet::Indirector::Terminus end end + # Is this instance fresh? Meaning, is the version we have equivalent or better + # than the version being asked about + def fresh?(key, vers) + raise Puppet::DevError.new("Cannot check update status when no 'version' method is defined") unless respond_to?(:version) + + if existing_version = version(key) + existing_version >= vers + else + false + end + end + + def indirection + self.class.indirection + end + def initialize if self.class.abstract_terminus? raise Puppet::DevError, "Cannot create instances of abstract terminus types" end end - def terminus_type - self.class.terminus_type + def model + self.class.model end def name self.class.name end - def model - self.class.model + def terminus_type + self.class.terminus_type end - def indirection - self.class.indirection + # Provide a default method for retrieving an instance's version. + # By default, just find the resource and get its version. Individual + # terminus types can override this method to provide custom definitions of + # 'versions'. + def version(name) + raise Puppet::DevError.new("Cannot retrieve an instance's version without a :find method") unless respond_to?(:find) + if instance = find(name) + instance.version + else + nil + end end end -- cgit From 7ac7872c74200d8818a9042d39296a21d857cc22 Mon Sep 17 00:00:00 2001 From: "Michael V. O'Brien" Date: Mon, 8 Oct 2007 11:13:50 -0500 Subject: Fixed #822. Applied patch provided by DavidS. --- lib/puppet/provider/service/debian.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/puppet') diff --git a/lib/puppet/provider/service/debian.rb b/lib/puppet/provider/service/debian.rb index d810eac1b..0ba7e1a79 100755 --- a/lib/puppet/provider/service/debian.rb +++ b/lib/puppet/provider/service/debian.rb @@ -17,7 +17,7 @@ Puppet::Type.type(:service).provide :debian, :parent => :init do # If it's enabled, then it will print output showing removal of # links. - if output =~ /etc\/rc[\dS].d|Nothing to do\./ + if output =~ /etc\/rc[\dS].d|not installed/ return :true else return :false -- cgit From ec58355ac0c42713bb1c661c94cb13793dd95768 Mon Sep 17 00:00:00 2001 From: "Michael V. O'Brien" Date: Mon, 8 Oct 2007 11:54:51 -0500 Subject: Fixed #819. Applied patch provided by matsuu. --- lib/puppet/provider/package/portage.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/provider/package/portage.rb b/lib/puppet/provider/package/portage.rb index 866ec8730..6a42444ef 100644 --- a/lib/puppet/provider/package/portage.rb +++ b/lib/puppet/provider/package/portage.rb @@ -55,7 +55,7 @@ Puppet::Type.type(:package).provide :portage, :parent => Puppet::Provider::Packa # The common package name format. def package_name - "%s/%s" % [@resource[:category], @resource[:name]] + @resource[:category] ? "%s/%s" % [@resource[:category], @resource[:name]] : @resource[:name] end def uninstall @@ -71,7 +71,7 @@ Puppet::Type.type(:package).provide :portage, :parent => Puppet::Provider::Packa result_fields = [:category, :name, :ensure, :version_available, :slot, :vendor, :description] search_field = @resource[:category] ? "--category-name" : "--name" - search_value = @resource[:category] ? package_name : @resource[:name] + search_value = package_name search_format = " [] [] " begin -- cgit From d24c1ccc56b912e0ff69f7572dd36912c8c739c2 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 8 Oct 2007 19:12:39 -0500 Subject: All tests should now pass again. This is the first real pass towards using caching. The `puppet` executable actually uses the indirection work, instead of handlers and such (and man! is it cleaner). Most of this work was a result of trying to get the client-side story working, with correct yaml caching of configurations, which means this commit also covers converting configurations to yaml, which was a much bigger PITA than it needed to be. I still need to write integration tests, and I also need to cover the server-side story of a normal configuration retrieval. --- lib/puppet/indirector/code/configuration.rb | 10 +++-- lib/puppet/indirector/indirection.rb | 9 ++-- lib/puppet/indirector/terminus.rb | 10 +++-- lib/puppet/indirector/yaml.rb | 18 ++++++-- lib/puppet/indirector/yaml/configuration.rb | 18 ++++++++ lib/puppet/network/handler/configuration.rb | 10 +---- lib/puppet/node/configuration.rb | 57 ++++++++++++++++++++++++ lib/puppet/pgraph.rb | 4 ++ lib/puppet/transportable.rb | 68 +++++++++++++++++++---------- lib/puppet/type/component.rb | 10 +---- 10 files changed, 162 insertions(+), 52 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/indirector/code/configuration.rb b/lib/puppet/indirector/code/configuration.rb index 2d8fedcc8..949926a3c 100644 --- a/lib/puppet/indirector/code/configuration.rb +++ b/lib/puppet/indirector/code/configuration.rb @@ -21,9 +21,13 @@ class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code node = find_node(key) end - configuration = compile(node) - - return configuration + if configuration = compile(node) + return configuration.to_transportable + else + # This shouldn't actually happen; we should either return + # a config or raise an exception. + return nil + end end def initialize diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb index cd71c4d66..5f7baa3da 100644 --- a/lib/puppet/indirector/indirection.rb +++ b/lib/puppet/indirector/indirection.rb @@ -91,12 +91,15 @@ class Puppet::Indirector::Indirection end def find(key, *args) - if cache? and terminus.fresh?(key, cache.version(key)) + if cache? and cache.has_most_recent?(key, terminus.version(key)) return cache.find(key, *args) end if result = terminus.find(key, *args) result.version ||= Time.now.utc - cache.save(result, *args) if cache? + if cache? + Puppet.info "Caching %s %s" % [self.name, key] + cache.save(result, *args) + end return result end end @@ -113,7 +116,7 @@ class Puppet::Indirector::Indirection def save(instance, *args) instance.version ||= Time.now.utc dest = cache? ? cache : terminus - return if dest.fresh?(instance.name, instance.version) + return if dest.has_most_recent?(instance.name, instance.version) cache.save(instance, *args) if cache? terminus.save(instance, *args) end diff --git a/lib/puppet/indirector/terminus.rb b/lib/puppet/indirector/terminus.rb index 5aeb4c6d7..37cfdc499 100644 --- a/lib/puppet/indirector/terminus.rb +++ b/lib/puppet/indirector/terminus.rb @@ -95,13 +95,15 @@ class Puppet::Indirector::Terminus end end - # Is this instance fresh? Meaning, is the version we have equivalent or better - # than the version being asked about - def fresh?(key, vers) + # Do we have an update for this object? This compares the provided version + # to our version, and returns true if our version is at least as high + # as the asked-about version. + def has_most_recent?(key, vers) raise Puppet::DevError.new("Cannot check update status when no 'version' method is defined") unless respond_to?(:version) if existing_version = version(key) - existing_version >= vers + #puts "%s fresh: %s (%s vs %s)" % [self.name, (existing_version.to_f >= vers.to_f).inspect, existing_version.to_f, vers.to_f] + existing_version.to_f >= vers.to_f else false end diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb index b9ea54f39..e11c88474 100644 --- a/lib/puppet/indirector/yaml.rb +++ b/lib/puppet/indirector/yaml.rb @@ -14,9 +14,9 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus return nil unless FileTest.exist?(file) begin - return YAML.load(File.read(file)) + return from_yaml(File.read(file)) rescue => detail - raise Puppet::Error, "Could not read YAML data for %s(%s): %s" % [indirection.name, name, detail] + raise Puppet::Error, "Could not read YAML data for %s %s: %s" % [indirection.name, name, detail] end end @@ -33,11 +33,23 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus Dir.mkdir(basedir) end - File.open(file, "w", 0660) { |f| f.print YAML.dump(object) } + begin + File.open(file, "w", 0660) { |f| f.print to_yaml(object) } + rescue TypeError => detail + Puppet.err "Could not save %s %s: %s" % [self.name, object.name, detail] + end end private + def from_yaml(text) + YAML.load(text) + end + + def to_yaml(object) + YAML.dump(object) + end + # Return the path to a given node's file. def path(name) File.join(Puppet[:yamldir], self.name.to_s, name.to_s + ".yaml") diff --git a/lib/puppet/indirector/yaml/configuration.rb b/lib/puppet/indirector/yaml/configuration.rb index 691f0e343..8e40ff9bf 100644 --- a/lib/puppet/indirector/yaml/configuration.rb +++ b/lib/puppet/indirector/yaml/configuration.rb @@ -2,4 +2,22 @@ require 'puppet/indirector/yaml' class Puppet::Indirector::Yaml::Configuration < Puppet::Indirector::Yaml desc "Store configurations as flat files, serialized using YAML." + + private + + # Override these, because yaml doesn't want to convert our self-referential + # objects. This is hackish, but eh. + def from_yaml(text) + if config = YAML.load(text) + # We can't yaml-dump classes. + #config.edgelist_class = Puppet::Relationship + return config + end + end + + def to_yaml(config) + # We can't yaml-dump classes. + #config.edgelist_class = nil + YAML.dump(config) + end end diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb index 09c4b971a..680304e2a 100644 --- a/lib/puppet/network/handler/configuration.rb +++ b/lib/puppet/network/handler/configuration.rb @@ -13,7 +13,7 @@ class Puppet::Network::Handler include Puppet::Util - attr_accessor :local + attr_accessor :local, :classes @interface = XMLRPC::Service::Interface.new("configuration") { |iface| iface.add_method("string configuration(string)") @@ -43,16 +43,10 @@ class Puppet::Network::Handler end def initialize(options = {}) - if options[:Local] - @local = options[:Local] - else - @local = false - end - options.each do |param, value| case param when :Classes: @classes = value - when :Local: self.local = true + when :Local: self.local = value else raise ArgumentError, "Configuration handler does not accept %s" % param end diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index 084bdf880..da8dc3a9a 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -22,6 +22,10 @@ class Puppet::Node::Configuration < Puppet::PGraph # How we should extract the configuration for sending to the client. attr_reader :extraction_format + # We need the ability to set this externally, so we can yaml-dump the + # configuration. + attr_accessor :edgelist_class + # Whether this is a host configuration, which behaves very differently. # In particular, reports are sent, graphs are made, and state is # stored in the state database. If this is set incorrectly, then you often @@ -355,6 +359,16 @@ class Puppet::Node::Configuration < Puppet::PGraph @tags.dup end + # Convert our configuration into a RAL configuration. + def to_ral + to_configuration :to_type + end + + # Turn our parser configuration into a transportable configuration. + def to_transportable + to_configuration :to_transobject + end + # Produce the graph files if requested. def write_graph(name) # We only want to graph the main host configuration. @@ -370,6 +384,14 @@ class Puppet::Node::Configuration < Puppet::PGraph } end + # LAK:NOTE We cannot yaml-dump the class in the edgelist_class, because classes cannot be + # dumped by default, nor does yaml-dumping # the edge-labels work at this point (I don't + # know why). + # Neither of these matters right now, but I suppose it could at some point. + def to_yaml_properties + instance_variables.reject { |v| %w{@edgelist_class @edge_labels}.include?(v) } + end + private def cleanup @@ -379,4 +401,39 @@ class Puppet::Node::Configuration < Puppet::PGraph @relationship_graph = nil end end + + # An abstracted method for converting one configuration into another type of configuration. + # This pretty much just converts all of the resources from one class to another, using + # a conversion method. + def to_configuration(convert) + result = self.class.new(self.name) + + vertices.each do |resource| + next if resource.respond_to?(:virtual?) and resource.virtual? + + result.add_resource resource.send(convert) + end + + message = convert.to_s.gsub "_", " " + edges.each do |edge| + # Skip edges between virtual resources. + next if edge.source.respond_to?(:virtual?) and edge.source.virtual? + next if edge.target.respond_to?(:virtual?) and edge.target.virtual? + + unless source = result.resource(edge.source.ref) + raise Puppet::DevError, "Could not find vertex for %s when converting %s" % [edge.source.ref, message] + end + + unless target = result.resource(edge.target.ref) + raise Puppet::DevError, "Could not find vertex for %s when converting %s" % [edge.target.ref, message] + end + + result.add_edge!(source, target, edge.label) + end + + result.add_class *self.classes + result.tag(*self.tags) + + return result + end end diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb index 2399d1651..ca45aa2b3 100644 --- a/lib/puppet/pgraph.rb +++ b/lib/puppet/pgraph.rb @@ -190,6 +190,10 @@ class Puppet::PGraph < GRATR::Digraph return result end + def to_yaml_properties + instance_variables + end + # A different way of walking a tree, and a much faster way than the # one that comes with GRATR. def tree_from_vertex2(start, direction = :out) diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb index ecd179ed7..2a600b50e 100644 --- a/lib/puppet/transportable.rb +++ b/lib/puppet/transportable.rb @@ -8,7 +8,7 @@ module Puppet # YAML. class TransObject include Enumerable - attr_accessor :type, :name, :file, :line, :collectable, :collected + attr_accessor :type, :name, :file, :line, :configuration attr_writer :tags @@ -25,7 +25,6 @@ module Puppet def initialize(name,type) @type = type @name = name - @collectable = false @params = {} @tags = [] end @@ -34,10 +33,44 @@ module Puppet return [@type,@name].join('--') end + def ref + unless defined? @ref + if @type == :component + @ref = @name + else + @ref = "%s[%s]" % [type_capitalized, name] + end + end + @ref + end + def tags return @tags end + # Convert a defined type into a component. + def to_component + tmpname = nil + + # Nodes have the same name and type + if self.name + tmpname = "%s[%s]" % [type_capitalized, self.name] + else + tmpname = @type + end + trans = TransObject.new(tmpname, :component) + if defined? @parameters + @parameters.each { |param,value| + Puppet.debug "Defining %s on %s of type %s" % + [param,@name,@type] + trans[param] = value + } + else + #Puppet.debug "%s[%s] has no parameters" % [@type, @name] + end + Puppet::Type::Component.create(trans) + end + def to_hash @params.dup end @@ -57,18 +90,18 @@ module Puppet end def to_yaml_properties - instance_variables + instance_variables.reject { |v| %w{@ref}.include?(v) } end def to_ref - unless defined? @ref + unless defined? @res_ref if self.type and self.name - @ref = "%s[%s]" % [self.type, self.name] + @res_ref = "%s[%s]" % [type_capitalized, self.name] else - @ref = nil + @res_ref = nil end end - @ref + @res_ref end def to_type @@ -85,11 +118,16 @@ module Puppet end end else - raise Puppet::Error.new("Could not find object type %s" % self.type) + return to_component end return retobj end + + # Return the type fully capitalized correctly. + def type_capitalized + type.split("::").collect { |s| s.capitalize }.join("::") + end end # Just a linear container for objects. Behaves mostly like an array, except @@ -107,20 +145,6 @@ module Puppet end } - # Remove all collectable objects from our tree, since the client - # should not see them. - def collectstrip! - @children.dup.each do |child| - if child.is_a? self.class - child.collectstrip! - else - if child.collectable and ! child.collected - @children.delete(child) - end - end - end - end - # Recursively yield everything. def delve(&block) @children.each do |obj| diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb index c5842e0e7..4dc542a65 100644 --- a/lib/puppet/type/component.rb +++ b/lib/puppet/type/component.rb @@ -109,14 +109,6 @@ Puppet::Type.newtype(:component) do @children = [] end - def parent=(parent) - if self.parentof?(parent) - devfail "%s[%s] is already the parent of %s[%s]" % - [self.class.name, self.title, parent.class.name, parent.title] - end - @parent = parent - end - # Add a hook for testing for recursion. def parentof?(child) if super(child) @@ -133,7 +125,7 @@ Puppet::Type.newtype(:component) do def pathbuilder tmp = [] myname = "" - if self.title =~ /^class\[(.+)\]$/ + if self.title =~ /^class\[(.+)\]$/i # 'main' is the top class, so we want to see '//' instead of # its name. unless $1 == "main" -- cgit