From 7cb884e44db412ed4cc19d9eb3e07d4b5b17f6b3 Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Tue, 15 Feb 2011 17:10:42 -0800 Subject: (#6346) Move the trap calls onto Signal so they're easier to stub Once you stub signal traps in tests, you can hit ctrl+c in the middle of a spec run and it will stop the run instead of puppet catching the SIGINT. I had trouble easily tracking down all the places to stub traps when the trap was being called as a private method on applications and daemons, but calling trap on Signal is equivalent since Kernel calls Signal.trap and Object mixes in Kernel to provide trap as a private method on all objects. A bigger solution would be to refactor everywhere we call trap into a method that's called consistently since right now we sprinkle SIGINT and SIGTERM trap handling over applications and daemons in inconsistent ways, returning different error codes and using different messages. I've captured this info in ticket #6345. Reviewed-by: Jacob Helwig --- lib/puppet/application/agent.rb | 2 +- lib/puppet/application/apply.rb | 2 +- lib/puppet/application/filebucket.rb | 2 +- lib/puppet/application/inspect.rb | 2 +- lib/puppet/application/kick.rb | 4 ++-- lib/puppet/application/master.rb | 2 +- lib/puppet/application/queue.rb | 4 ++-- 7 files changed, 9 insertions(+), 9 deletions(-) (limited to 'lib/puppet/application') diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb index 2b75505fd..895156f11 100644 --- a/lib/puppet/application/agent.rb +++ b/lib/puppet/application/agent.rb @@ -9,7 +9,7 @@ class Puppet::Application::Agent < Puppet::Application def preinit # Do an initial trap, so that cancels don't get a stack trace. - trap(:INT) do + Signal.trap(:INT) do $stderr.puts "Cancelling startup" exit(0) end diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb index 33a70ce8a..8f5aa86d0 100644 --- a/lib/puppet/application/apply.rb +++ b/lib/puppet/application/apply.rb @@ -143,7 +143,7 @@ class Puppet::Application::Apply < Puppet::Application client = nil server = nil - trap(:INT) do + Signal.trap(:INT) do $stderr.puts "Exiting" exit(1) end diff --git a/lib/puppet/application/filebucket.rb b/lib/puppet/application/filebucket.rb index 9c3c79bc3..5c91c4f64 100644 --- a/lib/puppet/application/filebucket.rb +++ b/lib/puppet/application/filebucket.rb @@ -52,7 +52,7 @@ class Puppet::Application::Filebucket < Puppet::Application @client = nil @server = nil - trap(:INT) do + Signal.trap(:INT) do $stderr.puts "Cancelling" exit(1) end diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb index 52ef97530..9e2aaed57 100644 --- a/lib/puppet/application/inspect.rb +++ b/lib/puppet/application/inspect.rb @@ -82,7 +82,7 @@ Licensed under the GNU General Public License version 2 Puppet::Util::Log.newdestination(@report) Puppet::Util::Log.newdestination(:console) unless options[:logset] - trap(:INT) do + Signal.trap(:INT) do $stderr.puts "Exiting" exit(1) end diff --git a/lib/puppet/application/kick.rb b/lib/puppet/application/kick.rb index 37aeb1ef2..b3c95e21d 100644 --- a/lib/puppet/application/kick.rb +++ b/lib/puppet/application/kick.rb @@ -151,7 +151,7 @@ class Puppet::Application::Kick < Puppet::Application def preinit [:INT, :TERM].each do |signal| - trap(signal) do + Signal.trap(signal) do $stderr.puts "Cancelling" exit(1) end @@ -195,7 +195,7 @@ class Puppet::Application::Kick < Puppet::Application # If we get a signal, then kill all of our children and get out. [:INT, :TERM].each do |signal| - trap(signal) do + Signal.trap(signal) do Puppet.notice "Caught #{signal}; shutting down" @children.each do |pid, host| Process.kill("INT", pid) diff --git a/lib/puppet/application/master.rb b/lib/puppet/application/master.rb index fde474907..6d1cdef1b 100644 --- a/lib/puppet/application/master.rb +++ b/lib/puppet/application/master.rb @@ -26,7 +26,7 @@ class Puppet::Application::Master < Puppet::Application end def preinit - trap(:INT) do + Signal.trap(:INT) do $stderr.puts "Cancelling startup" exit(0) end diff --git a/lib/puppet/application/queue.rb b/lib/puppet/application/queue.rb index 239f6b2ad..ede47d0a6 100644 --- a/lib/puppet/application/queue.rb +++ b/lib/puppet/application/queue.rb @@ -15,13 +15,13 @@ class Puppet::Application::Queue < Puppet::Application # Do an initial trap, so that cancels don't get a stack trace. # This exits with exit code 1 - trap(:INT) do + Signal.trap(:INT) do $stderr.puts "Caught SIGINT; shutting down" exit(1) end # This is a normal shutdown, so code 0 - trap(:TERM) do + Signal.trap(:TERM) do $stderr.puts "Caught SIGTERM; shutting down" exit(0) end -- cgit From de6a2052c2aeda1cd76ba828936a9d6f0ac7e907 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 22 Feb 2011 11:18:19 -0800 Subject: (#5552) Clean up subcommand handling inside puppet cert. We now have a regular, testable mechanism for handling the legacy '--' version of subcommands, as well as a modern bareword subcommand pattern. This makes it sensible to test command handling and avoid regressions. We identified a few quirks in the command line as part of this process. Pair-With: Jesse Wolfe Signed-off-by: Daniel Pittman --- lib/puppet/application/cert.rb | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) (limited to 'lib/puppet/application') diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb index 467b0c859..c8aad1833 100644 --- a/lib/puppet/application/cert.rb +++ b/lib/puppet/application/cert.rb @@ -5,17 +5,19 @@ class Puppet::Application::Cert < Puppet::Application should_parse_config run_mode :master - attr_accessor :cert_mode, :all, :ca, :digest, :signed + attr_accessor :all, :ca, :digest, :signed - def find_mode(opt) - require 'puppet/ssl/certificate_authority' - modes = Puppet::SSL::CertificateAuthority::Interface::INTERFACE_METHODS - tmp = opt.sub("--", '').to_sym - @cert_mode = modes.include?(tmp) ? tmp : nil + def subcommand + @subcommand + end + def subcommand=(name) + # Handle the nasty, legacy mapping of "clean" to "destroy". + sub = name.to_sym + @subcommand = (sub == :clean ? :destroy : sub) end option("--clean", "-c") do - @cert_mode = :destroy + self.subcommand = "destroy" end option("--all", "-a") do @@ -37,7 +39,7 @@ class Puppet::Application::Cert < Puppet::Application require 'puppet/ssl/certificate_authority/interface' Puppet::SSL::CertificateAuthority::Interface::INTERFACE_METHODS.reject {|m| m == :destroy }.each do |method| option("--#{method}", "-#{method.to_s[0,1]}") do - find_mode("--#{method}") + self.subcommand = method end end @@ -54,8 +56,8 @@ class Puppet::Application::Cert < Puppet::Application hosts = command_line.args.collect { |h| h.downcase } end begin - @ca.apply(:revoke, :to => hosts) if @cert_mode == :destroy - @ca.apply(@cert_mode, :to => hosts, :digest => @digest) + @ca.apply(:revoke, :to => hosts) if subcommand == :destroy + @ca.apply(subcommand, :to => hosts, :digest => @digest) rescue => detail puts detail.backtrace if Puppet[:trace] puts detail.to_s @@ -64,11 +66,12 @@ class Puppet::Application::Cert < Puppet::Application end def setup + require 'puppet/ssl/certificate_authority' exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs? Puppet::Util::Log.newdestination :console - if [:generate, :destroy].include? @cert_mode + if [:generate, :destroy].include? subcommand Puppet::SSL::Host.ca_location = :local else Puppet::SSL::Host.ca_location = :only @@ -82,4 +85,11 @@ class Puppet::Application::Cert < Puppet::Application exit(23) end end + + def parse_options + # handle the bareword subcommand pattern. + result = super + self.subcommand ||= self.command_line.args.shift + result + end end -- cgit From 309b9320feef3e1a9459c7a26d10955b4d6b549c Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 22 Feb 2011 11:25:03 -0800 Subject: (#5552) Display help when no subcommand is given. Previously, when the command line was empty we would try and invoke an empty method; this was less helpful than telling people what they could actually do, so we adapt our code to do precisely that. Paired-With: Jesse Wolfe Signed-off-by: Daniel Pittman --- lib/puppet/application/cert.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'lib/puppet/application') diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb index c8aad1833..ee59b7e56 100644 --- a/lib/puppet/application/cert.rb +++ b/lib/puppet/application/cert.rb @@ -89,7 +89,13 @@ class Puppet::Application::Cert < Puppet::Application def parse_options # handle the bareword subcommand pattern. result = super - self.subcommand ||= self.command_line.args.shift + unless self.subcommand then + if sub = self.command_line.args.shift then + self.subcommand = sub + else + help + end + end result end end -- cgit From 23eb77d999acb73021547c5ef86adf609e202605 Mon Sep 17 00:00:00 2001 From: Jesse Wolfe Date: Fri, 25 Feb 2011 11:45:38 -0800 Subject: (#6322) --noop should not suppress error codes The noop option has been suppressing exit statuses. This is counterintuitive, as per discussion at http://projects.puppetlabs.com/issues/6322 This patch causes noop runs to return the same exit codes as real runs. Reviewed-By: Daniel Pittman --- lib/puppet/application/agent.rb | 2 +- lib/puppet/application/apply.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/puppet/application') diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb index 895156f11..3749241f8 100644 --- a/lib/puppet/application/agent.rb +++ b/lib/puppet/application/agent.rb @@ -119,7 +119,7 @@ class Puppet::Application::Agent < Puppet::Application if not report exit(1) - elsif not Puppet[:noop] and options[:detailed_exitcodes] then + elsif options[:detailed_exitcodes] then exit(report.exit_status) else exit(0) diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb index 8f5aa86d0..cc733e1f5 100644 --- a/lib/puppet/application/apply.rb +++ b/lib/puppet/application/apply.rb @@ -125,7 +125,7 @@ class Puppet::Application::Apply < Puppet::Application configurer = Puppet::Configurer.new report = configurer.run(:skip_plugin_download => true, :catalog => catalog) - exit( Puppet[:noop] ? 0 : options[:detailed_exitcodes] ? report.exit_status : 0 ) + exit( options[:detailed_exitcodes] ? report.exit_status : 0 ) rescue => detail puts detail.backtrace if Puppet[:trace] $stderr.puts detail.message -- cgit