On Jun 8, 2009, at 6:39 AM, Stephan Gorget wrote: > > Luke Kanies a écrit : >> On Jun 5, 2009, at 9:12 AM, David Schmitt wrote: >> >> Yeah, I know this is a possibility, and would be required for more >> complicated types. >> >> For the simple types like package, I don't think it's worth it, but >> once you extend, yeah, you need to think about how to use the graph >> to >> solve some of these problems. I do expect it's a bit complicated, >> though. >> > I modified the patch so that it doesn't try to install non-synced > resource and it takes into account the dependency tree. This is > based on > the fact that a resource can only been installed if all its > dependencies > have the cached value :check setted. As I have to use > resource.evaluate() to do so I do not invoke query() (rpm -q) before > installing. It's only invoked after the yum call (it is what is done > without the patch), it's perfectible but it's not taking a long time.
I don't know what you mean about the 'checked' value needing to be cached. If that's required for the dependency system to work, then that needs to be reworked. The transaction class currently doesn't know anything about this 'checked' value, and it shouldn't need to. > > commit b6979292adb63a1f7a94eb15f4b218d96c7dc296 > Author: Stéphan Gorget <gor...@ocre.cea.fr> > Date: Mon Jun 8 13:26:53 2009 +0200 > > Features #2198 - Combine package installation > > Signed-off-by: Stéphan Gorget <gor...@ocre.cea.fr> > > diff --git a/lib/puppet/provider/package/yum.rb > b/lib/puppet/provider/package/yum.rb > index 6fdff69..52eec37 100755 > --- a/lib/puppet/provider/package/yum.rb > +++ b/lib/puppet/provider/package/yum.rb > @@ -2,6 +2,7 @@ Puppet::Type.type(:package).provide :yum, :parent => > :rpm, :source => :rpm do > desc "Support via ``yum``." > > has_feature :versionable > + has_feature :combineable > > commands :yum => "yum", :rpm => "rpm", :python => "python" > > @@ -104,5 +105,23 @@ Puppet::Type.type(:package).provide :yum, :parent > => :rpm, :source => :rpm do > def purge > yum "-y", :erase, @resource[:name] > end > - end > > + # Static function that enables multiple package to be install at > the same time > + # it returns the resources that haven't been installed correctly > + def self.install_multiple(resources) > + > + # check which resource is not installed > + begin > + # collect the names of all the resources that need to be > installed > + resources_name = resources.collect{ |r| r[:name]} > + > + # install package > + output = yum "-d", "0", "-e", "0", "-y", :install, > resources_name > + > + # resources that have not been installed > + resources.reject { |r| r.provider.query } > + rescue Puppet::ExecutionFailure > + raise Puppet::Error, "Failed to install packages" > + end > + end > + end > diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb > index f09ca80..fb5664e 100644 > --- a/lib/puppet/transaction.rb > +++ b/lib/puppet/transaction.rb > @@ -43,6 +43,16 @@ class Transaction > return true > end > > + # check if all the dependencies have been checked > + def dependencies_completed?(resource) > + deps = relationship_graph.dependencies(resource) > + deps.each { |r| > + # return false if a dependency have not been checked > + return false if r.cached(:checked).to_i > + } > + return true > + end > + > # Are there any failed resources in this transaction? > def any_failed? > failures = @failures.inject(0) { |failures, array| failures += > array[1]; failures } > @@ -92,18 +102,22 @@ class Transaction > resource.flush > end > > - # And set a trigger for refreshing this resource if > it's a > - # self-refresher > - if resource.self_refresh? and ! resource.deleting? > - # Create an edge with this resource as both the > source and > - # target. The triggering method treats these > specially for > - # logging. > - events = resourceevents.collect { |e| e.name } > - set_trigger(Puppet::Relationship.new(resource, > resource, :callback => :refresh, :event => events)) > - end > end > > - resourceevents > + resourceevents = set_trigger_and_refresh(resource, > resourceevents) > + end > + > + def set_trigger_and_refresh(resource, resourceevents) > + # And set a trigger for refreshing this resource if it's a > + # self-refresher > + if resource.self_refresh? and ! resource.deleting? > + # Create an edge with this resource as both the source > and > + # target. The triggering method treats these > specially for > + # logging. > + events = resourceevents.collect { |e| e.name } > + set_trigger(Puppet::Relationship.new(resource, resource, > :callback => :refresh, :event => events)) > + end > + resourceevents > end > > # Apply each change in turn. > @@ -115,7 +129,11 @@ class Transaction > begin > # use an array, so that changes can return more than > one > # event if they want > - events = [change.forward].flatten.reject { |e| > e.nil? } > + if resource.combine? > + events = evaluate_multiple_resource(resource) > + else > + events = [change.forward].flatten.reject { |e| > e.nil? } > + end > rescue => detail > if Puppet[:trace] > puts detail.backtrace > @@ -482,6 +500,109 @@ class Transaction > end > end > > + # Function that evaluate multiple resource at the same class if > they support it. > + def evaluate_multiple_resource(resource) > + provider_class = resource.provider.class > + > + if events = retrieve_combined_resource_events(resource) > + return events > + end > + > + resources = find_combineable_resources(resource) > + events = evaluate_multiple_resources(resources, > provider_class) > + set_triggers_from_resource(resources) > + > + @combine_events[provider_class][resource.name] > + end > + > + # retrieves events associated to a specific resource if they > exist > otherwise return nil > + def retrieve_combined_resource_events(resource) > + @combine_events ||= {} > + @combine_events[resource.provider.class] ||= {} > + @combine_events[resource.provider.class][resource.name] > + end > + > + # Find all resources matching the provider class that set combine > + def find_combineable_resources(resource) > + > + # collects the other resources that can be combined with > resource > + resources = catalog.vertices.find_all { |r| > + r != resource and > r.provider.class.equal?(resource.provider.class) and r.combine? and > !skip?(r) > + } > + > + # rejects the resources that are already synced or couldn't > been synced > + resources.reject! { |r| > + begin > + changes = r.evaluate > + rescue => detail > + if Puppet[:trace] > + puts detail.backtrace > + end > + r.err "Failed to retrieve current state of resource : > %s" % detail > + # Mark that it failed > + @failures[r] = 1 > + changes = [] > + end > + > + changes = [changes] unless changes.is_a?(Array) > + changes_not_empty = changes.length > 0 > + > + changes.each { |c| @changes << c } > + > + !changes_not_empty or !allow_processing?(r, changes) or > !dependencies_completed?(r) > + } > + > + # push the reference resource to the list > + resources.push(resource) > + end > + > + # it performs the multiple evaluation > + def evaluate_multiple_resources(resources, provider_class) > + # Uses the first resource to define noop or not and print > logs > + reference_resource = resources.first > + if resources.length > 0 > + begin > + # Define event for each resources > + failed_resources = > provider_class.install_multiple(resources) > + resources.each { |r| > + # Generate event > + if failed_resources.include?(r) > + r.err "Resource %s failed to install" % > r[:name] > + @failures[r] += 1 > + else > + if r.noop > + r.notice "Resource %s would have been > installed" % r[:name] > + @combine_events[provider_class][r.name] = > [Puppet::Transaction::Event.new(:noop, r)] > + else > + r.notice "Resource %s have been > installed" > % r[:name] > + @combine_events[provider_class][r.name] = > [Puppet::Transaction::Event.new(r.name, r)] > + end > + @count = 1 > + r.cache(:synced, Time.now) > + r.cache(:checked, Time.now) > + end > + } > + rescue => detail > + reference_resource.err "evaluate multiple resources > error: %s" % detail > + if Puppet[:trace] > + puts detail.backtrace > + end > + # All the resources failed > + resources.each { |r| > + @failures[r] += 1 > + } > + return nil > + end > + end > + end > + > + def set_triggers_from_resource(resources) > + resources.each { |r| > + events = retrieve_combined_resource_events(r) > + set_trigger_and_refresh(r, events) > + } > + end > + > # Prepare to evaluate the resources in a transaction. > def prepare > # Now add any dynamically generated resources > @@ -592,6 +713,8 @@ class Transaction > resource.debug "Not scheduled" > elsif failed_dependencies?(resource) > resource.warning "Skipping because of failed dependencies" > + elsif failed?(resource) > + resource.warning "Skipping because it has already > failed once" > else > return false > end > diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb > index d300338..fcc4991 100644 > --- a/lib/puppet/type.rb > +++ b/lib/puppet/type.rb > @@ -1647,6 +1647,11 @@ class Type > end > end > > + # returns true if the resource can be combined > + def combine? > + provider.class.respond_to?(:install_multiple) and > resource[:combine] == :true > + end > + > ############################### > # All of the relationship code. > > diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb > index bb3db28..6600cdd 100644 > --- a/lib/puppet/type/package.rb > +++ b/lib/puppet/type/package.rb > @@ -35,6 +35,9 @@ module Puppet > package database for installed version(s), and can > select > which out of a set of available versions of a > package to > install if asked." > + feature :combineable, "Multiple packages can be installed or > + uninstalled in one command, rather than requiring one > command per > + package." > > ensurable do > desc "What state the package should be in. > @@ -277,6 +280,14 @@ module Puppet > newvalues(:true, :false) > end > > + newparam(:combine, :boolean => true, :required_features => > :combineable) do > + desc "Tells package provider if the package installation > have to be > + perform in a single call by the package manager or if > it has to > + be executed separately." > + > + newvalues(:true, :false) > + end > + > autorequire(:file) do > autos = [] > [:responsefile, :adminfile].each { |param| > @@ -315,6 +326,11 @@ module Puppet > props > end > end > + > + def combine? > + provider.class.respond_to?(:install_multiple) and > self[:combine] == :true and > [:present,:latest,:installed].include?(self[:ensure]) > + end > + > end # Puppet::Type.type(:package) > end > > > > > -- The only thing that saves us from the bureaucracy is inefficiency. An efficient bureaucracy is the greatest threat to liberty. --Eugene McCarthy --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to puppet-dev@googlegroups.com To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---