summaryrefslogtreecommitdiffstats
path: root/lib/puppet/transaction
Commit message (Collapse)AuthorAgeFilesLines
* Fixed #4364 - Reduced audit msg from info to debugJames Turnbull2010-08-031-1/+1
|
* Code smell: Two space indentationMarkus Roberts2010-07-095-436/+436
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replaced 106806 occurances of ^( +)(.*$) with The ruby community almost universally (i.e. everyone but Luke, Markus, and the other eleven people who learned ruby in the 1900s) uses two-space indentation. 3 Examples: The code: end # Tell getopt which arguments are valid def test_get_getopt_args element = Setting.new :name => "foo", :desc => "anything", :settings => Puppet::Util::Settings.new assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args") becomes: end # Tell getopt which arguments are valid def test_get_getopt_args element = Setting.new :name => "foo", :desc => "anything", :settings => Puppet::Util::Settings.new assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args") The code: assert_equal(str, val) assert_instance_of(Float, result) end # Now test it with a passed object becomes: assert_equal(str, val) assert_instance_of(Float, result) end # Now test it with a passed object The code: end assert_nothing_raised do klass[:Yay] = "boo" klass["Cool"] = :yayness end becomes: end assert_nothing_raised do klass[:Yay] = "boo" klass["Cool"] = :yayness end
* Code smell: Avoid needless decorationsMarkus Roberts2010-07-091-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Replaced 704 occurances of (.*)\b([a-z_]+)\(\) with \1\2 3 Examples: The code: ctx = OpenSSL::SSL::SSLContext.new() becomes: ctx = OpenSSL::SSL::SSLContext.new The code: skip() becomes: skip The code: path = tempfile() becomes: path = tempfile * Replaced 31 occurances of ^( *)end *#.* with \1end 3 Examples: The code: becomes: The code: end # Dir.foreach becomes: end The code: end # def becomes: end
* Code smell: Avoid explicit returnsMarkus Roberts2010-07-093-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replaced 583 occurances of (DEF) (LINES) return (.*) end with 3 Examples: The code: def consolidate_failures(failed) filters = Hash.new { |h,k| h[k] = [] } failed.each do |spec, failed_trace| if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } filters[f] << spec break end end return filters end becomes: def consolidate_failures(failed) filters = Hash.new { |h,k| h[k] = [] } failed.each do |spec, failed_trace| if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } filters[f] << spec break end end filters end The code: def retrieve return_value = super return_value = return_value[0] if return_value && return_value.is_a?(Array) return return_value end becomes: def retrieve return_value = super return_value = return_value[0] if return_value && return_value.is_a?(Array) return_value end The code: def fake_fstab os = Facter['operatingsystem'] if os == "Solaris" name = "solaris.fstab" elsif os == "FreeBSD" name = "freebsd.fstab" else # Catchall for other fstabs name = "linux.fstab" end oldpath = @provider_class.default_target return fakefile(File::join("data/types/mount", name)) end becomes: def fake_fstab os = Facter['operatingsystem'] if os == "Solaris" name = "solaris.fstab" elsif os == "FreeBSD" name = "freebsd.fstab" else # Catchall for other fstabs name = "linux.fstab" end oldpath = @provider_class.default_target fakefile(File::join("data/types/mount", name)) end
* Code smell: Line modifiers are preferred to one-line blocks.Markus Roberts2010-07-093-9/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Replaced 6 occurances of (while .*?) *do$ with The do is unneeded in the block header form and causes problems with the block-to-one-line transformation. 3 Examples: The code: while line = f.gets do becomes: while line = f.gets The code: while line = shadow.gets do becomes: while line = shadow.gets The code: while wrapper = zeros.pop do becomes: while wrapper = zeros.pop * Replaced 19 occurances of ((if|unless) .*?) *then$ with The then is unneeded in the block header form and causes problems with the block-to-one-line transformation. 3 Examples: The code: if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } then becomes: if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } The code: unless defined?(@spec_command) then becomes: unless defined?(@spec_command) The code: if c == ?\n then becomes: if c == ?\n * Replaced 758 occurances of ((?:if|unless|while|until) .*) (.*) end with The one-line form is preferable provided: * The condition is not used to assign a variable * The body line is not already modified * The resulting line is not too long 3 Examples: The code: if Puppet.features.libshadow? has_feature :manages_passwords end becomes: has_feature :manages_passwords if Puppet.features.libshadow? The code: unless (defined?(@current_pool) and @current_pool) @current_pool = process_zpool_data(get_pool_data) end becomes: @current_pool = process_zpool_data(get_pool_data) unless (defined?(@current_pool) and @current_pool) The code: if Puppet[:trace] puts detail.backtrace end becomes: puts detail.backtrace if Puppet[:trace]
* Code smell: Use string interpolationMarkus Roberts2010-07-092-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Replaced 83 occurances of (.*)" *[+] *([$@]?[\w_0-9.:]+?)(.to_s\b)?(?! *[*(%\w_0-9.:{\[]) with \1#{\2}" 3 Examples: The code: puts "PUPPET " + status + ": " + process + ", " + state becomes: puts "PUPPET " + status + ": " + process + ", #{state}" The code: puts "PUPPET " + status + ": #{process}" + ", #{state}" becomes: puts "PUPPET #{status}" + ": #{process}" + ", #{state}" The code: }.compact.join( "\n" ) + "\n" + t + "]\n" becomes: }.compact.join( "\n" ) + "\n#{t}" + "]\n" * Replaced 21 occurances of (.*)" *[+] *" with \1 3 Examples: The code: puts "PUPPET #{status}" + ": #{process}" + ", #{state}" becomes: puts "PUPPET #{status}" + ": #{process}, #{state}" The code: puts "PUPPET #{status}" + ": #{process}, #{state}" becomes: puts "PUPPET #{status}: #{process}, #{state}" The code: res = self.class.name + ": #{@name}" + "\n" becomes: res = self.class.name + ": #{@name}\n" * Don't use string concatenation to split lines unless they would be very long. Replaced 11 occurances of (.*)(['"]) *[+] *(['"])(.*) with 3 Examples: The code: o.define_head "The check_puppet Nagios plug-in checks that specified " + "Puppet process is running and the state file is no " + becomes: o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " + The code: o.separator "Mandatory arguments to long options are mandatory for " + "short options too." becomes: o.separator "Mandatory arguments to long options are mandatory for short options too." The code: o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " + "older than specified interval." becomes: o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no older than specified interval." * Replaced no occurances of do (.*?) end with {\1} * Replaced 1488 occurances of "([^"\n]*%s[^"\n]*)" *% *(.+?)(?=$| *\b(do|if|while|until|unless|#)\b) with 20 Examples: The code: args[0].split(/\./).map do |s| "dc=%s"%[s] end.join(",") becomes: args[0].split(/\./).map do |s| "dc=#{s}" end.join(",") The code: puts "%s" % Puppet.version becomes: puts "#{Puppet.version}" The code: raise "Could not find information for %s" % node becomes: raise "Could not find information for #{node}" The code: raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)] becomes: raise Puppet::Error, "Cannot create #{dir}: basedir #{File.join(path)} is a file" The code: Puppet.err "Could not run %s: %s" % [client_class, detail] becomes: Puppet.err "Could not run #{client_class}: #{detail}" The code: raise "Could not find handler for %s" % arg becomes: raise "Could not find handler for #{arg}" The code: Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig] becomes: Puppet.err "Will not start without authorization file #{Puppet[:authconfig]}" The code: raise Puppet::Error, "Could not deserialize catalog from pson: %s" % detail becomes: raise Puppet::Error, "Could not deserialize catalog from pson: #{detail}" The code: raise "Could not find facts for %s" % Puppet[:certname] becomes: raise "Could not find facts for #{Puppet[:certname]}" The code: raise ArgumentError, "%s is not readable" % path becomes: raise ArgumentError, "#{path} is not readable" The code: raise ArgumentError, "Invalid handler %s" % name becomes: raise ArgumentError, "Invalid handler #{name}" The code: debug "Executing '%s' in zone %s with '%s'" % [command, @resource[:name], str] becomes: debug "Executing '#{command}' in zone #{@resource[:name]} with '#{str}'" The code: raise Puppet::Error, "unknown cert type '%s'" % hash[:type] becomes: raise Puppet::Error, "unknown cert type '#{hash[:type]}'" The code: Puppet.info "Creating a new certificate request for %s" % Puppet[:certname] becomes: Puppet.info "Creating a new certificate request for #{Puppet[:certname]}" The code: "Cannot create alias %s: object already exists" % [name] becomes: "Cannot create alias #{name}: object already exists" The code: return "replacing from source %s with contents %s" % [metadata.source, metadata.checksum] becomes: return "replacing from source #{metadata.source} with contents #{metadata.checksum}" The code: it "should have a %s parameter" % param do becomes: it "should have a #{param} parameter" do The code: describe "when registring '%s' messages" % log do becomes: describe "when registring '#{log}' messages" do The code: paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration%stest" % l } becomes: paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration#{l}test" } The code: assert_raise(Puppet::Error, "Check '%s' did not fail on false" % check) do becomes: assert_raise(Puppet::Error, "Check '#{check}' did not fail on false") do
* Code smell: Inconsistent indentation and related formatting issuesMarkus Roberts2010-07-092-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Replaced 163 occurances of defined\? +([@a-zA-Z_.0-9?=]+) with defined?(\1) This makes detecting subsequent patterns easier. 3 Examples: The code: if ! defined? @parse_config becomes: if ! defined?(@parse_config) The code: return @option_parser if defined? @option_parser becomes: return @option_parser if defined?(@option_parser) The code: if defined? @local and @local becomes: if defined?(@local) and @local * Eliminate trailing spaces. Replaced 428 occurances of ^(.*?) +$ with \1 1 file was skipped. test/ral/providers/host/parsed.rb because 0 * Replace leading tabs with an appropriate number of spaces. Replaced 306 occurances of ^(\t+)(.*) with Tabs are not consistently expanded in all environments. * Don't arbitrarily wrap on sprintf (%) operator. Replaced 143 occurances of (.*['"] *%) +(.*) with Splitting the line does nothing to aid clarity and hinders further refactorings. 3 Examples: The code: raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)] becomes: raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)] The code: Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig] becomes: Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig] The code: $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus] becomes: $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus] * Don't break short arrays/parameter list in two. Replaced 228 occurances of (.*) +(.*) with 3 Examples: The code: puts @format.wrap(type.provider(prov).doc, :indent => 4, :scrub => true) becomes: puts @format.wrap(type.provider(prov).doc, :indent => 4, :scrub => true) The code: assert(FileTest.exists?(daily), "Did not make daily graph for %s" % type) becomes: assert(FileTest.exists?(daily), "Did not make daily graph for %s" % type) The code: assert(prov.target_object(:first).read !~ /^notdisk/, "Did not remove thing from disk") becomes: assert(prov.target_object(:first).read !~ /^notdisk/, "Did not remove thing from disk") * If arguments must wrap, treat them all equally Replaced 510 occurances of lines ending in things like ...(foo, or ...(bar(1,3), with \1 \2 3 Examples: The code: midscope.to_hash(false), becomes: assert_equal( The code: botscope.to_hash(true), becomes: # bottomscope, then checking that we see the right stuff. The code: :path => link, becomes: * Replaced 4516 occurances of ^( *)(.*) with The present code base is supposed to use four-space indentation. In some places we failed to maintain that standard. These should be fixed regardless of the 2 vs. 4 space question. 15 Examples: The code: def run_comp(cmd) puts cmd results = [] old_sync = $stdout.sync $stdout.sync = true line = [] begin open("| #{cmd}", "r") do |f| until f.eof? do c = f.getc becomes: def run_comp(cmd) puts cmd results = [] old_sync = $stdout.sync $stdout.sync = true line = [] begin open("| #{cmd}", "r") do |f| until f.eof? do c = f.getc The code: s.gsub!(/.{4}/n, '\\\\u\&') } string.force_encoding(Encoding::UTF_8) string rescue Iconv::Failure => e raise GeneratorError, "Caught #{e.class}: #{e}" end else def utf8_to_pson(string) # :nodoc: string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&] } string.gsub!(/( becomes: s.gsub!(/.{4}/n, '\\\\u\&') } string.force_encoding(Encoding::UTF_8) string rescue Iconv::Failure => e raise GeneratorError, "Caught #{e.class}: #{e}" end else def utf8_to_pson(string) # :nodoc: string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&] } string.gsub!(/( The code: end } rvalues: rvalue | rvalues comma rvalue { if val[0].instance_of?(AST::ASTArray) result = val[0].push(val[2]) else result = ast AST::ASTArray, :children => [val[0],val[2]] end } becomes: end } rvalues: rvalue | rvalues comma rvalue { if val[0].instance_of?(AST::ASTArray) result = val[0].push(val[2]) else result = ast AST::ASTArray, :children => [val[0],val[2]] end } The code: #passwdproc = proc { @password } keytext = @key.export( OpenSSL::Cipher::DES.new(:EDE3, :CBC), @password ) File.open(@keyfile, "w", 0400) { |f| f << keytext } becomes: # passwdproc = proc { @password } keytext = @key.export( OpenSSL::Cipher::DES.new(:EDE3, :CBC), @password ) File.open(@keyfile, "w", 0400) { |f| f << keytext } The code: end def to_manifest "%s { '%s':\n%s\n}" % [self.type.to_s, self.name, @params.collect { |p, v| if v.is_a? Array " #{p} => [\'#{v.join("','")}\']" else " #{p} => \'#{v}\'" end }.join(",\n") becomes: end def to_manifest "%s { '%s':\n%s\n}" % [self.type.to_s, self.name, @params.collect { |p, v| if v.is_a? Array " #{p} => [\'#{v.join("','")}\']" else " #{p} => \'#{v}\'" end }.join(",\n") The code: via the augeas tool. Requires: - augeas to be installed (http://www.augeas.net) - ruby-augeas bindings Sample usage with a string:: augeas{\"test1\" : context => \"/files/etc/sysconfig/firstboot\", changes => \"set RUN_FIRSTBOOT YES\", becomes: via the augeas tool. Requires: - augeas to be installed (http://www.augeas.net) - ruby-augeas bindings Sample usage with a string:: augeas{\"test1\" : context => \"/files/etc/sysconfig/firstboot\", changes => \"set RUN_FIRSTBOOT YES\", The code: names.should_not be_include("root") end describe "when generating a purgeable resource" do it "should be included in the generated resources" do Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource] @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref) end end describe "when the instance's do not have an ensure property" do becomes: names.should_not be_include("root") end describe "when generating a purgeable resource" do it "should be included in the generated resources" do Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource] @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref) end end describe "when the instance's do not have an ensure property" do The code: describe "when the instance's do not have an ensure property" do it "should not be included in the generated resources" do @no_ensure_resource = Puppet::Type.type(:exec).new(:name => '/usr/bin/env echo') Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource] @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref) end end describe "when the instance's ensure property does not accept absent" do it "should not be included in the generated resources" do @no_absent_resource = Puppet::Type.type(:service).new(:name => 'foobar') becomes: describe "when the instance's do not have an ensure property" do it "should not be included in the generated resources" do @no_ensure_resource = Puppet::Type.type(:exec).new(:name => '/usr/bin/env echo') Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource] @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref) end end describe "when the instance's ensure property does not accept absent" do it "should not be included in the generated resources" do @no_absent_resource = Puppet::Type.type(:service).new(:name => 'foobar') The code: func = nil assert_nothing_raised do func = Puppet::Parser::AST::Function.new( :name => "template", :ftype => :rvalue, :arguments => AST::ASTArray.new( :children => [stringobj(template)] ) becomes: func = nil assert_nothing_raised do func = Puppet::Parser::AST::Function.new( :name => "template", :ftype => :rvalue, :arguments => AST::ASTArray.new( :children => [stringobj(template)] ) The code: assert( @store.allowed?("hostname.madstop.com", "192.168.1.50"), "hostname not allowed") assert( ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"), "subname name allowed") becomes: assert( @store.allowed?("hostname.madstop.com", "192.168.1.50"), "hostname not allowed") assert( ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"), "subname name allowed") The code: assert_nothing_raised { server = Puppet::Network::Handler.fileserver.new( :Local => true, :Config => false ) } becomes: assert_nothing_raised { server = Puppet::Network::Handler.fileserver.new( :Local => true, :Config => false ) } The code: 'yay', { :failonfail => false, :uid => @user.uid, :gid => @user.gid } ).returns('output') output = Puppet::Util::SUIDManager.run_and_capture 'yay', @user.uid, @user.gid becomes: 'yay', { :failonfail => false, :uid => @user.uid, :gid => @user.gid } ).returns('output') output = Puppet::Util::SUIDManager.run_and_capture 'yay', @user.uid, @user.gid The code: ).times(1) pkg.provider.expects( :aptget ).with( '-y', '-q', 'remove', 'faff' becomes: ).times(1) pkg.provider.expects( :aptget ).with( '-y', '-q', 'remove', 'faff' The code: johnny one two billy three four\n" # Just parse and generate, to make sure it's isomorphic. assert_nothing_raised do assert_equal(text, @parser.to_file(@parser.parse(text)), "parsing was not isomorphic") end end def test_valid_attrs becomes: johnny one two billy three four\n" # Just parse and generate, to make sure it's isomorphic. assert_nothing_raised do assert_equal(text, @parser.to_file(@parser.parse(text)), "parsing was not isomorphic") end end def test_valid_attrs The code: "testing", :onboolean => [true, "An on bool"], :string => ["a string", "A string arg"] ) result = [] should = [] assert_nothing_raised("Add args failed") do @config.addargs(result) end @config.each do |name, element| becomes: "testing", :onboolean => [true, "An on bool"], :string => ["a string", "A string arg"] ) result = [] should = [] assert_nothing_raised("Add args failed") do @config.addargs(result) end @config.each do |name, element|
* Fix for #4117 "Storing newly-audited value" messagesMarkus Roberts2010-07-091-1/+1
| | | | They're semantically info, not notifications, and now are handled as such.
* [#4110] Wrap Type#retrieve calls for backwards compatibilityJesse Wolfe2010-07-071-1/+1
| | | | | | | | This patch introduces Type#retrieve_resource as a wrapper for Type#resource, to coerce the return value from legacy types from Hash to Resource. Signed-off-by: Jesse Wolfe <jes5199@gmail.com>
* Fixing #3139 - all properties can now be auditedLuke Kanies2010-02-173-4/+45
| | | | | | | | | | | | | | | This provides a full audit trail for any parameter on any resource Puppet can manage. Just use: file { "/my/file": audit => [content, owner] } And Puppet will generate an event any time either of those properties change. This commit also deprecates the 'check' parameter in favor of a new 'audit' parameter. Signed-off-by: Luke Kanies <luke@puppetlabs.com>
* Working #3139 - ResourceHarness does cachingLuke Kanies2010-02-171-4/+14
| | | | | | | | | | | | | | | | | | This is again about moving transactional behaviour out of Puppet::Type and into the transactional code. I initially moved this code into Resource::Status, but it seemed to make more sense in the Harness - the Status object should be thin and have no code that doesn't make sense on both sides of the pipe, it seemed. The interface gets a bit uglier as a result, but this is all about good design != good OO (because we're increasing the argument count of methods by moving them outside of the target class). Signed-off-by: Luke Kanies <luke@puppetlabs.com>
* Working #3139 - scheduling moved to resource harnessLuke Kanies2010-02-171-0/+22
| | | | | | | | | | We previously had the schedule checking code in Puppet::Type, but it's more of a transactional function, and in order to do proper auditing in the transactional area, we need the cache checking done there. Scheduling is one of the few functions that actually uses cached data currently. Signed-off-by: Luke Kanies <luke@puppetlabs.com>
* Resolving conflicts with luke:tickets/master/2759Markus Roberts2010-02-171-1/+1
|
* Actually invoke the allow_changes? method in ResourceHarnessJesse Wolfe2010-02-171-0/+2
|
* Restore noop non-behavioursJesse Wolfe2010-02-171-2/+4
| | | | | | In the #2759 series, noop was altering the synced timestamp and calling the flush() method on nooped resources. This patch prevents those things from happening.
* Changing method profile for other event queueingLuke Kanies2010-02-171-6/+4
| | | | | | | | It wasn't clear in the first refactor if this was necessary, but doing the performance optimization made it clear it was. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Refactoring event queueing for performanceLuke Kanies2010-02-171-2/+14
| | | | | | | This does some normalization so we're not doing duplicate queries for large event collections. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Changing the method profile of EventManager#queue_eventLuke Kanies2010-02-171-17/+19
| | | | | | | It now takes multiple events instead of just one. This will help simplify a bunch of performance optimizations. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Moving Metric management to the reportsLuke Kanies2010-02-173-13/+80
| | | | | | | | | | | | This is one less bit that the transaction does. The resource status objects had nearly enough information to do everything, so I just added that last bit, and moved everything over. It's all much cleaner now. I had to change some existing, internal APIs, but mostly this should be hidden from outside users. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Fixing #2759 - reports now have complete change infoLuke Kanies2010-02-172-12/+6
| | | | | | | | This includes every event generated in the transaction and a Resource::Status object for each resource managed, with per-resource information in it. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* ResourceHarness now doesn't check params with no 'should'Luke Kanies2010-02-171-4/+6
| | | | | | | | | | | I hadn't been skipping parameters that didn't have a 'should' value set. This almost always resulted in the right behaviour, because most properties correctly just short-circuit to being in sync if the 'should' value is nil, but this encodes it at the harness, which is where it should be. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Changing Transaction to use the new ResourceHarnessLuke Kanies2010-02-171-4/+25
| | | | | | | | | | | | This is a much messier commit than I would like, mostly because of how 'file' works. I had to fix multiple special cases, and I had to move others. The whole system appears to now work, though, and we're ready to change reports to receive resource status instances rather than events. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Fixing log message when changes failLuke Kanies2010-02-171-3/+1
| | | | Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Renaming some methods in Transaction::ChangeLuke Kanies2010-02-171-16/+1
| | | | | | | | | | | Renaming 'go' to 'apply', which is a much more reasonable name. Also removing the 'backward' and 'forward' methods, since they're not actually used anywhere. (Well, 'forward' was used, but it just called 'go'.) Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Adding Transaction::ResourceHarness classLuke Kanies2010-02-171-0/+62
| | | | | | | | | This is the interface class between Transactions and Resources. It's a relatively ugly class, but it will hopefully allow us to move most/all of the messy interface code into this one, relatively small class. Signed-off-by: Luke Kanies <luke@reductivelabs.com>
* Solidifying the RAL/Event integration.Luke Kanies2010-02-171-1/+11
| | | | | | | | | | | | | | This has two changes: * Clarifies how we get the property and resource name (we pass the instance, the event converts to a string) * Logs at the resource's loglevel when there's no error These are related, because the event creator (resource) was passing in a string rather than an instance. Signed-off-by: Luke Kanies <luke@madstop.com>
* Refactoring the Change/Event/Property interfaceLuke Kanies2010-02-172-5/+29
| | | | | | | | This gives all logging responsibility to the event, which can now produce logs identical to those produced directly by the property. At this point, the events are entirely supersets of the logs.
* Correcting comments and making report timestamp internalLuke Kanies2010-02-171-0/+1
| | | | | | | | | We had some no-longer-correct comments in the Transaction class, which are now removed. This also moves the timestamp for reports into the report class, so it's created at initialization by the report, rather than by the transaction. Signed-off-by: Luke Kanies <luke@madstop.com>
* removing extraneous commentLuke Kanies2010-02-171-1/+0
| | | | Signed-off-by: Luke Kanies <luke@madstop.com>
* Adding Transaction events to Transaction reportsLuke Kanies2010-02-172-1/+12
| | | | | | | This means that every event generated during a transaction, with all of its metadata, will now be in the report. Signed-off-by: Luke Kanies <luke@madstop.com>
* Removing a redundant method in ReportLuke Kanies2010-02-171-5/+0
| | | | Signed-off-by: Luke Kanies <luke@madstop.com>
* Removing unused code and adding a couple of testsLuke Kanies2010-02-171-9/+0
| | | | Signed-off-by: Luke Kanies <luke@madstop.com>
* Extracting event management into a separate classLuke Kanies2010-02-171-0/+90
| | | | | | | | | | Thus pulls all event-related code out of Transaction. The Transaction class currently creates a single instance of this class, so it's nowhere near a "real" event manager, but at least it has very clean integration points and will be easy to upgrade as needed. Signed-off-by: Luke Kanies <luke@madstop.com>
* Cleaning up Event creationLuke Kanies2010-02-172-34/+50
| | | | | | | | | | The Property class is now completely responsible for creating the event, and it adds all of the metadata that a log message would normally have. This provides a cleaner definition of responsibility, and will allow further cleaning up in later commits. Signed-off-by: Luke Kanies <luke@madstop.com>
* Random code cleanupLuke Kanies2010-02-171-29/+30
| | | | Signed-off-by: Luke Kanies <luke@madstop.com>
* Switching transactions to callback-based eventsLuke Kanies2010-02-171-16/+3
| | | | | | | | | | | | | Events are now queued as they are created, and the queues are managed through simple interfaces, rather than collecting events over time and responding to them inline. This drastically simplifies event management, and will make moving it to a separate system essentially trivial. Signed-off-by: Luke Kanies <luke@madstop.com>
* Fix #1934 - detailed-exitcodes for puppetdDeepak Giridharagopal2009-11-191-1/+10
| | | | | | | | | | | | | | | | | | | | | | This option only works when --onetime is specified, as it doesn't make much sense to worry about exit codes in the context of a long-running daemon. This required a refactoring of the existing --detailed-exitcodes code, as "puppetd" wasn't directly creating a transaction object (like "puppet" does). Added Report::exit_status, which did what was previously hard-coded into the "puppet" executable. An Agent's "run" method now returns a value (the result of the individual client class' "run" method) The "puppetd" agent's "run" method now returns a transaction report, as that seems like the logical thing to return as the result of applying a catalog. Signed-off-by: Deepak Giridharagopal <deepak@brownman.org>
* Removed extra whitespace from end of linesIan Taylor2009-06-062-7/+7
|
* Fix #1483 - use REST to transmit reports over the wireBrice Figureau2008-12-061-1/+7
| | | | Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>
* Fixing #1764 - a property's 'sync' method is never considered a no-op.Luke Kanies2008-11-211-5/+3
| | | | | | | | | | | | | | | | *This is a behaviour change.* If the property does not return an event name, then one is generated based on the property name. Previously, the 'sync' method could return nil and it would be considered a noop, but if you need a noop, then you need to modify your 'insync?' method to return 'true' in the noop cases. Also modifying all of the builtin types that didn't handle this explicitly or returned nil in 'sync'. There should be no behaviour change in any of them. Signed-off-by: Luke Kanies <luke@madstop.com>
* Fixed #1442 - replaced use of Facter for report titling with certnameJames Turnbull2008-08-161-7/+1
|
* Testing and simplifying the Transaction::Change#backward method.Luke Kanies2008-07-041-14/+5
| | | | Signed-off-by: Luke Kanies <luke@madstop.com>
* Removing the Transaction::Change#transaction accessor.Luke Kanies2008-07-041-13/+2
| | | | | | | As with Events, this was never used (beyond being assigned), so I've gotten rid of it. Signed-off-by: Luke Kanies <luke@madstop.com>
* Refactoring the Transaction::Event class.Luke Kanies2008-07-042-10/+5
| | | | | | | | | | | The class had a 'transaction' accessor that was assigned but never used, and it is simple enough that it needed direct arguments rather than named arguments. The rest of the code is changing the other classes that use Events. Signed-off-by: Luke Kanies <luke@madstop.com>
* Adding tests to the Transaction::Change class.Luke Kanies2008-07-041-35/+19
| | | | | | | There's a small amount of refactoring here, mostly removing code that appears to not be used at all. Signed-off-by: Luke Kanies <luke@madstop.com>
* Renaming Puppet::Event to Puppet::Transaction::EventLuke Kanies2008-07-031-0/+22
| | | | Signed-off-by: Luke Kanies <luke@madstop.com>
* Renaming the Puppet::PropertyChange class to Puppet::Transaction::Change.Luke Kanies2008-07-031-0/+134
| | | | Signed-off-by: Luke Kanies <luke@madstop.com>
* Intermediate commit.Luke Kanies2008-04-081-0/+4
| | | | | | | | | | | | | | | | | This commit adds a Request instance into the indirection, pushing it all the way to the terminus instances. It's a big commit because it requires modifying every terminus class. There are still some thorny design issues. In particular, who should be responsible for making the request object? I've tried having both the indirection class and the Indirector module creating it, and both have their issues. Also, the Catalog class previously allowed passing Node instances directly to the find method, which is now no longer possible because the Request class would treat the node as the instance being found. We need the request class to have two modes, one when it's passed an instance and one when it's passed a key.
* Reorganizing the file structure for indirection terminus types.Luke Kanies2007-10-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | Previously, for example, the configuration terminus that was a subclass of 'code' would have been stored at lib/puppet/indirector/code/configuration and would have had to have been named 'configuration'. Now, the subclass can be named however the author prefers, and it must be stored at lib/puppet/indirector/configuration/<name>.rb, where <name> is the name you've chosen for the terminus type. The name only matters insomuch as it is used to load the file from disk and find the appropriate class when asked. The additional restriction is that the class constant for the terminus type must have its name as the last word, and the indirection must be the second to last word. Thus, in our example, we can choose any class constant that ends with Configuration::Code; given that there's only one Configuration class at this point, it makes the most sense to define the class as Puppet::Node::Configuration::Code. This is somewhat awkward, because of the class's location on disk, but the only other real option is to autogenerate a Puppet::Indirector::Configuration class constant, which is, I think, uglier.
* Translating the report handler to an indirected model.Luke Kanies2007-10-131-0/+5
| | | | | | | | | | | I've provided backward compatibility with the old handler. The only terminus type that currently exists for reports is the 'code' terminus, which is used to process reports in the style of the old handler. At some point, we should likely switch at least some of these report types (e.g., 'store') to terminus types.