From 4b1847acf30c8ef46d499c41ed54736277f232aa Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 9 Jul 2015 21:23:42 -0600 Subject: [PATCH 1/2] Use Chef Client for Chef Zero Fixes GH-5619 --- plugins/provisioners/chef/command_builder.rb | 68 +++++---- plugins/provisioners/chef/provisioner/base.rb | 7 - .../chef/provisioner/chef_client.rb | 5 +- .../chef/provisioner/chef_solo.rb | 15 +- .../chef/provisioner/chef_zero.rb | 132 ++---------------- templates/locales/en.yml | 4 +- 6 files changed, 66 insertions(+), 165 deletions(-) diff --git a/plugins/provisioners/chef/command_builder.rb b/plugins/provisioners/chef/command_builder.rb index 25d47c3d6..ad5093831 100644 --- a/plugins/provisioners/chef/command_builder.rb +++ b/plugins/provisioners/chef/command_builder.rb @@ -1,52 +1,66 @@ module VagrantPlugins module Chef class CommandBuilder - def initialize(config, client_type, is_windows = false, is_ui_colored = false) - @client_type = client_type - @config = config - @is_windows = is_windows - @is_ui_colored = is_ui_colored + def self.command(type, config, options = {}) + new(type, config, options).command + end - if client_type != :solo && client_type != :client - raise 'Invalid client_type, expected solo or client' + attr_reader :type + attr_reader :config + attr_reader :options + + def initialize(type, config, options = {}) + @type = type + @config = config + @options = options.dup + + if type != :client && type != :solo + raise "Unknown type `#{type.inspect}'!" end end - def build_command - "#{command_env}#{chef_binary_path} #{chef_arguments}" + def command + if config.binary_env + "#{config.binary_env} #{binary_path} #{args}" + else + "#{binary_path} #{args}" + end end protected - def command_env - @config.binary_env ? "#{@config.binary_env} " : "" - end + def binary_path + binary_path = "chef-#{type}" - def chef_binary_path - binary_path = "chef-#{@client_type}" - if @config.binary_path - binary_path = File.join(@config.binary_path, binary_path) + if config.binary_path + binary_path = File.join(config.binary_path, binary_path) if windows? binary_path = windows_friendly_path(binary_path) end end + binary_path end - def chef_arguments - chef_arguments = "-c #{provisioning_path("#{@client_type}.rb")}" - chef_arguments << " -j #{provisioning_path("dna.json")}" - chef_arguments << " #{@config.arguments}" if @config.arguments - chef_arguments << " --no-color" unless color? - chef_arguments.strip + def args + args = "" + args << " --config #{provisioning_path("#{type}.rb")}" + args << " --json-attributes #{provisioning_path("dna.json")}" + args << " --local-mode" if options[:local_mode] + args << " --log_level #{config.log_level}" if config.log_level + args << " --force-formatter" + args << " --no-color" if !options[:colored] + args << " #{config.arguments.strip}" if config.arguments + + args.strip end def provisioning_path(file) if windows? - path = @config.provisioning_path || "C:/vagrant-chef" + path = config.provisioning_path || "C:/vagrant-chef" return windows_friendly_path(File.join(path, file)) else - path = @config.provisioning_path || "/tmp/vagrant-chef" + path = config.provisioning_path || "/tmp/vagrant-chef" return File.join(path, file) end end @@ -58,11 +72,7 @@ module VagrantPlugins end def windows? - !!@is_windows - end - - def color? - !!@is_ui_colored + !!options[:windows] end end end diff --git a/plugins/provisioners/chef/provisioner/base.rb b/plugins/provisioners/chef/provisioner/base.rb index 726b93078..e0b2b3fc4 100644 --- a/plugins/provisioners/chef/provisioner/base.rb +++ b/plugins/provisioners/chef/provisioner/base.rb @@ -51,13 +51,6 @@ module VagrantPlugins ) end - # This returns the command to run Chef for the given client - # type. - def build_command(client) - builder = CommandBuilder.new(@config, client, windows?, @machine.env.ui.color?) - return builder.build_command - end - # Returns the path to the Chef binary, taking into account the # `binary_path` configuration option. def chef_binary_path(binary) diff --git a/plugins/provisioners/chef/provisioner/chef_client.rb b/plugins/provisioners/chef/provisioner/chef_client.rb index f2751eb68..da44fce78 100644 --- a/plugins/provisioners/chef/provisioner/chef_client.rb +++ b/plugins/provisioners/chef/provisioner/chef_client.rb @@ -69,7 +69,10 @@ module VagrantPlugins @machine.guest.capability(:wait_for_reboot) end - command = build_command(:client) + command = CommandBuilder.command(:client, @config, + windows: windows?, + colored: @machine.env.ui.color?, + ) @config.attempts.times do |attempt| if attempt == 0 diff --git a/plugins/provisioners/chef/provisioner/chef_solo.rb b/plugins/provisioners/chef/provisioner/chef_solo.rb index c21811e9e..f3b52f178 100644 --- a/plugins/provisioners/chef/provisioner/chef_solo.rb +++ b/plugins/provisioners/chef/provisioner/chef_solo.rb @@ -41,7 +41,7 @@ module VagrantPlugins share_folders(root_config, "cse", @environments_folders, existing) end - def provision(mode = :solo) + def provision install_chef # Verify that the proper shared folders exist. check = [] @@ -58,7 +58,7 @@ module VagrantPlugins upload_encrypted_data_bag_secret setup_json setup_solo_config - run_chef(mode) + run_chef_solo delete_encrypted_data_bag_secret end @@ -164,7 +164,7 @@ module VagrantPlugins } end - def run_chef(mode) + def run_chef_solo if @config.run_list && @config.run_list.empty? @machine.ui.warn(I18n.t("vagrant.chef_run_list_empty")) end @@ -173,13 +173,16 @@ module VagrantPlugins @machine.guest.capability(:wait_for_reboot) end - command = build_command(:solo) + command = CommandBuilder.command(:solo, @config, + windows: windows?, + colored: @machine.env.ui.color?, + ) @config.attempts.times do |attempt| if attempt == 0 - @machine.ui.info I18n.t("vagrant.provisioners.chef.running_#{mode}") + @machine.ui.info I18n.t("vagrant.provisioners.chef.running_solo") else - @machine.ui.info I18n.t("vagrant.provisioners.chef.running_#{mode}_again") + @machine.ui.info I18n.t("vagrant.provisioners.chef.running_solo_again") end opts = { error_check: false, elevated: true } diff --git a/plugins/provisioners/chef/provisioner/chef_zero.rb b/plugins/provisioners/chef/provisioner/chef_zero.rb index 1e5f61766..48078a09f 100644 --- a/plugins/provisioners/chef/provisioner/chef_zero.rb +++ b/plugins/provisioners/chef/provisioner/chef_zero.rb @@ -6,42 +6,19 @@ require "log4r" require "vagrant/util/counter" -require_relative "base" +require_relative "chef_solo" module VagrantPlugins module Chef module Provisioner # This class implements provisioning via chef-zero. - class ChefZero < Base - extend Vagrant::Util::Counter - include Vagrant::Util::Counter - include Vagrant::Action::Builtin::MixinSyncedFolders - - attr_reader :environments_folders - attr_reader :cookbook_folders - attr_reader :role_folders - attr_reader :data_bags_folders - + class ChefZero < ChefSolo def initialize(machine, config) super @logger = Log4r::Logger.new("vagrant::provisioners::chef_zero") - @shared_folders = [] end - def configure(root_config) - @cookbook_folders = expanded_folders(@config.cookbooks_path, "cookbooks") - @role_folders = expanded_folders(@config.roles_path, "roles") - @data_bags_folders = expanded_folders(@config.data_bags_path, "data_bags") - @environments_folders = expanded_folders(@config.environments_path, "environments") - - existing = synced_folders(@machine, cached: true) - share_folders(root_config, "csc", @cookbook_folders, existing) - share_folders(root_config, "csr", @role_folders, existing) - share_folders(root_config, "csdb", @data_bags_folders, existing) - share_folders(root_config, "cse", @environments_folders, existing) - end - - def provision(mode = :client) + def provision install_chef # Verify that the proper shared folders exist. check = [] @@ -58,99 +35,10 @@ module VagrantPlugins upload_encrypted_data_bag_secret setup_json setup_zero_config - run_chef(mode) + run_chef_zero delete_encrypted_data_bag_secret end - # Converts paths to a list of properly expanded paths with types. - def expanded_folders(paths, appended_folder=nil) - # Convert the path to an array if it is a string or just a single - # path element which contains the folder location (:host or :vm) - paths = [paths] if paths.is_a?(String) || paths.first.is_a?(Symbol) - - results = [] - paths.each do |type, path| - # Create the local/remote path based on whether this is a host - # or VM path. - local_path = nil - remote_path = nil - if type == :host - # Get the expanded path that the host path points to - local_path = File.expand_path(path, @machine.env.root_path) - - if File.exist?(local_path) - # Path exists on the host, setup the remote path. We use - # the MD5 of the local path so that it is predictable. - key = Digest::MD5.hexdigest(local_path) - remote_path = "#{guest_provisioning_path}/#{key}" - else - @machine.ui.warn(I18n.t( - "vagrant.provisioners.chef.cookbook_folder_not_found_warning", - path: local_path.to_s)) - next - end - else - # Path already exists on the virtual machine. Expand it - # relative to where we're provisioning. - remote_path = File.expand_path(path, guest_provisioning_path) - - # Remove drive letter if running on a windows host. This is a bit - # of a hack but is the most portable way I can think of at the moment - # to achieve this. Otherwise, Vagrant attempts to share at some crazy - # path like /home/vagrant/c:/foo/bar - remote_path = remote_path.gsub(/^[a-zA-Z]:/, "") - end - - # If we have specified a folder name to append then append it - remote_path += "/#{appended_folder}" if appended_folder - - # Append the result - results << [type, local_path, remote_path] - end - - results - end - - # Shares the given folders with the given prefix. The folders should - # be of the structure resulting from the `expanded_folders` function. - def share_folders(root_config, prefix, folders, existing=nil) - existing_set = Set.new - (existing || []).each do |_, fs| - fs.each do |id, data| - existing_set.add(data[:guestpath]) - end - end - - folders.each do |type, local_path, remote_path| - next if type != :host - - # If this folder already exists, then we don't share it, it means - # it was already put down on disk. - # - # NOTE: This is currently commented out because it was causing - # major bugs (GH-5199). We will investigate why this is in more - # detail for 1.8.0, but we wanted to fix this in a patch release - # and this was the hammer that did that. -=begin - if existing_set.include?(remote_path) - @logger.debug("Not sharing #{local_path}, exists as #{remote_path}") - next - end -=end - - key = Digest::MD5.hexdigest(remote_path) - key = key[0..8] - - opts = {} - opts[:id] = "v-#{prefix}-#{key}" - opts[:type] = @config.synced_folder_type if @config.synced_folder_type - - root_config.vm.synced_folder(local_path, remote_path, opts) - end - - @shared_folders += folders - end - def setup_zero_config setup_config("provisioners/chef_zero/zero", "client.rb", { local_mode: true, @@ -162,7 +50,7 @@ module VagrantPlugins }) end - def run_chef(mode) + def run_chef_zero if @config.run_list && @config.run_list.empty? @machine.ui.warn(I18n.t("vagrant.chef_run_list_empty")) end @@ -171,13 +59,17 @@ module VagrantPlugins @machine.guest.capability(:wait_for_reboot) end - command = build_command(:client) + command = CommandBuilder.command(:client, @config, + windows: windows?, + colored: @machine.env.ui.color?, + local_mode: true, + ) @config.attempts.times do |attempt| if attempt == 0 - @machine.ui.info I18n.t("vagrant.provisioners.chef.running_#{mode}") + @machine.ui.info I18n.t("vagrant.provisioners.chef.running_zero") else - @machine.ui.info I18n.t("vagrant.provisioners.chef.running_#{mode}_again") + @machine.ui.info I18n.t("vagrant.provisioners.chef.running_zero_again") end opts = { error_check: false, elevated: true } diff --git a/templates/locales/en.yml b/templates/locales/en.yml index b8c91aa64..b906e8f0b 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -1849,8 +1849,8 @@ en: running_apply: "Running chef-apply..." running_solo: "Running chef-solo..." running_solo_again: "Running chef-solo again (failed to converge)..." - running_zero: "Running chef-zero..." - running_zero_again: "Running chef-zero again (failed to converge)..." + running_zero: "Running chef-client (local-mode)..." + running_zero_again: "Running chef-client (local-mode) again (failed to converge)..." missing_shared_folders: |- Shared folders that Chef requires are missing on the virtual machine. This is usually due to configuration changing after already booting the From d72306b6d505ff0106d9485e6c505422882075c7 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Thu, 9 Jul 2015 21:36:50 -0600 Subject: [PATCH 2/2] Fix command builder tests --- .../provisioners/chef/command_builder_test.rb | 85 +++++++++++-------- 1 file changed, 49 insertions(+), 36 deletions(-) diff --git a/test/unit/plugins/provisioners/chef/command_builder_test.rb b/test/unit/plugins/provisioners/chef/command_builder_test.rb index 3fb53693b..655c9041f 100644 --- a/test/unit/plugins/provisioners/chef/command_builder_test.rb +++ b/test/unit/plugins/provisioners/chef/command_builder_test.rb @@ -8,97 +8,110 @@ describe VagrantPlugins::Chef::CommandBuilder do let(:chef_config) { double("chef_config") } before(:each) do - allow(chef_config).to receive(:provisioning_path).and_return('/tmp/vagrant-chef-1') + allow(chef_config).to receive(:provisioning_path).and_return("/tmp/vagrant-chef-1") allow(chef_config).to receive(:arguments).and_return(nil) allow(chef_config).to receive(:binary_env).and_return(nil) allow(chef_config).to receive(:binary_path).and_return(nil) allow(chef_config).to receive(:binary_env).and_return(nil) + allow(chef_config).to receive(:log_level).and_return(:info) end - describe '.initialize' do - it 'should raise when chef type is not client or solo' do + describe ".initialize" do + it "raises an error when chef type is not client or solo" do expect { VagrantPlugins::Chef::CommandBuilder.new(chef_config, :client_bad) }. to raise_error end + + it "does not raise an error for :client" do + expect { + VagrantPlugins::Chef::CommandBuilder.new(:client, chef_config) + }.to_not raise_error + end + + it "does not raise an error for :solo" do + expect { + VagrantPlugins::Chef::CommandBuilder.new(:solo, chef_config) + }.to_not raise_error + end end - describe 'build_command' do - describe 'windows' do + describe "#command" do + describe "windows" do subject do - VagrantPlugins::Chef::CommandBuilder.new(chef_config, :client, true) + VagrantPlugins::Chef::CommandBuilder.new(:client, chef_config, windows: true) end it "executes the chef-client in PATH by default" do - expect(subject.build_command()).to match(/^chef-client/) + expect(subject.command).to match(/^chef-client/) end it "executes the chef-client using full path if binary_path is specified" do allow(chef_config).to receive(:binary_path).and_return( "c:\\opscode\\chef\\bin\\chef-client") - expect(subject.build_command()).to match(/^c:\\opscode\\chef\\bin\\chef-client\\chef-client/) + expect(subject.command).to match(/^c:\\opscode\\chef\\bin\\chef-client\\chef-client/) end it "builds a guest friendly client.rb path" do - expect(subject.build_command()).to include( - '-c c:\\tmp\\vagrant-chef-1\\client.rb') + expect(subject.command).to include( + '--config c:\\tmp\\vagrant-chef-1\\client.rb') end it "builds a guest friendly solo.json path" do - expect(subject.build_command()).to include( - '-j c:\\tmp\\vagrant-chef-1\\dna.json') + expect(subject.command).to include( + '--json-attributes c:\\tmp\\vagrant-chef-1\\dna.json') end - it 'includes Chef arguments if specified' do - allow(chef_config).to receive(:arguments).and_return("-l DEBUG") - expect(subject.build_command()).to include( - '-l DEBUG') + it "includes Chef arguments if specified" do + allow(chef_config).to receive(:arguments).and_return("bacon pants") + expect(subject.command).to include( + " bacon pants") end - it 'includes --no-color if UI is not colored' do - expect(subject.build_command()).to include( - ' --no-color') + it "includes --no-color if UI is not colored" do + expect(subject.command).to include( + " --no-color") end end - describe 'linux' do + describe "linux" do subject do - VagrantPlugins::Chef::CommandBuilder.new(chef_config, :client, false) + VagrantPlugins::Chef::CommandBuilder.new(:client, chef_config, windows: false) end it "executes the chef-client in PATH by default" do - expect(subject.build_command()).to match(/^chef-client/) + expect(subject.command).to match(/^chef-client/) end it "executes the chef-client using full path if binary_path is specified" do allow(chef_config).to receive(:binary_path).and_return( "/opt/chef/chef-client") - expect(subject.build_command()).to match(/^\/opt\/chef\/chef-client/) + expect(subject.command).to match(/^\/opt\/chef\/chef-client/) end it "builds a guest friendly client.rb path" do - expect(subject.build_command()).to include( - '-c /tmp/vagrant-chef-1/client.rb') + expect(subject.command).to include( + "--config /tmp/vagrant-chef-1/client.rb") end it "builds a guest friendly solo.json path" do - expect(subject.build_command()).to include( - '-j /tmp/vagrant-chef-1/dna.json') + expect(subject.command).to include( + "--json-attributes /tmp/vagrant-chef-1/dna.json") end - it 'includes Chef arguments if specified' do - allow(chef_config).to receive(:arguments).and_return("-l DEBUG") - expect(subject.build_command()).to include( - '-l DEBUG') + it "includes Chef arguments if specified" do + allow(chef_config).to receive(:arguments).and_return("bacon") + expect(subject.command).to include( + " bacon") end - it 'includes --no-color if UI is not colored' do - expect(subject.build_command()).to include( - ' --no-color') + it "includes --no-color if UI is not colored" do + expect(subject.command).to include( + " --no-color") end - it 'includes environment variables if specified' do + it "includes environment variables if specified" do allow(chef_config).to receive(:binary_env).and_return("ENVVAR=VAL") - expect(subject.build_command()).to match(/^ENVVAR=VAL /) + expect(subject.command).to match(/^ENVVAR=VAL /) end end end