Merge pull request #5916 from mitchellh/sethvargo/chef_zero

Refactor Chef provisioners

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)
+      def self.command(type, config, options = {})
-        @client_type   = client_type
+        new(type, config, options).command
-        @config        = config
+      end
-        @is_windows    = is_windows
-        @is_ui_colored = is_ui_colored
 
-        if client_type != :solo && client_type != :client
+      attr_reader :type
-          raise 'Invalid client_type, expected solo or client'
+      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
+      def command
-        "#{command_env}#{chef_binary_path} #{chef_arguments}"
+        if config.binary_env
+          "#{config.binary_env} #{binary_path} #{args}"
+        else
+          "#{binary_path} #{args}"
+        end
       end
 
       protected
 
-      def command_env
+      def binary_path
-        @config.binary_env ? "#{@config.binary_env} " : ""
+        binary_path = "chef-#{type}"
-      end
 
-      def chef_binary_path
+        if config.binary_path
-        binary_path = "chef-#{@client_type}"
+          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
+      def args
-        chef_arguments = "-c #{provisioning_path("#{@client_type}.rb")}"
+        args =  ""
-        chef_arguments << " -j #{provisioning_path("dna.json")}"
+        args << " --config #{provisioning_path("#{type}.rb")}"
-        chef_arguments << " #{@config.arguments}" if @config.arguments
+        args << " --json-attributes #{provisioning_path("dna.json")}"
-        chef_arguments << " --no-color" unless color?
+        args << " --local-mode" if options[:local_mode]
-        chef_arguments.strip
+        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
+        !!options[:windows]
-      end
-
-      def color?
-        !!@is_ui_colored
       end
     end
   end

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)

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

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 }

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
+      class ChefZero < ChefSolo
-        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
-
         def initialize(machine, config)
           super
           @logger = Log4r::Logger.new("vagrant::provisioners::chef_zero")
-          @shared_folders = []
         end
 
-        def configure(root_config)
+        def provision
-          @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)
           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 }

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: "Running chef-client (local-mode)..."
-        running_zero_again: "Running chef-zero again (failed to converge)..."
+        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

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
+  describe ".initialize" do
-    it 'should raise when chef type is not client or solo' 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 "#command" do
-    describe 'windows' 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(
+        expect(subject.command).to include(
-          '-c c:\\tmp\\vagrant-chef-1\\client.rb')
+          '--config c:\\tmp\\vagrant-chef-1\\client.rb')
       end
 
       it "builds a guest friendly solo.json path" do
-        expect(subject.build_command()).to include(
+        expect(subject.command).to include(
-          '-j c:\\tmp\\vagrant-chef-1\\dna.json')
+          '--json-attributes c:\\tmp\\vagrant-chef-1\\dna.json')
       end
 
-      it 'includes Chef arguments if specified' do
+      it "includes Chef arguments if specified" do
-        allow(chef_config).to receive(:arguments).and_return("-l DEBUG")
+        allow(chef_config).to receive(:arguments).and_return("bacon pants")
-        expect(subject.build_command()).to include(
+        expect(subject.command).to include(
-          '-l DEBUG')
+          " bacon pants")
       end
 
-      it 'includes --no-color if UI is not colored' do
+      it "includes --no-color if UI is not colored" do
-        expect(subject.build_command()).to include(
+        expect(subject.command).to include(
-          ' --no-color')
+          " --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(
+        expect(subject.command).to include(
-          '-c /tmp/vagrant-chef-1/client.rb')
+          "--config /tmp/vagrant-chef-1/client.rb")
       end
 
       it "builds a guest friendly solo.json path" do
-        expect(subject.build_command()).to include(
+        expect(subject.command).to include(
-          '-j /tmp/vagrant-chef-1/dna.json')
+          "--json-attributes /tmp/vagrant-chef-1/dna.json")
       end
 
-      it 'includes Chef arguments if specified' do
+      it "includes Chef arguments if specified" do
-        allow(chef_config).to receive(:arguments).and_return("-l DEBUG")
+        allow(chef_config).to receive(:arguments).and_return("bacon")
-        expect(subject.build_command()).to include(
+        expect(subject.command).to include(
-          '-l DEBUG')
+          " bacon")
       end
 
-      it 'includes --no-color if UI is not colored' do
+      it "includes --no-color if UI is not colored" do
-        expect(subject.build_command()).to include(
+        expect(subject.command).to include(
-          ' --no-color')
+          " --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