Please review pull request #494: (#12403) Always create a default log destination opened by (joshcooper)
Description:
This patch series removes duplicate log setup code and ensures we always have a default log destination, even if the syslog feature is not present, e.g. puppet agent on Windows.
- Opened: Tue Feb 14 08:50:47 UTC 2012
- Based on: puppetlabs:2.7.x (53380b98a2899ef94b1f50027d5fb6b5d04e9ba4)
- Requested merge: joshcooper:ticket/2.7.x/12403-windows-default-log-destination (6fc9ccd30073df4a96354e25ee200aee8a223d9e)
Diff follows:
diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index c940d06..cccaa17 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -318,7 +318,10 @@ def run_command
end
def setup
- # Handle the logging settings
+ setup_logs
+ end
+
+ def setup_logs
if options[:debug] or options[:verbose]
Puppet::Util::Log.newdestination(:console)
if options[:debug]
@@ -328,7 +331,7 @@ def setup
end
end
- Puppet::Util::Log.newdestination(:syslog) unless options[:setdest]
+ Puppet::Util::Log.setup_default unless options[:setdest]
end
def configure_indirector_routes
diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb
index fdd93a6..001342e 100644
--- a/lib/puppet/application/agent.rb
+++ b/lib/puppet/application/agent.rb
@@ -377,20 +377,6 @@ def setup_test
options[:detailed_exitcodes] = true
end
- # Handle the logging settings.
- def setup_logs
- if options[:debug] or options[:verbose]
- Puppet::Util::Log.newdestination(:console)
- if options[:debug]
- Puppet::Util::Log.level = :debug
- else
- Puppet::Util::Log.level = :info
- end
- end
-
- Puppet::Util::Log.newdestination(:syslog) unless options[:setdest]
- end
-
def enable_disable_client(agent)
if options[:enable]
agent.enable
diff --git a/lib/puppet/application/device.rb b/lib/puppet/application/device.rb
index 7a602f2..c56850f 100644
--- a/lib/puppet/application/device.rb
+++ b/lib/puppet/application/device.rb
@@ -201,20 +201,6 @@ def main
end
end
- # Handle the logging settings.
- def setup_logs
- if options[:debug] or options[:verbose]
- Puppet::Util::Log.newdestination(:console)
- if options[:debug]
- Puppet::Util::Log.level = :debug
- else
- Puppet::Util::Log.level = :info
- end
- end
-
- Puppet::Util::Log.newdestination(:syslog) unless options[:setdest]
- end
-
def setup_host
@host = Puppet::SSL::Host.new
waitforcert = options[:waitforcert] || (Puppet[:onetime] ? 0 : 120)
diff --git a/lib/puppet/application/queue.rb b/lib/puppet/application/queue.rb
index 2a9d3e7..b11be48 100644
--- a/lib/puppet/application/queue.rb
+++ b/lib/puppet/application/queue.rb
@@ -118,16 +118,6 @@ def help
end
end
- option("--logdest DEST", "-l DEST") do |arg|
- begin
- Puppet::Util::Log.newdestination(arg)
- options[:setdest] = true
- rescue => detail
- puts detail.backtrace if Puppet[:debug]
- $stderr.puts detail.to_s
- end
- end
-
def main
require 'puppet/indirector/catalog/queue' # provides Puppet::Indirector::Queue.subscribe
Puppet.notice "Starting puppetqd #{Puppet.version}"
@@ -148,19 +138,6 @@ def main
Thread.list.each { |thread| thread.join }
end
- # Handle the logging settings.
- def setup_logs
- if options[:debug] or options[:verbose]
- Puppet::Util::Log.newdestination(:console)
- if options[:debug]
- Puppet::Util::Log.level = :debug
- else
- Puppet::Util::Log.level = :info
- end
- end
- Puppet::Util::Log.newdestination(:syslog) unless options[:setdest]
- end
-
def setup
unless Puppet.features.stomp?
raise ArgumentError, "Could not load the 'stomp' library, which must be present for queueing to work. You must install the required library."
diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index 67eaf64..c0a94f9 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -189,12 +189,16 @@ def Log.reopen
}
rescue => detail
if @destinations.empty?
- Log.newdestination(:syslog)
+ Log.setup_default
Puppet.err detail.to_s
end
end
end
+ def self.setup_default
+ Log.newdestination(Puppet.features.syslog? ? :syslog : Puppet[:puppetdlog])
+ end
+
# Is the passed level a valid log level?
def self.validlevel?(level)
@levels.include?(level)
diff --git a/spec/unit/application/agent_spec.rb b/spec/unit/application/agent_spec.rb
index 13be1a5..44162bf 100755
--- a/spec/unit/application/agent_spec.rb
+++ b/spec/unit/application/agent_spec.rb
@@ -260,10 +260,10 @@
end
end
- it "should set syslog as the log destination if no --logdest" do
+ it "should set a default log destination if no --logdest" do
@puppetd.options.stubs(:[]).with(:setdest).returns(false)
- Puppet::Util::Log.expects(:newdestination).with(:syslog)
+ Puppet::Util::Log.expects(:setup_default)
@puppetd.setup_logs
end
@@ -286,7 +286,7 @@
it "should set a central log destination with --centrallogs" do
@puppetd.options.stubs(:[]).with(:centrallogs).returns(true)
Puppet[:server] = "puppet.reductivelabs.com"
- Puppet::Util::Log.stubs(:newdestination).with(:syslog)
+ Puppet::Util::Log.stubs(:setup_default)
Puppet::Util::Log.expects(:newdestination).with("puppet.reductivelabs.com")
diff --git a/spec/unit/application/device_spec.rb b/spec/unit/application/device_spec.rb
index fa200b1..d66cb6f 100755
--- a/spec/unit/application/device_spec.rb
+++ b/spec/unit/application/device_spec.rb
@@ -163,10 +163,10 @@
end
end
- it "should set syslog as the log destination if no --logdest" do
+ it "should set a default log destination if no --logdest" do
@device.options.stubs(:[]).with(:setdest).returns(false)
- Puppet::Util::Log.expects(:newdestination).with(:syslog)
+ Puppet::Util::Log.expects(:setup_default)
@device.setup_logs
end
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index efd5b46..b26e8e0 100755
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -68,6 +68,7 @@ def run_command
end
end
+ Puppet.features.stubs(:syslog?).returns(true)
Puppet[:run_mode].should == "user"
expect {
@@ -390,7 +391,7 @@ def run_command
it "should honor setdest option" do
@app.options.stubs(:[]).with(:setdest).returns(false)
- Puppet::Util::Log.expects(:newdestination).with(:syslog)
+ Puppet::Util::Log.expects(:setup_default)
@app.setup
end
diff --git a/spec/unit/util/log_spec.rb b/spec/unit/util/log_spec.rb
index 68728fe..3b9718f 100755
--- a/spec/unit/util/log_spec.rb
+++ b/spec/unit/util/log_spec.rb
@@ -14,6 +14,22 @@
message.should == "foo"
end
+ describe ".setup_default" do
+ it "should default to :syslog" do
+ Puppet.features.stubs(:syslog?).returns(true)
+ Puppet::Util::Log.expects(:newdestination).with(:syslog)
+
+ Puppet::Util::Log.setup_default
+ end
+
+ it "should fall back to :file" do
+ Puppet.features.stubs(:syslog?).returns(false)
+ Puppet::Util::Log.expects(:newdestination).with(Puppet[:puppetdlog])
+
+ Puppet::Util::Log.setup_default
+ end
+ end
+
describe Puppet::Util::Log::DestConsole do
before do
@console = Puppet::Util::Log::DestConsole.new
-- 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.
