diff options
| author | Luke Kanies <luke@madstop.com> | 2007-11-12 22:08:44 -0600 |
|---|---|---|
| committer | Luke Kanies <luke@madstop.com> | 2007-11-12 22:08:44 -0600 |
| commit | 72510bfaa65e97f4eaaf246ef8f1c155716967b6 (patch) | |
| tree | 978aa0e92812f5854978048162c6e2ab752dad72 | |
| parent | dd7caa76e160ed51c8b0e123c18f7526b575bfec (diff) | |
Fixing #800 by refactoring how configurations are retrieved
from the server. The real problem was getting all of the validation
done before any caching, which required a good bit more refactoring
than I expected.
In actuality, this commit is relatively small even though it covers
many files; most of the changes just make the code clearer or shorter.
30 files changed, 788 insertions, 563 deletions
@@ -1,3 +1,14 @@ + Fixed #800 -- invalid configurations are no longer + cached. This was done partially by adding a relationship + validation step once the entire configuration is created, + but it also required the previously-mentioned changes + to how the configuration retrieval process works. + + Removed some functionality from the Master client, + since the local functionality has been replaced + with the Indirector already, and rearranging how configuration + retrieval is done to fix ordering and caching bugs. + The node scope is now above all other scopes besides the 'main' scope, which should help make its variables visible to other classes, assuming those classes were diff --git a/lib/puppet/metatype/instances.rb b/lib/puppet/metatype/instances.rb index 2f9918067..8cc648e8f 100644 --- a/lib/puppet/metatype/instances.rb +++ b/lib/puppet/metatype/instances.rb @@ -99,6 +99,8 @@ class Puppet::Type end end + # If they've specified a type and called on the base, then + # delegate to the subclass. if type if typeklass = self.type(type) return typeklass.create(hash) @@ -233,19 +235,22 @@ class Puppet::Type hash.delete :name end - unless title - raise Puppet::Error, - "You must specify a title for objects of type %s" % self.to_s + if configuration = hash[:configuration] + hash.delete(:configuration) end + raise(Puppet::Error, "You must specify a title for objects of type %s" % self.to_s) unless title + if hash.include? :type unless self.validattr? :type hash.delete :type end end + # okay, now make a transobject out of hash begin trans = Puppet::TransObject.new(title, self.name.to_s) + trans.configuration = configuration if configuration hash.each { |param, value| trans[param] = value } diff --git a/lib/puppet/metatype/metaparams.rb b/lib/puppet/metatype/metaparams.rb index 1ab3f26c1..eb158a47d 100644 --- a/lib/puppet/metatype/metaparams.rb +++ b/lib/puppet/metatype/metaparams.rb @@ -94,7 +94,7 @@ class Puppet::Type # We've got four relationship metaparameters, so this method is used # to reduce code duplication between them. - def store_relationship(param, values) + def munge_relationship(param, values) # We need to support values passed in as an array or as a # resource reference. result = [] @@ -194,20 +194,24 @@ class Puppet::Type unless aliases.is_a?(Array) aliases = [aliases] end - @resource.info "Adding aliases %s" % aliases.collect { |a| - a.inspect - }.join(", ") + + raise(ArgumentError, "Cannot add aliases without a configuration") unless @resource.configuration + + @resource.info "Adding aliases %s" % aliases.collect { |a| a.inspect }.join(", ") + aliases.each do |other| - if obj = @resource.class[other] - unless obj == @resource - self.fail( - "%s can not create alias %s: object already exists" % - [@resource.title, other] - ) + if obj = @resource.configuration.resource(@resource.class.name, other) + unless obj.object_id == @resource.object_id + self.fail("%s can not create alias %s: object already exists" % [@resource.title, other]) end next end + + # LAK:FIXME Old-school, add the alias to the class. @resource.class.alias(other, @resource) + + # Newschool, add it to the configuration. + @resource.configuration.alias(@resource, other) end end end @@ -247,7 +251,16 @@ class Puppet::Type end def munge(rels) - @resource.store_relationship(self.class.name, rels) + @resource.munge_relationship(self.class.name, rels) + end + + def validate_relationship + @value.each do |value| + unless @resource.configuration.resource(*value) + description = self.class.direction == :in ? "dependency" : "dependent" + raise Puppet::Error, "Could not find #{description} %s[%s]" % [value[0].to_s.capitalize, value[1]] + end + end end # Create edges from each of our relationships. :in diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb index 5408cabe4..ea351ddc3 100644 --- a/lib/puppet/network/client/master.rb +++ b/lib/puppet/network/client/master.rb @@ -139,63 +139,57 @@ class Puppet::Network::Client::Master < Puppet::Network::Client facts = self.class.facts end - if self.configuration or FileTest.exists?(self.cachefile) - if self.fresh?(facts) - Puppet.info "Config is up to date" - if self.configuration - return - end - if oldtext = self.retrievecache - begin - @configuration = YAML.load(oldtext).to_configuration - rescue => detail - Puppet.warning "Could not load cached configuration: %s" % detail - end - return - end - end - end - Puppet.debug("getting config") + raise Puppet::Network::ClientError.new("Could not retrieve any facts") unless facts.length > 0 # Retrieve the plugins. - if Puppet[:pluginsync] - getplugins() - end + getplugins() if Puppet[:pluginsync] - unless facts.length > 0 - raise Puppet::Network::ClientError.new( - "Could not retrieve any facts" - ) + if (self.configuration or FileTest.exist?(self.cachefile)) and self.fresh?(facts) + Puppet.info "Configuration is up to date" + return if use_cached_config end - unless objects = get_actual_config(facts) - @configuration = nil + Puppet.debug("Retrieving configuration") + + # If we can't retrieve the configuration, just return, which will either + # fail, or use the in-memory configuration. + unless yaml_objects = get_actual_config(facts) + use_cached_config(true) return end - unless objects.is_a?(Puppet::TransBucket) - raise NetworkClientError, - "Invalid returned objects of type %s" % objects.class + begin + objects = YAML.load(yaml_objects) + rescue => detail + msg = "Configuration could not be translated from yaml" + msg += "; using cached configuration" if use_cached_config(true) + Puppet.warning msg + return end self.setclasses(objects.classes) # Clear all existing objects, so we can recreate our stack. - if self.configuration - clear() - end + clear() if self.configuration # Now convert the objects to a puppet configuration graph. - @configuration = objects.to_configuration + begin + @configuration = objects.to_configuration + rescue => detail + clear() + puts detail.backtrace if Puppet[:trace] + msg = "Configuration could not be instantiated: %s" % detail + msg += "; using cached configuration" if use_cached_config(true) + Puppet.warning msg + return + end - if @configuration.nil? - raise Puppet::Error, "Configuration could not be processed" + if ! @configuration.from_cache + self.cache(yaml_objects) end # Keep the state database up to date. @configuration.host_config = true - - return @configuration end # A simple proxy method, so it's easy to test. @@ -270,11 +264,9 @@ class Puppet::Network::Client::Master < Puppet::Network::Client Puppet.err "Could not retrieve configuration: %s" % detail end - if defined? @configuration and @configuration + if self.configuration @configuration.retrieval_duration = duration - unless @local - Puppet.notice "Starting configuration run" - end + Puppet.notice "Starting configuration run" unless @local benchmark(:notice, "Finished configuration run") do @configuration.apply(options) end @@ -500,34 +492,16 @@ class Puppet::Network::Client::Master < Puppet::Network::Client # Actually retrieve the configuration, either from the server or from a # local master. def get_actual_config(facts) - if @local - return get_local_config(facts) - else - begin - Timeout::timeout(self.class.timeout) do - return get_remote_config(facts) - end - rescue Timeout::Error - Puppet.err "Configuration retrieval timed out" - return nil + begin + Timeout::timeout(self.class.timeout) do + return get_remote_config(facts) end + rescue Timeout::Error + Puppet.err "Configuration retrieval timed out" + return nil end end - # Retrieve a configuration from a local master. - def get_local_config(facts) - # If we're local, we don't have to do any of the conversion - # stuff. - objects = @driver.getconfig(facts, "yaml") - @compile_time = Time.now - - if objects == "" - raise Puppet::Error, "Could not retrieve configuration" - end - - return objects - end - # Retrieve a config from a remote master. def get_remote_config(facts) textobjects = "" @@ -545,45 +519,18 @@ 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] - @configuration = nil - Puppet.warning "Not using cache on failed configuration" - return - end + return nil end end - fromcache = false - if textobjects == "" - unless textobjects = self.retrievecache - raise Puppet::Error.new( - "Cannot connect to server and there is no cached configuration" - ) - end - Puppet.warning "Could not get config; using cached copy" - fromcache = true - else - @compile_time = Time.now - Puppet::Util::Storage.cache(:configuration)[:facts] = facts - Puppet::Util::Storage.cache(:configuration)[:compile_time] = @compile_time - end + return nil if textobjects == "" - begin - objects = YAML.load(textobjects) - rescue => detail - raise Puppet::Error, - "Could not understand configuration: %s" % - detail.to_s - end + @compile_time = Time.now + Puppet::Util::Storage.cache(:configuration)[:facts] = facts + Puppet::Util::Storage.cache(:configuration)[:compile_time] = @compile_time - if @cache and ! fromcache - self.cache(textobjects) - end - - return objects + return textobjects end def lockfile @@ -609,4 +556,32 @@ class Puppet::Network::Client::Master < Puppet::Network::Client Puppet.info "Sleeping for %s seconds (splay is enabled)" % time sleep(time) end + + private + + # Use our cached config, optionally specifying whether this is + # necessary because of a failure. + def use_cached_config(because_of_failure = false) + return true if self.configuration + + if because_of_failure and ! Puppet[:usecacheonfailure] + @configuration = nil + Puppet.warning "Not using cache on failed configuration" + return false + end + + return false unless oldtext = self.retrievecache + + begin + @configuration = YAML.load(oldtext).to_configuration + @configuration.from_cache = true + @configuration.host_config = true + rescue => detail + puts detail.backtrace if Puppet[:trace] + Puppet.warning "Could not load cached configuration: %s" % detail + clear + return false + end + return true + end end diff --git a/lib/puppet/network/handler/runner.rb b/lib/puppet/network/handler/runner.rb index a8d0da9ce..c97e4791a 100755 --- a/lib/puppet/network/handler/runner.rb +++ b/lib/puppet/network/handler/runner.rb @@ -43,7 +43,7 @@ class Puppet::Network::Handler end if ignoreschedules - msg += " without schedules" + msg += " ignoring schedules" end Puppet.notice msg diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb index e49090d70..804f357d1 100644 --- a/lib/puppet/node/configuration.rb +++ b/lib/puppet/node/configuration.rb @@ -38,6 +38,10 @@ class Puppet::Node::Configuration < Puppet::PGraph # relationship graph. attr_accessor :is_relationship_graph + # Whether this configuration was retrieved from the cache, which affects + # whether it is written back out again. + attr_accessor :from_cache + # Add classes to our class list. def add_class(*classes) classes.each do |klass| @@ -66,6 +70,16 @@ class Puppet::Node::Configuration < Puppet::PGraph end end + # Create an alias for a resource. + def alias(resource, name) + resource.ref =~ /^(.+)\[/ + + newref = "%s[%s]" % [$1 || resource.class.name, name] + raise(ArgumentError, "Cannot alias %s to %s; resource %s already exists" % [resource.ref, name, newref]) if @resource_table[newref] + @resource_table[newref] = resource + @aliases[resource.ref] << newref + end + # Apply our configuration to the local host. Valid options # are: # :tags - set the tags that restrict what resources run @@ -274,6 +288,8 @@ class Puppet::Node::Configuration < Puppet::PGraph @applying = false @relationship_graph = nil + @aliases = Hash.new { |hash, key| hash[key] = [] } + if block_given? yield(self) finalize() @@ -331,7 +347,9 @@ class Puppet::Node::Configuration < Puppet::PGraph # references to the resource instances. def remove_resource(*resources) resources.each do |resource| - @resource_table.delete(resource.ref) if @resource_table.include?(resource.ref) + @resource_table.delete(resource.ref) + @aliases[resource.ref].each { |res_alias| @resource_table.delete(res_alias) } + @aliases[resource.ref].clear remove_vertex!(resource) if vertex?(resource) @relationship_graph.remove_vertex!(resource) if @relationship_graph and @relationship_graph.vertex?(resource) resource.remove diff --git a/lib/puppet/parser/compile.rb b/lib/puppet/parser/compile.rb index 45d60a58c..7f568f1b3 100644 --- a/lib/puppet/parser/compile.rb +++ b/lib/puppet/parser/compile.rb @@ -107,7 +107,7 @@ class Puppet::Parser::Compile # Evaluate all of the classes specified by the node. def evaluate_node_classes - evaluate_classes(@node.classes, @topscope) + evaluate_classes(@node.classes, topscope) end # Evaluate each specified class in turn. If there are any classes we can't @@ -142,9 +142,7 @@ class Puppet::Parser::Compile # Return a resource by either its ref or its type and title. def findresource(string, name = nil) - if name - string = "%s[%s]" % [string.capitalize, name] - end + string = "%s[%s]" % [string.capitalize, name] if name @resource_table[string] end @@ -173,7 +171,7 @@ class Puppet::Parser::Compile # using the top scope. Adds an edge between the scope and # its parent to the graph. def newscope(parent, options = {}) - parent ||= @topscope + parent ||= topscope options[:compile] = self options[:parser] ||= self.parser scope = Puppet::Parser::Scope.new(options) @@ -419,7 +417,6 @@ class Puppet::Parser::Compile # A graph for maintaining scope relationships. @scope_graph = GRATR::Digraph.new - @scope_graph.add_vertex!(@topscope) # For maintaining the relationship between scopes and their resources. @configuration = Puppet::Node::Configuration.new(@node.name) diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index c8fc2f199..ef53889cf 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -311,7 +311,7 @@ class Transaction ret = eval_resource(resource) end - if Puppet[:evaltrace] + if Puppet[:evaltrace] and @configuration.host_config? resource.info "Evaluated in %0.2f seconds" % seconds end ret diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb index 281ad00d3..6a573489c 100644 --- a/lib/puppet/transportable.rb +++ b/lib/puppet/transportable.rb @@ -59,15 +59,11 @@ module Puppet 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 + @params.each { |param,value| + next unless Puppet::Type::Component.validattr?(param) + Puppet.debug "Defining %s on %s of type %s" % [param,@name,@type] + trans[param] = value + } Puppet::Type::Component.create(trans) end @@ -107,16 +103,7 @@ module Puppet def to_type retobj = nil if typeklass = Puppet::Type.type(self.type) - # FIXME This should really be done differently, but... - if retobj = typeklass[self.name] - self.each do |param, val| - retobj[param] = val - end - else - unless retobj = typeklass.create(self) - return nil - end - end + return typeklass.create(self) else return to_component end @@ -135,7 +122,7 @@ module Puppet class TransBucket include Enumerable - attr_accessor :name, :type, :file, :line, :classes, :keyword, :top + attr_accessor :name, :type, :file, :line, :classes, :keyword, :top, :configuration %w{delete shift include? length empty? << []}.each { |method| define_method(method) do |*args| @@ -218,11 +205,13 @@ module Puppet def to_configuration configuration = Puppet::Node::Configuration.new(Facter.value("hostname")) do |config| delver = proc do |obj| + obj.configuration = config unless container = config.resource(obj.to_ref) container = obj.to_type config.add_resource container end obj.each do |child| + child.configuration = config unless resource = config.resource(child.to_ref) next unless resource = child.to_type config.add_resource resource @@ -252,65 +241,25 @@ module Puppet end def to_type - # this container will contain the equivalent of all objects at - # this level - #container = Puppet::Component.new(:name => @name, :type => @type) - #unless defined? @name - # raise Puppet::DevError, "TransBuckets must have names" - #end unless defined? @type Puppet.debug "TransBucket '%s' has no type" % @name end - usetrans = true - if usetrans - tmpname = nil - - # Nodes have the same name and type - if self.name - tmpname = "%s[%s]" % [@type, 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 - container = Puppet::Type::Component.create(trans) + # Nodes have the same name and type + if self.name + tmpname = "%s[%s]" % [@type, self.name] else - hash = { - :name => self.name, - :type => @type - } - if defined? @parameters - @parameters.each { |param,value| - Puppet.debug "Defining %s on %s of type %s" % - [param,@name,@type] - hash[param] = value - } - else - #Puppet.debug "%s[%s] has no parameters" % [@type, @name] - end - - container = Puppet::Type::Component.create(hash) + tmpname = @type end - #Puppet.info container.inspect - - # unless we successfully created the container, return an error - unless container - Puppet.warning "Got no container back" - return nil + 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 + } end - - # at this point, no objects at are level are still Transportable - # objects - return container + return Puppet::Type::Component.create(trans) end def param(param,value) diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index b7ff1f664..f5dd0f8dd 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -182,7 +182,7 @@ class Type # directly from it. This is the main object instantiation mechanism. if hash.is_a?(Puppet::TransObject) #self[:name] = hash[:name] - [:file, :line, :tags].each { |getter| + [:file, :line, :tags, :configuration].each { |getter| if hash.respond_to?(getter) setter = getter.to_s + "=" if val = hash.send(getter) @@ -194,10 +194,11 @@ class Type # XXX This will need to change when transobjects change to titles. @title = hash.name hash = hash.to_hash - elsif hash[:title] - # XXX This should never happen - @title = hash[:title] - hash.delete(:title) + else + if hash[:title] + @title = hash[:title] + hash.delete(:title) + end end # Before anything else, set our parent if it was included @@ -221,7 +222,7 @@ class Type if attrs.include?(namevar) attrs.delete(namevar) else - self.devfail "My namevar isn\'t a valid attribute...?" + self.devfail "My namevar isn't a valid attribute...?" end else self.devfail "I was not passed a namevar" @@ -284,6 +285,14 @@ class Type # Scheduling has to be done when the whole config is instantiated, so # that file order doesn't matter in finding them. self.schedule + + # Make sure all of our relationships are valid. Again, must be done + # when the entire configuration is instantiated. + self.class.relationship_params.collect do |klass| + if param = @parameters[klass.name] + param.validate_relationship + end + end.flatten.reject { |r| r.nil? } end # Return a cached value diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb index 4dc542a65..7aa24a302 100644 --- a/lib/puppet/type/component.rb +++ b/lib/puppet/type/component.rb @@ -93,9 +93,9 @@ Puppet::Type.newtype(:component) do end # Initialize a new component - def initialize(args) + def initialize(*args) @children = [] - super(args) + super # If the title isn't a full resource reference, assume # we're a class and make an alias for that. diff --git a/lib/puppet/type/pfile/group.rb b/lib/puppet/type/pfile/group.rb index 9625b6354..5f7caf342 100755 --- a/lib/puppet/type/pfile/group.rb +++ b/lib/puppet/type/pfile/group.rb @@ -6,6 +6,10 @@ module Puppet name or group ID." @event = :file_changed + validate do |group| + raise(Puppet::Error, "Invalid group name '%s'" % group.inspect) unless group and group != "" + end + def id2name(id) if id > 70000 return nil diff --git a/spec/unit/network/client/master.rb b/spec/unit/network/client/master.rb new file mode 100755 index 000000000..dca923994 --- /dev/null +++ b/spec/unit/network/client/master.rb @@ -0,0 +1,365 @@ +#!/usr/bin/env ruby +# +# Created by Luke Kanies on 2007-11-12. +# Copyright (c) 2007. All rights reserved. + +require File.dirname(__FILE__) + '/../../../spec_helper' +require 'puppet/network/client/master' + +describe Puppet::Network::Client::Master, " when retrieving the configuration" do + before do + @master = mock 'master' + @client = Puppet::Network::Client.master.new( + :Master => @master + ) + @facts = {"one" => "two", "three" => "four"} + end + + it "should initialize the metadata store" do + @client.class.stubs(:facts).returns(@facts) + @client.expects(:dostorage) + @master.stubs(:getconfig).returns(nil) + @client.getconfig + end + + it "should collect facts to use for configuration retrieval" do + @client.stubs(:dostorage) + @client.class.expects(:facts).returns(@facts) + @master.stubs(:getconfig).returns(nil) + @client.getconfig + end + + it "should fail if no facts could be collected" do + @client.stubs(:dostorage) + @client.class.expects(:facts).returns({}) + @master.stubs(:getconfig).returns(nil) + proc { @client.getconfig }.should raise_error(Puppet::Network::ClientError) + end + + it "should use the cached configuration if it is up to date" do + file = "/path/to/cachefile" + @client.stubs(:cachefile).returns(file) + FileTest.expects(:exist?).with(file).returns(true) + @client.expects(:fresh?).with(@facts).returns true + @client.class.stubs(:facts).returns(@facts) + @client.expects(:use_cached_config).returns(true) + Puppet.stubs(:info) + + @client.getconfig + end + + it "should log that the configuration does not need a recompile" do + file = "/path/to/cachefile" + @client.stubs(:cachefile).returns(file) + FileTest.stubs(:exist?).with(file).returns(true) + @client.stubs(:fresh?).with(@facts).returns true + @client.stubs(:use_cached_config).returns(true) + @client.class.stubs(:facts).returns(@facts) + Puppet.expects(:info).with { |m| m.include?("up to date") } + + @client.getconfig + end + + it "should retrieve plugins if :pluginsync is enabled" do + file = "/path/to/cachefile" + @client.stubs(:cachefile).returns(file) + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + Puppet.settings.expects(:value).with(:pluginsync).returns(true) + @client.expects(:getplugins) + @client.stubs(:get_actual_config).returns(nil) + FileTest.stubs(:exist?).with(file).returns(true) + @client.stubs(:fresh?).with(@facts).returns true + @client.stubs(:use_cached_config).returns(true) + @client.class.stubs(:facts).returns(@facts) + @client.getconfig + end + + it "should use the cached configuration if no configuration could be retrieved" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).raises(ArgumentError.new("whev")) + @client.expects(:use_cached_config).with(true) + @client.getconfig + end + + it "should load the retrieved configuration using YAML" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + config = mock 'config' + YAML.expects(:load).with("myconfig").returns(config) + + @client.stubs(:setclasses) + + config.stubs(:classes) + config.stubs(:to_configuration).returns(config) + config.stubs(:host_config=) + config.stubs(:from_cache).returns(true) + + @client.getconfig + end + + it "should use the cached configuration if the retrieved configuration cannot be converted from YAML" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + YAML.expects(:load).with("myconfig").raises(ArgumentError) + + @client.expects(:use_cached_config).with(true) + + @client.getconfig + end + + it "should set the classes.txt file with the classes listed in the retrieved configuration" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + config = mock 'config' + YAML.expects(:load).with("myconfig").returns(config) + + config.expects(:classes).returns(:myclasses) + @client.expects(:setclasses).with(:myclasses) + + config.stubs(:to_configuration).returns(config) + config.stubs(:host_config=) + config.stubs(:from_cache).returns(true) + + @client.getconfig + end + + it "should convert the retrieved configuration to a RAL configuration" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + yamlconfig = mock 'yaml config' + YAML.stubs(:load).returns(yamlconfig) + + @client.stubs(:setclasses) + + config = mock 'config' + + yamlconfig.stubs(:classes) + yamlconfig.expects(:to_configuration).returns(config) + + config.stubs(:host_config=) + config.stubs(:from_cache).returns(true) + + @client.getconfig + end + + it "should use the cached configuration if the retrieved configuration cannot be converted to a RAL configuration" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + yamlconfig = mock 'yaml config' + YAML.stubs(:load).returns(yamlconfig) + + @client.stubs(:setclasses) + + config = mock 'config' + + yamlconfig.stubs(:classes) + yamlconfig.expects(:to_configuration).raises(ArgumentError) + + @client.expects(:use_cached_config).with(true) + + @client.getconfig + end + + it "should clear the failed configuration if using the cached configuration after failing to instantiate the retrieved configuration" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + yamlconfig = mock 'yaml config' + YAML.stubs(:load).returns(yamlconfig) + + @client.stubs(:setclasses) + + config = mock 'config' + + yamlconfig.stubs(:classes) + yamlconfig.stubs(:to_configuration).raises(ArgumentError) + + @client.stubs(:use_cached_config).with(true) + + @client.expects(:clear) + + @client.getconfig + end + + it "should cache the retrieved yaml configuration if it is not from the cache and is valid" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + yamlconfig = mock 'yaml config' + YAML.stubs(:load).returns(yamlconfig) + + @client.stubs(:setclasses) + + config = mock 'config' + + yamlconfig.stubs(:classes) + yamlconfig.expects(:to_configuration).returns(config) + + config.stubs(:host_config=) + + config.expects(:from_cache).returns(false) + + @client.expects(:cache).with("myconfig") + + @client.getconfig + end + + it "should mark the configuration as a host configuration" do + @client.stubs(:dostorage) + @client.class.stubs(:facts).returns(@facts) + @master.stubs(:getconfig).returns("myconfig") + + yamlconfig = mock 'yaml config' + YAML.stubs(:load).returns(yamlconfig) + + @client.stubs(:setclasses) + + config = mock 'config' + + yamlconfig.stubs(:classes) + yamlconfig.expects(:to_configuration).returns(config) + + config.stubs(:from_cache).returns(true) + + config.expects(:host_config=).with(true) + + @client.getconfig + end +end + +describe Puppet::Network::Client::Master, " when using the cached configuration" do + before do + @master = mock 'master' + @client = Puppet::Network::Client.master.new( + :Master => @master + ) + @facts = {"one" => "two", "three" => "four"} + end + + it "should return do nothing and true if there is already an in-memory configuration" do + @client.configuration = :whatever + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config.should be_true + end + end + + it "should return do nothing and false if it has been told there is a failure and :nocacheonfailure is enabled" do + Puppet.settings.expects(:value).with(:usecacheonfailure).returns(false) + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config(true).should be_false + end + end + + it "should return false if no cached configuration can be found" do + @client.expects(:retrievecache).returns(nil) + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config().should be_false + end + end + + it "should return false if the cached configuration cannot be instantiated" do + YAML.expects(:load).raises(ArgumentError) + @client.expects(:retrievecache).returns("whatever") + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config().should be_false + end + end + + it "should warn if the cached configuration cannot be instantiated" do + YAML.stubs(:load).raises(ArgumentError) + @client.stubs(:retrievecache).returns("whatever") + Puppet.expects(:warning).with { |m| m.include?("Could not load cache") } + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config().should be_false + end + end + + it "should clear the client if the cached configuration cannot be instantiated" do + YAML.stubs(:load).raises(ArgumentError) + @client.stubs(:retrievecache).returns("whatever") + @client.expects(:clear) + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config().should be_false + end + end + + it "should return true if the cached configuration can be instantiated" do + config = mock 'config' + YAML.stubs(:load).returns(config) + + ral_config = mock 'ral config' + ral_config.stubs(:from_cache=) + ral_config.stubs(:host_config=) + config.expects(:to_configuration).returns(ral_config) + + @client.stubs(:retrievecache).returns("whatever") + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config().should be_true + end + end + + it "should set the configuration instance variable if the cached configuration can be instantiated" do + config = mock 'config' + YAML.stubs(:load).returns(config) + + ral_config = mock 'ral config' + ral_config.stubs(:from_cache=) + ral_config.stubs(:host_config=) + config.expects(:to_configuration).returns(ral_config) + + @client.stubs(:retrievecache).returns("whatever") + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config() + end + + @client.configuration.should equal(ral_config) + end + + it "should mark the configuration as a host_config if valid" do + config = mock 'config' + YAML.stubs(:load).returns(config) + + ral_config = mock 'ral config' + ral_config.stubs(:from_cache=) + ral_config.expects(:host_config=).with(true) + config.expects(:to_configuration).returns(ral_config) + + @client.stubs(:retrievecache).returns("whatever") + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config() + end + + @client.configuration.should equal(ral_config) + end + + it "should mark the configuration as from the cache if valid" do + config = mock 'config' + YAML.stubs(:load).returns(config) + + ral_config = mock 'ral config' + ral_config.expects(:from_cache=).with(true) + ral_config.stubs(:host_config=) + config.expects(:to_configuration).returns(ral_config) + + @client.stubs(:retrievecache).returns("whatever") + Puppet::Network::Client::Master.publicize_methods :use_cached_config do + @client.use_cached_config() + end + + @client.configuration.should equal(ral_config) + end +end diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb index e9dc6b85d..5780d4fbb 100755 --- a/spec/unit/node/configuration.rb +++ b/spec/unit/node/configuration.rb @@ -301,9 +301,9 @@ end describe Puppet::Node::Configuration, " when functioning as a resource container" do before do @config = Puppet::Node::Configuration.new("host") - @one = stub 'resource1', :ref => "Me[you]", :configuration= => nil - @two = stub 'resource2', :ref => "Me[him]", :configuration= => nil - @dupe = stub 'resource3', :ref => "Me[you]", :configuration= => nil + @one = stub 'resource1', :ref => "Me[one]", :configuration= => nil + @two = stub 'resource2', :ref => "Me[two]", :configuration= => nil + @dupe = stub 'resource3', :ref => "Me[one]", :configuration= => nil end it "should provide a method to add one or more resources" do @@ -376,7 +376,7 @@ describe Puppet::Node::Configuration, " when functioning as a resource container it "should be able to find resources by reference or by type/title tuple" do @config.add_resource @one - @config.resource("me", "you").should equal(@one) + @config.resource("me", "one").should equal(@one) end it "should have a mechanism for removing resources" do @@ -386,6 +386,32 @@ describe Puppet::Node::Configuration, " when functioning as a resource container @config.resource(@one.ref).should be_nil @config.vertex?(@one).should be_false end + + it "should have a method for creating aliases for resources" do + @config.add_resource @one + @config.alias(@one, "other") + @config.resource("me", "other").should equal(@one) + end + + # This test is the same as the previous, but the behaviour should be explicit. + it "should alias using the class name from the resource reference, not the resource class name" do + @config.add_resource @one + @config.alias(@one, "other") + @config.resource("me", "other").should equal(@one) + end + + it "should fail to add an alias if the aliased name already exists" do + @config.add_resource @one + proc { @config.alias @two, "one" }.should raise_error(ArgumentError) + end + + it "should remove resource aliases when the target resource is removed" do + @config.add_resource @one + @config.alias(@one, "other") + @one.expects :remove + @config.remove_resource(@one) + @config.resource("me", "other").should be_nil + end end module ApplyingConfigurations diff --git a/spec/unit/other/transbucket.rb b/spec/unit/other/transbucket.rb index 8cb9abaa4..0da808460 100755 --- a/spec/unit/other/transbucket.rb +++ b/spec/unit/other/transbucket.rb @@ -102,6 +102,18 @@ describe Puppet::TransBucket, " when generating a configuration" do @top.to_configuration end + it "should set each TransObject's configuration before converting to a RAL resource" do + @middleobj.expects(:configuration=).with { |c| c.is_a?(Puppet::Node::Configuration) } + @top.to_configuration + end + + it "should set each TransBucket's configuration before converting to a RAL resource" do + # each bucket is seen twice in the loop, so we have to handle the case where the config + # is set twice + @bottom.expects(:configuration=).with { |c| c.is_a?(Puppet::Node::Configuration) }.at_least_once + @top.to_configuration + end + after do Puppet::Type.allclear end diff --git a/spec/unit/other/transobject.rb b/spec/unit/other/transobject.rb index 144940b7e..eaca855db 100755 --- a/spec/unit/other/transobject.rb +++ b/spec/unit/other/transobject.rb @@ -2,114 +2,73 @@ require File.dirname(__FILE__) + '/../../spec_helper' -describe Puppet::TransObject, " when building its search path" do -end - -describe Puppet::TransObject, " when building its search path" do -end -#!/usr/bin/env ruby - -$:.unshift("../lib").unshift("../../lib") if __FILE__ =~ /\.rb$/ - -require 'puppet' require 'puppet/transportable' -require 'puppettest' -require 'puppettest/parsertesting' -require 'yaml' -class TestTransportable < Test::Unit::TestCase - include PuppetTest::ParserTesting - - def test_yamldumpobject - obj = mk_transobject - obj.to_yaml_properties - str = nil - assert_nothing_raised { - str = YAML.dump(obj) - } - - newobj = nil - assert_nothing_raised { - newobj = YAML.load(str) - } - - assert(newobj.name, "Object has no name") - assert(newobj.type, "Object has no type") +describe Puppet::TransObject, " when serializing" do + before do + @resource = Puppet::TransObject.new("/my/file", "file") + @resource["one"] = "test" + @resource["two"] = "other" end - def test_yamldumpbucket - objects = %w{/etc/passwd /etc /tmp /var /dev}.collect { |d| - mk_transobject(d) - } - bucket = mk_transbucket(*objects) - str = nil - assert_nothing_raised { - str = YAML.dump(bucket) - } - - newobj = nil - assert_nothing_raised { - newobj = YAML.load(str) - } - - assert(newobj.name, "Bucket has no name") - assert(newobj.type, "Bucket has no type") + it "should be able to be dumped to yaml" do + proc { YAML.dump(@resource) }.should_not raise_error end - # Verify that we correctly strip out collectable objects, since they should - # not be sent to the client. - def test_collectstrip - top = mk_transtree do |object, depth, width| - if width % 2 == 1 - object.collectable = true - end - end + it "should produce an equivalent yaml object" do + text = YAML.dump(@resource) - assert(top.flatten.find_all { |o| o.collectable }.length > 0, - "Could not find any collectable objects") + newresource = YAML.load(text) + newresource.name.should == "/my/file" + newresource.type.should == "file" + %w{one two}.each do |param| + newresource[param].should == @resource[param] + end + end +end - # Now strip out the collectable objects - top.collectstrip! +describe Puppet::TransObject, " when converting to a RAL resource" do + before do + @resource = Puppet::TransObject.new("/my/file", "file") + @resource["one"] = "test" + @resource["two"] = "other" + end - # And make sure they're actually gone - assert_equal(0, top.flatten.find_all { |o| o.collectable }.length, - "Still found collectable objects") + it "should use the resource type's :create method to create the resource" do + type = mock 'resource type' + type.expects(:create).with(@resource).returns(:myresource) + Puppet::Type.expects(:type).with("file").returns(type) + @resource.to_type.should == :myresource end - # Make sure our 'delve' command is working - def test_delve - top = mk_transtree do |object, depth, width| - if width % 2 == 1 - object.collectable = true - end - end + it "should convert to a component instance if the resource type cannot be found" do + Puppet::Type.expects(:type).with("file").returns(nil) + @resource.expects(:to_component).returns(:mycomponent) + @resource.to_type.should == :mycomponent + end +end - objects = [] - buckets = [] - collectable = [] +describe Puppet::TransObject, " when converting to a RAL component instance" do + before do + @resource = Puppet::TransObject.new("/my/file", "one::two") + @resource["one"] = "test" + @resource["noop"] = "other" + end - count = 0 - assert_nothing_raised { - top.delve do |object| - count += 1 - if object.is_a? Puppet::TransBucket - buckets << object - else - objects << object - if object.collectable - collectable << object - end - end - end - } + it "should use a new TransObject whose name is a resource reference of the type and title of the original TransObject" do + Puppet::Type::Component.expects(:create).with { |resource| resource.type == :component and resource.name == "One::Two[/my/file]" }.returns(:yay) + @resource.to_component.should == :yay + end - top.flatten.each do |obj| - assert(objects.include?(obj), "Missing obj %s[%s]" % [obj.type, obj.name]) - end + it "should pass the resource parameters on to the newly created TransObject" do + Puppet::Type::Component.expects(:create).with { |resource| resource["noop"] == "other" }.returns(:yay) + @resource.to_component.should == :yay + end - assert_equal(collectable.length, - top.flatten.find_all { |o| o.collectable }.length, - "Found incorrect number of collectable objects") + # LAK:FIXME This really isn't the design we want going forward, but it's + # good enough for now. + it "should not pass resource paramaters that are not metaparams" do + Puppet::Type::Component.expects(:create).with { |resource| resource["one"].nil? }.returns(:yay) + @resource.to_component.should == :yay end end - diff --git a/test/language/parser.rb b/test/language/parser.rb index 04cd3a095..bc89eff20 100755 --- a/test/language/parser.rb +++ b/test/language/parser.rb @@ -694,6 +694,7 @@ file { "/tmp/yayness": manifest = File.join(modpath, "manifest.pp") manifest_texts.each do |txt| + Puppet::Type.allclear File.open(manifest, "w") { |f| f.puts txt } assert_nothing_raised { diff --git a/test/lib/puppettest.rb b/test/lib/puppettest.rb index 92981b557..6447b80fb 100755 --- a/test/lib/puppettest.rb +++ b/test/lib/puppettest.rb @@ -20,6 +20,17 @@ if ARGV.include?("-d") $console = true end +# Some monkey-patching to allow us to test private methods. +class Class + def publicize_methods(*methods) + saved_private_instance_methods = methods.empty? ? self.private_instance_methods : methods + + self.class_eval { public *saved_private_instance_methods } + yield + self.class_eval { private *saved_private_instance_methods } + end +end + module PuppetTest # Munge cli arguments, so we can enable debugging if we want # and so we can run just specific methods. @@ -196,7 +207,7 @@ module PuppetTest Puppet[:ignoreschedules] = true - @start = Time.now + #@start = Time.now end def tempfile @@ -246,8 +257,8 @@ module PuppetTest end def teardown - @stop = Time.now - File.open("/tmp/test_times.log", ::File::WRONLY|::File::CREAT|::File::APPEND) { |f| f.puts "%0.4f %s %s" % [@stop - @start, @method_name, self.class] } + #@stop = Time.now + #File.open("/tmp/test_times.log", ::File::WRONLY|::File::CREAT|::File::APPEND) { |f| f.puts "%0.4f %s %s" % [@stop - @start, @method_name, self.class] } @@cleaners.each { |cleaner| cleaner.call() } @@tmpfiles.each { |file| diff --git a/test/lib/puppettest/support/assertions.rb b/test/lib/puppettest/support/assertions.rb index 7e3e5ca2b..906bb3c76 100644 --- a/test/lib/puppettest/support/assertions.rb +++ b/test/lib/puppettest/support/assertions.rb @@ -1,7 +1,9 @@ require 'puppettest' +require 'puppettest/support/utils' require 'fileutils' module PuppetTest + include PuppetTest::Support::Utils def assert_logged(level, regex, msg = nil) # Skip verifying logs that we're not supposed to send. return unless Puppet::Util::Log.sendlevel?(level) diff --git a/test/network/client/master.rb b/test/network/client/master.rb index 4f6956470..7216af936 100755 --- a/test/network/client/master.rb +++ b/test/network/client/master.rb @@ -7,38 +7,6 @@ require 'mocha' class TestMasterClient < Test::Unit::TestCase include PuppetTest::ServerTest - - class FakeTrans - def initialize - @counters = Hash.new { |h,k| h[k] = 0 } - end - [:evaluate, :report, :cleanup, :addtimes, :tags, :ignoreschedules].each do |m| - define_method(m.to_s + "=") do |*args| - @counters[m] += 1 - end - define_method(m) do |*args| - @counters[m] += 1 - end - define_method(m.to_s + "?") do - @counters[m] - end - end - end - class FakeComponent - attr_accessor :trans - def evaluate - @trans = FakeTrans.new - @trans - end - - def finalize - @finalized = true - end - - def finalized? - @finalized - end - end def setup super @@ -67,66 +35,6 @@ class TestMasterClient < Test::Unit::TestCase return client end - - def mk_fake_client - server = Puppet::Network::Handler.master.new :Code => "" - master = Puppet::Network::Client.master.new :Master => server, :Local => true - - # Now create some objects - objects = FakeComponent.new - - master.send(:instance_variable_set, "@objects", objects) - - class << master - def report(r) - @reported ||= 0 - @reported += 1 - end - def reported - @reported ||= 0 - @reported - end - end - return master, objects - end - - def test_getconfig - client = mkclient - - $methodsrun = [] - cleanup { $methodsrun = nil } - client.meta_def(:getplugins) do - $methodsrun << :getplugins - end - client.meta_def(:get_actual_config) do - $methodsrun << :get_actual_config - result = Puppet::TransBucket.new() - result.type = "testing" - result.name = "yayness" - result - end - - assert_nothing_raised do - client.getconfig - end - [:get_actual_config].each do |method| - assert($methodsrun.include?(method), "method %s was not run" % method) - end - assert(! $methodsrun.include?(:getplugins), "plugins were synced even tho disabled") - - # Now set pluginsync - Puppet[:pluginsync] = true - $methodsrun.clear - - assert_nothing_raised do - client.getconfig - end - [:getplugins, :get_actual_config].each do |method| - assert($methodsrun.include?(method), "method %s was not run" % method) - end - - assert_instance_of(Puppet::Node::Configuration, client.configuration, "Configuration was not created") - end def test_disable FileUtils.mkdir_p(Puppet[:statedir]) @@ -136,27 +44,22 @@ class TestMasterClient < Test::Unit::TestCase client = mkclient(master) - assert(! FileTest.exists?(@createdfile)) - - assert_nothing_raised { + assert_nothing_raised("Could not disable client") { client.disable } - assert_nothing_raised { - client.run - } + client.expects(:getconfig).never - assert(! FileTest.exists?(@createdfile), "Disabled client ran") + client.run - assert_nothing_raised { - client.enable - } + client = mkclient(master) - assert_nothing_raised { - client.run - } + client.expects(:getconfig) - assert(FileTest.exists?(@createdfile), "Enabled client did not run") + assert_nothing_raised("Could not enable client") { + client.enable + } + client.run end # Make sure we're getting the client version in our list of facts @@ -652,15 +555,16 @@ end client = mkclient ftype = Puppet::Type.type(:file) + file = ftype.create :title => "/what/ever", :ensure => :present + config = Puppet::Node::Configuration.new + config.add_resource(file) - assert_nil(ftype[@createdfile], "file object already exists") - assert(! FileTest.exists?(@createdfile), "File already exists on disk") + config.expects :apply - assert_nothing_raised("Could not apply config") do - client.run - end + client.configuration = config + client.expects(:getconfig) + client.run - assert(FileTest.exists?(@createdfile), "File does not exist on disk") assert_nil(ftype[@createdfile], "file object was not removed from memory") end @@ -675,20 +579,20 @@ end end end - # #800 -- we cannot fix this using the current design. - def disabled_test_invalid_relationships_do_not_get_cached - # Make a master with an invalid relationship + def test_invalid_configurations_do_not_get_cached master = mkmaster :Code => "notify { one: require => File[yaytest] }" master.local = false # so it gets cached client = mkclient(master) + client.stubs(:facts).returns({}) client.local = false - client.getconfig - # Doesn't throw an exception, but definitely fails. - client.apply + Puppet::Node::Facts.indirection.stubs(:terminus_class).returns(:memory) # Make sure the config is not cached. - config = Puppet.settings[:localconfig] + ".yaml" - assert(! File.exists?(config), "Cached an invalid configuration") + client.expects(:cache).never + + client.getconfig + # Doesn't throw an exception, but definitely fails. + client.run end end diff --git a/test/network/client/resource.rb b/test/network/client/resource.rb index 83c195035..eb8e829eb 100755 --- a/test/network/client/resource.rb +++ b/test/network/client/resource.rb @@ -3,10 +3,13 @@ require File.dirname(__FILE__) + '/../../lib/puppettest' require 'puppettest' +require 'puppettest/support/utils' +require 'puppettest/support/assertions' require 'puppet/network/client/resource' class TestResourceClient < Test::Unit::TestCase include PuppetTest::ServerTest + include PuppetTest::Support::Utils def mkresourceserver Puppet::Network::Handler.resource.new @@ -35,6 +38,7 @@ class TestResourceClient < Test::Unit::TestCase assert_instance_of(Puppet::TransObject, tobj) + Puppet::Type.allclear obj = nil assert_nothing_raised { obj = tobj.to_type @@ -45,6 +49,7 @@ class TestResourceClient < Test::Unit::TestCase File.unlink(file) # Now test applying + Puppet::Type.allclear result = nil assert_nothing_raised { result = client.apply(tobj) @@ -52,6 +57,7 @@ class TestResourceClient < Test::Unit::TestCase assert(FileTest.exists?(file), "File was not created on apply") # Lastly, test "list" + Puppet::Type.allclear list = nil assert_nothing_raised { list = client.list("user") @@ -64,12 +70,14 @@ class TestResourceClient < Test::Unit::TestCase break if count > 3 assert_instance_of(Puppet::TransObject, tobj) + Puppet::Type.allclear tobj2 = nil assert_nothing_raised { tobj2 = client.describe(tobj.type, tobj.name) } obj = nil + Puppet::Type.allclear assert_nothing_raised { obj = tobj2.to_type } diff --git a/test/network/handler/master.rb b/test/network/handler/master.rb index 694888f4d..25117030e 100755 --- a/test/network/handler/master.rb +++ b/test/network/handler/master.rb @@ -13,39 +13,11 @@ class TestMaster < Test::Unit::TestCase Puppet::Indirector::Indirection.clear_cache end - def test_defaultmanifest - textfiles { |file| - Puppet[:manifest] = file - client = nil - master = nil - assert_nothing_raised() { - # this is the default server setup - master = Puppet::Network::Handler.master.new( - :Manifest => file, - :UseNodes => false, - :Local => true - ) - } - assert_nothing_raised() { - client = Puppet::Network::Client.master.new( - :Master => master - ) - } - - # pull our configuration - assert_nothing_raised() { - client.getconfig - stopservices - Puppet::Type.allclear - } - - break - } - end - + # Make sure that files are reread when they change. def test_filereread # Start with a normal setting Puppet[:filetimeout] = 15 + manifest = mktestmanifest() facts = Puppet::Network::Client.master.facts @@ -63,35 +35,12 @@ class TestMaster < Test::Unit::TestCase :Local => true ) } - assert_nothing_raised() { - client = Puppet::Network::Client.master.new( - :Master => master - ) - } - assert(client, "did not create master client") - # The client doesn't have a config, so it can't be up to date - assert(! client.fresh?(facts), - "Client is incorrectly up to date") - - Puppet.settings.use(:main) - config = nil - assert_nothing_raised { - config = client.getconfig - config.apply - } - - # Now it should be up to date - assert(client.fresh?(facts), "Client is not up to date") + config = master.getconfig({"hostname" => "blah"}) # Cache this value for later parse1 = master.freshness("mynode") - # Verify the config got applied - assert(FileTest.exists?(@createdfile), - "Created file %s does not exist" % @createdfile) - Puppet::Type.allclear - sleep 1.5 # Create a new manifest File.open(manifest, "w") { |f| @@ -101,7 +50,6 @@ class TestMaster < Test::Unit::TestCase # Verify that the master doesn't immediately reparse the file; we # want to wait through the timeout assert_equal(parse1, master.freshness("mynode"), "Master did not wait through timeout") - assert(client.fresh?(facts), "Client is not up to date") # Then eliminate it Puppet[:filetimeout] = 0 @@ -109,16 +57,8 @@ class TestMaster < Test::Unit::TestCase # Now make sure the master does reparse #Puppet.notice "%s vs %s" % [parse1, master.freshness] assert(parse1 != master.freshness("mynode"), "Master did not reparse file") - assert(! client.fresh?(facts), "Client is incorrectly up to date") - - # Retrieve and apply the new config - assert_nothing_raised { - config = client.getconfig - config.apply - } - assert(client.fresh?(facts), "Client is not up to date") - assert(FileTest.exists?(file2), "Second file %s does not exist" % file2) + assert(master.getconfig({"hostname" => "blah"}) != config, "Did not use reloaded config") end # Make sure we're correctly doing clientname manipulations. diff --git a/test/network/handler/runner.rb b/test/network/handler/runner.rb index 402b27ffc..171458ffa 100755 --- a/test/network/handler/runner.rb +++ b/test/network/handler/runner.rb @@ -29,7 +29,8 @@ class TestHandlerRunner < Test::Unit::TestCase client end - def test_runner + def setup + super FileUtils.mkdir_p(Puppet[:statedir]) Puppet[:ignoreschedules] = false # Okay, make our manifest @@ -37,7 +38,7 @@ class TestHandlerRunner < Test::Unit::TestCase created = tempfile() # We specify the schedule here, because I was having problems with # using default schedules. - code = %{ + @code = %{ class yayness { schedule { "yayness": period => weekly } file { "#{created}": ensure => file, schedule => yayness } @@ -46,59 +47,24 @@ class TestHandlerRunner < Test::Unit::TestCase include yayness } - client = mkclient(code) + @client = mkclient(@code) - runner = nil - assert_nothing_raised { - runner = Puppet::Network::Handler.runner.new - } - # First: tags - # Second: ignore schedules true/false - # Third: background true/false - # Fourth: whether file should exist true/false - [ - ["with no backgrounding", - nil, true, true, true], - ["in the background", - nil, true, false, true], - ["with a bad tag", - ["coolness"], true, false, false], - ["with another bad tag", - "coolness", true, false, false], - ["with a good tag", - ["coolness", "yayness"], true, false, true], - ["with another good tag", - ["yayness"], true, false, true], - ["with a third good tag", - "yayness", true, false, true], - ["with no tags", - "", true, false, true], - ["not ignoring schedules", - nil, false, false, false], - ["ignoring schedules", - nil, true, false, true], - ].each do |msg, tags, ignore, fg, shouldexist| - if FileTest.exists?(created) - File.unlink(created) - end - assert_nothing_raised { - # Try it without backgrounding - runner.run(tags, ignore, fg) - } + @runner = Puppet::Network::Handler.runner.new + end + + def test_runner_when_in_foreground + @client.expects(:run).with(:tags => "mytags", :ignoreschedules => true) + + Process.expects(:newthread).never - unless fg - Puppet.join - end - - if shouldexist - assert(FileTest.exists?(created), "File did not get created " + - msg) - else - assert(!FileTest.exists?(created), "File got created incorrectly " + - msg) - end - end + @runner.run("mytags", true, true) end -end + def test_runner_when_in_background + @client.expects(:run).with(:tags => "mytags", :ignoreschedules => true) + + Puppet.expects(:newthread).yields + @runner.run("mytags", true, false) + end +end diff --git a/test/other/relationships.rb b/test/other/relationships.rb index bfe7e88d7..0f2a103fe 100755 --- a/test/other/relationships.rb +++ b/test/other/relationships.rb @@ -111,7 +111,7 @@ class TestRelationships < Test::Unit::TestCase end end - def test_store_relationship + def test_munge_relationship file = Puppet::Type.newfile :path => tempfile(), :mode => 0755 execs = [] 3.times do |i| @@ -122,7 +122,7 @@ class TestRelationships < Test::Unit::TestCase result = nil [execs[0], [:exec, "yay0"], ["exec", "yay0"]].each do |target| assert_nothing_raised do - result = file.send(:store_relationship, :require, target) + result = file.send(:munge_relationship, :require, target) end assert_equal([[:exec, "yay0"]], result) @@ -133,7 +133,7 @@ class TestRelationships < Test::Unit::TestCase strings = execs.collect { |e| [e.class.name.to_s, e.title] } [execs, symbols, strings].each do |target| assert_nothing_raised do - result = file.send(:store_relationship, :require, target) + result = file.send(:munge_relationship, :require, target) end assert_equal(symbols, result) @@ -141,7 +141,7 @@ class TestRelationships < Test::Unit::TestCase # Make sure we can mix it up, even though this shouldn't happen assert_nothing_raised do - result = file.send(:store_relationship, :require, [execs[0], [execs[1].class.name, execs[1].title]]) + result = file.send(:munge_relationship, :require, [execs[0], [execs[1].class.name, execs[1].title]]) end assert_equal([[:exec, "yay0"], [:exec, "yay1"]], result) @@ -151,7 +151,7 @@ class TestRelationships < Test::Unit::TestCase file[:require] = execs[0] assert_nothing_raised do - result = file.send(:store_relationship, :require, [execs[1], execs[2]]) + result = file.send(:munge_relationship, :require, [execs[1], execs[2]]) end assert_equal(symbols, result) diff --git a/test/ral/manager/instances.rb b/test/ral/manager/instances.rb index 6ac4322f5..88f766038 100755 --- a/test/ral/manager/instances.rb +++ b/test/ral/manager/instances.rb @@ -93,7 +93,8 @@ class TestTypeInstances < Test::Unit::TestCase # Make sure resources are entirely deleted. def test_delete aliases = %w{one} - obj = @type.create(:name => "testing", :alias => "two") + config = mk_configuration + obj = @type.create(:name => "testing", :alias => "two", :configuration => config) aliases << "two" @type.alias("two", obj) diff --git a/test/ral/manager/type.rb b/test/ral/manager/type.rb index 57248159b..350d3dd15 100755 --- a/test/ral/manager/type.rb +++ b/test/ral/manager/type.rb @@ -131,27 +131,60 @@ class TestType < Test::Unit::TestCase } end - # Verify that aliasing works - def test_aliasing - file = tempfile() + def test_aliases_to_self_are_not_failures + resource = Puppet.type(:file).create( + :name => "/path/to/some/missing/file", + :ensure => "file" + ) + resource.stubs(:path).returns("") - baseobj = nil - assert_nothing_raised { - baseobj = Puppet.type(:file).create( - :name => file, - :ensure => "file", - :alias => ["funtest"] - ) - } + configuration = stub 'configuration' + configuration.expects(:resource).with(:file, "/path/to/some/missing/file").returns(resource) + resource.configuration = configuration # Verify our adding ourselves as an alias isn't an error. - assert_nothing_raised { - baseobj[:alias] = file + assert_nothing_raised("Could not add alias") { + resource[:alias] = "/path/to/some/missing/file" + } + + assert_equal(resource.object_id, Puppet.type(:file)["/path/to/some/missing/file"].object_id, "Could not retrieve alias to self") + end + + def test_aliases_are_added_to_class_and_configuration + resource = Puppet.type(:file).create( + :name => "/path/to/some/missing/file", + :ensure => "file" + ) + resource.stubs(:path).returns("") + + configuration = stub 'configuration' + configuration.stubs(:resource).returns(nil) + configuration.expects(:alias).with(resource, "funtest") + resource.configuration = configuration + + assert_nothing_raised("Could not add alias") { + resource[:alias] = "funtest" } - assert_instance_of(Puppet.type(:file), Puppet.type(:file)["funtest"], - "Could not retrieve alias") + assert_equal(resource.object_id, Puppet.type(:file)["funtest"].object_id, "Could not retrieve alias") + end + + def test_aliasing_fails_without_a_configuration + resource = Puppet.type(:file).create( + :name => "/no/such/file", + :ensure => "file" + ) + + assert_raise(Puppet::Error, "Did not fail to alias when no configuration was available") { + resource[:alias] = "funtest" + } + end + def test_configurations_are_set_during_initialization_if_present_on_the_transobject + trans = Puppet::TransObject.new("/path/to/some/file", :file) + trans.configuration = :my_config + resource = trans.to_type + assert_equal(resource.configuration, trans.configuration, "Did not set configuration on initialization") end # Verify that requirements don't depend on file order diff --git a/test/ral/types/file.rb b/test/ral/types/file.rb index aca9d3b9c..73095a783 100755 --- a/test/ral/types/file.rb +++ b/test/ral/types/file.rb @@ -104,6 +104,12 @@ class TestFile < Test::Unit::TestCase } end + def test_groups_fails_when_invalid + assert_raise(Puppet::Error, "did not fail when the group was empty") do + Puppet::Type.type(:file).create :path => "/some/file", :group => "" + end + end + if Puppet::Util::SUIDManager.uid == 0 def test_createasuser dir = tmpdir() diff --git a/test/ral/types/host.rb b/test/ral/types/host.rb index 69e37da0f..1013651c5 100755 --- a/test/ral/types/host.rb +++ b/test/ral/types/host.rb @@ -38,12 +38,16 @@ class TestHost < Test::Unit::TestCase else @hcount = 1 end + + @configuration ||= mk_configuration + host = nil assert_nothing_raised { host = Puppet.type(:host).create( :name => "fakehost%s" % @hcount, :ip => "192.168.27.%s" % @hcount, - :alias => "alias%s" % @hcount + :alias => "alias%s" % @hcount, + :configuration => @configuration ) } diff --git a/test/ral/types/parameter.rb b/test/ral/types/parameter.rb index 89c8b944d..1d402cb85 100755 --- a/test/ral/types/parameter.rb +++ b/test/ral/types/parameter.rb @@ -113,6 +113,9 @@ class TestParameter < Test::Unit::TestCase inst = type.create(:name => "test") + config = mk_configuration + inst.configuration = config + assert_nothing_raised("Could not create shadowed param") { inst[:alias] = "foo" } @@ -133,7 +136,7 @@ class TestParameter < Test::Unit::TestCase # Now try it during initialization other = nil assert_nothing_raised("Could not create instance with shadow") do - other = type.create(:name => "rah", :alias => "one") + other = type.create(:name => "rah", :alias => "one", :configuration => config) end params = other.instance_variable_get("@parameters") obj = params[:alias] diff --git a/test/ral/types/sshkey.rb b/test/ral/types/sshkey.rb index c610eb9e9..c99f7562a 100755 --- a/test/ral/types/sshkey.rb +++ b/test/ral/types/sshkey.rb @@ -47,12 +47,15 @@ class TestSSHKey < Test::Unit::TestCase @kcount = 1 end + @config ||= mk_configuration + assert_nothing_raised { key = @sshkeytype.create( :name => "host%s.madstop.com" % @kcount, :key => "%sAAAAB3NzaC1kc3MAAACBAMnhSiku76y3EGkNCDsUlvpO8tRgS9wL4Eh54WZfQ2lkxqfd2uT/RTT9igJYDtm/+UHuBRdNGpJYW1Nw2i2JUQgQEEuitx4QKALJrBotejGOAWxxVk6xsh9xA0OW8Q3ZfuX2DDitfeC8ZTCl4xodUMD8feLtP+zEf8hxaNamLlt/AAAAFQDYJyf3vMCWRLjTWnlxLtOyj/bFpwAAAIEAmRxxXb4jjbbui9GYlZAHK00689DZuX0EabHNTl2yGO5KKxGC6Esm7AtjBd+onfu4Rduxut3jdI8GyQCIW8WypwpJofCIyDbTUY4ql0AQUr3JpyVytpnMijlEyr41FfIb4tnDqnRWEsh2H7N7peW+8DWZHDFnYopYZJ9Yu4/jHRYAAACAERG50e6aRRb43biDr7Ab9NUCgM9bC0SQscI/xdlFjac0B/kSWJYTGVARWBDWug705hTnlitY9cLC5Ey/t/OYOjylTavTEfd/bh/8FkAYO+pWdW3hx6p97TBffK0b6nrc6OORT2uKySbbKOn0681nNQh4a6ueR3JRppNkRPnTk5c=" % @kcount, :type => "ssh-dss", - :alias => ["192.168.0.%s" % @kcount] + :alias => ["192.168.0.%s" % @kcount], + :configuration => @config ) } |
