provisioners/chef: put global lock around knife exec

CHANGELOG.md

@@ -23,6 +23,7 @@ BUG FIXES:
   - hosts/arch: NFS works with latest versions. [GH-4224]
   - hosts/windows: RDP command works without crash. [GH-3962]
   - providers/virtualbox: Increase network device limit to 36. [GH-4206]
+  - provisioners/chef: Chef client cleanup should work. [GH-4099]
   - provisioners/puppet: Manifest file can be a directory. [GH-4169]
   - provisioners/puppet: Properly escape facter variables for PowerShell
       on Windows guests. [GH-3959]

lib/vagrant/shared_helpers.rb

@@ -1,7 +1,10 @@
 require "pathname"
 require "tempfile"
+require "thread"
 
 module Vagrant
+  @@global_lock = Mutex.new
+
   # This is the default endpoint of the Vagrant Cloud in
   # use. API calls will be made to this for various functions
   # of Vagrant that may require remote access.
@@ -9,6 +12,15 @@ module Vagrant
   # @return [String]
   DEFAULT_SERVER_URL = "https://vagrantcloud.com"
 
+  # This holds a global lock for the duration of the block. This should
+  # be invoked around anything that is modifying process state (such as
+  # environmental variables).
+  def self.global_lock
+    @@global_lock.synchronize do
+      return yield
+    end
+  end
+
   # This returns a true/false showing whether we're running from the
   # environment setup by the Vagrant installers.
   #

plugins/provisioners/chef/provisioner/chef_client.rb

@@ -1,4 +1,6 @@
 require 'pathname'
+
+require 'vagrant'
 require 'vagrant/util/subprocess'
 
 require File.expand_path("../base", __FILE__)
@@ -106,15 +108,17 @@ module VagrantPlugins
 
           # Knife is not part of the current Vagrant bundle, so it needs to run
           # in the context of the system.
-          Bundler.with_clean_env do
+          Vagrant.global_lock do
-            command = ["knife", deletable, "delete", "--yes", node_name]
+            Bundler.with_clean_env do
-            r = Vagrant::Util::Subprocess.execute(*command)
+              command = ["knife", deletable, "delete", "--yes", node_name]
-            if r.exit_code != 0
+              r = Vagrant::Util::Subprocess.execute(*command)
-              @machine.ui.error(I18n.t(
+              if r.exit_code != 0
-                "vagrant.chef_client_cleanup_failed",
+                @machine.ui.error(I18n.t(
-                deletable: deletable,
+                  "vagrant.chef_client_cleanup_failed",
-                stdout: r.stdout,
+                  deletable: deletable,
-                stderr: r.stderr))
+                  stdout: r.stdout,
+                  stderr: r.stderr))
+              end
             end
           end
         end

test/unit/vagrant/shared_helpers_test.rb

@@ -8,6 +8,13 @@ describe Vagrant do
 
   subject { described_class }
 
+  describe "#global_lock" do
+    it "yields to the block" do
+      result = subject.global_lock { 42 }
+      expect(result).to eq(42)
+    end
+  end
+
   describe "#in_installer?" do
     it "is not if env is not set" do
       with_temp_env("VAGRANT_INSTALLER_ENV" => nil) do