diff options
| -rw-r--r-- | lib/chef/application.rb | 3 | ||||
| -rw-r--r-- | lib/chef/config.rb | 24 | ||||
| -rw-r--r-- | spec/unit/application_spec.rb | 12 | ||||
| -rw-r--r-- | spec/unit/config_spec.rb | 30 | ||||
| -rw-r--r-- | spec/unit/monologger_spec.rb | 39 |
5 files changed, 52 insertions, 56 deletions
diff --git a/lib/chef/application.rb b/lib/chef/application.rb index 98bdedff7f..4e6b29cec6 100644 --- a/lib/chef/application.rb +++ b/lib/chef/application.rb @@ -118,6 +118,9 @@ class Chef::Application configure_stdout_logger end Chef::Log.level = resolve_log_level + rescue => error + Chef::Log.fatal("Failed to open or create log file at #{Chef::Config[:log_location]}: #{error.class} (#{error.message})") + Chef::Application.fatal!("Aborting due to invalid 'log_location' configuration", 2) end def configure_stdout_logger diff --git a/lib/chef/config.rb b/lib/chef/config.rb index f8bfa3ac21..f5cba9e0ed 100644 --- a/lib/chef/config.rb +++ b/lib/chef/config.rb @@ -127,26 +127,6 @@ class Chef # properly. configurable(:daemonize).writes_value { |v| v } - # Override the config dispatch to set the value of log_location configuration option - # - # === Parameters - # location<IO||String>:: Logging location as either an IO stream or string representing log file path - # - config_attr_writer :log_location do |location| - if location.respond_to? :sync= - location.sync = true - location - elsif location.respond_to? :to_str - begin - f = File.new(location.to_str, "a") - f.sync = true - rescue Errno::ENOENT - raise Chef::Exceptions::ConfigurationError, "Failed to open or create log file at #{location.to_str}" - end - f - end - end - # The root where all local chef object data is stored. cookbooks, data bags, # environments are all assumed to be in separate directories under this. # chef-solo uses these directories for input data. knife commands @@ -299,6 +279,9 @@ class Chef # logger is the primary mode of output, and the log level is set to :info default :log_level, :auto + # Logging location as either an IO stream or string representing log file path + default :log_location, STDOUT + # Using `force_formatter` causes chef to default to formatter output when STDOUT is not a tty default :force_formatter, false @@ -310,7 +293,6 @@ class Chef default :interval, nil default :once, nil default :json_attribs, nil - default :log_location, STDOUT # toggle info level log items that can create a lot of output default :verbose_logging, true default :node_name, nil diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb index 1606c82531..426dbd5242 100644 --- a/spec/unit/application_spec.rb +++ b/spec/unit/application_spec.rb @@ -163,6 +163,13 @@ describe Chef::Application do @app.configure_logging end + it "should raise fatals if log location is invalid" do + Chef::Config[:log_location] = "/tmp/non-existing-dir/logfile" + Chef::Log.should_receive(:fatal).at_least(:once) + Process.should_receive(:exit) + @app.configure_logging + end + shared_examples_for "log_level_is_auto" do context "when STDOUT is to a tty" do before do @@ -306,11 +313,6 @@ describe Chef::Application do @config_file.unlink end - it "should raise informative fatals for missing log file dir" do - create_config_file('log_location "/tmp/non-existing-dir/logfile"') - raises_informative_fatals_on_configure_chef - end - it "should raise informative fatals for badly written config" do create_config_file("text that should break the config parsing") raises_informative_fatals_on_configure_chef diff --git a/spec/unit/config_spec.rb b/spec/unit/config_spec.rb index 77870e84d8..2bf003bbdb 100644 --- a/spec/unit/config_spec.rb +++ b/spec/unit/config_spec.rb @@ -121,29 +121,6 @@ describe Chef::Config do end - describe "config attribute writer: log_method=" do - describe "when given an object that responds to sync= e.g. IO" do - it "should configure itself to use the IO as log_location" do - Chef::Config.log_location = STDOUT - Chef::Config.log_location.should == STDOUT - end - end - - describe "when given an object that is stringable (to_str)" do - before do - @mockfile = mock("File", :path => "/var/log/chef/client.log", :sync= => true) - File.should_receive(:new). - with("/var/log/chef/client.log", "a"). - and_return(@mockfile) - end - - it "should configure itself to use a File object based upon the String" do - Chef::Config.log_location = "/var/log/chef/client.log" - Chef::Config.log_location.path.should == "/var/log/chef/client.log" - end - end - end - describe "class method: plaform_specific_path" do it "should return given path on non-windows systems" do platform_mock :unix do @@ -354,13 +331,6 @@ describe Chef::Config do end end - describe "Chef::Config[:log_location]" do - it "raises ConfigurationError when log_location directory is missing" do - missing_path = "/tmp/non-existing-dir/file" - expect{Chef::Config.log_location = missing_path}.to raise_error Chef::Exceptions::ConfigurationError - end - end - describe "Chef::Config[:event_handlers]" do it "sets a event_handlers to an empty array by default" do Chef::Config[:event_handlers].should eq([]) diff --git a/spec/unit/monologger_spec.rb b/spec/unit/monologger_spec.rb new file mode 100644 index 0000000000..38b23f22c6 --- /dev/null +++ b/spec/unit/monologger_spec.rb @@ -0,0 +1,39 @@ +# +# Copyright:: Copyright (c) 2014 Opscode, Inc. +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'chef/monologger' +require 'tempfile' +require 'spec_helper' + +describe MonoLogger do + describe "when given an object that responds to write and close e.g. IO" do + it "should use the object directly" do + stream = StringIO.new + MonoLogger.new(stream).fatal("Houston, we've had a problem.") + stream.string.should =~ /Houston, we've had a problem./ + end + end + + describe "when given an object that is stringable (to_str)" do + it "should open a File object with the given path" do + temp_file = Tempfile.new("rspec-monologger-log") + temp_file.close + MonoLogger.new(temp_file.path).fatal("Do, or do not. There is no try.") + File.read(temp_file.path).should =~ /Do, or do not. There is no try./ + end + end +end |