diff --git a/CHANGELOG b/CHANGELOG index 3f3d87fa4..09f74eb96 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,8 @@ 0.22.2 (grover) + All parameter instances are stored in a single @parameters instance variable + hash within resource type instances. We used to use separate hashes for + each parameter type. + Added the concept of provider features. Eventually these should be able to express the full range of provider functionality, but for now they can test a provider to see what methods it has set and determine what features it diff --git a/lib/puppet/metatype/attributes.rb b/lib/puppet/metatype/attributes.rb index a96533964..b4cc5f687 100644 --- a/lib/puppet/metatype/attributes.rb +++ b/lib/puppet/metatype/attributes.rb @@ -102,26 +102,28 @@ class Puppet::Type end end - # A similar function but one that yields the name, type, and class. + # A similar function but one that yields the class and type. # This is mainly so that setdefaults doesn't call quite so many functions. def self.eachattr(*ary) - # now get all of the arguments, in a specific order - # Cache this, since it gets called so many times - if ary.empty? ary = nil end - self.properties.each { |property| - yield(property, :property) if ary.nil? or ary.include?(property.name) - } - @parameters.each { |param| - yield(param, :param) if ary.nil? or ary.include?(param.name) - } - - @@metaparams.each { |param| - yield(param, :meta) if ary.nil? or ary.include?(param.name) - } + # We have to do this in a specific order, so that defaults are + # created in that order (e.g., providers should be set up before + # anything else). + allattrs.each do |name| + next unless ary.nil? or ary.include?(name) + if obj = @properties.find { |p| p.name == name } + yield obj, :property + elsif obj = @parameters.find { |p| p.name == name } + yield obj, :param + elsif obj = @@metaparams.find { |p| p.name == name } + yield obj, :meta + else + raise Puppet::DevError, "Could not find parameter %s" % name + end + end end def self.eachmetaparam @@ -627,6 +629,7 @@ class Puppet::Type # set, set them now. This method can be handed a list of attributes, # and if so it will only set defaults for those attributes. def setdefaults(*ary) + #self.class.eachattr(*ary) { |klass, type| self.class.eachattr(*ary) { |klass, type| # not many attributes will have defaults defined, so we short-circuit # those away diff --git a/lib/puppet/metatype/providers.rb b/lib/puppet/metatype/providers.rb index c078ced24..98b5caf43 100644 --- a/lib/puppet/metatype/providers.rb +++ b/lib/puppet/metatype/providers.rb @@ -45,74 +45,6 @@ class Puppet::Type return @defaultprovider end - # Define one or more features. Currently, features are just a list of - # methods; if all methods are defined as instance methods on the provider, - # then the provider has that feature, otherwise it does not. - def self.dis_features(hash) - @features ||= {} - hash.each do |name, methods| - name = symbolize(name) - methods = methods.collect { |m| symbolize(m) } - if @features.include?(name) - raise Puppet::DevError, "Feature %s is already defined" % name - end - @features[name] = methods - end - end - - # Generate a module that sets up the boolean methods to test for given - # features. - def self.dis_feature_module - unless defined? @feature_module - @features ||= {} - @feature_module = ::Module.new - const_set("FeatureModule", @feature_module) - features = @features - @feature_module.send(:define_method, :feature?) do |name| - method = name.to_s + "?" - if respond_to?(method) and send(method) - return true - else - return false - end - end - @feature_module.send(:define_method, :features) do - return false unless defined?(features) - features.keys.find_all { |n| feature?(n) }.sort { |a,b| - a.to_s <=> b.to_s - } - end - #if defined?(@features) - @features.each do |name, methods| - method = name.to_s + "?" - @feature_module.send(:define_method, method) do - set = nil - methods.each do |m| - if is_a?(Class) - unless public_method_defined?(m) - set = false - break - end - else - unless respond_to?(m) - set = false - break - end - end - end - - if set.nil? - true - else - false - end - end - end - #end - end - @feature_module - end - # Convert a hash, as provided by, um, a provider, into an instance of self. def self.hash2obj(hash) obj = nil @@ -267,7 +199,9 @@ class Puppet::Type }.join("\n") end - defaultto { @parent.class.defaultprovider.name } + defaultto { + @parent.class.defaultprovider.name + } validate do |value| value = value[0] if value.is_a? Array diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb index 6ae7d55b3..d568b8ae7 100644 --- a/lib/puppet/network/client/master.rb +++ b/lib/puppet/network/client/master.rb @@ -128,27 +128,27 @@ class Puppet::Network::Client::Master < Puppet::Network::Client end end - # Have the facts changed since we last compiled? - def facts_changed?(facts) - oldfacts = Puppet::Util::Storage.cache(:configuration)[:facts] - newfacts = self.class.facts - if oldfacts == newfacts - return false - else - return true - end - end - # Check whether our configuration is up to date def fresh?(facts) - return false if Puppet[:ignorecache] - return false unless self.compile_time - return false if self.facts_changed?(facts) + if Puppet[:ignorecache] + Puppet.notice "Ignoring cache" + return false + end + unless self.compile_time + Puppet.debug "No cached compile time" + return false + end + if facts_changed?(facts) + Puppet.info "Facts have changed; recompiling" + return false + end # We're willing to give a 2 second drift - if @driver.freshness - @compile_time.to_i < 1 + newcompile = @driver.freshness + if newcompile - @compile_time.to_i < 1 return true else + Puppet.debug "Server compile time is %s vs %s" % [newcompile, @compile_time] return false end end @@ -174,12 +174,15 @@ class Puppet::Network::Client::Master < Puppet::Network::Client if self.objects or FileTest.exists?(self.cachefile) if self.fresh?(facts) Puppet.info "Config is up to date" - begin - @objects = YAML.load(self.retrievecache).to_type - rescue => detail - Puppet.warning "Could not load cached configuration: %s" % detail + unless self.objects + oldtext = self.retrievecache + begin + @objects = YAML.load(oldtext).to_type + rescue => detail + Puppet.warning "Could not load cached configuration: %s" % detail + end + return end - return end end Puppet.debug("getting config") @@ -513,6 +516,26 @@ class Puppet::Network::Client::Master < Puppet::Network::Client loadfacts() private + + # Have the facts changed since we last compiled? + def facts_changed?(facts) + oldfacts = Puppet::Util::Storage.cache(:configuration)[:facts] + newfacts = facts + if oldfacts == newfacts + return false + else +# unless oldfacts +# puts "no old facts" +# return true +# end +# newfacts.keys.each do |k| +# unless newfacts[k] == oldfacts[k] +# puts "%s: %s vs %s" % [k, newfacts[k], oldfacts[k]] +# end +# end + return true + end + end # Actually retrieve the configuration, either from the server or from a # local master. diff --git a/lib/puppet/network/handler/filebucket.rb b/lib/puppet/network/handler/filebucket.rb index 96b9a1e9a..f0d389aea 100755 --- a/lib/puppet/network/handler/filebucket.rb +++ b/lib/puppet/network/handler/filebucket.rb @@ -98,9 +98,11 @@ class Puppet::Network::Handler # :nodoc: self.info msg # ...then just create the file - File.open(bfile, File::WRONLY|File::CREAT, 0440) { |of| - of.print contents - } + Puppet::Util.withumask(0007) do + File.open(bfile, File::WRONLY|File::CREAT, 0440) { |of| + of.print contents + } + end # Write the path to the paths file. add_path(path, pathpath) @@ -132,6 +134,10 @@ class Puppet::Network::Handler # :nodoc: end end + def paths(md5) + self.class(@path, md5) + end + def to_s self.name end diff --git a/lib/puppet/provider/cron/crontab.rb b/lib/puppet/provider/cron/crontab.rb index b7caa4a21..bd03c21e3 100755 --- a/lib/puppet/provider/cron/crontab.rb +++ b/lib/puppet/provider/cron/crontab.rb @@ -114,10 +114,11 @@ Puppet::Type.type(:cron).provide(:crontab, break end - # FIXME It'd be great if I could somehow reuse how the - # fields are turned into text, but.... + # Yay differing definitions of absent. next if (hash[field] == :absent and obj.value(field) == "*") - next if (hash[field].join(",") == obj.value(field)) + + # Everything should be in the form of arrays, not the normal text. + next if (hash[field] == obj.value(field)) Puppet.info "Did not match %s: %s vs %s" % [field, obj.value(field).inspect, hash[field].inspect] matched = false diff --git a/lib/puppet/provider/package/apt.rb b/lib/puppet/provider/package/apt.rb index d19276cc1..034a406f1 100755 --- a/lib/puppet/provider/package/apt.rb +++ b/lib/puppet/provider/package/apt.rb @@ -69,7 +69,7 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg do end end - cmd << 'install' << str + cmd << :install << str aptget(*cmd) end diff --git a/lib/puppet/provider/package/aptitude.rb b/lib/puppet/provider/package/aptitude.rb index 240c3fd82..c4f8023d2 100755 --- a/lib/puppet/provider/package/aptitude.rb +++ b/lib/puppet/provider/package/aptitude.rb @@ -15,7 +15,7 @@ Puppet::Type.type(:package).provide :aptitude, :parent => :apt do output = aptitude(*args) # Yay, stupid aptitude doesn't throw an error when the package is missing. - if args.include?(:install) and output =~ /0 newly installed/ + if args.include?(:install) and output =~ /Couldn't find any package/ raise Puppet::Error.new( "Could not find package %s" % self.name ) @@ -24,7 +24,7 @@ Puppet::Type.type(:package).provide :aptitude, :parent => :apt do def purge aptitude '-y', 'purge', @model[:name] - end + end end # $Id$ diff --git a/test/network/client/master.rb b/test/network/client/master.rb index 221c2409f..c9f785580 100755 --- a/test/network/client/master.rb +++ b/test/network/client/master.rb @@ -595,8 +595,10 @@ end facts = master.class.facts assert_equal("two", Facter.value(:testfact), "fact did not change") - assert(master.facts_changed?(facts), "master does not think facts changed") - assert(! master.fresh?(facts), "master is considered fresh after facts changed") + assert(master.send(:facts_changed?, facts), + "master does not think facts changed") + assert(! master.fresh?(facts), + "master is considered fresh after facts changed") assert_nothing_raised("Could not recompile when facts changed") do master.getconfig diff --git a/test/network/handler/master.rb b/test/network/handler/master.rb index eb3db0959..7e1a32396 100755 --- a/test/network/handler/master.rb +++ b/test/network/handler/master.rb @@ -97,6 +97,10 @@ class TestMaster < Test::Unit::TestCase Puppet[:filetimeout] = 15 manifest = mktestmanifest() + facts = Puppet::Network::Client.master.facts + # Store them, so we don't determine frshness based on facts. + Puppet::Util::Storage.cache(:configuration)[:facts] = facts + file2 = @createdfile + "2" @@tmpfiles << file2 @@ -117,7 +121,8 @@ class TestMaster < Test::Unit::TestCase 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?, "Client is incorrectly up to date") + assert(! client.fresh?(facts), + "Client is incorrectly up to date") Puppet.config.use(:puppet) assert_nothing_raised { @@ -126,7 +131,7 @@ class TestMaster < Test::Unit::TestCase } # Now it should be up to date - assert(client.fresh?, "Client is not up to date") + assert(client.fresh?(facts), "Client is not up to date") # Cache this value for later parse1 = master.freshness @@ -145,7 +150,7 @@ 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, "Master did not wait through timeout") - assert(client.fresh?, "Client is not up to date") + assert(client.fresh?(facts), "Client is not up to date") # Then eliminate it Puppet[:filetimeout] = 0 @@ -153,14 +158,14 @@ class TestMaster < Test::Unit::TestCase # Now make sure the master does reparse #Puppet.notice "%s vs %s" % [parse1, master.freshness] assert(parse1 != master.freshness, "Master did not reparse file") - assert(! client.fresh?, "Client is incorrectly up to date") + assert(! client.fresh?(facts), "Client is incorrectly up to date") # Retrieve and apply the new config assert_nothing_raised { client.getconfig client.apply } - assert(client.fresh?, "Client is not up to date") + assert(client.fresh?(facts), "Client is not up to date") assert(FileTest.exists?(file2), "Second file %s does not exist" % file2) end diff --git a/test/network/handler/runner.rb b/test/network/handler/runner.rb index 5c3acde2b..b6df9bab9 100755 --- a/test/network/handler/runner.rb +++ b/test/network/handler/runner.rb @@ -36,10 +36,13 @@ class TestHandlerRunner < Test::Unit::TestCase # Okay, make our manifest file = tempfile() created = tempfile() + # We specify the schedule here, because I was having problems with + # using default schedules. File.open(file, "w") do |f| f.puts %{ class yayness { - file { "#{created}": ensure => file, schedule => weekly } + schedule { "yayness": period => weekly } + file { "#{created}": ensure => file, schedule => yayness } } include yayness diff --git a/test/ral/manager/attributes.rb b/test/ral/manager/attributes.rb index f27b0513a..43d64f367 100755 --- a/test/ral/manager/attributes.rb +++ b/test/ral/manager/attributes.rb @@ -11,14 +11,9 @@ class TestTypeAttributes < Test::Unit::TestCase include PuppetTest def mktype - Puppet::Type.newtype(:faketype) {} - end - - def teardown - super - if Puppet::Type.type(:faketype) - Puppet::Type.rmtype(:faketype) - end + type = Puppet::Type.newtype(:faketype) {} + cleanup { Puppet::Type.rmtype(:faketype) } + type end def test_bracket_methods @@ -227,6 +222,28 @@ class TestTypeAttributes < Test::Unit::TestCase assert_equal(val, inst.value(old), "Incorrect orig value for %s" % old) end end + + # Make sure eachattr is called in the parameter order. + def test_eachattr + type = mktype + name = type.newparam(:name) {} + one = type.newparam(:one) {} + two = type.newproperty(:two) {} + type.provide(:testing) {} + provider = type.attrclass(:provider) + should = [[name, :param], [provider, :param], [two, :property], [one, :param]] + + assert_nothing_raised do + result = nil + type.eachattr do |obj, name| + result = [obj, name] + shouldary = should.shift + assert_equal(shouldary, result, "Did not get correct parameter") + break if should.empty? + end + end + assert(should.empty?, "Did not get all of the parameters.") + end end # $Id$ diff --git a/test/ral/types/cron.rb b/test/ral/types/cron.rb index ce9631d97..941f8d072 100755 --- a/test/ral/types/cron.rb +++ b/test/ral/types/cron.rb @@ -426,12 +426,16 @@ class TestCron < Test::Unit::TestCase crons = [] users = ["root", nonrootuser.name] users.each do |user| - crons << Puppet::Type.type(:cron).create( + cron = Puppet::Type.type(:cron).create( :name => "testcron-#{user}", :user => user, :command => "/bin/echo", :minute => [0,30] ) + crons << cron + + assert_equal(cron.should(:user), cron.should(:target), + "Target was not set correctly for %s" % user) end provider = crons[0].provider.class diff --git a/test/ral/types/file.rb b/test/ral/types/file.rb index caf01fecd..9f7861787 100755 --- a/test/ral/types/file.rb +++ b/test/ral/types/file.rb @@ -1131,9 +1131,10 @@ class TestFile < Test::Unit::TestCase obj[:content] = "New content" assert_apply(obj) - bucketedpath = File.join(bpath, "18cc17fa3047fcc691fdf49c0a7f539a", "contents") + md5 = "18cc17fa3047fcc691fdf49c0a7f539a" + dir, file, pathfile = Puppet::Network::Handler.filebucket.paths(bpath, md5) - assert_equal(0440, filemode(bucketedpath)) + assert_equal(0440, filemode(file)) end def test_largefilechanges diff --git a/test/ral/types/filebucket.rb b/test/ral/types/filebucket.rb index 32b8a0bd5..5b0093425 100755 --- a/test/ral/types/filebucket.rb +++ b/test/ral/types/filebucket.rb @@ -82,7 +82,9 @@ class TestFileBucket < Test::Unit::TestCase assert(md5) - assert(FileTest.directory?(File.join(bucketpath, md5)), + dir, file, pathfile = Puppet::Network::Handler.filebucket.paths(bucketpath, md5) + + assert(FileTest.directory?(dir), "MD5 directory does not exist") newmd5 = nil