CHEF-3832: Fix for InSpec Parallel fails to fetch remote profiles due to cache contention. (#6546)

* CHEF-3832: Fix for InSpec Parallel fails to fetch remote profiles due to cache contention.
This fix creates the write exclusive lock on the cache directory of profile till write operation is finished for git fetcher

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

* Refactored to have locking and unlocking logic inside cache. Cache directory is not required for archived directories and this is currently only required by git fetcher

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

* REFACTOR: adds method in fetcher to return true if requires cache locking mechanism to avoid cache contention

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

* Adds exception handling

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

* Handles interruption in cache creation process to avoid empty cache directory creation or currupted data

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

* Refactored to handle interruption through SystemExit

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

* Fixed the failing test. While creating lock cache_key need to be extracted from the respective fetcher.

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

---------

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

lib/inspec/cached_fetcher.rb

@@ -39,12 +39,33 @@ module Inspec
     end
 
     def fetch
-      if cache.exists?(cache_key)
+      if cache.exists?(cache_key) && cache.locked?(cache_key)
+        Inspec::Log.debug "Waiting for lock to be released on the cache dir ...."
+        counter = 0
+        until cache.locked?(cache_key) == false
+          if (counter += 1) > 300
+            Inspec::Log.warn "Giving up waiting on cache lock at #{cache_key}"
+            exit 1
+          end
+          sleep 0.1
+        end
+        fetch
+      elsif cache.exists?(cache_key) && !cache.locked?(cache_key)
         Inspec::Log.debug "Using cached dependency for #{target}"
         [cache.prefered_entry_for(cache_key), false]
       else
+        begin
           Inspec::Log.debug "Dependency does not exist in the cache #{target}"
+          cache.lock(cache.base_path_for(fetcher.cache_key)) if fetcher.requires_locking?
           fetcher.fetch(cache.base_path_for(fetcher.cache_key))
+        rescue SystemExit => e
+          exit_code = e.status || 1
+          Inspec::Log.error "Error while creating cache for dependency ... #{e.message}"
+          FileUtils.rm_rf(cache.base_path_for(fetcher.cache_key))
+          exit(exit_code)
+        ensure
+          cache.unlock(cache.base_path_for(fetcher.cache_key)) if fetcher.requires_locking?
+        end
         assert_cache_sanity!
         [fetcher.archive_path, fetcher.writable?]
       end

lib/inspec/dependencies/cache.rb

@@ -70,5 +70,38 @@ module Inspec
     def base_path_for(cache_key)
       File.join(@path, cache_key)
     end
+
+    #
+    # For given cache key, return true if the
+    # cache path is locked
+    def locked?(key)
+      locked = false
+      path = base_path_for(key)
+      # For archive there is no need to lock the directory so we skip those and return false for archive formatted cache
+      if File.directory?(path)
+        locked = File.exist?("#{path}/.lock")
+      end
+      locked
+    end
+
+    def lock(cache_path)
+      lock_file_path = File.join(cache_path, ".lock")
+      begin
+        FileUtils.mkdir_p(cache_path)
+        Inspec::Log.debug("Locking cache ..... #{cache_path}")
+        FileUtils.touch(lock_file_path)
+      rescue Errno::EACCES
+        raise "Permission denied while creating cache lock #{cache_path}/.lock."
+      end
+    end
+
+    def unlock(cache_path)
+      Inspec::Log.debug("Unlocking cache..... #{cache_path}")
+      begin
+        FileUtils.rm("#{cache_path}/.lock") if File.exist?("#{cache_path}/.lock")
+      rescue Errno::EACCES
+        raise "Permission denied while removing cache lock #{cache_path}/.lock"
+      end
+    end
   end
 end

lib/inspec/fetcher/git.rb

@@ -115,6 +115,11 @@ module Inspec::Fetcher
       %i{branch tag ref}.map { |opt_name| update_ivar_from_opt(opt_name, opts) }.any?
     end
 
+    # Git fetcher is sensitive to cache contention so it needs cache locking mechanism.
+    def requires_locking?
+      true
+    end
+
     private
 
     def resolved_ref

lib/inspec/plugin/v1/plugin_types/fetcher.rb

@@ -105,6 +105,13 @@ module Inspec
         file_provider = Inspec::FileProvider.for_path(archive_path)
         file_provider.relative_provider
       end
+
+      # Returns false by default
+      # This is used to regulate cache contention.
+      # Fetchers that are sensitive to cache contention should return true.
+      def requires_locking?
+        false
+      end
     end
   end
 end