diff options
42 files changed, 416 insertions, 310 deletions
@@ -1,3 +1,18 @@ + Significantly reworked the type => provider interface with respect to + listing existing provider instances. The class method on both + class heirarchies has been renamed to 'instances', to start. Providers + are now expected to return provider instances, instead of creating + resources, and the resource's 'instances' method is expected to + find the matching resource, if any, and set the resource's + provider appropriately. This *significantly* reduces the reliance on + effectively global state (resource references in the resource classes). + This global state will go away soon. + + Along with this change, the 'prefetch' class method on providers now + accepts the list of resources for prefetching. This again reduces + reliance on global state, and makes the execution path much easier + to follow. + Fixed #532 -- reparsing config files now longer throws an exception. Added some warnings and logs to the service type so diff --git a/lib/puppet/metatype/instances.rb b/lib/puppet/metatype/instances.rb index 2d408f750..be7d1be1c 100644 --- a/lib/puppet/metatype/instances.rb +++ b/lib/puppet/metatype/instances.rb @@ -248,6 +248,24 @@ class Puppet::Type return trans end + # Retrieve all known instances. Either requires providers or must be overridden. + def self.instances + unless defined?(@providers) and ! @providers.empty? + raise Puppet::DevError, "%s has no providers and has not overridden 'instances'" % self.name + end + + # Put the default provider first, then the rest of the suitable providers. + sources = [] + [defaultprovider, suitableprovider].flatten.uniq.collect do |provider| + next if sources.include?(provider.source) + + sources << provider.source + provider.instances.collect do |instance| + create(:name => instance.name, :provider => instance, :check => :all) + end + end.flatten.compact + end + # Create the path for logging and such. def pathbuilder if defined? @parent and @parent diff --git a/lib/puppet/metatype/providers.rb b/lib/puppet/metatype/providers.rb index 33506e06f..72459d3af 100644 --- a/lib/puppet/metatype/providers.rb +++ b/lib/puppet/metatype/providers.rb @@ -86,26 +86,6 @@ class Puppet::Type return obj end - # Create a list method that just calls our providers. - def self.mkprovider_list - unless respond_to?(:list) - meta_def(:list) do - suitableprovider.find_all { |p| p.respond_to?(:list) }.collect { |prov| - prov.list.each { |h| h[:provider] = prov.name } - }.flatten.collect do |hash| - if hash.is_a?(Hash) - hash2obj(hash) - elsif hash.is_a?(self) - hash - else - raise Puppet::DevError, "Provider %s returned object of type %s in list" % - [prov.name, hash.class] - end - end - end - end - end - # Retrieve a provider by name. def self.provider(name) name = Puppet::Util.symbolize(name) @@ -172,7 +152,6 @@ class Puppet::Type def self.providify return if @paramhash.has_key? :provider - mkprovider_list() newparam(:provider) do desc "The specific backend for #{self.name.to_s} to use. You will seldom need to specify this -- Puppet will usually discover the @@ -197,17 +176,21 @@ class Puppet::Type @resource.class.defaultprovider.name } - validate do |value| - value = value[0] if value.is_a? Array - if provider = @resource.class.provider(value) + validate do |provider_class| + provider_class = provider_class[0] if provider_class.is_a? Array + if provider_class.is_a?(Puppet::Provider) + provider_class = provider_class.class.name + end + + if provider = @resource.class.provider(provider_class) unless provider.suitable? raise ArgumentError, "Provider '%s' is not functional on this platform" % - [value] + [provider_class] end else raise ArgumentError, "Invalid %s provider '%s'" % - [@resource.class.name, value] + [@resource.class.name, provider_class] end end @@ -244,10 +227,13 @@ class Puppet::Type end def provider=(name) - if klass = self.class.provider(name) + if name.is_a?(Puppet::Provider) + @provider = name + @provider.resource = self + elsif klass = self.class.provider(name) @provider = klass.new(self) else - raise UnknownProviderError, "Could not find %s provider of %s" % + raise ArgumentError, "Could not find %s provider of %s" % [name, self.class.name] end end diff --git a/lib/puppet/network/handler/resource.rb b/lib/puppet/network/handler/resource.rb index 349f73537..ca492bd81 100755 --- a/lib/puppet/network/handler/resource.rb +++ b/lib/puppet/network/handler/resource.rb @@ -145,7 +145,7 @@ class Puppet::Network::Handler bucket = Puppet::TransBucket.new bucket.type = typeklass.name - typeklass.list.each do |obj| + typeklass.instances.each do |obj| next if ignore.include? obj.name #object = Puppet::TransObject.new(obj.name, typeklass.name) diff --git a/lib/puppet/provider.rb b/lib/puppet/provider.rb index 1f18ae730..a725611d0 100644 --- a/lib/puppet/provider.rb +++ b/lib/puppet/provider.rb @@ -109,6 +109,12 @@ class Puppet::Provider end end + # The method for returning a list of provider instances. Note that it returns providers, preferably with values already + # filled in, not resources. + def self.instances + raise Puppet::DevError, "Provider %s has not defined the 'instances' class method" % self.name + end + # Create the methods for a given command. def self.make_command_methods(name) # Now define a method for that command @@ -323,16 +329,34 @@ class Puppet::Provider self.class.command(name) end - def initialize(resource) - @resource = resource - - # LAK 2007-05-09: Keep the model stuff around for backward compatibility - @model = resource - @property_hash = {} + def initialize(resource = nil) + if resource.is_a?(Hash) + @property_hash = resource.dup + elsif resource + @resource = resource if resource + # LAK 2007-05-09: Keep the model stuff around for backward compatibility + @model = resource + @property_hash = {} + else + @property_hash = {} + end end def name - @resource.name + if n = @property_hash[:name] + return n + elsif self.resource + resource.name + else + raise Puppet::DevError, "No resource and no name in property hash" + end + end + + # Set passed params as the current values. + def set(params) + params.each do |param, value| + @property_hash[symbolize(param)] = value + end end def to_s diff --git a/lib/puppet/provider/nameservice/netinfo.rb b/lib/puppet/provider/nameservice/netinfo.rb index c44f105e8..0323807e8 100644 --- a/lib/puppet/provider/nameservice/netinfo.rb +++ b/lib/puppet/provider/nameservice/netinfo.rb @@ -45,6 +45,12 @@ class NetInfo < Puppet::Provider::NameService end end + def self.instances + report(@resource_type.validproperties).collect do |hash| + self.new(hash) + end + end + # Convert a NetInfo line into a hash of data. def self.line2hash(line, params) values = line.split(/\t/) @@ -61,12 +67,6 @@ class NetInfo < Puppet::Provider::NameService hash end - def self.list - report(@resource_type.validproperties).collect do |hash| - @resource_type.hash2obj(hash) - end - end - # What field the value is stored under. def self.netinfokey(name) name = symbolize(name) diff --git a/lib/puppet/provider/package/apple.rb b/lib/puppet/provider/package/apple.rb index ce845670f..e9fb00482 100755 --- a/lib/puppet/provider/package/apple.rb +++ b/lib/puppet/provider/package/apple.rb @@ -11,7 +11,17 @@ Puppet::Type.type(:package).provide :apple do defaultfor :operatingsystem => :darwin - def self.listbyname + def self.instances + instance_by_name.collect do |name| + self.new( + :name => name, + :provider => :apple, + :ensure => :installed + ) + end + end + + def self.instance_by_name Dir.entries("/Library/Receipts").find_all { |f| f =~ /\.pkg$/ }.collect { |f| @@ -22,16 +32,6 @@ Puppet::Type.type(:package).provide :apple do } end - def self.list - listbyname.collect do |name| - Puppet.type(:package).installedpkg( - :name => name, - :provider => :apple, - :ensure => :installed - ) - end - end - def query if FileTest.exists?("/Library/Receipts/#{@resource[:name]}.pkg") return {:name => @resource[:name], :ensure => :present} diff --git a/lib/puppet/provider/package/blastwave.rb b/lib/puppet/provider/package/blastwave.rb index 9f544dbd5..07f904eef 100755 --- a/lib/puppet/provider/package/blastwave.rb +++ b/lib/puppet/provider/package/blastwave.rb @@ -23,10 +23,10 @@ Puppet::Type.type(:package).provide :blastwave, :parent => :sun do end end - def self.list(hash = {}) + def self.instances(hash = {}) blastlist(hash).collect do |bhash| bhash.delete(:avail) - Puppet::Type.type(:package).installedpkg(bhash) + new(bhash) end end diff --git a/lib/puppet/provider/package/darwinport.rb b/lib/puppet/provider/package/darwinport.rb index 70e6ed388..8c0bbb891 100755 --- a/lib/puppet/provider/package/darwinport.rb +++ b/lib/puppet/provider/package/darwinport.rb @@ -31,12 +31,11 @@ Puppet::Type.type(:package).provide :darwinport do } end - def self.list + def self.instances packages = [] eachpkgashash do |hash| - pkg = Puppet.type(:package).installedpkg(hash) - packages << pkg + packages << new(hash) end return packages diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb index d309ae9e5..e09638669 100755 --- a/lib/puppet/provider/package/dpkg.rb +++ b/lib/puppet/provider/package/dpkg.rb @@ -6,7 +6,7 @@ Puppet::Type.type(:package).provide :dpkg do commands :dpkg => "/usr/bin/dpkg" commands :dpkgquery => "/usr/bin/dpkg-query" - def self.list + def self.instances packages = [] # list out all of the packages @@ -29,7 +29,7 @@ Puppet::Type.type(:package).provide :dpkg do hash[:provider] = self.name - packages.push Puppet.type(:package).installedpkg(hash) + packages << new(hash) else Puppet.warning "Failed to match dpkg-query line %s" % line.inspect diff --git a/lib/puppet/provider/package/gem.rb b/lib/puppet/provider/package/gem.rb index c902bcbe5..fb5e243e9 100755 --- a/lib/puppet/provider/package/gem.rb +++ b/lib/puppet/provider/package/gem.rb @@ -56,9 +56,9 @@ Puppet::Type.type(:package).provide :gem do end end - def self.list(justme = false) + def self.instances(justme = false) gemlist(:local => true).collect do |hash| - Puppet::Type.type(:package).installedpkg(hash) + new(hash) end end diff --git a/lib/puppet/provider/package/openbsd.rb b/lib/puppet/provider/package/openbsd.rb index e910a41ba..0e238f546 100755 --- a/lib/puppet/provider/package/openbsd.rb +++ b/lib/puppet/provider/package/openbsd.rb @@ -7,7 +7,7 @@ Puppet::Type.type(:package).provide :openbsd do defaultfor :operatingsystem => :openbsd confine :operatingsystem => :openbsd - def self.list + def self.instances packages = [] begin @@ -29,8 +29,7 @@ Puppet::Type.type(:package).provide :openbsd do hash[:provider] = self.name - pkg = Puppet.type(:package).installedpkg(hash) - packages << pkg + packages << new(hash) else # Print a warning on lines we can't match, but move # on, since it should be non-fatal @@ -38,12 +37,6 @@ Puppet::Type.type(:package).provide :openbsd do end } end - # Mark as absent any packages we didn't find - Puppet.type(:package).each do |pkg| - unless packages.include? pkg - pkg.is = [:ensure, :absent] - end - end return packages rescue Puppet::ExecutionFailure diff --git a/lib/puppet/provider/package/pkgdmg.rb b/lib/puppet/provider/package/pkgdmg.rb index e3d211fef..e0864764a 100644 --- a/lib/puppet/provider/package/pkgdmg.rb +++ b/lib/puppet/provider/package/pkgdmg.rb @@ -23,7 +23,7 @@ Puppet::Type.type(:package).provide :pkgdmg do commands :curl => "/usr/bin/curl" # JJM We store a cookie for each installed .pkg.dmg in /var/db - def self.listbyname + def self.instance_by_name Dir.entries("/var/db").find_all { |f| f =~ /^\.puppet_pkgdmg_installed_/ }.collect do |f| @@ -33,9 +33,9 @@ Puppet::Type.type(:package).provide :pkgdmg do end end - def self.list - listbyname.collect do |name| - Puppet.type(:package).installedpkg( + def self.instances + instance_by_name.collect do |name| + new( :name => name, :provider => :pkgdmg, :ensure => :installed diff --git a/lib/puppet/provider/package/portage.rb b/lib/puppet/provider/package/portage.rb index dd69f23a4..3b4c1a44f 100644 --- a/lib/puppet/provider/package/portage.rb +++ b/lib/puppet/provider/package/portage.rb @@ -7,7 +7,7 @@ Puppet::Type.type(:package).provide :portage do defaultfor :operatingsystem => :gentoo - def self.list + def self.instances result_format = /(\S+) (\S+) \[(.*)\] \[[^0-9]*([^\s:]*)(:\S*)?\] ([\S]*) (.*)/ result_fields = [:category, :name, :ensure, :version_available, :slot, :vendor, :description] @@ -28,7 +28,7 @@ Puppet::Type.type(:package).provide :portage do package[:provider] = :portage package[:ensure] = package[:ensure].split.last - packages.push(Puppet.type(:package).installedpkg(package)) + packages << new(package) end end diff --git a/lib/puppet/provider/package/rpm.rb b/lib/puppet/provider/package/rpm.rb index 13f2589d7..3b9ac8b15 100755 --- a/lib/puppet/provider/package/rpm.rb +++ b/lib/puppet/provider/package/rpm.rb @@ -10,7 +10,7 @@ Puppet::Type.type(:package).provide :rpm do commands :rpm => "rpm" - def self.list + def self.instances packages = [] # list out all of the packages @@ -30,7 +30,7 @@ Puppet::Type.type(:package).provide :rpm do hash[field] = value } hash[:provider] = self.name - packages.push Puppet.type(:package).installedpkg(hash) + packages << new(hash) else raise "failed to match rpm line %s" % line end diff --git a/lib/puppet/provider/package/sun.rb b/lib/puppet/provider/package/sun.rb index ea169548d..c5b954881 100755 --- a/lib/puppet/provider/package/sun.rb +++ b/lib/puppet/provider/package/sun.rb @@ -9,7 +9,7 @@ Puppet::Type.type(:package).provide :sun do defaultfor :operatingsystem => :solaris - def self.list + def self.instances packages = [] hash = {} names = { @@ -40,7 +40,7 @@ Puppet::Type.type(:package).provide :sun do when /^$/: hash[:provider] = :sun - packages.push Puppet.type(:package).installedpkg(hash) + packages << new(hash) hash.clear when /\s*(\w+):\s+(.+)/: name = $1 diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb index 09233069b..43c53601e 100755 --- a/lib/puppet/provider/parsedfile.rb +++ b/lib/puppet/provider/parsedfile.rb @@ -107,17 +107,13 @@ class Puppet::Provider::ParsedFile < Puppet::Provider end # Return a list of all of the records we can find. - def self.list + def self.instances prefetch() @records.find_all { |r| r[:record_type] == self.name }.collect { |r| - clean(r) + new(clean(r)) } end - def self.list_by_name - list.collect { |r| r[:name] } - end - # Override the default method with a lot more functionality. def self.mk_resource_methods [resource_type.validproperties, resource_type.parameters].flatten.each do |attr| @@ -175,11 +171,24 @@ class Puppet::Provider::ParsedFile < Puppet::Provider # set. We need to turn those three locations into a list of files, # prefetch each one, and make sure they're associated with each appropriate # resource instance. - def self.prefetch + def self.prefetch(resources = nil) # Reset the record list. @records = [] - targets().each do |target| - prefetch_target(target) + targets(resources).each do |target| + @records += prefetch_target(target) + end + + if resources + @records.each do |record| + if name = record[:name] and resource = resources[name] + resource.provider = new(record) + elsif respond_to?(:match) + if instance = match(record, resources) + record[:name] = instance[:name] + instance.provider = new(record) + end + end + end end end @@ -196,25 +205,10 @@ class Puppet::Provider::ParsedFile < Puppet::Provider end unless target_records - raise Puppet::DevError, "Prefetch hook for provider %s returned nil" % - self.name + raise Puppet::DevError, "Prefetching %s for provider %s returned nil" % [target, self.name] end - @records += target_records - - # Set current property on any existing resource instances. - target_records(target).find_all { |i| i.is_a?(Hash) }.each do |record| - # Find any resource instances whose names match our instances. - if instance = self.resource_type[record[:name]] - next unless instance.provider.is_a?(self) - instance.provider.property_hash = record - elsif respond_to?(:match) - if instance = match(record) - record[:name] = instance[:name] - instance.provider.property_hash = record - end - end - end + target_records end # Is there an existing record with this name? @@ -256,7 +250,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider # Find a list of all of the targets that we should be reading. This is # used to figure out what targets we need to prefetch. - def self.targets + def self.targets(resources = nil) targets = [] # First get the default target unless self.default_target @@ -268,17 +262,13 @@ class Puppet::Provider::ParsedFile < Puppet::Provider targets += @target_objects.keys # Lastly, check the file from any resource instances - self.resource_type.each do |resource| - targets << resource.value(:target) - - # This is only the case for properties, and targets should always - # be properties. - #if resource.respond_to?(:is) - # targets << resource.is(:target) - #end + if resources + resources.each do |name, resource| + targets << resource.value(:target) + end end - targets.uniq.reject { |t| t.nil? } + targets.uniq.compact end def self.to_file(records) diff --git a/lib/puppet/provider/service/init.rb b/lib/puppet/provider/service/init.rb index c5bcc26eb..79e5a2c69 100755 --- a/lib/puppet/provider/service/init.rb +++ b/lib/puppet/provider/service/init.rb @@ -29,7 +29,7 @@ Puppet::Type.type(:service).provide :init, :parent => :base do end # List all services of this type. - def self.list(name) + def self.instances(name) # We need to find all paths specified for our type or any parent types paths = Puppet.type(:service).paths(name) @@ -44,7 +44,7 @@ Puppet::Type.type(:service).provide :init, :parent => :base do end end - paths.each do |path| + paths.collect do |path| unless FileTest.directory?(path) Puppet.notice "Service path %s does not exist" % path next @@ -59,14 +59,8 @@ Puppet::Type.type(:service).provide :init, :parent => :base do Dir.entries(path).reject { |e| fullpath = File.join(path, e) e =~ /^\./ or ! FileTest.executable?(fullpath) - }.each do |name| - if obj = Puppet::Type.type(:service)[name] - obj[:check] = check - else - Puppet::Type.type(:service).create( - :name => name, :check => check, :path => path - ) - end + }.collect do |name| + new(:name => name, :path => path) end end end diff --git a/lib/puppet/provider/zone/solaris.rb b/lib/puppet/provider/zone/solaris.rb index 7ace69047..d178679fe 100644 --- a/lib/puppet/provider/zone/solaris.rb +++ b/lib/puppet/provider/zone/solaris.rb @@ -21,18 +21,11 @@ Puppet::Type.type(:zone).provide(:solaris) do return hash end - def self.list + def self.instances adm(:list, "-cp").split("\n").collect do |line| hash = line2hash(line) - obj = nil - unless obj = @resource[hash[:name]] - obj = @resource.create(:name => hash[:name]) - end - - obj.setstatus(hash) - - obj + new(hash) end end diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index 7c046c898..d27f4038a 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -473,16 +473,20 @@ class Transaction # Prefetch any providers that support it. We don't support prefetching # types, just providers. def prefetch - @resources.collect { |obj| - if pro = obj.provider - pro.class - else - nil + prefetchers = {} + @resources.each do |resource| + if provider = resource.provider and provider.class.respond_to?(:prefetch) + prefetchers[provider.class] ||= {} + prefetchers[provider.class][resource.title] = resource end - }.reject { |o| o.nil? }.uniq.each do |klass| - # XXX We need to do something special here in case of failure. - if klass.respond_to?(:prefetch) - klass.prefetch + end + + # Now call prefetch, passing in the resources so that the provider instances can be replaced. + prefetchers.each do |provider, resources| + begin + provider.prefetch(resources) + rescue => detail + Puppet.err "Could not prefetch % provider %s: %s" % [resources[0].class.name, provider.name, detail] end end end diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb index e5071b484..dabd98830 100755 --- a/lib/puppet/type/exec.rb +++ b/lib/puppet/type/exec.rb @@ -444,8 +444,8 @@ module Puppet end end - def self.list - self.collect { |i| i } + def self.instances + [] end # Verify that we pass all of the checks. The argument determines whether diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb index 5e82140e7..8ca687904 100755 --- a/lib/puppet/type/group.rb +++ b/lib/puppet/type/group.rb @@ -120,22 +120,6 @@ module Puppet defaultto false end - def self.list_by_name - users = [] - defaultprovider.listbyname do |user| - users << user - end - return users - end - - def self.list - defaultprovider.list - - self.collect do |user| - user - end - end - def retrieve if self.provider and @provider.exists? return super diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb index 2c3cdc41d..d116238a1 100644 --- a/lib/puppet/type/package.rb +++ b/lib/puppet/type/package.rb @@ -361,23 +361,6 @@ module Puppet return object end - # List all package instances - def self.list - # XXX For now, just list the default provider, but we should list - # all suitables or something, but we need to not list a parent - # type if a child type gets listed. - #suitableprovider.each do |provider| - # p provider.name - # provider.list - #end - - defaultprovider.list - - self.collect do |pkg| - pkg - end - end - # Iterate across all packages of a given type and mark them absent # if they are not in the list def self.markabsent(pkgtype, packages) diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb index bec0a4dd2..3a07a9342 100644 --- a/lib/puppet/type/pfile.rb +++ b/lib/puppet/type/pfile.rb @@ -259,7 +259,7 @@ module Puppet end # List files, but only one level deep. - def self.list(base = "/") + def self.instances(base = "/") unless FileTest.directory?(base) return [] end diff --git a/lib/puppet/type/pfilebucket.rb b/lib/puppet/type/pfilebucket.rb index ec1803ff8..fd0f131a1 100755 --- a/lib/puppet/type/pfilebucket.rb +++ b/lib/puppet/type/pfilebucket.rb @@ -70,8 +70,8 @@ module Puppet end end - def self.list - self.collect do |obj| obj.name end + def self.instances + [] end def bucket diff --git a/lib/puppet/type/resources.rb b/lib/puppet/type/resources.rb index ca88a7c31..39fa1bc34 100644 --- a/lib/puppet/type/resources.rb +++ b/lib/puppet/type/resources.rb @@ -31,8 +31,8 @@ Puppet::Type.newtype(:resources) do validate do |value| if [:true, true, "true"].include?(value) - unless @resource.resource_type.respond_to?(:list) - raise ArgumentError, "Purging resources of type %s is not supported, since they cannot be listed" % @resource[:name] + unless @resource.resource_type.respond_to?(:instances) + raise ArgumentError, "Purging resources of type %s is not supported, since they cannot be queried from the system" % @resource[:name] end unless @resource.resource_type.validproperty?(:ensure) raise ArgumentError, "Purging is only supported on types that accept 'ensure'" @@ -91,7 +91,7 @@ Puppet::Type.newtype(:resources) do return [] unless self.purge? hascheck = false method = - resource_type.list.find_all do |resource| + resource_type.instances.find_all do |resource| ! resource.managed? end.find_all do |resource| check(resource) diff --git a/lib/puppet/type/schedule.rb b/lib/puppet/type/schedule.rb index 08bada3a7..2e3c126e4 100755 --- a/lib/puppet/type/schedule.rb +++ b/lib/puppet/type/schedule.rb @@ -307,8 +307,8 @@ module Puppet end end - def self.list - self.collect do |obj| obj end + def self.instances + [] end def self.mkdefaultschedules diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb index c582b17ee..2df311633 100644 --- a/lib/puppet/type/service.rb +++ b/lib/puppet/type/service.rb @@ -265,20 +265,6 @@ module Puppet newvalues(:true, :false) end - # List all available services - def self.list - defprov = defaultprovider - - names = [] - if defprov.respond_to? :list - defprov.list(defprov.name) - else - Puppet.debug "Type %s does not respond to list" % defprov.name - end - - self.collect { |s| s } - end - # Add a new path to our list of paths that services could be in. def self.newpath(type, path) type = type.intern if type.is_a? String diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb index 8d9cda170..75a68170d 100755 --- a/lib/puppet/type/tidy.rb +++ b/lib/puppet/type/tidy.rb @@ -254,8 +254,8 @@ module Puppet validate do end - def self.list - self.collect { |t| t } + def self.instances + [] end @depthfirst = true diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index 87a57cddd..a9634b6f7 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -322,22 +322,6 @@ module Puppet autos end - def self.list_by_name - users = [] - defaultprovider.listbyname do |user| - users << user - end - return users - end - - def self.list - defaultprovider.list - - self.collect do |user| - user - end - end - def retrieve absent = false properties().inject({}) { |prophash, property| diff --git a/lib/puppet/type/yumrepo.rb b/lib/puppet/type/yumrepo.rb index 395a33068..5efb3a307 100644 --- a/lib/puppet/type/yumrepo.rb +++ b/lib/puppet/type/yumrepo.rb @@ -80,7 +80,7 @@ module Puppet # Where to put files for brand new sections @defaultrepodir = nil - def self.list + def self.instances l = [] check = validproperties clear diff --git a/test/network/handler/resource.rb b/test/network/handler/resource.rb index 8d1fa1087..589d22d83 100755 --- a/test/network/handler/resource.rb +++ b/test/network/handler/resource.rb @@ -183,8 +183,8 @@ class TestResourceServer < Test::Unit::TestCase Puppet::Type.type(:schedule).mkdefaultschedules Puppet::Type.eachtype do |type| - unless type.respond_to? :list - Puppet.warning "%s does not respond to :list" % type.name + unless type.respond_to? :instances + Puppet.warning "%s does not respond to :instances" % type.name next end next unless type.name == :package @@ -212,7 +212,7 @@ class TestResourceServer < Test::Unit::TestCase count = 0 described = {} Puppet.info "listing again" - type.list.each do |obj| + type.instances.each do |obj| assert_instance_of(type, obj) break if count > 5 diff --git a/test/other/transactions.rb b/test/other/transactions.rb index 1e0b6377d..4cb65dbe3 100755 --- a/test/other/transactions.rb +++ b/test/other/transactions.rb @@ -124,8 +124,8 @@ class TestTransactions < Test::Unit::TestCase # Now create a provider type.provide(:prefetch) do - def self.prefetch - $prefetched = true + def self.prefetch(resources) + $prefetched = resources end end @@ -140,7 +140,7 @@ class TestTransactions < Test::Unit::TestCase trans.prefetch end - assert_equal(true, $prefetched, "type prefetch was not called") + assert_equal({inst.title => inst}, $prefetched, "type prefetch was not called") # Now make sure it gets called from within evaluate() $prefetched = false @@ -148,7 +148,7 @@ class TestTransactions < Test::Unit::TestCase trans.evaluate end - assert_equal(true, $prefetched, "evaluate did not call prefetch") + assert_equal({inst.title => inst}, $prefetched, "evaluate did not call prefetch") end def test_refreshes_generate_events diff --git a/test/ral/manager/instances.rb b/test/ral/manager/instances.rb new file mode 100755 index 000000000..142b85c5c --- /dev/null +++ b/test/ral/manager/instances.rb @@ -0,0 +1,82 @@ +#!/usr/bin/env ruby +# +# Created by Luke A. Kanies on 2007-06-10. +# Copyright (c) 2007. All rights reserved. + +$:.unshift("../../lib") if __FILE__ =~ /\.rb$/ + +require 'puppettest' + +class TestTypeInstances < Test::Unit::TestCase + include PuppetTest + + def setup + super + @type = Puppet::Type.newtype(:instance_test) do + newparam(:name) {} + ensurable + end + cleanup { Puppet::Type.rmtype(:instance_test) } + end + + # Make sure the instances class method works as expected. + def test_instances + # First make sure it throws an error when there are no providers + assert_raise(Puppet::DevError, "Did not fail when no providers are present") do + @type.instances + end + + # Now add a couple of providers + + # The default + @type.provide(:default) do + defaultfor :operatingsystem => Facter.value(:operatingsystem) + mk_resource_methods + class << self + attr_accessor :names + end + def self.instance(name) + new(:name => name, :ensure => :present) + end + def self.instances + @instances ||= names.collect { |name| instance(name) } + @instances + end + + @names = [:one] + end + + # A provider with the same source + @type.provide(:sub, :source => :default, :parent => :default) do + @names = [:two] + end + + # An unsuitable provider + @type.provide(:nope, :parent => :default) do + confine :exists => "/no/such/file" + @names = [:three] + end + + # Another suitable, non-default provider + @type.provide(:yep, :parent => :default) do + @names = [:four] + end + + result = nil + assert_nothing_raised("Could not get instance list") do + result = @type.instances + end + + assert_equal(:one, result[0].name, "Did not get default instances first") + assert_equal(@type.provider(:default).instances[0], result[0].provider, "Provider instances were not maintained") + + resources = result.inject({}) { |hash, res| hash[res.name] = res; hash } + assert(resources.include?(:four), "Did not get resources from other suitable providers") + assert(! resources.include?(:three), "Got resources from unsuitable providers") + + # Now make sure the resources have an 'ensure' property to go with the value in the provider + assert(resources[:one].send(:instance_variable_get, "@parameters").include?(:ensure), "Did not create ensure property") + end +end + +# $Id$ diff --git a/test/ral/manager/provider.rb b/test/ral/manager/provider.rb index f3a7d719f..a9748e12a 100755 --- a/test/ral/manager/provider.rb +++ b/test/ral/manager/provider.rb @@ -8,47 +8,84 @@ require 'mocha' class TestTypeProviders < Test::Unit::TestCase include PuppetTest - # Make sure default providers behave correctly - def test_defaultproviders - # Make a fake type - type = Puppet::Type.newtype(:defaultprovidertest) do - newparam(:name) do end + def setup + super + @type = Puppet::Type.newtype(:provider_test) do + newparam(:name) {} + ensurable end + cleanup { Puppet::Type.rmtype(:provider_test) } + end - cleanup { Puppet::Type.rmtype(:defaultprovidertest) } - - basic = type.provide(:basic) do + # Make sure default providers behave correctly + def test_defaultproviders + basic = @type.provide(:basic) do defaultfor :operatingsystem => :somethingelse, :operatingsystemrelease => :yayness end - assert_equal(basic, type.defaultprovider) - type.defaultprovider = nil + assert_equal(basic, @type.defaultprovider) + @type.defaultprovider = nil - greater = type.provide(:greater) do + greater = @type.provide(:greater) do defaultfor :operatingsystem => Facter.value("operatingsystem") end - assert_equal(greater, type.defaultprovider) + assert_equal(greater, @type.defaultprovider) end # Make sure the provider is always the first parameter created. def test_provider_sorting - type = Puppet::Type.newtype(:sorttest) do - newparam(:name) {} - ensurable - end - cleanup { Puppet::Type.rmtype(:sorttest) } - should = [:name, :ensure] - assert_equal(should, type.allattrs.reject { |p| ! should.include?(p) }, + assert_equal(should, @type.allattrs.reject { |p| ! should.include?(p) }, "Got wrong order of parameters") - type.provide(:yay) { } + @type.provide(:yay) { } should = [:name, :provider, :ensure] - assert_equal(should, type.allattrs.reject { |p| ! should.include?(p) }, + assert_equal(should, @type.allattrs.reject { |p| ! should.include?(p) }, "Providify did not reorder parameters") end + + # Make sure that provider instances can be passed in directly. + def test_name_or_provider + provider = @type.provide(:testing) do + end + + # first make sure we can pass the name in + resource = nil + assert_nothing_raised("Could not create provider instance by name") do + resource = @type.create :name => "yay", :provider => :testing + end + + assert_instance_of(provider, resource.provider, "Did not create provider instance") + + # Now make sure we can pass in an instance + provinst = provider.new(:name => "foo") + assert_nothing_raised("Could not pass in provider instance") do + resource = @type.create :name => "foo", :provider => provinst + end + + assert_equal(provinst, resource.provider, "Did not retain provider instance") + + # Now make sure unsuitable provider instances still throw errors + provider = @type.provide(:badprov) do + confine :exists => "/no/such/file" + end + + inst = provider.new(:name => "bar") + assert_raise(Puppet::Error, "Did not fail on unsuitable provider instance") do + resource = @type.create :name => "bar", :provider => inst + end + + # And make sure the provider must be a valid provider type for this resource + pkgprov = Puppet::Type.type(:package).create(:name => "yayness").provider + assert(provider, "did not get package provider") + + assert_raise(Puppet::Error, "Did not fail on invalid provider instance") do + resource = @type.create :name => "bar", :provider => pkgprov + end + + end end # $Id$ diff --git a/test/ral/providers/package.rb b/test/ral/providers/package.rb index 345fbb0e6..ab7c0130d 100755 --- a/test/ral/providers/package.rb +++ b/test/ral/providers/package.rb @@ -234,17 +234,17 @@ class TestPackageProvider < Test::Unit::TestCase # Make sure all of the suitable providers on our platform can successfully # list. - def test_listing + def test_instances Puppet::Type.type(:package).suitableprovider.each do |provider| result = nil assert_nothing_raised("Could not list %s packages" % provider.name) do - result = provider.list + result = provider.instances end result.each do |pkg| - assert_instance_of(Puppet::Type.type(:package), pkg, - "%s returned non-package" % provider.name) - assert_equal(provider.name, pkg.provider.class.name, - "%s did not set provider correctly" % provider.name) + assert_instance_of(provider, pkg, + "%s returned non-provider" % provider.name) + assert_equal(provider.name, pkg.class.name, + "Provider %s returned an instance of a different provider" % provider.name) end end end diff --git a/test/ral/providers/parsedfile.rb b/test/ral/providers/parsedfile.rb index efe8105ca..2d9f7ae9d 100755 --- a/test/ral/providers/parsedfile.rb +++ b/test/ral/providers/parsedfile.rb @@ -90,8 +90,9 @@ class TestParsedFile < Test::Unit::TestCase fakeresource = fakeresource(:testparsedfiletype, "yay") file = prov.new(fakeresource) + assert(file, "Could not make provider") - assert_nothing_raised do + assert_nothing_raised("Could not set provider name") do file.name = :yayness end @@ -206,31 +207,30 @@ class TestParsedFile < Test::Unit::TestCase resource = mkresource "bill", :target => :file1 default = mkresource "will", :target => :default + resources = {"bill" => resource, "will" => default} + assert_nothing_raised do - prov.prefetch + prov.prefetch(resources) end # Make sure we prefetched our resources. - assert_equal("b", resource.provider.one) - assert_equal("b", default.provider.one) - assert_equal("d", default.provider.two) + assert_equal("b", resource.provider.one, "did not prefetch resource from file1") + assert_equal("b", default.provider.one, "did not prefetch resource from default") + assert_equal("d", default.provider.two, "did not prefetch resource from default") # Now list all of them and make sure we get everything back - hashes = nil + providers = nil assert_nothing_raised do - hashes = prov.list + providers = prov.instances end - names = nil - assert_nothing_raised do - names = prov.list_by_name + providers.each do |provider| + assert_instance_of(prov, provider, "'instances' class method did not return providers") end %w{bill jill will}.each do |name| - assert(hashes.find { |r| r[:name] == name}, + assert(providers.find { |provider| provider.name == name}, "Did not return %s in list" % name) - assert(names.include?(name), - "Did not return %s in list_by_name" % name) end end @@ -242,25 +242,27 @@ class TestParsedFile < Test::Unit::TestCase target = :yayness prov.target_object(target).write "yay b d" - resource = mkresource "yay", :target => :yayness - + records = nil assert_nothing_raised do - prov.prefetch_target(:yayness) + records = prov.prefetch_target(:yayness) end # Now make sure we correctly got the hash - mprov = resource.provider - assert_equal("b", mprov.one) - assert_equal("d", mprov.two) + record = records.find { |r| r[:name] == "yay" } + assert(record, "Did not get record in prefetch_target") + assert_equal("b", record[:one]) + assert_equal("d", record[:two]) end def test_prefetch_match prov = mkprovider - prov.meta_def(:match) do |record| + prov.meta_def(:match) do |record, resources| # Look for matches on :one - self.resource_type.find do |m| - m.should(:one).to_s == record[:one].to_s + if res = resources.find { |name, resource| resource.should(:one).to_s == record[:one].to_s } + res[1] + else + nil end end @@ -271,7 +273,7 @@ class TestParsedFile < Test::Unit::TestCase resource = mkresource "yay", :target => :yayness, :one => "b" assert_nothing_raised do - prov.prefetch_target(:yayness) + prov.prefetch("yay" => resource) end # Now make sure we correctly got the hash @@ -301,22 +303,35 @@ class TestParsedFile < Test::Unit::TestCase # Lastly, create a resource with separate is and should values mtarget = tempfile() - # istarget = tempfile() files[:resources] = mtarget - # files[:isresources] = istarget resource = mkresource "yay", :target => mtarget - # resource.is = [:target, istarget] assert(resource.should(:target), "Did not get a value for target") - # assert(resource.is(:target), "Did not get a value for target") list = nil + + # First run it without the resource assert_nothing_raised do list = prov.targets end + # Make sure it got the first two, but not the resources file files.each do |name, file| - assert(list.include?(file), "Provider did not find %s file" % name) + if name == :resources + assert(! list.include?(file), "Provider somehow found resource target when no resource was passed") + else + assert(list.include?(file), "Provider did not find %s file" % name) + end + end + + # Now list with the resource passed + assert_nothing_raised do + list = prov.targets("yay" => resource) + end + + # And make sure we get all three files + files.each do |name, file| + assert(list.include?(file), "Provider did not find %s file when resource was passed" % name) end end @@ -331,11 +346,12 @@ class TestParsedFile < Test::Unit::TestCase # Create some resources. one = mkresource "one", :one => "a", :two => "c", :target => :yayness two = mkresource "two", :one => "b", :two => "d", :target => :yayness + resources = {"one" => one, "two" => two} # Write out a file with different data. prov.target_object(:yayness).write "one b d\ntwo a c" - prov.prefetch + prov.prefetch(resources) # Apply and flush the first resource. assert_nothing_raised do @@ -343,7 +359,7 @@ class TestParsedFile < Test::Unit::TestCase end assert_nothing_raised { one.flush } - # Make sure it changed our file + # Make sure it didn't clear out our property hash assert_equal(:a, one.provider.one) assert_equal(:c, one.provider.two) @@ -360,7 +376,7 @@ class TestParsedFile < Test::Unit::TestCase "Wrote out other resource") # Now fetch the data again and make sure we're still right - assert_nothing_raised { prov.prefetch } + assert_nothing_raised { prov.prefetch(resources) } assert_equal("a", one.provider.one) assert_equal("a", two.provider.one) @@ -368,9 +384,11 @@ class TestParsedFile < Test::Unit::TestCase assert_nothing_raised { apply(two) } assert_nothing_raised { two.flush } + # And make sure it didn't clear our hash assert_equal(:b, two.provider.one) end + # Make sure it works even if the file does not currently exist def test_creating_file prov = mkprovider prov.clear @@ -387,7 +405,7 @@ class TestParsedFile < Test::Unit::TestCase resource.flush end - assert(prov.target_object(:basic).read.include?("yay a c"), + assert_equal("yay a c\n", prov.target_object(:basic).read, "Did not create file") # Make a change @@ -399,8 +417,8 @@ class TestParsedFile < Test::Unit::TestCase end # And make sure our resource doesn't appear twice in the file. - assert_equal("yay b c\n", - prov.target_object(:basic).read) + assert_equal("yay b c\n", prov.target_object(:basic).read, + "Wrote record to file twice") end # Make sure a record can switch targets. @@ -472,7 +490,7 @@ class TestParsedFile < Test::Unit::TestCase notdisk = mkresource "notdisk", :target => :first prov.target_object(:first).write "ondisk a c\n" - prov.prefetch + prov.prefetch("ondisk" => ondisk, "notdisk" => notdisk) assert_equal(:present, notdisk.should(:ensure), "Did not get default ensure value") @@ -544,7 +562,7 @@ class TestParsedFile < Test::Unit::TestCase prov.target_object(:yayness).write "bill a c\njill b d" - list = @type.list + list = @type.instances bill = list.find { |r| r[:name] == "bill" } jill = list.find { |r| r[:name] == "jill" } @@ -639,7 +657,6 @@ class TestParsedFile < Test::Unit::TestCase targeted = {:target => "target"} prov.send(:instance_variable_set, "@records", records) prov.expects(:retrieve).with(target).returns([targeted]) - prov.expects(:target_records).with(target).returns([targeted]) prov.expects(:prefetch_hook).with([targeted]).returns([targeted]) diff --git a/test/ral/providers/provider.rb b/test/ral/providers/provider.rb index 9e59f2008..7fcf3f59d 100755 --- a/test/ral/providers/provider.rb +++ b/test/ral/providers/provider.rb @@ -293,6 +293,34 @@ class TestProvider < Test::Unit::TestCase assert_equal(:base, other.source, "source did not override") assert_equal(:base, other.source, "source did not override") end + + # Make sure we can initialize with either a resource or a hash, or none at all. + def test_initialize + test = @type.provide(:test) + + inst = @type.create :name => "boo" + prov = nil + assert_nothing_raised("Could not init with a resource") do + prov = test.new(inst) + end + assert_equal(prov.resource, inst, "did not set resource correctly") + assert_equal(inst.name, prov.name, "did not get resource name") + + params = {:name => :one, :ensure => :present} + assert_nothing_raised("Could not init with a hash") do + prov = test.new(params) + end + assert_equal(params, prov.send(:instance_variable_get, "@property_hash"), "did not set resource correctly") + assert_equal(:one, prov.name, "did not get name from hash") + + assert_nothing_raised("Could not init with no argument") do + prov = test.new() + end + + assert_raise(Puppet::DevError, "did not fail when no name is present") do + prov.name + end + end end class TestProviderFeatures < Test::Unit::TestCase diff --git a/test/ral/types/cron.rb b/test/ral/types/cron.rb index ead815071..be94463d2 100755 --- a/test/ral/types/cron.rb +++ b/test/ral/types/cron.rb @@ -287,9 +287,11 @@ class TestCron < Test::Unit::TestCase # Write it to our file assert_apply(cron) + @crontype.clear + crons = [] assert_nothing_raised { - @crontype.list.each do |cron| + @crontype.instances.each do |cron| crons << cron end } diff --git a/test/ral/types/package.rb b/test/ral/types/package.rb index 9aafc3468..e37cdfe06 100755 --- a/test/ral/types/package.rb +++ b/test/ral/types/package.rb @@ -11,7 +11,6 @@ class TestPackages < Test::Unit::TestCase include PuppetTest::FileTesting def setup super - #@list = Puppet.type(:package).getpkglist Puppet.type(:package).clear @type = Puppet::Type.type(:package) end diff --git a/test/ral/types/resources.rb b/test/ral/types/resources.rb index 56603c611..82d4e225d 100755 --- a/test/ral/types/resources.rb +++ b/test/ral/types/resources.rb @@ -13,7 +13,7 @@ class TestResources < Test::Unit::TestCase def add_purge_lister # Now define the list method class << @purgetype - def list + def instances $purgemembers.values end end @@ -85,21 +85,9 @@ class TestResources < Test::Unit::TestCase purger = @type.create :name => "purgetest", :noop => true, :loglevel => :warning end assert(purger, "did not get purger manager") - - # Make sure we throw an error, because the purger type does - # not support listing. - - # It should work when we set it to false - assert_nothing_raised do - purger[:purge] = false - end - # but not true - assert_raise(ArgumentError) do - purger[:purge] = true - end add_purge_lister() - assert_equal($purgemembers.values.sort, @purgetype.list.sort) + assert_equal($purgemembers.values.sort, @purgetype.instances.sort) # and it should now succeed assert_nothing_raised do diff --git a/test/ral/types/sshkey.rb b/test/ral/types/sshkey.rb index e8ee575d9..9a20dc251 100755 --- a/test/ral/types/sshkey.rb +++ b/test/ral/types/sshkey.rb @@ -59,9 +59,9 @@ class TestSSHKey < Test::Unit::TestCase return key end - def test_list + def test_instances assert_nothing_raised { - Puppet.type(:sshkey).defaultprovider.list + Puppet.type(:sshkey).instances } count = 0 |