Merge pull request #3 from RoboticCheese/jdh-make-safer-for-concurrency

Use a class Mutex to not run multiple driver instances concurrently

.kitchen.yml

@@ -9,9 +9,9 @@ platforms:
   - name: localhost
 
 suites:
-  - name: tempfile
+  - name: create_file
     run_list:
-      - recipe[kitchen-localhost-test]
-    attributes:
-      kitchen_localhost_test:
-        test_file: testing_kitchen_localhost
+      - recipe[kitchen-localhost-test::create_file]
+  - name: delete_file
+    run_list:
+      - recipe[kitchen-localhost-test::delete_file]

.travis.yml

@@ -4,7 +4,7 @@ cache: bundler
 sudo: true
 
 script:
-  - bundle exec rake && bundle exec rake kitchen:all
+  - bundle exec rake && bundle exec kitchen test -c 2
 
 rvm:
 - ruby-head

CHANGELOG.md

@@ -3,6 +3,8 @@ Kitchen-Localhost CHANGELOG
 
 v?.?.? (????-??-??)
 -------------------
+* Try to catch instances where Kitchen is running in concurrency mode and force
+  instances of this driver to run serially.
 
 v0.1.1 (2015-04-29)
 -------------------

README.md

@@ -29,6 +29,10 @@ understanding that this driver will be running against _your local machine_. If
 you write a cookbook that formats a hard drive and run it with this driver, bad
 things will happen.
 
+Also note that this driver's very nature makes it likely there will be problems
+if you try to do a Test Kitchen run with multiple suites and/or with
+concurrency enabled.
+
 Installation and Setup
 ----------------------
 

kitchen-localhost.gemspec

@@ -10,7 +10,7 @@ Gem::Specification.new do |spec|
   spec.authors       = ['Jonathan Hartman']
   spec.email         = %w(j@p4nt5.com)
   spec.description   = 'A Test Kitchen Driver for localhost'
-  spec.summary       = 'Use Test Kitchen to run Chef on your local machine!'
+  spec.summary       = 'Use Test Kitchen on your local machine!'
   spec.homepage      = 'https://github.com/test-kitchen/kitchen-localhost'
   spec.license       = 'Apache 2.0'
 

lib/kitchen/driver/localhost.rb

@@ -31,14 +31,42 @@ module Driver
     # @author Jonathan Hartman <j@p4nt5.com>
     class Localhost < Kitchen::Driver::Base
       kitchen_driver_api_version 2
-
       plugin_version Kitchen::Localhost::VERSION
 
+      #
+      # Define a Mutex at the class level so we can prevent instances using
+      # this driver from colliding.
+      #
+      # @return [Mutex]
+      #
+      def self.lock
+        @lock ||= Mutex.new
+      end
+
+      #
+      # Lock the class-level Mutex, whatever state it's in currently.
+      #
+      def self.lock!
+        lock.lock
+      end
+
+      #
+      # Unlock the class-level Mutex, whatever state it's in currently.
+      #
+      def self.unlock!
+        # Mutex#unlock raises an exception if lock is owned by another thread
+        # and Mutex#owned? isn't available in Ruby 1.9.
+        lock.unlock if lock.locked?
+      rescue ThreadError
+        nil
+      end
+
       #
       # Create the temp dirs on the local filesystem for Kitchen.
       #
       # (see Base#create)
       def create(state)
+        self.class.lock!
         state[:hostname] = Socket.gethostname
         logger.info("[Localhost] Instance #{instance} ready.")
       end
@@ -56,6 +84,7 @@ def destroy(_)
           FileUtils.rm_rf(p)
           logger.info("[Localhost] Deleted temp dir '#{p}'.")
         end
+        self.class.unlock!
       end
     end
   end

spec/kitchen/driver/localhost_spec.rb

@@ -22,6 +22,69 @@
     expect(res).to match(expected)
   end
 
+  describe '.lock' do
+    it 'returns a Mutex' do
+      expect(described_class.lock).to be_an_instance_of(Mutex)
+    end
+  end
+
+  describe '.lock!' do
+    let(:lock) { double(lock: true) }
+
+    before(:each) do
+      allow(described_class).to receive(:lock).and_return(lock)
+    end
+
+    it 'requests a lock from the class-level Mutex' do
+      expect(lock).to receive(:lock)
+      described_class.lock!
+    end
+  end
+
+  describe '.unlock!' do
+    let(:locked?) { nil }
+    let(:owned?) { nil }
+    let(:lock) { double(locked?: locked?) }
+
+    before(:each) do
+      if owned?
+        allow(lock).to receive(:unlock)
+      else
+        allow(lock).to receive(:unlock).and_raise(ThreadError)
+      end
+      allow(described_class).to receive(:lock).and_return(lock)
+    end
+
+    context 'already locked and owned' do
+      let(:locked?) { true }
+      let(:owned?) { true }
+
+      it 'tries to unlock without error' do
+        expect(lock).to receive(:unlock)
+        described_class.unlock!
+      end
+    end
+
+    context 'locked and not owned' do
+      let(:locked?) { true }
+      let(:owned?) { false }
+
+      it 'tries to unlock without error' do
+        expect(lock).to receive(:unlock)
+        described_class.unlock!
+      end
+    end
+
+    context 'not locked' do
+      let(:locked!) { false }
+
+      it 'does nothing' do
+        expect(lock).not_to receive(:unlock)
+        described_class.unlock!
+      end
+    end
+  end
+
   describe '#create' do
     let(:state) { {} }
     let(:hostname) { 'example.com' }
@@ -30,6 +93,11 @@
       allow(Socket).to receive(:gethostname).and_return(hostname)
     end
 
+    it 'locks the class-level Mutex' do
+      expect(described_class).to receive(:lock!)
+      driver.create(state)
+    end
+
     it 'adds the system hostname to the state' do
       s = state
       driver.create(s)
@@ -61,5 +129,10 @@
       expect(FileUtils).to receive(:rm_rf).with(verifier_path)
       driver.destroy(state)
     end
+
+    it 'unlocks the class-level Mutex' do
+      expect(described_class).to receive(:unlock!)
+      driver.destroy(state)
+    end
   end
 end

test/fixtures/cookbooks/kitchen-localhost-test/recipes/create_file.rb

@@ -0,0 +1,10 @@
+# Encoding: UTF-8
+#
+# Cookbook Name:: kitchen-localhost-test
+# Recipe:: create_file
+#
+
+file '/tmp/testing_kitchen_localhost' do
+  content 'This file generated by Chef'
+  action :create
+end

test/fixtures/cookbooks/kitchen-localhost-test/recipes/delete_file.rb

@@ -0,0 +1,9 @@
+# Encoding: UTF-8
+#
+# Cookbook Name:: kitchen-localhost-test
+# Recipe:: delete_file
+#
+
+file '/tmp/testing_kitchen_localhost' do
+  action :delete
+end

test/integration/delete_file/serverspec/localhost/file_spec.rb

@@ -0,0 +1,9 @@
+# Encoding: UTF-8
+
+require 'spec_helper'
+
+describe file('/tmp/testing_kitchen_localhost') do
+  it 'does not exist' do
+    expect(subject).not_to be_file
+  end
+end

test/integration/delete_file/serverspec/spec_helper.rb

@@ -0,0 +1 @@
+../../create_file/serverspec/spec_helper.rb