diff options
author | danielsdeleo <dan@opscode.com> | 2014-01-10 17:37:13 -0800 |
---|---|---|
committer | danielsdeleo <dan@opscode.com> | 2014-01-14 17:51:36 -0800 |
commit | cc8a504d26c882b688004e8a9627a103d2fa3f53 (patch) | |
tree | a9c5578c8fde1578a9d1cb2ecf28d39b93b5c6bc | |
parent | f7763fa5ae9ee07ba83e5dadf606a2d105684dc7 (diff) | |
download | chef-cc8a504d26c882b688004e8a9627a103d2fa3f53.tar.gz |
Extract policy building concerns from Chef::Client
Chef::Client has too many responsibilities that are difficult to test in
isolation. Refactor them out to an implementation class. This is a
prerequsite for providing alternative policy building strategies.
-rw-r--r-- | lib/chef/client.rb | 168 | ||||
-rw-r--r-- | lib/chef/policy_builder.rb | 214 | ||||
-rw-r--r-- | spec/unit/client_spec.rb | 11 |
3 files changed, 238 insertions, 155 deletions
diff --git a/lib/chef/client.rb b/lib/chef/client.rb index 31def49a3d..390dc247ab 100644 --- a/lib/chef/client.rb +++ b/lib/chef/client.rb @@ -43,6 +43,7 @@ require 'chef/formatters/minimal' require 'chef/version' require 'chef/resource_reporter' require 'chef/run_lock' +require 'chef/policy_builder' require 'ohai' require 'rbconfig' @@ -147,7 +148,6 @@ class Chef @events = EventDispatch::Dispatcher.new(*event_handlers) @override_runlist = args.delete(:override_runlist) @specific_recipes = args.delete(:specific_recipes) - runlist_override_sanity_check! end def configure_formatters @@ -226,38 +226,28 @@ class Chef raise Exceptions::ChildConvergeError, message end + def load_node + policy_builder.load_node + @node = policy_builder.node + end + def build_node + policy_builder.build_node + @run_status = Chef::RunStatus.new(node, events) + end - # Configures the Chef::Cookbook::FileVendor class to fetch file from the - # server or disk as appropriate, creates the run context for this run, and - # sanity checks the cookbook collection. - #===Returns - # Chef::RunContext:: the run context for this run. def setup_run_context - if Chef::Config[:solo] - Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::FileSystemFileVendor.new(manifest, Chef::Config[:cookbook_path]) } - cl = Chef::CookbookLoader.new(Chef::Config[:cookbook_path]) - cl.load_cookbooks - cookbook_collection = Chef::CookbookCollection.new(cl) - run_context = Chef::RunContext.new(node, cookbook_collection, @events) - else - Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::RemoteFileVendor.new(manifest, rest) } - cookbook_hash = sync_cookbooks - cookbook_collection = Chef::CookbookCollection.new(cookbook_hash) - run_context = Chef::RunContext.new(node, cookbook_collection, @events) - end - run_status.run_context = run_context - - run_context.load(@run_list_expansion) - if @specific_recipes - @specific_recipes.each do |recipe_file| - run_context.load_recipe_file(recipe_file) - end - end + run_context = policy_builder.setup_run_context(@specific_recipes) assert_cookbook_path_not_empty(run_context) + run_status.run_context = run_context run_context end + def policy_builder + @policy_builder ||= Chef::PolicyBuilder.new(node_name, ohai.data, json_attribs, @override_runlist, events) + end + + def save_updated_node unless Chef::Config[:solo] Chef::Log.debug("Saving the current state of node #{node_name}") @@ -291,85 +281,6 @@ class Chef name end - # Applies environment, external JSON attributes, and override run list to - # the node, Then expands the run_list. - # - # === Returns - # node<Chef::Node>:: The modified @node object. @node is modified in place. - def build_node - # Allow user to override the environment of a node by specifying - # a config parameter. - if Chef::Config[:environment] && !Chef::Config[:environment].chop.empty? - @node.chef_environment(Chef::Config[:environment]) - end - - # consume_external_attrs may add items to the run_list. Save the - # expanded run_list, which we will pass to the server later to - # determine which versions of cookbooks to use. - @node.reset_defaults_and_overrides - @node.consume_external_attrs(ohai.data, @json_attribs) - - unless(@override_runlist.empty?) - @original_runlist = @node.run_list.run_list_items.dup - runlist_override_sanity_check! - @node.run_list(*@override_runlist) - Chef::Log.warn "Run List override has been provided." - Chef::Log.warn "Original Run List: [#{@original_runlist.join(', ')}]" - Chef::Log.warn "Overridden Run List: [#{@node.run_list}]" - end - - @run_list_expansion = expand_run_list - - # @run_list_expansion is a RunListExpansion. - # - # Convert @expanded_run_list, which is an - # Array of Hashes of the form - # {:name => NAME, :version_constraint => Chef::VersionConstraint }, - # into @expanded_run_list_with_versions, an - # Array of Strings of the form - # "#{NAME}@#{VERSION}" - @expanded_run_list_with_versions = @run_list_expansion.recipes.with_version_constraints_strings - - Chef::Log.info("Run List is [#{@node.run_list}]") - Chef::Log.info("Run List expands to [#{@expanded_run_list_with_versions.join(', ')}]") - - @run_status = Chef::RunStatus.new(@node, @events) - - @events.node_load_completed(node, @expanded_run_list_with_versions, Chef::Config) - - @node - end - - # In client-server operation, loads the node state from the server. In - # chef-solo operation, builds a new node object. - def load_node - @events.node_load_start(node_name, Chef::Config) - Chef::Log.debug("Building node object for #{node_name}") - - if Chef::Config[:solo] - @node = Chef::Node.build(node_name) - else - @node = Chef::Node.find_or_create(node_name) - end - rescue Exception => e - # TODO: wrap this exception so useful error info can be given to the - # user. - @events.node_load_failed(node_name, e, Chef::Config) - raise - end - - def expand_run_list - if Chef::Config[:solo] - @node.expand!('disk') - else - @node.expand!('server') - end - rescue Exception => e - # TODO: wrap/munge exception with useful error output. - @events.run_list_expand_failed(node, e) - raise - end - # # === Returns # rest<Chef::REST>:: returns Chef::REST connection object @@ -397,37 +308,6 @@ class Chef raise end - # Sync_cookbooks eagerly loads all files except files and - # templates. It returns the cookbook_hash -- the return result - # from /environments/#{node.chef_environment}/cookbook_versions, - # which we will use for our run_context. - # - # === Returns - # Hash:: The hash of cookbooks with download URLs as given by the server - def sync_cookbooks - Chef::Log.debug("Synchronizing cookbooks") - - begin - @events.cookbook_resolution_start(@expanded_run_list_with_versions) - cookbook_hash = rest.post_rest("environments/#{@node.chef_environment}/cookbook_versions", - {:run_list => @expanded_run_list_with_versions}) - rescue Exception => e - # TODO: wrap/munge exception to provide helpful error output - @events.cookbook_resolution_failed(@expanded_run_list_with_versions, e) - raise - else - @events.cookbook_resolution_complete(cookbook_hash) - end - - synchronizer = Chef::CookbookSynchronizer.new(cookbook_hash, @events) - synchronizer.sync_cookbooks - - # register the file cache path in the cookbook path so that CookbookLoader actually picks up the synced cookbooks - Chef::Config[:cookbook_path] = File.join(Chef::Config[:file_cache_path], "cookbooks") - - cookbook_hash - end - # Converges the node. # # === Returns @@ -533,22 +413,6 @@ class Chef true end - # Ensures runlist override contains RunListItem instances - def runlist_override_sanity_check! - # Convert to array and remove whitespace - if @override_runlist.is_a?(String) - @override_runlist = @override_runlist.split(',').map { |e| e.strip } - end - @override_runlist = [@override_runlist].flatten.compact - @override_runlist.map! do |item| - if(item.is_a?(Chef::RunList::RunListItem)) - item - else - Chef::RunList::RunListItem.new(item) - end - end - end - def empty_directory?(path) !File.exists?(path) || (Dir.entries(path).size <= 2) end diff --git a/lib/chef/policy_builder.rb b/lib/chef/policy_builder.rb new file mode 100644 index 0000000000..bc91cfff83 --- /dev/null +++ b/lib/chef/policy_builder.rb @@ -0,0 +1,214 @@ +require 'chef/log' +require 'chef/rest' +require 'chef/run_context' +require 'chef/config' +require 'chef/node' + +class Chef + + # Class that handles fetching policy from server or disk and resolving any + # indirection (e.g. expanding run_list). + # + # INPUTS + # * event stream object + # * node object/run_list + # * json_attribs + # * override_runlist + # + # OUTPUTS + # * mutated node object (implicit) + # * a new RunStatus (probably doesn't need to be here) + # * cookbooks sync'd to disk + # * cookbook_hash is stored in run_context + class PolicyBuilder + + attr_reader :events + attr_reader :node + attr_reader :node_name + attr_reader :ohai_data + attr_reader :json_attribs + attr_reader :override_runlist + attr_reader :original_runlist + attr_reader :run_context + + def initialize(node_name, ohai_data, json_attribs, override_runlist, events) + @node_name = node_name + @ohai_data = ohai_data + @json_attribs = json_attribs + @override_runlist = override_runlist + @events = events + + @node = nil + @original_runlist = nil + end + + def setup_run_context(specific_recipes=nil) + if Chef::Config[:solo] + Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::FileSystemFileVendor.new(manifest, Chef::Config[:cookbook_path]) } + cl = Chef::CookbookLoader.new(Chef::Config[:cookbook_path]) + cl.load_cookbooks + cookbook_collection = Chef::CookbookCollection.new(cl) + run_context = Chef::RunContext.new(node, cookbook_collection, @events) + else + Chef::Cookbook::FileVendor.on_create { |manifest| Chef::Cookbook::RemoteFileVendor.new(manifest, rest) } + cookbook_hash = sync_cookbooks + cookbook_collection = Chef::CookbookCollection.new(cookbook_hash) + run_context = Chef::RunContext.new(node, cookbook_collection, @events) + end + + # TODO: this is not the place for this. It should be in Runner or + # CookbookCompiler or something. + run_context.load(@run_list_expansion) + if specific_recipes + specific_recipes.each do |recipe_file| + run_context.load_recipe_file(recipe_file) + end + end + run_context + end + + + # In client-server operation, loads the node state from the server. In + # chef-solo operation, builds a new node object. + def load_node + events.node_load_start(node_name, Chef::Config) + Chef::Log.debug("Building node object for #{node_name}") + + if Chef::Config[:solo] + @node = Chef::Node.build(node_name) + else + @node = Chef::Node.find_or_create(node_name) + end + rescue Exception => e + # TODO: wrap this exception so useful error info can be given to the + # user. + events.node_load_failed(node_name, e, Chef::Config) + raise + end + + + # Applies environment, external JSON attributes, and override run list to + # the node, Then expands the run_list. + # + # === Returns + # node<Chef::Node>:: The modified node object. node is modified in place. + def build_node + # Allow user to override the environment of a node by specifying + # a config parameter. + if Chef::Config[:environment] && !Chef::Config[:environment].chop.empty? + node.chef_environment(Chef::Config[:environment]) + end + + # consume_external_attrs may add items to the run_list. Save the + # expanded run_list, which we will pass to the server later to + # determine which versions of cookbooks to use. + node.reset_defaults_and_overrides + node.consume_external_attrs(ohai_data, @json_attribs) + + setup_run_list_override + + @run_list_expansion = expand_run_list + + # @run_list_expansion is a RunListExpansion. + # + # Convert @expanded_run_list, which is an + # Array of Hashes of the form + # {:name => NAME, :version_constraint => Chef::VersionConstraint }, + # into @expanded_run_list_with_versions, an + # Array of Strings of the form + # "#{NAME}@#{VERSION}" + @expanded_run_list_with_versions = @run_list_expansion.recipes.with_version_constraints_strings + + Chef::Log.info("Run List is [#{node.run_list}]") + Chef::Log.info("Run List expands to [#{@expanded_run_list_with_versions.join(', ')}]") + + + events.node_load_completed(node, @expanded_run_list_with_versions, Chef::Config) + + node + end + + ######################################## + # Internal public API + ######################################## + + def expand_run_list + if Chef::Config[:solo] + node.expand!('disk') + else + node.expand!('server') + end + rescue Exception => e + # TODO: wrap/munge exception with useful error output. + events.run_list_expand_failed(node, e) + raise + end + + # Sync_cookbooks eagerly loads all files except files and + # templates. It returns the cookbook_hash -- the return result + # from /environments/#{node.chef_environment}/cookbook_versions, + # which we will use for our run_context. + # + # === Returns + # Hash:: The hash of cookbooks with download URLs as given by the server + def sync_cookbooks + Chef::Log.debug("Synchronizing cookbooks") + + begin + events.cookbook_resolution_start(@expanded_run_list_with_versions) + cookbook_hash = api_service.post("environments/#{node.chef_environment}/cookbook_versions", + {:run_list => @expanded_run_list_with_versions}) + rescue Exception => e + # TODO: wrap/munge exception to provide helpful error output + events.cookbook_resolution_failed(@expanded_run_list_with_versions, e) + raise + else + events.cookbook_resolution_complete(cookbook_hash) + end + + synchronizer = Chef::CookbookSynchronizer.new(cookbook_hash, events) + synchronizer.sync_cookbooks + + # register the file cache path in the cookbook path so that CookbookLoader actually picks up the synced cookbooks + Chef::Config[:cookbook_path] = File.join(Chef::Config[:file_cache_path], "cookbooks") + + cookbook_hash + end + + def setup_run_list_override + runlist_override_sanity_check! + unless(override_runlist.empty?) + @original_runlist = node.run_list.run_list_items.dup + node.run_list(*override_runlist) + Chef::Log.warn "Run List override has been provided." + Chef::Log.warn "Original Run List: [#{original_runlist.join(', ')}]" + Chef::Log.warn "Overridden Run List: [#{node.run_list}]" + end + end + + # Ensures runlist override contains RunListItem instances + def runlist_override_sanity_check! + # Convert to array and remove whitespace + if override_runlist.is_a?(String) + @override_runlist = override_runlist.split(',').map { |e| e.strip } + end + @override_runlist = [override_runlist].flatten.compact + override_runlist.map! do |item| + if(item.is_a?(Chef::RunList::RunListItem)) + item + else + Chef::RunList::RunListItem.new(item) + end + end + end + + def api_service + @api_service ||= Chef::REST.new(config[:chef_server_url]) + end + + def config + Chef::Config + end + + end +end diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb index eb705e0386..6b7dd36930 100644 --- a/spec/unit/client_spec.rb +++ b/spec/unit/client_spec.rb @@ -169,6 +169,7 @@ shared_examples_for Chef::Client do it "should identify the node and run ohai, then register the client" do mock_chef_rest_for_node = mock("Chef::REST (node)") + mock_chef_rest_for_cookbook_sync = mock("Chef::REST (cookbook sync)") mock_chef_rest_for_node_save = mock("Chef::REST (node save)") mock_chef_runner = mock("Chef::Runner") @@ -201,7 +202,8 @@ shared_examples_for Chef::Client do # ---Client#sync_cookbooks -- downloads the list of cookbooks to sync # Chef::CookbookSynchronizer.any_instance.should_receive(:sync_cookbooks) - mock_chef_rest_for_node.should_receive(:post_rest).with("environments/_default/cookbook_versions", {:run_list => []}).and_return({}) + Chef::REST.should_receive(:new).with(Chef::Config[:chef_server_url]).and_return(mock_chef_rest_for_cookbook_sync) + mock_chef_rest_for_cookbook_sync.should_receive(:post).with("environments/_default/cookbook_versions", {:run_list => []}).and_return({}) # --Client#converge Chef::Runner.should_receive(:new).and_return(mock_chef_runner) @@ -274,6 +276,7 @@ shared_examples_for Chef::Client do Chef::Client.clear_notifications Chef::Node.stub!(:find_or_create).and_return(@node) @node.stub!(:save) + @client.load_node @client.build_node end @@ -329,7 +332,8 @@ shared_examples_for Chef::Client do @node[:roles].should be_nil @node[:recipes].should be_nil - @client.build_node + @client.policy_builder.stub!(:node).and_return(@node) + @client.policy_builder.build_node # check post-conditions. @node[:roles].should_not be_nil @@ -434,7 +438,8 @@ shared_examples_for Chef::Client do @node.should_receive(:save).and_return(nil) - @client.build_node + @client.policy_builder.stub(:node).and_return(@node) + @client.policy_builder.build_node @node[:roles].should_not be_nil @node[:roles].should eql(['test_role']) |