On Fri, Jul 24, 2009 at 7:38 AM, Stéphan Gorget <[email protected]> wrote: > > Here is the version with the modifications. > I took a look at spec/unit/transaction.rb and test/util/transaction.rb > and I do not really feel confident about how to test it well, I'd be > glad to have some advice.
Anyone willing to step up and help Stéphan with this patch? Being able to combine package installs together should improve client run time significantly... > > commit 8de5272cbab0c92980feb1f5ca3189ee0e4d278a > Author: Stéphan Gorget <[email protected]> > Date: Mon Jun 22 21:32:18 2009 +0200 > > Features #2198 - Combine package installation > > Signed-off-by: Stéphan Gorget <[email protected]> > > 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 c0b4c0e..eff4cc7 100644 > --- a/lib/puppet/transaction.rb > +++ b/lib/puppet/transaction.rb > @@ -43,6 +43,11 @@ class Transaction > return true > end > > + # check if all the dependencies have been synced > + def has_dependencies?(resource) > + return !relationship_graph.dependencies(resource).empty? > + end > + > # Are there any failed resources in this transaction? > def any_failed? > failures = @failures.inject(0) { |failures, array| failures > += array[1]; failures } > @@ -92,18 +97,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 > > + set_trigger_and_refresh(resource, resourceevents) > 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 > end > > # Apply each change in turn. > @@ -115,7 +124,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 +495,125 @@ 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) > + 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 > has_dependencies?(r) > + } > + > + # push the reference resource to the list > + resources.push(resource) > + end > + > + # it performs the multiple evaluation > + def evaluate_multiple_resources(resources) > + # Uses the first resource as the one who notified errors > + reference_resource = resources.first > + provider_class = reference_resource.provider.class > + > + if resources.length > 0 > + begin > + # Define event for each resources > + failed_resources = provider_class.install_multiple(resources) > + > + resources.each { |r| > + set_combined_resource_metrics(r, failed_resources) > + } > + > + 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 > + > + # Set information about the modification applied > + def set_combined_resource_metrics(r, failed_resources) > + provider_class = r.provider.class > + > + # Generate event > + if failed_resources.include?(r) > + r.err "Resource %s failed to evaluate" % r[:name] > + �...@failures[r] += 1 > + > + else > + if r.noop > + r.notice "Resource %s would have been evaluated" % r[:name] > + �...@combine_events[provider_class][r.name] = > [Puppet::Transaction::Event.new(:noop, r)] > + > + else > + r.notice "Resource %s have been evaluated" % r[:name] > + r.cache(:synced, Time.now) > + �...@combine_events[provider_class][r.name] = > [Puppet::Transaction::Event.new(r.name, r)] > + end > + > + �...@count += 1 > + 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 > @@ -590,6 +722,8 @@ class Transaction > resource.warning "Skipping because of failed dependencies" > elsif resource.exported? > resource.debug "Skipping because exported" > + 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 098d832..6668568 100644 > --- a/lib/puppet/type.rb > +++ b/lib/puppet/type.rb > @@ -1656,6 +1656,11 @@ class Type > end > end > > + # returns true if the resource can be combined > + def combine? > + provider.class.respond_to?(:install_multiple) and resource.combine? > + 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 > > > > -- > Stéphan > > --~--~---------~--~----~------------~-------~--~----~ > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to [email protected] > To unsubscribe from this group, send email to > [email protected] > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en > -~----------~----~----~----~------~----~------~--~--- > > -- nigel -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
