From 954a3ffd0c4a5079f50b6119a65c442df928bc39 Mon Sep 17 00:00:00 2001 From: Fletcher Nichol Date: Wed, 12 Feb 2014 11:10:27 -0700 Subject: [PATCH 1/3] Load needed (dynamic) dependencies for provisioners at creation time. The need for this change was initated with #293 which runs every instance action in a thread of execution, even in serial mode. The issue is that on the Mac platform MRI Ruby 1.9 and 2.0 (not confirmed for 2.1), dynamic (i.e. native) code needs to be loaded in the main thread otherwise causing a "Trace/BPT trap: 5" crash. References: * http://stackoverflow.com/questions/9933969/matlab-ruby-gem-doesnt-work-when-called-from-thread#answer-12319780 * https://bugs.launchpad.net/sbcl/+bug/1014409 For Berkshelf 2.0.x at least, there are multiple transitive dependencies that *could* load native code, meaning that `require 'berkshelf'` could very well lead to a VM crash if performed in a thread other than main. This commit pushes library loading code ahead to Provisioner creation time. In other words, by the time you have a Provisioner object reference either Berkshelf, Librarian-Chef, or nothing has been required (assuming a Chef provisioner). A drawback to this approach is that these dynamic dependencies will be eagerly loaded when Test Kitchen is booting for trivial CLI commands such as `kitchen list` and `kitchen diagnose`. Further work is needed to ensure that `kitchen diagnose` remains viable even in the face of a failure to load these dependencies. Finally, the loaded version of Berkshelf or Librarian-Chef will be displayed to the user on INFO for converge action and DEBUG on code loading for troubleshooting. Closes #357 /cc @reset, @ivey, @sethvargo --- lib/kitchen/provisioner/base.rb | 3 +++ lib/kitchen/provisioner/chef/berkshelf.rb | 22 ++++++++++++++++------ lib/kitchen/provisioner/chef/librarian.rb | 23 +++++++++++++++++------ lib/kitchen/provisioner/chef_base.rb | 10 ++++++++++ 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/kitchen/provisioner/base.rb b/lib/kitchen/provisioner/base.rb index b9ee8fdbe..2957e6c36 100644 --- a/lib/kitchen/provisioner/base.rb +++ b/lib/kitchen/provisioner/base.rb @@ -41,6 +41,7 @@ def initialize(config = {}) def instance=(instance) @instance = instance expand_paths! + load_needed_dependencies! end # Returns the name of this driver, suitable for display in a CLI. @@ -128,6 +129,8 @@ def expand_paths! end end + def load_needed_dependencies! ; end + def logger instance ? instance.logger : Kitchen.logger end diff --git a/lib/kitchen/provisioner/chef/berkshelf.rb b/lib/kitchen/provisioner/chef/berkshelf.rb index 3cbdc27d9..d76443107 100644 --- a/lib/kitchen/provisioner/chef/berkshelf.rb +++ b/lib/kitchen/provisioner/chef/berkshelf.rb @@ -39,12 +39,15 @@ def initialize(berksfile, path, logger = Kitchen.logger) @logger = logger end + def self.load!(logger = Kitchen.logger) + load_berkshelf!(logger) + end + def resolve - info("Resolving cookbook dependencies with Berkshelf...") + version = ::Berkshelf::VERSION + info("Resolving cookbook dependencies with Berkshelf #{version}...") debug("Using Berksfile from #{berksfile}") - load_berkshelf! - ::Berkshelf.ui.mute do if ::Berkshelf::Berksfile.method_defined?(:vendor) # Berkshelf 3.0 requires the directory to not exist @@ -60,10 +63,17 @@ def resolve attr_reader :berksfile, :path, :logger - def load_berkshelf! - require 'berkshelf' + def self.load_berkshelf!(logger) + first_load = require 'berkshelf' + + version = ::Berkshelf::VERSION + if first_load + logger.debug("Berkshelf #{version} library loaded") + else + logger.debug("Berkshelf #{version} previously loaded") + end rescue LoadError => e - fatal("The `berkshelf' gem is missing and must be installed" + + logger.fatal("The `berkshelf' gem is missing and must be installed" + " or cannot be properly activated. Run" + " `gem install berkshelf` or add the following to your" + " Gemfile if you are using Bundler: `gem 'berkshelf'`.") diff --git a/lib/kitchen/provisioner/chef/librarian.rb b/lib/kitchen/provisioner/chef/librarian.rb index e25624137..e07669737 100644 --- a/lib/kitchen/provisioner/chef/librarian.rb +++ b/lib/kitchen/provisioner/chef/librarian.rb @@ -33,18 +33,22 @@ class Librarian include Logging + def initialize(cheffile, path, logger = Kitchen.logger) @cheffile = cheffile @path = path @logger = logger end + def self.load!(logger = Kitchen.logger) + load_librarian!(logger) + end + def resolve - info("Resolving cookbook dependencies with Librarian-Chef") + version = ::Librarian::Chef::VERSION + info("Resolving cookbook dependencies with Librarian-Chef #{version}...") debug("Using Cheffile from #{cheffile}") - load_librarian! - env = ::Librarian::Chef::Environment.new( :project_path => File.dirname(cheffile)) env.config_db.local["path"] = path @@ -54,12 +58,19 @@ def resolve attr_reader :cheffile, :path, :logger - def load_librarian! - require 'librarian/chef/environment' + def self.load_librarian!(logger) + first_load = require 'librarian/chef/environment' require 'librarian/action/resolve' require 'librarian/action/install' + + version = ::Librarian::Chef::VERSION + if first_load + logger.debug("Librarian-Chef #{version} library loaded") + else + logger.debug("Librarian-Chef #{version} previously loaded") + end rescue LoadError => e - fatal("The `librarian-chef' gem is missing and must be installed" + + logger.fatal("The `librarian-chef' gem is missing and must be installed" + " or cannot be properly activated. Run" + " `gem install librarian-chef` or add the following to your" + " Gemfile if you are using Bundler: `gem 'librarian-chef'`.") diff --git a/lib/kitchen/provisioner/chef_base.rb b/lib/kitchen/provisioner/chef_base.rb index 2d2d80867..67a7cc17d 100644 --- a/lib/kitchen/provisioner/chef_base.rb +++ b/lib/kitchen/provisioner/chef_base.rb @@ -129,6 +129,16 @@ def create_sandbox protected + def load_needed_dependencies! + if File.exists?(berksfile) + debug("Berksfile found at #{berksfile}, loading Berkshelf") + Chef::Berkshelf.load!(logger) + elsif File.exists?(cheffile) + debug("Cheffile found at #{cheffile}, loading Librarian-Chef") + Chef::Librarian.load!(logger) + end + end + def format_config_file(data) data.each.map { |attr, value| [attr, (value.is_a?(Array) ? value.to_s : %{"#{value}"})].join(" ") From c8e6f12002d5cbf28358837f09d39e50836e312c Mon Sep 17 00:00:00 2001 From: Fletcher Nichol Date: Wed, 12 Feb 2014 13:21:50 -0700 Subject: [PATCH 2/3] Record failures to build instances when running `kitchen diagnose`. In resolving #357 some code loading logic needed to be moved early in an object's life meaning that an Instance (or a collaborator object) could crash before `#diagnose` was callable. A design goal for `kitchen diagnose` is that it not crash itself and to provide some minimum useful information in a failure scenario. This commit will return an error hash in the :instances and :loader keys if an exception was raised. --- lib/kitchen/command/diagnose.rb | 38 ++++++++++++++++++++++++++------- lib/kitchen/diagnostic.rb | 16 ++++++++++++-- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/lib/kitchen/command/diagnose.rb b/lib/kitchen/command/diagnose.rb index 81877e4d9..6de7dc4c4 100644 --- a/lib/kitchen/command/diagnose.rb +++ b/lib/kitchen/command/diagnose.rb @@ -31,20 +31,42 @@ module Command class Diagnose < Kitchen::Command::Base def call - loader = if options[:all] || options[:loader] - @loader - else - nil - end + instances = record_failure { load_instances } + + loader = record_failure { load_loader } + + puts Kitchen::Diagnostic.new( + :loader => loader, :instances => instances).read.to_yaml + end + + private - instances = if options[:all] || options[:instances] + def load_instances + if options[:all] || options[:instances] parse_subcommand(args.first) else [] end + end - puts Kitchen::Diagnostic.new( - :loader => loader, :instances => instances).read.to_yaml + def load_loader + if options[:all] || options[:loader] + @loader + else + nil + end + end + + def record_failure + yield + rescue => e + { + :error => { + :exception => e.inspect, + :message => e.message, + :backtrace => e.backtrace + } + } end end end diff --git a/lib/kitchen/diagnostic.rb b/lib/kitchen/diagnostic.rb index 8b06e7eab..1c9aaf91d 100644 --- a/lib/kitchen/diagnostic.rb +++ b/lib/kitchen/diagnostic.rb @@ -50,12 +50,24 @@ def prepare_common end def prepare_loader - result[:loader] = loader.diagnose if loader + if error_hash?(loader) + result[:loader] = loader + else + result[:loader] = loader.diagnose if loader + end end def prepare_instances result[:instances] = Hash.new - Array(instances).each { |i| result[:instances][i.name] = i.diagnose } + if error_hash?(instances) + result[:instances][:error] = instances[:error] + else + Array(instances).each { |i| result[:instances][i.name] = i.diagnose } + end + end + + def error_hash?(obj) + obj.is_a?(Hash) && obj.has_key?(:error) end end end From 42e38d0ce07fc8b5fb34307967d282676b3ab727 Mon Sep 17 00:00:00 2001 From: Fletcher Nichol Date: Wed, 12 Feb 2014 13:24:47 -0700 Subject: [PATCH 3/3] On non-expected crashes, tell the user to try `kitchen diagnose --all`. --- lib/kitchen/errors.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/kitchen/errors.rb b/lib/kitchen/errors.rb index 7cacd90a6..26577244a 100644 --- a/lib/kitchen/errors.rb +++ b/lib/kitchen/errors.rb @@ -122,7 +122,8 @@ def self.handle_instance_failure(e) def self.handle_error(e) stderr_log(Error.formatted_exception(e)) - stderr_log("Please see .kitchen/logs/kitchen.log for more details\n") + stderr_log("Please see .kitchen/logs/kitchen.log for more details") + stderr_log("Also try running `kitchen diagnose --all` for configuration\n") file_log(:error, Error.formatted_trace(e)) end end